-
Notifications
You must be signed in to change notification settings - Fork 39
refactor(atomic): migrate atomic-tab to Lit #6603
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
Conversation
Co-authored-by: alexprudhomme <[email protected]>
|
@copilot Retry this please ! |
Co-authored-by: alexprudhomme <[email protected]>
Co-authored-by: alexprudhomme <[email protected]>
✅ Migration complete! The Completed:
Notes:
The component is ready for use! |
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 successfully migrates the atomic-tab component from Stencil to Lit, moving it from tabs/atomic-tab/atomic-tab.tsx to atomic-tab/atomic-tab.ts. The component is a simple light DOM data container used exclusively as a child of atomic-tab-manager to define tab configurations (label, name, and expression).
Key Changes:
- Migrated component implementation to Lit using
LightDomMixin - Added property validation with
ValidatePropsControllerfor requiredlabelandnameprops - Created comprehensive unit tests (8 tests) and Storybook documentation
- Updated all package exports (index.ts, lazy-index.ts) and React wrapper
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/atomic/src/components/search/tabs/atomic-tab/atomic-tab.tsx | Removed Stencil implementation (file deleted) |
| packages/atomic/src/components/search/atomic-tab/atomic-tab.ts | New Lit implementation with property validation and light DOM rendering |
| packages/atomic/src/components/search/atomic-tab/atomic-tab.spec.ts | Unit tests covering properties, reflection, and slots (missing validation error tests) |
| packages/atomic/src/components/search/atomic-tab/atomic-tab.new.stories.tsx | Storybook stories showing component within tab-manager context |
| packages/atomic/src/components/search/atomic-tab/atomic-tab.mdx | Documentation with usage examples and parent component reference |
| packages/atomic/src/components/search/index.ts | Added AtomicTab export for public API |
| packages/atomic/src/components/search/lazy-index.ts | Added lazy-loading entry for atomic-tab |
| packages/atomic/src/components.d.ts | Removed Stencil-generated type definitions |
| packages/atomic-react/src/components/search/components.ts | Added React wrapper component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/atomic/src/components/search/atomic-tab/atomic-tab.mdx
Outdated
Show resolved
Hide resolved
…/ui-kit into copilot/migrate-atomic-tab
✅ Checklist
.mdxfileindex.tsandlazy-index.tsfiles. (N/A - child component used via HTML)https://coveord.atlassian.net/browse/KIT-5026
Description
Successfully migrated the
atomic-tabcomponent from Stencil to Lit. The component is a simple light DOM data container used exclusively as a child ofatomic-tab-manager.Changes Made
Component Migration
atomic-tab.tsusing Lit withLightDomMixinlabel,name,expressionerrorstate property for validationstencil-atomic-tab.tsxTesting
atomic-tab.spec.ts) - 8 tests covering rendering, property reflection, slot content, and property updatesatomic-tab-managerE2E testsDocumentation
atomic-tab.mdxwith usage examplesatomic-tab-managerfor contextDesign Decisions
atomic-tabis documented throughatomic-tab-managerstoriesatomic-tab-managervia HTML (similar to other nested components likeatomic-result-icon)Testing
The component maintains full backward compatibility and is ready for use.
Fixes #6375
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.