Repository navigation
Resolve the pilot package drift the gate, the lint rules and decision 0004 each found - #1175
Closed
schickling-assistant wants to merge 2 commits into
Conversation
… findings 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 `<input>` 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 `<header>`, and a `<header>` 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.
schickling-assistant
changed the title
Collaborator
Author
|
Superseded by #1191, which collapses this stack onto Closing rather than merging: propagating This PR's content is in #1191, verified with This body stays as the record of the per-change evidence, which #1191 summarises but does not reproduce in full. Posted on behalf of @schickling
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the pilot package's three independent drift findings. One PR rather
than three because all three were deferred on the same prerequisite — a working
visual gate for this package — which #1170 now supplies. Closes #1171.
What was wrong, and how each part was proved
on-primaryandshadow-raisedsemantic tokens, defaults from named scale stepsstylex.propsargumentsprimaryblue500->blue600Header-> plain element +aria-labelon the groupAll 17
oxlint-disablesuppressions are gone; all eight rules gate the packageagain.
Gate adjudication
Baseline
98037687a(#1172 tip): 39 compared, 28/39 passed at baseline, 20changed. Non-degenerate, so the pre-existing subtraction is not masking
anything.
43,127,255->21,93,252, confined to selected segments, checkbox boxes and the accenttick.
blue500measured 3.76:1 against white body text where AA requires4.5:1;
blue600measures 5.25:1. Renaming the literal toon-primaryalonewould have preserved the violation behind a better name.
not assumed: recapturing the unchanged baseline tree reproduces both diffs
identically —
URLn=689,With Hintn=693, same bounding boxes, max channeldelta 2/255 on one border hairline. Comparing that recapture against this
branch yields exactly 18 and no fringe. Reported to
GateFixes, includingthat it is a deterministic first-capture effect rather than random noise.
The zero-pixel rows are the substantive result. The recorded hazard was that
nesting a pseudo-class changes which condition wins; the gate shows the
refactor was behaviour-preserving.
Two things deliberately NOT done
boxShadowrather than being converted tooutline.The partitioning invariant already holds — nothing else on these elements sets
outline*, andstylex-outline-focus-visible-onlyconfirms it — and no storyrenders a focused element, so repainting the ring would be a visual change the
gate structurally cannot adjudicate. Named follow-up: add a story that
renders a focused element, then convert with the gate watching. That turns an
unverifiable change into a verified one and leaves a permanent regression test
for a hazard that is otherwise invisible to both the type checker and the
linter.
shadow-raisedtoken (no story opens the dropdown) and the focus-visible move(no story focuses anything).
Verification
lint:check:oxlint: clean.<header>gone,aria-labelpresent).Note the reproducible
close timed out after 10000ms / something prevents the main process from exitingon every gate run: known upstream StyleX bundlerplugin defect (leaked handles), not ours, recorded so nobody investigates it
from scratch.
Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile