-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Respect area type in filters #344
Respect area type in filters #344
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 256-multiple-geographies #344 +/- ##
============================================================
+ Coverage 82.82% 82.93% +0.10%
============================================================
Files 90 92 +2
Lines 2725 2742 +17
Branches 278 279 +1
============================================================
+ Hits 2257 2274 +17
Misses 404 404
Partials 64 64 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve added a fixup commit that simplifies the styling for the "unavailable" filters (no need to show the form controls, if they’ll have no effect). I guess this same styling / markup will need to be replicated for the shaders in the shaderContainer
, and the columns in the columnContainer
(which currently has no areas_available
logic at all).
I‘ve also added a commit that generates the list of area type <option>
s from data on the JavaScript side, rather than being hard-coded in the HTML markup. Felt like that’s more along the lines of the way the rest of the logic on this page works – but feel free to revert it, if it’s not suitable.
One more thing that needs fixing, I think, is that datasets should only appear in the "Add dataset" or "Add shader" modals if they apply to the current area type. No sense in displaying datasets that people can’t/shouldn’t choose. We’ll need to do this filtering before the favouriteDatasets
, featuredDatasets
, and otherDatasets
arrays are passed to add-data-modal.html
, so that any "empty" arrays are properly skipped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit tricky to test this out locally as I don’t have any WMC23-only datasets, but from what I can see, this looks good to go 👍
863476e
to
228f87c
Compare
Otherwise you end up with no areas returned if you select a dataset that's not available for the area type Fixes #335
Include area types available with filter list returned from server and use this to highlight is a filter isn't available for the chosen area type Part of #336
Also indicate in the UI if it's not available.
We wrap this in a v-cloak div to prevent the naked option text "${ t.label }" being visible before vue.js loads.
Adds an availableOtherDatasets property which is the list of datasets available for the current area type.
56df20f
to
50570ce
Compare
merged into #350 |
This handles both the back end task only filtering on available filters for the area type, and also the front end of indicating when a selected filter type is not available when switching area types.
This also updates the filter list to indicate if a filter is available for the current area type.