Update default FilterSearch behavior #333
Conversation
|
Current unit coverage is 88.20861678004535% |
| @@ -123,14 +128,33 @@ export function FilterSearch({ | |||
| ); | |||
|
|
|||
| useEffect(() => { | |||
| if (matchingFilters.length > 1 && !onSelect) { | |||
There was a problem hiding this comment.
I feel like it may be beneficial to warn user regardless if onSelect is provided here?
There was a problem hiding this comment.
this was a deliberate decision because the thinking is that if you provide your own onSelect prop, you might be fine with having multiple filters selected in state at once and you wouldn't want this warning to to be logged. it's only assumed to be a problem for the default component behavior. people shouldn't have to fork the component just to remove this warning if they don't want clobbering behavior
src/components/FilterSearch.tsx
Outdated
| @@ -123,14 +128,33 @@ export function FilterSearch({ | |||
| ); | |||
|
|
|||
| useEffect(() => { | |||
| if (matchingFilters.length > 1 && !onSelect) { | |||
| console.warn('More than one selected static filter found that matches the filter search fields.' | |||
There was a problem hiding this comment.
nit: can we add to the message that we will use the first selected static filter found?
There was a problem hiding this comment.
what do you think about adding "Picking one filter to display in the input." instead? because technically, it will only pick the first filter it finds if the currentFilter is not selected in state anymore. or does that seem too vague?
Edit: updated to "Picking one filter to display in the input." and I also added the specific search fields to make it more clear in case there are multiple filter search components on the page
### Features - Default behavior of `FilterSearch` was changed to better support Locators and Doctor Finders. Additionally, a new `onSelect` prop was added to the Component. The `searchOnSelect` prop is now deprecated. (#323, #343, #333) - A new CSS bundle without the Tailwind resets is exported. (#322) - We've added a `MapboxMap` Component, powered by v2 of their JavaScript API. (#332) ### Changes - Assorted updates to improve our GH Actions. - Styling of Facet Headers is now exposed in `FilterGroupCssClasses`. (#321) ### Bug Fixes - Vulnerabilities were addressed for the repo and its test-site. - Fixed the Dropdown Component to invoke `preventDefault` only when it is active. (#307) - Corrected a small error in the generation of SSR Hydration IDs. (#315)
Update the default behavior of
FilterSearch:fieldIdthat matches with one of the search fields forFilterSearch. If there are multiple filters that were clobbered, log a console warning.onSelectprop passed in.Note: We only look for field value filters in State for the "matching" filters. We don't prescribe how compound filters should be handled when comparing one to a search field. In the UCSD use case, a developer would pass in a custom
onSelectfunction that would set compound static filters in State. In this case, thecurrentFiltermay not be part of thematchingFiltersarray, which is why the concept of acurrentFilteris still needed.J=SLAP-2432
TEST=auto, manual
See that the added Jest tests pass. Spin up the test-site and test that the above situations match the expected behavior with different combinations of filters in State and passing an
onSelectprop or not.