-
Notifications
You must be signed in to change notification settings - Fork 40
refactor(atomic): migrate FacetSegmentedValue functional component to Lit #6545
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: louis-bompart <[email protected]>
louis-bompart
left a comment
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.
@copilot some work on the ut, make them more "stable" & reliable.
Do consider also that this is a functionnal component. We Unit Test those: if you need more mocks, go ahead.
...ages/atomic/src/components/search/facets/facet-segmented-value/facet-segmented-value.spec.ts
Outdated
Show resolved
Hide resolved
...ages/atomic/src/components/search/facets/facet-segmented-value/facet-segmented-value.spec.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: louis-bompart <[email protected]>
Done! I've improved the tests to be more stable and reliable:
All tests now have specific, predictable expectations. Commit: 7d87568 |
…/github.com/coveo/ui-kit into copilot/migrate-facet-segmented-value-again
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.
Pull request overview
This PR migrates the FacetSegmentedValue functional component from Stencil to Lit. The component renders segmented facet values with a button, label, and compact count formatting used by atomic-segmented-facet.
Key changes:
- New Lit implementation using
FunctionalComponent<FacetValuePropsBase>type signature - Applies
keyeddirective for list stability and usesrenderButtoninstead of Stencil'sButton - Comprehensive test suite with 21 test cases covering rendering, props, ARIA attributes, and styling
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
stencil-facet-segmented-value.tsx |
Renamed original Stencil implementation with deprecation notice for backward compatibility |
facet-segmented-value.ts |
New Lit functional component replacing Stencil version with equivalent functionality |
facet-segmented-value.spec.ts |
New unit tests for Lit component covering rendering, accessibility, and interaction scenarios |
atomic-segmented-facet.tsx |
Updated import to use prefixed Stencil version maintaining backward compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
packages/atomic/src/components/search/facets/facet-segmented-value/facet-segmented-value.ts
Show resolved
Hide resolved
...ages/atomic/src/components/search/facets/facet-segmented-value/facet-segmented-value.spec.ts
Show resolved
Hide resolved
...ages/atomic/src/components/search/facets/facet-segmented-value/facet-segmented-value.spec.ts
Show resolved
Hide resolved
packages/atomic/src/components/search/facets/facet-segmented-value/facet-segmented-value.ts
Outdated
Show resolved
Hide resolved
fbeaudoincoveo
left a comment
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.
GG!
Description
Migrates the
FacetSegmentedValuefunctional component from Stencil to Lit. This is a rendering function used byatomic-segmented-facet, not a standalone web component.Changes
Lit Implementation (
facet-segmented-value.ts)FunctionalComponent<FacetValuePropsBase>type signatureButtonwith Lit'srenderButtonkeyeddirective for list item stabilityBackward Compatibility
stencil-facet-segmented-value.tsxatomic-segmented-facet.tsximport to use prefixed versionUnit Tests (
facet-segmented-value.spec.ts)renderFunctionFixturepattern consistent with other functional component testscreateTestI18n()1234→(1.2K),987654321→(988M))✅ Checklist
.mdxfile (N/A - functional component)index.tsandlazy-index.tsfiles (N/A - internal functional component)https://coveord.atlassian.net/browse/KIT-4956
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.