-
Notifications
You must be signed in to change notification settings - Fork 841
Bubble validate state change for textield #608
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
…ging on-the-wire a2UI JSON and actions
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… into md-componentslistagent
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.
Code Review
This pull request introduces a valuable feature by dispatching a custom event, a2ui-validation-input, from the TextField component to communicate its validation status. This is an effective solution for propagating state across Shadow DOM boundaries. The implementation is clean and well-tested within the component gallery.
I've identified a couple of areas for improvement: one is a minor code-safety enhancement in the event handler, and the other is a schema duplication issue in the v0.9 specification. Additionally, there's a small typo in the pull request title ("textield" should be "textfield").
| "type": "string", | ||
| "description": "The display style of the component.", | ||
| "enum": [ | ||
| "checkbox", | ||
| "chips" | ||
| ] |
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.
There appears to be a duplicated property in the ChoicePicker component definition. Both displayStyle (lines 739-746) and type (lines 747-752) are defined with the same description ("The display style of the component.") and enum values. This seems to be an error and could lead to confusion in the schema. Please remove the duplicated type property to maintain a clean and unambiguous API specification.
| #handleValidationInput = (e: Event) => { | ||
| const detail = (e as CustomEvent).detail; |
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 type assertion (e as CustomEvent) is unsafe. If an event with the same name but a different type is dispatched, this could lead to runtime errors. It's safer to use a type guard with instanceof to ensure the event is a CustomEvent before accessing its detail property.
#handleValidationInput = (e: Event) => {
if (!(e instanceof CustomEvent)) {
return;
}
const detail = e.detail;
This PR introduces the a2ui-validation-input event to the
TextField
component to bubble validation state changes.
Changes
TextField (
text-field.ts
):
Added dispatch of a2ui-validation-input on every input event.
Payload: { componentId, value, valid: boolean }.
Reason: Native invalid events do not cross the Shadow DOM boundary. This custom event allows parent components (like the Shell) to observe real-time validation status.
Component Gallery (
component-gallery.ts
):
Added a listener for a2ui-validation-input at the document root.
Logs validation events to both the Console and the internal Debug Panel for visual verification.
Verification
Verified manually in the Component Gallery:
Typing in "TextField (Regex)" now triggers logs in the Debug Panel.
Valid/Invalid states strictly match the regex rules.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.