change filter from is to is one of #3529
Conversation
| const newFilters = [] | ||
| values.forEach(element => { | ||
| const filter = { | ||
| match_phrase: { | ||
| [element.value]: element.label | ||
| } | ||
| } | ||
| newFilters.push(filter); | ||
| }); |
There was a problem hiding this comment.
nitpick: You could use Array.prototype.map instead of Array.prototype.forEach + Array.prototype.push
const newFilters = values.map(element => ({
match: {
[element.value]: {
query: element.label,
type: 'phrase'
}
}
});The match_phrase gives problems with some app logic. Use:
match: {
[element.value]: {
query: element.label,
type: 'phrase'
}
}
| filters.forEach(item => { | ||
| item.params.forEach(element => { | ||
| filterCustom.push({ label: element, value: item.key }) | ||
| }) |
There was a problem hiding this comment.
nitpick: We could use Array.prototype.map instead of Array.protype.forEach + Array.prototype.push
| useEffect(() => { | ||
| let filterSubscriber = filterManager.getUpdates$().subscribe(() => { | ||
| onFiltersUpdated(); | ||
| return () => { | ||
| filterSubscriber.unsubscribe(); | ||
| }; | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
nitpick: The returned function to unsubscribe to the changes on the filterManager should be the returned on the useEffect callback, not the subscription function.
suggestion: You could use the useFilterManager hook instead of use the filterManager.getUpdates$().subscribe and listen to the chagens of filters variable returned by the mentioned hook to apply the effect.
|
|
||
| const getComponent = (item: any) => { | ||
| var types = { |
There was a problem hiding this comment.
nitpick: You could use const instead of var here. Anyways, we prefer let over var in the variable declaration.
|
issue: We should be able to remove the filters applied to the selectors. The button to remove the filter is hidden with CSS |
| const prevValues = usePrevious(values); | ||
|
|
||
| useEffect(() => { | ||
| if (prevValues != values) { |
There was a problem hiding this comment.
Why check for differences when values is already a dependence?
Hi team, this resolve:
closes #3521