Enable filtering for all available metadata #1743
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nextstrain-bot
temporarily deployed
to
auspice-james-search-al-xnd7bk
jameshadfield
force-pushed
the
james/search-all
branch
from
6c9bbbb to
29c6a90
Compare
nextstrain-bot
temporarily deployed
to
auspice-james-search-al-xnd7bk
joverlee521
approved these changes
Jan 31, 2024
There was a problem hiding this comment.
The new filters work as expected in the test app! I tested filtering by accessions in the ncov and seasonal-flu builds.
+1 for removing a feature that has been broken for ~4 years 😅
I left some comments to change the order of the filter options, but overall looks good to me.
This fixes a bug introduced in 8b1b9a2 (September 2019) where we moved the data structure from an Object to a Map. The test summary appears at the top of the page, the download modal and within a exported SVG.
This behaviour was restored by the previous commit but had been broken for over 4 years. For early datasets, which used relatively few filters, this text was mostly appropriate. For many of today's datasets it's cumbersome, with phrases like "comprising 3 symptoms and 3 recencys". The removal of `visibleStateCounts` should improve performance a little, but we may wish to reinstate it in the future if we ever want to display information about the number of visible tips per filter.
Deferring the value lookup will be slightly faster, especially for the (common) terminal-nodes-only case. We have no use for counting invalid values, so skip these.
The sidebar filtering now surfaces all valid node-attrs defined across
the (terminal) nodes. URL queries (`?f_${attrName}=value1,value2,...`)
also work for all known attrs. Attributes which are known to be
continuous (via a colorings definition) are excluded, as the filtering
UI is not (yet) able to handle these; if a non-coloring continuous
attribute is set on the nodes then this will end up as a multitude of
numerical options in the sidebar.
As part of this implementation we have removed `stateCountAttrs` from
redux state and improved the validation of (filtering) URL queries.
The behaviour of filtering, and the restriction to collecting attributes
from terminal nodes only, is unchanged. See #1275 for more context.
Closes #1251
jameshadfield
force-pushed
the
james/search-all
branch
from
29c6a90 to
70665c8
Compare
nextstrain-bot
temporarily deployed
to
auspice-james-search-al-xnd7bk
This was referenced Feb 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit messages for more details, but here's the main change:
The sidebar filtering now surfaces all valid node-attrs defined across
the (terminal) nodes. URL queries (
?f_${attrName}=value1,value2,...)also work for all known attrs. Attributes which are known to be
continuous (via a colorings definition) are excluded, as the filtering
UI is not (yet) able to handle these; if a non-coloring continuous
attribute is set on the nodes then this will end up as a multitude of
numerical options in the sidebar.
As part of this implementation we have removed
stateCountAttrsfromredux state and improved the validation of (filtering) URL queries.
The behaviour of filtering, and the restriction to collecting attributes
from terminal nodes only, is unchanged. See #1275 for more context.
Closes #1251