[combobox][autocomplete][select] Fix listbox separator semantics - #5399
Conversation
commit: |
Bundle size
PerformanceTotal duration: 1,159.50 ms ▼-325.25 ms(-21.9%) | Renders: 78 (+0) | Paint: 1,820.06 ms ▼-497.54 ms(-21.5%)
…and 4 more (+6 within noise) — details 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. |
1b344c9 to
80f2c77
Compare
80f2c77 to
4b0fc02
Compare
PR reviewNothing is merge-blocking: the semantic fix is valid, Simplifications (1)1. 🟡 Share the listbox separator implementationLocation: export const SelectSeparator = React.forwardRef(function SelectSeparator(
componentProps: SelectSeparator.Props,
forwardedRef: React.ForwardedRef<HTMLDivElement>,
) {
Failure scenario: Each listbox primitive ships and maintains a duplicate component body, adding bundle bytes and allowing the implementations to drift even though their behavior is identical. Fix: Extract one internal Docs (1)1. 🟡 Make the shared separator description neutralLocation: /**
* A visual separator between combobox items.
* Renders a `<div>` element.
*/Autocomplete reuses Failure scenario: A reader of the Autocomplete API sees Combobox-specific terminology and may reasonably wonder whether the documented component or generated API mapping is incorrect. Fix: Use a neutral shared description such as “A visual separator between items.” VerdictApprove after nits - the accessibility approach is correct; only the duplicated runtime implementation and shared wording should be cleaned up before merging. 🤖 Review generated with Codex |
4b0fc02 to
a3f8d1d
Compare
a3f8d1d to
d956016
Compare
|
@atomiks Thanks for the review. I extracted the shared listbox separator implementation into |
atomiks
left a comment
There was a problem hiding this comment.
Thanks for the fix @sarthakmalik0810
Select.Separator,Combobox.Separator, andAutocomplete.Separatorcurrently reuseSeparator, which exposesrole="separator"when rendered inside a listbox. This is flagged by axe DevTools and ARC Toolkit.Changes
role="presentation".Select,Combobox, andAutocomplete.orientation, anddata-orientation.Fixes #5398