Filter bugfixes #848
Filter bugfixes #848
Conversation
src/js/components/filters.js
Outdated
| @@ -88,8 +88,13 @@ class Filters { | |||
| techSelector.before(errorMsg); | |||
| } | |||
|
|
|||
| /* Get a list of technologies */ | |||
| const techs = this.technologies; | |||
| const sortedTechs = techs.sort((a, b) => a.technology - b.technology); | |||
There was a problem hiding this comment.
Per #845 (comment) let's revert back to sorting by # sites
There was a problem hiding this comment.
I'll just revert back for now, and suggest we add more user-friendly dropdowns as a separate action item (so deciding on a sort order and design for it doesn't block the other bugfixes in this PR). If we keep it by origins, then we should implement a component more similar to the old report, where we can display the numbers as well.
src/js/components/filters.js
Outdated
| @@ -107,7 +112,8 @@ class Filters { | |||
| updateGeo() { | |||
| const select = document.querySelector('select#geo'); | |||
| select.innerHTML = ''; | |||
| this.geos.forEach((geo) => { | |||
| const sortedGeos = this.geos.sort((a, b) => a.geo !== b.geo ? a.geo < b.geo ? -1 : 1 : 0); | |||
There was a problem hiding this comment.
Per #845 (comment) let's revert back to sorting by # sites
There was a problem hiding this comment.
See: #848 (comment)
For geos I'm wondering if alphabetical might make it easier to find specific countries, as that's how most country selectors on the web are sorted alphabetically. If I want to find Norway, I know I still have to scroll further when I see "Finland" but am close when I see "Netherlands" or "North Korea". Maybe a good one to look for some more feedback/user tests on, or try out two different designs?
There was a problem hiding this comment.
I do also like how the Data Studio select list includes a search filter. More complex to reimplement but could give us the best of both.
src/js/components/filters.js
Outdated
| selectedTechs.forEach(selectedTech => selectedTechInfo.push(this.technologies.find(tech => tech.technology === selectedTech))); | ||
| // Get the techs associated with the selected category | ||
| const selectedCategory = this.categories.find(category => category.category === event.target.value); | ||
| const selectedTechs = selectedCategory?.technologies; |
There was a problem hiding this comment.
I think we need a fallback in case selectedCategory is null, to avoid breaking the forEach below
| const selectedTechs = selectedCategory?.technologies; | |
| const selectedTechs = selectedCategory?.technologies || []; |
Fixed: