milestone 23: amend two rulings the s2 review found the code in breach of
This commit is contained in:
+34
-2
@@ -452,6 +452,35 @@ so a select labelled "Type" showing "A" has the accessible name "A Type", and
|
||||
listbox. RAC also renders a hidden native `<select>` for form submission; it sits
|
||||
inside an `aria-hidden` container, so role and label queries skip it.
|
||||
|
||||
### The RAC anti-requirement forbids parts of a component it requires
|
||||
|
||||
`Select.tsx` imports RAC `Button`, `Popover`, `Label`, `ListBox`, `ListBoxItem`
|
||||
and `SelectValue`. The anti-requirement below names `Button` and `Popover`
|
||||
among the components RAC must not bring. Those two are not adopted components
|
||||
here — RAC composes a Select out of exactly these parts, and there is no RAC
|
||||
Select without them. The anti-requirement is amended: it forbids adopting a RAC
|
||||
component as an app-wide primitive, and the internal parts of the four adopted
|
||||
components are permitted where no file outside their wrapper imports them.
|
||||
That last clause is the check that keeps the scope honest, and it holds:
|
||||
`react-aria-components` is imported only by the four `ui/` wrappers. A RAC
|
||||
`Button` in a feature file would still be a breach.
|
||||
|
||||
### The focus-ring floor does not reach a listbox option
|
||||
|
||||
`Select.tsx` gives the trigger the shared focus ring, but the popover's options
|
||||
show keyboard focus as a background inversion — `primary` behind `primaryText`
|
||||
— with the outline removed, and the listbox itself has no ring. That is the
|
||||
selection-highlight convention for a listbox and a strong-contrast indicator,
|
||||
but it is not ruling 3's 2px focus-token outline, and the code is in breach as
|
||||
the ruling is written. Two things make the ring the wrong mechanism here rather
|
||||
than a missing one: an option inside a scrolling popover clips its own outline,
|
||||
and RAC drives option focus through its `isFocused` render prop, so a
|
||||
`:focus-visible` rule — which is how the shared `focusRing` is written — may
|
||||
never match the element at all. The floor is amended to cover controls that
|
||||
take DOM focus; a listbox option instead requires a focus indicator with a
|
||||
contrast change against both schemes, which the inversion provides. Every other
|
||||
interactive element, the trigger included, keeps the ring.
|
||||
|
||||
### Two notes for S3
|
||||
|
||||
`textMuted` resolves to zinc-400 in dark mode. Several current `text-zinc-500`
|
||||
@@ -467,8 +496,11 @@ and exits 0. It should not be allowed to become the accepted baseline.
|
||||
- No router or query changes: TanStack Router and TanStack Query stay,
|
||||
untouched. That decision is closed.
|
||||
- No RAC beyond Dialog, AlertDialog, Tabs, Select — no RAC Button, TextField,
|
||||
Table, Menu, Popover, DatePicker. Native elements styled with StyleX are
|
||||
the end state.
|
||||
Table, Menu, DatePicker adopted as an app primitive. Native elements styled
|
||||
with StyleX are the end state. The internal parts of the four adopted
|
||||
components (Button, Popover, Label, ListBox, ListBoxItem, SelectValue inside
|
||||
Select) are permitted, and only inside their own wrapper file — see the
|
||||
recorded amendment above.
|
||||
- No runtime style injection (`runtimeInjection: false` stays), no CSS-in-JS
|
||||
at runtime, no styled-components-like patterns.
|
||||
- No dynamic class-name assembly, no `clsx`/`classnames` dependency, no
|
||||
|
||||
Reference in New Issue
Block a user