Review 5344

Review 5344 — in-listbox dividers as presentation

#5344 · gonzoblasco · community Head reviewed 5636b22f252a9c9ea2564051d33f17227ab5298f Verdict declined — closed in favour of #5107, merged 405f1eed A/B final vibe test; B won — and this review produced R6d.

Problem

role="listbox" permits only option/group children, but both selectors push a Divider (role="separator") into the popup for every {type:'divider'} option. Measured on main in Chromium, axe-core 4.12.1: aria-required-children critical ×1. The section-heading and empty-state cases were already fixed on main, so #4994 overstated what was left.

Solution

Divider destructures role (default 'separator') and renders it instead of hardcoding — the prop was already typed via BaseProps and silently dropped, because role="separator" sat after {...props}. Both selectors pass role="presentation". The search-row divider, outside the listbox, correctly untouched.

API

~ Divider.role?: React.AriaRole = 'separator'   (public via BaseProps; now HONORED,
                                                 previously spread then overridden.
                                                 NOT declared on DividerProps,
                                                 absent from Divider.doc.mjs)

Breaking · Theme · Performance

API no compile break, one silent behavior change (a call site passing role had it dropped, now applied; no in-repo call site does) · Visual no, frames identical · Theme no · Performance nothing.

Judgement

1. [BLOCKS] Divider writes aria-orientation unconditionally, and
   presentation does not allow it
   → axe trades one critical for another on the same <div>:
     aria-required-children becomes aria-allowed-attr, critical ×2   · Divider.tsx:171
2. [not blocking] role is honored but never declared                 · Divider.tsx:153
3. [not blocking] no story opens a listbox containing a divider      · MultiSelector.stories.tsx:590

The set, measured on one axis (axe-core 4.12.1, scoped to the open listbox):

candidate	result
main — do nothing	aria-required-children critical ×1
#5344 — role="presentation"	aria-allowed-attr critical ×2
#5107 — aria-hidden	clean
presentation without aria-orientation	clean

The PR makes the audit worse. #5107, open six days longer, fixes the same two lines without touching a shared component. Both arms found the blocking finding independently; only the arm with the radius table found the competing PR.

What it cost us to learn

The loop reviewed this PR in full, concluded "land #5107 instead", and had measured #5107 only as a foil inside this review — nobody had reviewed it, and it had sat six days with zero human reviews. → R6d: competing PRs are ONE review. Hold every judgement in the set until the last is reviewed, then post them all at once, winner first. On detecting a competitor, promote it.

Order actually followed: #5107 reviewed on its own merits, approved, merged — then this one declined and closed, naming the merged commit.