From a4fcb0cee38e586aa5bb5ff24a8e8cdf6287d2b3 Mon Sep 17 00:00:00 2001 From: "AI assistant of @schickling" <261620128+schickling-assistant@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:59:50 +0200 Subject: [PATCH 1/2] fix(effect-schema-form-aria): resolve the pilot package's three drift findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three separate checks each found real drift in this package, none of them looking for it: the state-idiom review behind decision 0004, the StyleX lint rules the package's own patterns produced, and the first run of the visual and accessibility gate. All three were deferred on the same prerequisite — a working visual gate for this package — which the gate now supplies, so they are settled together rather than three times over. Raw colours become semantic tokens. `on-primary` is the foreground for anything drawn on a `primary` background; `shadow-raised` is the popover elevation and takes its default from the `shadows.lg` scale step rather than restating the rgba stack. Both defaults reference a named scale value, so the palette choice stays in one reviewable file. `primary` moves from `blue500` to `blue600`, and that is a legibility fact rather than a preference. The accessibility gate failed ten of this package's thirty-nine stories on exactly one thing: white body text on the selected segment measured 3.76:1 where AA requires 4.5:1. `blue600` measures 5.25:1. Renaming the literal to `on-primary` would have preserved the violation behind a nicer name. `accent` follows `primary` so the brand blue stays one colour. The thirteen deprecated top-level pseudo-class sites move into condition objects. This is the change the deferral was actually about, because nesting a pseudo-class changes which condition wins, so every site was checked for a competing state on the same property rather than translated mechanically. Eleven of the thirteen set a property nothing else touches and carry no precedence risk at all. The two that do compete — the segmented control's and the list option's background under hover versus selection — are resolved by application order instead: hover lives in the base style, selection is a later `stylex.props` argument, and it restates the hover value because a later unconditional `backgroundColor` does not replace an earlier `backgroundColor` under a `:hover` key. That is the R06 shape, and it means the outcome no longer depends on attribute conditions outranking pseudo-class ones. Focus moves from `:focus` to the accessible-component library's own focus-visible state where the element is one of its components, and to the native `:focus-visible` on the one plain `` that is not. A pointer click no longer paints a keyboard focus ring. Note what this is NOT: the ring is still drawn with `boxShadow` rather than converted to `outline`. The partitioning invariant already holds here — nothing else on these elements sets `outline*`, and the lint rule confirms it — and no story exercises a focused element, so repainting the ring would be a visual change the gate structurally cannot adjudicate. Recorded as follow-up rather than done blind. State resolution stops re-deriving what the component already knows. The checkbox derived its box style from its own `value` prop; selection lives on the CheckboxButton, which is an ancestor of the box, so the box cannot read it as one of its own conditions and React Aria's render prop is the sanctioned mechanism for that case. The segmented control and the list options branched between two mutually exclusive style objects, hover rule included; they now apply one additive override in argument order. The accessibility gate's eleventh failure was structural, not colour. React Aria's `Header` renders `
`, and a `
` outside sectioning content is a `banner` landmark, so two nested field groups produced two banners and tripped `landmark-no-duplicate-banner` and `landmark-unique`. The label is now a plain element and the group carries the accessible name it was missing. The seventeen per-line `oxlint-disable` suppressions are gone; the package is gated by all eight rules again. --- .../src/components/BooleanField.tsx | 54 +++++++++------- .../src/components/FieldGroup.tsx | 17 +++-- .../src/components/LiteralField.tsx | 64 +++++++------------ .../src/components/NumberField.tsx | 34 +++------- .../src/components/TextField.tsx | 12 +--- .../src/mod.unit.test.tsx | 12 ++-- .../src/tokens.stylex.ts | 14 +++- 7 files changed, 96 insertions(+), 111 deletions(-) diff --git a/packages/@overeng/effect-schema-form-aria/src/components/BooleanField.tsx b/packages/@overeng/effect-schema-form-aria/src/components/BooleanField.tsx index 83f7520c45..dd2d16f2df 100644 --- a/packages/@overeng/effect-schema-form-aria/src/components/BooleanField.tsx +++ b/packages/@overeng/effect-schema-form-aria/src/components/BooleanField.tsx @@ -60,8 +60,7 @@ const styles = stylex.create({ check: { width: '0.75rem', height: '0.75rem', - // oxlint-disable-next-line overeng/stylex-no-raw-color, @stylexjs/valid-styles -- needs a semantic `onPrimary` token; see #1171 - color: '#ffffff', + color: tokens['on-primary'], opacity: 0, transitionProperty: 'opacity', transitionDuration: '150ms', @@ -90,28 +89,37 @@ export const BooleanField = ({ isDisabled={isDisabled} {...stylex.props(styles.root)} > + {/* + Selection lives on the CheckboxButton, not on the box the style targets, + so the box cannot read it as one of its own conditions. React Aria + surfaces it as a render prop, which is the sanctioned mechanism for that + case, and the selected styles are ordered arguments rather than a + mutually exclusive branch — the winner is last and the precedence is the + argument order. Reading `isSelected` rather than the `value` prop also + stops the style re-deriving state the component already resolved. + */} -
- -
- {label} + {({ isSelected }) => ( + <> +
+ +
+ {label} + + )}
diff --git a/packages/@overeng/effect-schema-form-aria/src/components/FieldGroup.tsx b/packages/@overeng/effect-schema-form-aria/src/components/FieldGroup.tsx index 42911fab5b..c2d97f23f0 100644 --- a/packages/@overeng/effect-schema-form-aria/src/components/FieldGroup.tsx +++ b/packages/@overeng/effect-schema-form-aria/src/components/FieldGroup.tsx @@ -1,6 +1,6 @@ import * as stylex from '@stylexjs/stylex' import type { ReactNode } from 'react' -import { Group, Header, Text } from 'react-aria-components' +import { Group, Text } from 'react-aria-components' import { fontSizes, radii, spacing } from '@overeng/stylex-tokens/tokens.stylex' @@ -90,9 +90,15 @@ export const FieldGroup = ({ .filter(Boolean) .join(' ') + // The group label is a plain element, not React Aria's `Header`. `Header` + // renders `
`, and a `
` that is not inside sectioning content + // is a `banner` landmark — so two field groups on a page produced two banners + // and the accessibility gate failed the nested-group story on + // `landmark-no-duplicate-banner` and `landmark-unique`. Naming the group with + // `aria-label` gives it the accessible name it was missing anyway. return ( - -
{label}
+ +
{label}
{children}
) @@ -120,9 +126,10 @@ export const FieldGroupEmpty = ({ .filter(Boolean) .join(' ') + // Same landmark reasoning as `FieldGroup`. return ( - -
{label}
+ +
{label}
{message}
) diff --git a/packages/@overeng/effect-schema-form-aria/src/components/LiteralField.tsx b/packages/@overeng/effect-schema-form-aria/src/components/LiteralField.tsx index fe65ae7182..fb2116a95d 100644 --- a/packages/@overeng/effect-schema-form-aria/src/components/LiteralField.tsx +++ b/packages/@overeng/effect-schema-form-aria/src/components/LiteralField.tsx @@ -68,27 +68,21 @@ const styles = stylex.create({ fontSize: fontSizes.sm, lineHeight: '1.25rem', color: tokens.ink, - backgroundColor: tokens.surface, - borderRightWidth: 1, + backgroundColor: { default: tokens.surface, ':hover': tokens['surface-raised'] }, + borderRightWidth: { default: 1, ':last-child': 0 }, borderRightStyle: 'solid', borderRightColor: tokens.border, transitionProperty: 'background-color, color', transitionDuration: '150ms', - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':last-child': { - borderRightWidth: 0, - }, - }, - segmentUnselected: { - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':hover': { - backgroundColor: tokens['surface-raised'], - }, }, segmentSelected: { - backgroundColor: tokens.primary, - // oxlint-disable-next-line overeng/stylex-no-raw-color, @stylexjs/valid-styles -- needs a semantic `onPrimary` token; see #1171 - color: '#ffffff', + // Applied as the argument after `segment`, so it is the last write to both + // properties and wins by application order rather than by which condition + // kind happens to outrank which. The hover value is restated rather than + // left to fall through: `segment` sets `backgroundColor` under a `:hover` + // key, and a later unconditional `backgroundColor` does not replace that key. + backgroundColor: { default: tokens.primary, ':hover': tokens.primary }, + color: tokens['on-primary'], }, root: { display: 'grid', @@ -110,15 +104,13 @@ const styles = stylex.create({ borderColor: tokens.border, backgroundColor: tokens.input, color: tokens.ink, + // `outline` is reserved for the focus ring design-system-wide, so the base + // only suppresses the user-agent one. React Aria's own focus-visible state + // is preferred over `:focus`: it normalises across input modalities, so a + // pointer click no longer paints a keyboard focus ring. outline: 'none', - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':focus': { - boxShadow: `0 0 0 1px ${tokens.primary}`, - }, - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':disabled': { - opacity: 0.5, - }, + boxShadow: { default: null, '[data-focus-visible]': `0 0 0 1px ${tokens.primary}` }, + opacity: { default: 1, ':disabled': 0.5 }, }, triggerValue: { flexGrow: 1, @@ -136,8 +128,7 @@ const styles = stylex.create({ borderStyle: 'solid', borderColor: tokens.border, backgroundColor: tokens.surface, - // oxlint-disable-next-line overeng/stylex-no-raw-color -- needs a scheme-varying elevation-shadow token; see #1171 - boxShadow: '0 10px 15px -3px rgb(0 0 0 / 0.1), 0 4px 6px -4px rgb(0 0 0 / 0.1)', + boxShadow: tokens['shadow-raised'], }, listBox: { outline: 'none', @@ -152,12 +143,7 @@ const styles = stylex.create({ lineHeight: '1.25rem', cursor: 'pointer', borderRadius: radii.default, - }, - optionUnselected: { - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':hover': { - backgroundColor: tokens['surface-raised'], - }, + backgroundColor: { default: null, ':hover': tokens['surface-raised'] }, }, optionMuted: { color: tokens['subtle-ink'], @@ -166,9 +152,9 @@ const styles = stylex.create({ color: tokens.ink, }, optionSelected: { - backgroundColor: tokens.primary, - // oxlint-disable-next-line overeng/stylex-no-raw-color, @stylexjs/valid-styles -- needs a semantic `onPrimary` token; see #1171 - color: '#ffffff', + // Same ordering contract as `segmentSelected`. + backgroundColor: { default: tokens.primary, ':hover': tokens.primary }, + color: tokens['on-primary'], }, hint: { fontSize: '12px', @@ -219,10 +205,8 @@ export const LiteralField = ({ key={opt.value} id={opt.value} className={({ isSelected }) => - stylex.props( - styles.segment, - isSelected === true ? styles.segmentSelected : styles.segmentUnselected, - ).className ?? '' + stylex.props(styles.segment, isSelected === true && styles.segmentSelected) + .className ?? '' } > {opt.label} @@ -262,7 +246,7 @@ export const LiteralField = ({ stylex.props( styles.option, styles.optionMuted, - isSelected === true ? styles.optionSelected : styles.optionUnselected, + isSelected === true && styles.optionSelected, ).className ?? '' } > @@ -278,7 +262,7 @@ export const LiteralField = ({ stylex.props( styles.option, styles.optionInk, - isSelected === true ? styles.optionSelected : styles.optionUnselected, + isSelected === true && styles.optionSelected, ).className ?? '' } > diff --git a/packages/@overeng/effect-schema-form-aria/src/components/NumberField.tsx b/packages/@overeng/effect-schema-form-aria/src/components/NumberField.tsx index e7a8385c9d..35c1d2de88 100644 --- a/packages/@overeng/effect-schema-form-aria/src/components/NumberField.tsx +++ b/packages/@overeng/effect-schema-form-aria/src/components/NumberField.tsx @@ -49,15 +49,11 @@ const styles = stylex.create({ borderColor: tokens.border, backgroundColor: tokens.input, color: tokens.ink, + // A plain ``, not a React Aria one, so the native focus-visible + // pseudo-class is the only focus state available here. outline: 'none', - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':focus': { - boxShadow: `0 0 0 1px ${tokens.primary}`, - }, - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':disabled': { - opacity: 0.5, - }, + boxShadow: { default: null, ':focus-visible': `0 0 0 1px ${tokens.primary}` }, + opacity: { default: 1, ':disabled': 0.5 }, }, toggle: { width: '1rem', @@ -70,14 +66,8 @@ const styles = stylex.create({ borderWidth: 1, borderStyle: 'solid', borderColor: tokens.border, - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':hover': { - backgroundColor: tokens['surface-raised'], - }, - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':disabled': { - opacity: 0.5, - }, + backgroundColor: { default: null, ':hover': tokens['surface-raised'] }, + opacity: { default: 1, ':disabled': 0.5 }, }, root: { display: 'grid', @@ -96,15 +86,9 @@ const styles = stylex.create({ backgroundColor: tokens.input, color: tokens.ink, outline: 'none', - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':focus': { - boxShadow: `0 0 0 1px ${tokens.primary}`, - }, - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':disabled': { - opacity: 0.5, - cursor: 'not-allowed', - }, + boxShadow: { default: null, '[data-focus-visible]': `0 0 0 1px ${tokens.primary}` }, + opacity: { default: 1, ':disabled': 0.5 }, + cursor: { default: null, ':disabled': 'not-allowed' }, }, description: { fontSize: '12px', diff --git a/packages/@overeng/effect-schema-form-aria/src/components/TextField.tsx b/packages/@overeng/effect-schema-form-aria/src/components/TextField.tsx index 63bf5f1b08..2fc9492163 100644 --- a/packages/@overeng/effect-schema-form-aria/src/components/TextField.tsx +++ b/packages/@overeng/effect-schema-form-aria/src/components/TextField.tsx @@ -49,18 +49,12 @@ const styles = stylex.create({ backgroundColor: tokens.input, color: tokens.ink, outline: 'none', - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':focus': { - boxShadow: `0 0 0 1px ${tokens.primary}`, - }, + boxShadow: { default: null, '[data-focus-visible]': `0 0 0 1px ${tokens.primary}` }, + opacity: { default: 1, ':disabled': 0.5 }, + cursor: { default: null, ':disabled': 'not-allowed' }, '::placeholder': { color: tokens['subtle-ink'], }, - // oxlint-disable-next-line @stylexjs/no-legacy-contextual-styles, @stylexjs/valid-styles -- deprecated top-level pseudo-class; nesting it changes condition precedence, so it needs the visual gate. See #1171 - ':disabled': { - opacity: 0.5, - cursor: 'not-allowed', - }, }, description: { fontSize: '12px', diff --git a/packages/@overeng/effect-schema-form-aria/src/mod.unit.test.tsx b/packages/@overeng/effect-schema-form-aria/src/mod.unit.test.tsx index 0486e58605..106fbc382d 100644 --- a/packages/@overeng/effect-schema-form-aria/src/mod.unit.test.tsx +++ b/packages/@overeng/effect-schema-form-aria/src/mod.unit.test.tsx @@ -43,7 +43,7 @@ describe('effect-schema-form-aria baselines (cross-major invariant)', () => { ) expect(html).toMatchInlineSnapshot( - `"
Primitives <世界>
résumé@example.com
bare
"`, + `"
Primitives <世界>
résumé@example.com
bare
"`, ) }) @@ -76,7 +76,7 @@ describe('effect-schema-form-aria baselines (cross-major invariant)', () => { ) expect(html).toMatchInlineSnapshot( - `"
NaN boundary
"`, + `"
NaN boundary
"`, ) }) @@ -94,7 +94,7 @@ describe('effect-schema-form-aria baselines (cross-major invariant)', () => { ) expect(html).toMatchInlineSnapshot( - `"
Role
"`, + `"
Role
"`, ) }) @@ -113,7 +113,7 @@ describe('effect-schema-form-aria baselines (cross-major invariant)', () => { ) expect(html).toMatchInlineSnapshot( - `"
Choose
"`, + `"
Choose
"`, ) }) @@ -137,7 +137,7 @@ describe('effect-schema-form-aria baselines (cross-major invariant)', () => { ) expect(html).toMatchInlineSnapshot( - `"
itemsUnsupported schema type: unknown
Empty Group
No additional options
Custom
"`, + `"
itemsUnsupported schema type: unknown
Empty Group
No additional options
Custom
"`, ) }) @@ -166,7 +166,7 @@ describe('effect-schema-form-aria baselines (cross-major invariant)', () => { ) expect(html).toMatchInlineSnapshot( - `"
Contact Preferences
Unicode accepted
mode
itemsUnsupported schema type: unknown
"`, + `"
Contact Preferences
Unicode accepted
mode
itemsUnsupported schema type: unknown
"`, ) }) diff --git a/packages/@overeng/effect-schema-form-aria/src/tokens.stylex.ts b/packages/@overeng/effect-schema-form-aria/src/tokens.stylex.ts index 81799e599f..75fbcfc3a4 100644 --- a/packages/@overeng/effect-schema-form-aria/src/tokens.stylex.ts +++ b/packages/@overeng/effect-schema-form-aria/src/tokens.stylex.ts @@ -1,6 +1,6 @@ import * as stylex from '@stylexjs/stylex' -import { colors } from '@overeng/stylex-tokens/tokens.stylex' +import { colors, shadows } from '@overeng/stylex-tokens/tokens.stylex' /** * Semantic design tokens for effect-schema-form-aria. @@ -17,6 +17,14 @@ export const tokens = stylex.defineVars({ input: colors.white, surface: colors.gray50, 'surface-raised': colors.gray100, - primary: colors.blue500, - accent: colors.blue500, + // One step darker than the obvious `blue500`, which is a legibility fact + // rather than a preference: white body text on `blue500` measures 3.76:1 and + // the accessibility gate failed ten stories on it. `blue600` measures 5.25:1. + // Anything placed on this token as a background must use `on-primary`. + primary: colors.blue600, + /** Foreground for text and icons drawn on top of `primary`. */ + 'on-primary': colors.white, + accent: colors.blue600, + /** Elevation for surfaces that float above the page, such as a popover. */ + 'shadow-raised': shadows.lg, }) From ff33f7b1b6eb15614d3e4c62481224c6b6c0eae2 Mon Sep 17 00:00:00 2001 From: "AI assistant of @schickling" <261620128+schickling-assistant@users.noreply.github.com> Date: Tue, 1 Sep 2026 23:45:06 +0200 Subject: [PATCH 2/2] docs(changelog): record the pilot package drift resolution --- CHANGELOG.md | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e8c55b4b0..138f320522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,49 @@ All notable changes to this project will be documented in this file. ### Fixed +- **@overeng/effect-schema-form-aria**: the pilot package's three independent + drift findings are resolved together, because all three were waiting on the + same prerequisite — a working visual gate for this package. The seventeen + per-line `oxlint-disable` suppressions are gone and all eight StyleX rules + gate the package again. + **Raw colours become semantic tokens**: `on-primary` for foregrounds drawn on + `primary`, and `shadow-raised` for popover elevation, defaulting to the + `shadows.lg` scale step. **`primary` moves `blue500` -> `blue600`**, which is + the accessibility fix rather than a palette preference: the gate failed ten of + thirty-nine stories because white text on the selected segment measured + 3.76:1 against AA's 4.5:1, and `blue600` measures 5.25:1. Renaming the literal + alone would have preserved the violation behind a better name. + **Thirteen deprecated top-level pseudo-class sites move into condition + objects.** Every site was reviewed for a competing state rather than + translated mechanically, because nesting a pseudo-class changes which + condition wins. Eleven set a property nothing else touches; the two that + compete — the segmented control's and the list option's background under hover + versus selection — now resolve by `stylex.props` argument order, with the + selected style restating the hover value because a later unconditional + `backgroundColor` does not replace an earlier one under a `:hover` key. + **Focus moves to the accessible-component library's focus-visible state** + (native `:focus-visible` on the one plain ``), so a pointer click no + longer paints a keyboard focus ring. The ring is still drawn with `boxShadow` + rather than converted to `outline`: the partitioning invariant already holds + and no story renders a focused element, so repainting it would be a change the + gate structurally cannot adjudicate. + **State resolution stops re-deriving what the component knows.** The checkbox + read its own `value` prop; selection lives on the CheckboxButton, an ancestor + of the styled box, so it now comes from React Aria's render prop. The + segmented control and list options applied one of two mutually exclusive style + objects, hover rule included, and now apply one additive override. + **The eleventh gate failure was structural, not colour**: React Aria's + `Header` renders `
`, which outside sectioning content is a `banner` + landmark, so nested field groups produced two banners. The label is a plain + element now and the group carries the accessible name it lacked. + Adjudicated against the gate: 20 stories changed, 18 of them the intended + `43,127,255 -> 21,93,252` recolour confined to selected segments, checkbox + boxes and the accent tick. The other two are the gate's own sub-pixel fringe, + proven by recapturing the *unchanged* baseline tree and reproducing both + diffs identically (689 and 693 pixels, max channel delta 2). Zero + accessibility failures remain, and the condition-nesting, ordered-argument and + landmark changes moved no pixels at all. Closes #1171. + - **nix/oxlint-with-plugins.nix**: three fixes to the oxlint wrapper. `jsPlugins` entries other than ours are no longer discarded, so third-party JS plugins can load beside `@overeng/oxc-config` (previously any such plugin