-
Notifications
You must be signed in to change notification settings - Fork 4
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
DT-781: Use logical AND to combine filters in the DUOS UI #2715
Conversation
…lso combine some common code to avoid duplication
@@ -104,60 +104,48 @@ export const DatasetSearchTable = (props) => { | |||
queryChunks.push(...searchModifier); | |||
} | |||
|
|||
var filterQuery = {}; | |||
let filterQuery = {}; |
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 think var
is not recommended now that we have let
and const
. The difference is that var
has function scope, whereas let
and const
have block scope, which is usually what you want.
'bool': { | ||
'must': queryChunks | ||
} | ||
return { |
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.
This change wasn't required, but I thought it would be nice to
- combine the code fragments to avoid duplication
- test
filterQuery
instead of recomputingnumSelectedFilters(filters)
, sincefilterQuery
is the value actually used in the code block
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.
👍🏽
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.
See below:
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.
👍🏽
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.
looks good, thank you! 👍
Addresses
https://broadworkbench.atlassian.net/browse/DT-781
Users would like filters in the DUOS UI to combine as a set intersection operation, so an item is only shown if it's valid for all selected filters.
Summary
By changing the root node of filter query to use
must
instead ofshould
, the filters are combined using logicalAND
instead ofOR
.Video of feature in action
filter.mov