-
Notifications
You must be signed in to change notification settings - Fork 7
Fix allowCreate for Select #684
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
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 fixes the allowCreate
functionality for the Select component, specifically addressing error handling when creating new options fails. The changes improve the user experience by properly restoring the previous state and displaying a meaningful error message when option creation fails.
Key changes:
- Removed complex materialization logic that was duplicating option creation functionality
- Added proper error handling with state restoration and user-friendly error messages
- Simplified the option creation flow to handle failures more gracefully
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/form/select/Select.ts | Removes duplicate option creation logic, simplifies flow, and adds error handling with state restoration |
src/locales/*.ts | Adds new error message for failed option creation in multiple languages |
xliff/*.xlf | Updates translation files with new error message entry |
src/form/select/Select.ts
Outdated
@@ -943,6 +880,7 @@ export class Select<T extends SelectOption> extends FieldElement { | |||
} | |||
|
|||
public handleOptionSelection(event: CustomEvent) { | |||
const previous = [...this.values]; |
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.
This change lacks test coverage. According to the coding guidelines, 100% code coverage is expected for all touched code. Tests should verify that the previous state is properly restored when option creation fails.
Copilot generated this review using guidance from repository custom instructions.
src/form/select/Select.ts
Outdated
this.values = previous; | ||
this.errors = [msg('There was a problem creating your selection.')]; |
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.
This error handling logic lacks test coverage. According to the coding guidelines, 100% code coverage is expected for all touched code. Tests should verify that errors are properly set and the previous values are restored when the POST request fails.
Copilot generated this review using guidance from repository custom instructions.
📊 Coverage Report
Summary
|
No description provided.