Skip to content

FilterSearch updates for builtin.location #343

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

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

nmanu1
Copy link
Contributor

@nmanu1 nmanu1 commented Dec 5, 2022

Update FilterSearch to account for special backend logic when searching on the builtin.location field. In this case, the backend can return filters on builtin.region or address.countryCode as well, so these fieldIds are also considered "matching" for the component. Filters on these fields can be used to populate the input and should be unselected when a new filter is selected from FilterSearch.

J=SLAP-2495
TEST=auto, manual

See that the added Jest tests pass. Spin up the test-site and add static filters on builtin.region. See that they are used to populate the FilterSearch input, warnings are logged, and the filters are unselected by FilterSearch as expected.

@nmanu1 nmanu1 requested a review from a team as a code owner December 5, 2022 19:29
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 85.72% when pulling 5a135cd on dev/filtersearch-location into f364882 on release/v1.1.0.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

Current unit coverage is 88.8646288209607%
Current visual coverage is 77.78894472361809%
Current combined coverage is 89.51965065502183%

Copy link
Contributor

@oshi97 oshi97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if we'd want to put this weird business logic into headless, since this is the kind of weird business logic that's easy to forget about, but at least right now I'm pretty sure we wouldn't want to do that.

@nmanu1
Copy link
Contributor Author

nmanu1 commented Dec 5, 2022

I was wondering if we'd want to put this weird business logic into headless, since this is the kind of weird business logic that's easy to forget about, but at least right now I'm pretty sure we wouldn't want to do that.

Yeah, I didn't think we'd want to bake this into headless because it seems to mostly affect the UI for now and it sounds like Product is thinking of making a separate endpoint for this case anyways. I'm not a big fan of having this kind of weird logic in search-ui-react either, but I'm hoping we'll be able to remove it soon and having it higher in the stack will probably make it easier to isolate/remove

@nmanu1 nmanu1 merged commit fedd93e into release/v1.1.0 Dec 5, 2022
@nmanu1 nmanu1 deleted the dev/filtersearch-location branch December 5, 2022 21:01
@tmeyer2115 tmeyer2115 mentioned this pull request Dec 15, 2022
tmeyer2115 added a commit that referenced this pull request Dec 15, 2022
### 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants