-
Notifications
You must be signed in to change notification settings - Fork 39
refactor(atomic): refactor SmartSnippetFeedbackModal functional components #6600
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
…tional components Co-authored-by: alexprudhomme <[email protected]>
Co-authored-by: alexprudhomme <[email protected]>
...mmon/smart-snippets/atomic-smart-snippet-feedback-modal/smart-snippet-feedback-modal.spec.ts
Fixed
Show fixed
Hide fixed
...ts/common/smart-snippets/atomic-smart-snippet-feedback-modal/smart-snippet-feedback-modal.ts
Outdated
Show resolved
Hide resolved
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 six SmartSnippet Feedback Modal functional components from Stencil to Lit. The migration follows the established pattern of creating new Lit versions with a render* naming convention while preserving the original Stencil versions with a stencil- prefix for backward compatibility. Comprehensive unit tests were added for all migrated components.
Key Changes
- Created Lit functional components: All six functional components (Header, Body, Options, Option, Details, Footer) were migrated to Lit with proper TypeScript types and reactive directives
- Preserved Stencil versions: Original components were renamed with
stencil-prefix to maintain compatibility with existing parent components - Added comprehensive tests: Each functional component has thorough unit test coverage following Vitest conventions
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
stencil-smart-snippet-feedback-modal-common.tsx |
Renamed original Stencil file with deprecation notices |
modal-header.ts |
New Lit functional component for modal header |
modal-body.ts |
New Lit functional component for form body wrapper |
modal-options.ts |
New Lit functional component for options fieldset |
modal-option.ts |
New Lit functional component for individual radio option |
modal-details.ts |
New Lit functional component for details textarea (has issues) |
modal-footer.ts |
New Lit functional component for footer with buttons |
feedback-options.ts |
Extracted feedback options data structure |
*.spec.ts (7 files) |
Comprehensive unit tests for all components |
atomic-smart-snippet-feedback-modal.tsx |
Updated import path to renamed Stencil file |
atomic-insight-smart-snippet-feedback-modal.tsx |
Updated import path to renamed Stencil file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name="answer-details" | ||
| ${detailsInputRef ? ref(detailsInputRef) : ''} | ||
| class="border-neutral mt-2 w-full resize-none rounded border p-2 text-base leading-5" | ||
| rows="4" |
Copilot
AI
Nov 20, 2025
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.
The rows attribute should be a number, not a string. Use rows=${4} instead of rows="4" to maintain type consistency with Lit's property binding.
| rows="4" | |
| rows=${4} |
| <textarea | ||
| part="details-input" | ||
| name="answer-details" | ||
| ${detailsInputRef ? ref(detailsInputRef) : ''} |
Copilot
AI
Nov 20, 2025
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.
The conditional ref directive pattern ${detailsInputRef ? ref(detailsInputRef) : ''} is problematic. When detailsInputRef is undefined, this renders an empty string in the template, which can cause rendering issues. Use ${detailsInputRef ? ref(detailsInputRef) : nothing} instead, importing nothing from lit.
Migration Plan: SmartSnippet Feedback Modal Functional Components
This is a Functional Component Migration (not a custom element migration).
Components to Migrate
Located in
/packages/atomic/src/components/common/smart-snippets/atomic-smart-snippet-feedback-modal/:SmartSnippetFeedbackModalHeader- Renders feedback modal headerSmartSnippetFeedbackModalBody- Renders form body wrapperSmartSnippetFeebackModalOptions- Renders fieldset for feedback options (note: typo in name)SmartSnippetFeedbackModalOption- Renders individual radio button optionSmartSnippetFeedbackModalDetails- Renders textarea for additional detailsSmartSnippetFeedbackModalFooter- Renders footer with cancel/submit buttonsMigration Steps
smart-snippet-feedback-modal.ts)stencil-prefix (stencil-smart-snippet-feedback-modal-common.tsx)Parent Components (Stencil - not migrating)
These components now import from the renamed stencil file:
atomic-smart-snippet-feedback-modal(search)atomic-insight-smart-snippet-feedback-modal(insight)Dependencies
renderButton- Lit version used in new componentsOriginal 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.