-
-
Notifications
You must be signed in to change notification settings - Fork 369
[combobox][autocomplete] Remove aria-readonly prop from Clear and Popup components when readOnly #3907
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
commit: |
Bundle size report
Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Greptile OverviewGreptile SummaryRemoved Key changes:
Note: The Confidence Score: 5/5
Important Files Changed
|
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.
2 files reviewed, 1 comment
| @@ -90,7 +90,6 @@ export const ComboboxClear = React.forwardRef(function ComboboxClear( | |||
| { | |||
| tabIndex: -1, | |||
| children: 'x', | |||
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.
For consistency with this change, consider also removing aria-readonly from ComboboxChipRemove component (line 57 in packages/react/src/combobox/chip-remove/ComboboxChipRemove.tsx) and updating its tests. The ComboboxChipRemove component is also a button that should not have aria-readonly for the same reasons as Clear and Trigger buttons.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react/src/combobox/clear/ComboboxClear.tsx
Line: 92:92
Comment:
For consistency with this change, consider also removing `aria-readonly` from `ComboboxChipRemove` component (line 57 in `packages/react/src/combobox/chip-remove/ComboboxChipRemove.tsx`) and updating its tests. The `ComboboxChipRemove` component is also a button that should not have `aria-readonly` for the same reasons as Clear and Trigger buttons.
How can I resolve this? If you propose a fix, please make it concise.|
Failing test:
|
|
Thanks for the fix @markocupic024! |
Closes #3906
readOnly has no effect these buttons