feat(search):ES-5892 Introduce displayName and displayKey fields to facets #2690
Conversation
🦋 Changeset detectedLatest commit: 44c9f28 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
chanceaclark
left a comment
chanceaclark
left a comment
There was a problem hiding this comment.
Hey @jfugalde thanks for this! In addition to the CI run, would you also add a changeset for this PR?
I tested that this works on another locale: https://catalyst-canary-git-es-5892-bigcommerce-platform.vercel.app/es/shop-all/?attr_Color=Verde
…for improved labeling
chanceaclark
left a comment
chanceaclark
left a comment
There was a problem hiding this comment.
Looks like the lint task is still failing. You might need to add our store 1002923827 to any feature flags that are driving this feature.
Additionally, could we add functional tests to our local functional test suite, under /core/tests/? This test suite is for merchant consumption for context.
| edges { | ||
| node { | ||
| __typename | ||
| name |
There was a problem hiding this comment.
Can we remove the name field from the request now?
| "@bigcommerce/catalyst-core": patch | ||
| --- | ||
|
|
||
| feat(search):ES-5892 Introduce displayName and displayKey fields to facets |
There was a problem hiding this comment.
This will be used as part of the changelog generation, so can you tweak this entire changeset to be changelog friendly (e.g. omit the JIRA key, more user friendly wording, etc...).
Also, as you can tell from the pull request description, we like to include a little bit of a migration guide which we also like to include in the changeset so the changelog has migration notes. This migration guide should be written from a perspective that you have an existing Catalyst repository and have made a ton of customizations, so how would you pull in new code to get this change. Would you mind updating the PR description with a better migration guide + include it within this changeset?
| @@ -0,0 +1,9 @@ | |||
| --- | |||
| "@bigcommerce/catalyst-core": patch | |||
There was a problem hiding this comment.
We should probably include this within our 1.4 release so let's change this to a minor.
| "@bigcommerce/catalyst-core": patch | |
| "@bigcommerce/catalyst-core": minor |
What/Why?
This pull request updates how facet filters are handled and displayed in the product search functionality. The main focus is on switching from using the
nameproperty since it's now a deprecated field and usedisplayNameproperty across both the GraphQL query and the data transformer logic, ensuring consistency and improved labeling in the UI. Additionally, new fields such asdisplayNameandfilterKeyare added to the GraphQL query for richer data support.Facet filter field improvements:
displayNamefield to all facet filter types (BrandSearchFilter,CategorySearchFilter,ProductAttributeSearchFilter,RatingSearchFilter, andPriceSearchFilter) in the GraphQL query infetch-faceted-search.tsfor more descriptive labels.Facet transformer logic updates:
facets-transformer.tsto usedisplayNameinstead ofnamefor matching and labeling facets, improving user-facing labels and consistency.filterKeyfor parameter naming and to preferdisplayNamefor labels, falling back tofilterNameifdisplayNameis not present.Testing
Functionality remained the same after the change
Migration
Revert this PR or consider using deprecated name field