-
Notifications
You must be signed in to change notification settings - Fork 0
Review 4689
#4689 · AKnassa · aged-PR cleanup Step 3, batch 3, 2026-08-23
Verdict: changes requested
The trigger carried aria-required, which ARIA does not allow on role=button — a critical axe aria-allowed-attr baseline entry.
role="combobox" on the trigger button, following DateInput/DateTimeInput; test queries move to the combobox role; one baseline entry removed.
no API change (an AT-visible role change; the changeset calls out that getByRole('button') queries must move)
no new theme targets
API no (no TS signature moves; consumer tests querying the trigger by button role break, and the changeset says so) · Visual no · Theme no
DateRangeInput.tsx:692 carries aria-required on a <button> on main and the baseline entry is still there. Read the whole trigger JSX: no onKeyDown anywhere on it. DateInput.tsx:663-670 implements ArrowDown and Alt+ArrowDown with an explicit APG citation in the comment, via popover.show({skipAutoFocus: true}) — a real signature (usePopover.tsx:239).
request changes. [BLOCKS] the role now promises a keyboard affordance the trigger does not implement → a screen-reader user hears 'combobox, collapsed', presses Down Arrow, and nothing happens, where the same press on the sibling DateInput opens the picker. Enter and Space still work, so this is incompleteness, not a break — but it is the half of the pattern the new announcement advertises.
Asked one real question rather than prescribing: the popup here is a dialog, not a listbox, so should ArrowDown move focus into the calendar (what handleToggle does) or hold it on the trigger the way DateInput does?
See the review on #4689. Attribution: [Reviewed by Robohands].
(posted directly by the loop under the aged-PR brief; nothing edited)