fix(select): prevent default browser error UI from appearing #3984
Conversation
🦋 Changeset detectedLatest commit: 5bf7332 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@chirokas is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces patches for the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@wingkwong please review |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
packages/components/select/__tests__/select.test.tsx(1 hunks)packages/components/select/src/hidden-select.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/select/src/hidden-select.tsx
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts (1)
74-75: Document validation timing for maintainability.Consider adding a comment explaining when validation is committed and why, as this is a crucial part of the component's behavior.
} + // Commit validation after selection changes to ensure internal validation state is updated validationState.commitValidation(); },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
.changeset/quick-buses-kick.md(1 hunks)packages/components/select/__tests__/select.test.tsx(1 hunks)packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/quick-buses-kick.md
- packages/components/select/tests/select.test.tsx
🔇 Additional comments (1)
packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts (1)
74-75: LGTM! Verify validation behavior in edge cases.
The addition of validationState.commitValidation() after selection changes aligns with the PR objective of preventing default browser error UI. This ensures validation state is managed internally.
Let's verify the validation behavior in related components:
✅ Verification successful
Let me gather more context about the validation behavior and its integration.
Validation behavior is correctly implemented and consistent
The implementation of validationState.commitValidation() in the multiselect component is appropriate and aligns with the codebase's validation patterns:
- It uses the standard
useFormValidationStatehook with "native" validation behavior - The validation state is properly tied to the selected keys
- The commit timing (after selection changes) matches the expected behavior
- The validation state is correctly exposed through the component's API (
isInvalid,validationErrors,validationDetails)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other validation commit calls to ensure consistent behavior
rg "commitValidation" --type ts
# Look for other validation state management
ast-grep --pattern 'useFormValidationState({$$$})'
Length of output: 896
Script:
#!/bin/bash
# Search for the useFormValidationState hook implementation
rg "useFormValidationState" -A 5 --type ts
# Look for other validation-related code in the multiselect component
rg "validation" packages/hooks/use-aria-multiselect --type ts
# Check for any validation-related tests
fd "test|spec" --type f --exec rg "validation" {} \;
Length of output: 5034
|
@wingkwong Select doesn't support the #3913 (comment) |
|
Opening a new one - #4281 |
Closes #3913
📝 Description
See #3913 (comment)
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
Bug Fixes
onChangehandler to properly update the selected keys in the state.Tests