feat(advanced search): adding advanced search filter component & prereqs for it #6055
Conversation
| values: [String!] | ||
|
|
||
| """ | ||
| Value of the field to filter by | ||
| """ | ||
| value: String! | ||
| """ | ||
| If the filter should or should not be matched | ||
| """ | ||
| negated: Boolean | ||
|
|
||
| """ | ||
| Condition for the values. How to If unset, assumed to be equality | ||
| """ | ||
| condition: SearchCondition |
There was a problem hiding this comment.
this pr reformats things, but this is the only actual change to this file ^ aside from formatting.
There was a problem hiding this comment.
Thanks for the fyi!
| values: [String!] | ||
|
|
||
| """ | ||
| Value of the field to filter by | ||
| """ | ||
| value: String! | ||
| """ | ||
| If the filter should or should not be matched | ||
| """ | ||
| negated: Boolean | ||
|
|
||
| """ | ||
| Condition for the values. How to If unset, assumed to be equality | ||
| """ | ||
| condition: SearchCondition |
There was a problem hiding this comment.
Thanks for the fyi!
| </CloseSpan> | ||
| </FieldFilterSection> | ||
| <ValueFilterSection> | ||
| {TEXT_FILTERS.indexOf(filter.field) === -1 && |
There was a problem hiding this comment.
can we factor out this TEXT_FILTERS.indexOf(filter.field) === -1 and rename it as a boolean variable? it's a little confusing to read, and we do use it multiple times. I imagine this just means that the filters don't contain that filter field
There was a problem hiding this comment.
also i think we should just use .includes()
There was a problem hiding this comment.
thanks for brining this up-- i realize we didn't need this logic at all
| </FieldFilterSection> | ||
| <ValueFilterSection> | ||
| {TEXT_FILTERS.indexOf(filter.field) === -1 && | ||
| filter?.values?.map((value) => { |
There was a problem hiding this comment.
can this also be moved to not be inline and given a more descriptive name?
There was a problem hiding this comment.
i can make this its own component
| value: '', | ||
| values: values as string[], | ||
| condition: filter.condition, | ||
| negated: filter.negated || false, |
There was a problem hiding this comment.
won't this always be filter.negated? if it's optional you could just do filter?.negated
| ]; | ||
|
|
||
| function getLabelsForField(field: string) { | ||
| if (FIELDS_WHO_USE_CONTAINS_OPERATOR.indexOf(field) >= 0) { |
There was a problem hiding this comment.
can we just use .includes() here instead of indexOf? I think it makes the code a lot more confusing
| const entityRegistry = useEntityRegistry(); | ||
| const countText = aggregation.count === MAX_COUNT_VAL ? '10000+' : aggregation.count; | ||
| const countText = hideCount |
There was a problem hiding this comment.
thanks for refactoring this, it's a little cleaner!
| origin: 'Environment', | ||
| }; | ||
|
|
||
| export const FIELDS_WHO_USE_CONTAINS_OPERATOR = ['description', 'fieldDescriptions']; |
There was a problem hiding this comment.
Maybe FIELDS_THAT_USE_CONTAINS_OPERATOR?
| margin: 4px; | ||
| `; | ||
|
|
||
| export const AdvancedSearchFilterValuesSection = ({ facet, filter }: Props) => { |
There was a problem hiding this comment.
Thanks, I think this is much more readable!
This PR takes care of:
valuesin a single filterChecklist