Validations for select component.#2510
Conversation
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces form validation logic for the ix-select component, including handling for the required attribute, form submission events, and visual error states. It also adds a suite of component tests covering various validation scenarios. The review identified several technical issues: potential memory leaks due to improper event listener cleanup and anonymous functions in connectedCallback, incorrect validity reporting to formInternals which could bypass browser validation, and the need to use standard HTMLFormElement.noValidate properties instead of manual attribute checks.
khathija-ahamadi
changed the title
|
| if (this.required && !this.hasValue()) { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| this.hostElement.focus(); |
There was a problem hiding this comment.
Calling focus here introduces potential UX issues if there is more than one invalid element during form submission. Also in invalid listener in l377
| await expect(select).toHaveClass(/ix-invalid--required/); | ||
|
|
||
| const tabIndex = await select.getAttribute('tabindex'); | ||
| expect(tabIndex).toBe('0'); |
There was a problem hiding this comment.
Why do we check tab index here?
2 participants
💡 What is the current behavior?
There are some errors which prevent validations from happening as expected.
GitHub Issue Number: #1799
JIRA: IX-2595
🆕 What is the new behavior?
The acceptance criteria for this ticket has been met for select component:
Required input:
Invalid input > Removing value with keyboard > Stays invalid
Invalid input > Remove touched state (e.g. clear button) > Valid again
Invalid input > Programmatically setting it to empty > Stays invalid
Valid input > Removing value with keyboard > It is invalid
Valid input > Remove touched state (e.g. autoclear button) > Valid
Valid input > Programmatically setting it to empty > It is invalid
Not required input:
Invalid input > Removing value with keyboard > Valid
Invalid input > Remove touched state (e.g. clear button) > Valid again
Invalid input > Programmatically setting it to empty > Valid
Valid input > Removing value with keyboard > Valid
Valid input > Remove touched state (e.g. clear button) > Valid
Valid input > Programmatically setting it to empty > Valid
Also form 'novalidate' cases is handled.
🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support