Feature/update special filter UI and create any/none/fuzzy query params #238
Conversation
…er-app into feature/update-special-filters
aswallace
requested review from
kmitcham,
pgarrison,
SeanLeRoy,
lynwilhelm and
BrianWhitneyAI
…er-app into feature/update-special-filters
| setSearchMaxValue(""); | ||
| setSearchMinValue(overallMin); | ||
| setSearchMaxValue(overallMax); | ||
| onSearch(`RANGE(${overallMin},${overallMax})`); |
There was a problem hiding this comment.
Debatable if this is in the scope of this PR but I think its odd we pass a string version of a range filter to the parent component rather than something like onSearch(min, max)
There was a problem hiding this comment.
Agreed, although onSearch is a pre-existing function also used for searchboxes so would need some additional changes to accept other args. RANGE as a filter value in general could use refactoring, we have to do some additional processing in number-formatter and date-formatter as well
There was a problem hiding this comment.
I feel like the decision to make the ANY, EXCLUDE EVERYTHING, and FUZZY filters totally separate has made the state logic of other components much more complicated. I like the idea of each having their own class, but could they each have a "type" property that we could then use in the components to separate out the filters we need? I'm unsure what we gain by having them separated since we can create composed selectors to separate them out of the state branch if need by like:
const getAnnotationsFilteredOut = createSelector(getFileFilters, filters) => (
filters
.filter(filter => filter.type === FilterType.EXCLUDE)
.map(filter => filter.annotationName)
))
const getAnnotationsRequired = createSelector(getFileFilters, filters) => (
filters
.filter(filter => filter.type === FilterType.ANY)
.map(filter => filter.annotationName)
))
There was a problem hiding this comment.
I agree, but would need to think about how we'd incorporate fuzzy filters into this, since a filter can be both a regular FileFilter with a value AND a fuzzy filter
There was a problem hiding this comment.
That could be another type I think
…er-app into feature/update-special-filters
CHANGES FROM PREVIOUS VERSIONAdded a new optional This means the FileExplorerUrl (our local BFF url parsing class) will NOT have the fuzzy/include/exclude params that I'd added in before, since now all filters are part of the
Also no longer includes the unrelated UX changes to the modals since was merged in #250 |
| dispatch(selection.actions.addFileFilter(createFileFilter(item))); | ||
| }; | ||
|
|
||
| // TODO: Should this select ALL or just the visible items in list? |
There was a problem hiding this comment.
I think just visible items
| .map((parsedFilter) => new FileFilter(parsedFilter.name, parsedFilter.value)), | ||
| .map( | ||
| (parsedFilter) => | ||
| new FileFilter(parsedFilter.name, parsedFilter.value, parsedFilter.type) |
There was a problem hiding this comment.
This works with type being undefined? Just triple checking that we aren't making links created before this PR unusable - our links have to always be backwards compatible now that we are tied to the EMT paper publication
There was a problem hiding this comment.
yep! I gave the parent class a default value. Many of our unit tests still use FileFilter(name, value) without a type, so the constructor provides it with FilterType.DEFAULT if no arg is given.
Added tests for this in d685394
Description
A Big Ticket! This PR includes:
UX updates to the special filter types (date, numerical range, search box with fuzzy search)closes #53 , closes #74
TODO: fix font (#235 )Testing
Screenshots
EDIT: Updated screenshots 9/30/2024
Any/none option
The radio buttons are a little off center with Helvetica, but I think it will be fixed when we're using the correct Open Sans linkSearch box with new fuzzy search toggle (exact/non-fuzzy by default)
Date range with filter type choice group
Numerical range with filter type choice group