-
Notifications
You must be signed in to change notification settings - Fork 9
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
Create Facets component #360
Conversation
by the search. If any hierarchical facets are returned, they are rendered as standard facets. J=BACK-2220 TEST=manual,auto Manually tested this component on the test site and confirmed it rendered all types of facets. Also added a storybook page and automated tests and confirmed they passed.
Current unit coverage is 89.07922912205568% |
by the search. If any hierarchical facets are returned, they are rendered as standard facets. J=BACK-2220 TEST=manual,auto Manually tested this component on the test site and confirmed it rendered all types of facets. Also added a storybook page and automated tests and confirmed they passed.
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.
LGTM, just two minor things that mostly are just questions
@@ -0,0 +1,5 @@ | |||
import { DisplayableFacetOption } from '@yext/search-headless-react'; | |||
|
|||
export function getOptionLabelText(option: DisplayableFacetOption) { |
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.
can you add a an example of what this is trying to match?
<StaticFilters | ||
fieldId='c_hierarchicalFacet' | ||
title='Static Hierarchical Facets' | ||
filterOptions={hierarchicalFilterConfigs} | ||
/> | ||
<FilterDivider /> |
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.
why is this only wrapping StaticFilters and br tag is used on line 73?
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.
These aren't wrapping StaticFilters and are instead standalone filter dividers. The facets dynamically include dividers where needed, but static filters do not
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.
Lgtm as well, just another minor comment
* | ||
* @remarks | ||
* This component is a quick way of getting facets on the page, and it will render standard facets, | ||
* numerical facets, and hierarchical facets. The {@link StandardFacets}, {@link NumericalFacets}, |
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.
it won't currently use hierarchical facets right?
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.
It will render them but as standard facets
# Version 1.3.0 ### Features - Introduce new Facet components including the new Facets (#360), StandardFacet (#370), NumericalFacet (#375), and HierarchicalFacet (#373) components which support more customizability than the previous Facet components. - Support the new RichTextV2 (Lexical) and Markdown fields which have been converted to HTML as FeaturedSnippetDirectAnswers (#379) - Remove the Yext logo from the search bar (#376) - Deprecate LocationBias (#338) component in favor of the new Geolocation component (#336) ### Bug fixes - Improve the UX of the search bar when long autocomplete text is returned which includes the truncation of long text (#378)
Create a facets component which renders any facets returned by the search. If any hierarchical facets are returned, they are rendered as standard facets.
J=BACK-2220
TEST=manual, auto
Manually tested this component on the test site and confirmed it rendered all types of facets. Also added a storybook page and automated tests and confirmed they passed.