fix(theme): colour every focus outline from the design system's focus token - #474
Merged
Conversation
… token #191 reads as an inconsistency — four colours doing one job across the theme files — and the scope agreed was colour only: form stays per component, and the `ring-(--b24ui-border-color)` pattern on inputs is left alone, because there the ring is the field's own border rather than a focus accent. Measuring the four turned it into an accessibility fix. WCAG 2.2 SC 1.4.11 asks 3:1 of a focus indicator; against the background it renders on: outline-primary #2fc6f6 1.99:1 on white fails --ui-color-accent-soft-element-blue #0056bf 2.22:1 on #262626 fails --ui-color-design-outline-focused-stroke 4.21:1 / 4.24:1 passes Seven of the nine outline-coloured sites failed in at least one theme. On chat-reasoning, chat-tool and footer-columns the focus ring is nearly invisible in the light theme, and on accordion, splitter and table it is nearly invisible in the dark one. `outline-primary` is the worst of them because it does not look like a problem: it resolves to `--color-primary`, a legacy Bitrix cyan with no relationship to the design system at all. `--ui-color-design-outline-focused-stroke` is the design system's own name for this — `design/outline/focused/design-outline-focused-stroke` in `design-tokens/app.json` — and it is the only one of the four defined in all four theme contexts, which is why it adapts and the others do not. Two of the nine sites already used what it resolves to in light contexts, so those change name without changing pixels. Two things are deliberately left. The `isAction` link's red focus matches its hover state, and repainting it blue would break the variant rather than fix it; the real defect there is the token having no dark value, filed as #473, which costs that link's hover *text* 4.5:1 as well. And the fifteen `--b24ui-border-color` rings on inputs are out of scope by the agreed reading. Also taken, both design-agnostic and both from upstream: `outline-offset: 0` on `a:focus-visible`, so an inline link's ring is measured from the link rather than landing on the line above; and `overflow: hidden` on every frame of the accordion and collapsible height animations, so content is not drawn outside the box while it collapses. A dead class went with them: `hover:text(` in `link.ts`, missing its dash, so Tailwind generated nothing for it. The correct spelling sits beside it on the same line, so nothing changes. None of this is visible to the suite as it stands — happy-dom neither resolves custom properties nor computes contrast, so a port swapping the token back changes one string in a class list and nothing goes red. `test/utils/focus-accent-token.spec.ts` holds it, including that the token stays defined in every context: a theme pointing at a token no context defines resolves to an invalid declaration, silently, and the outline falls back to the browser's. Resolves #191 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc
436 class-list occurrences across nine components, and nothing else: every changed line carries a `focus-visible:outline-*` class, and the old colours account for the new ones exactly — 208 `soft-element-blue`, 151 `outline-primary`, 77 `accent-main-primary`, against 436 of the focus token. Worth noting what that count says about the shape of the problem. Three colours, one job, and the two that lost the most ground were the two that failed contrast: `outline-primary` in the light theme and `soft-element-blue` in the dark one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc
The same 436 occurrences, in the `vue` project's copies. Both projects render the same themes, so the counts match exactly: 208 `soft-element-blue`, 151 `outline-primary`, 77 `accent-main-primary`, replaced by 436 of the focus token, with no line changing for any other reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc
The ledger recorded the upstream focus commit as skipped and said the two generic bits — the `a:focus-visible` reset and `overflow: hidden` in the keyframes — were "also deferred". They are not deferred any more, so the entry now describes a state that has not been true since the previous commit on this branch. The skip itself stands, and the entry still says so: the upstream pattern rewrites form across ~50 themes onto a model this fork does not have, and form stays per component by maintainer decision. What changed is narrower and worth recording because it was not the reason the commit was skipped — the colour consolidation turned out to fix contrast failures rather than inconsistency. No new key, so the upstream-reference check does not ask this commit to name a port. That is the case it was written for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc
The docstring gave two numbers, light and dark, and left the impression that they characterise the token. They do not, and the two contexts left out are the ones that make the case. light #0075ff 4.21:1 was #0154c8 at 6.77:1 dark #1587fa 4.24:1 was #0056bf at 2.22:1 edge-light #000000 at 35% was #0056bf edge-dark #ffffff at 40% was #0056bf Two things worth having written down. In the light theme the contrast **falls** — 6.77:1 to 4.21:1 — which is a real cost, disclosed rather than buried: still comfortably past the 3:1 the criterion asks, and the price of one colour that works everywhere over one that is excellent in half the contexts and unusable in the rest. And in the edge contexts both the surface and the token are translucent, so no static ratio exists to quote. What can be said is stronger than a number: the token inverts, black at 35% over a light surface and white at 40% over a dark one, while `soft-element-blue` stayed the same opaque dark blue in all four. That is what "the only one defined in every context" buys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc
The sweep in this branch was a grep for three known colours, so it found three known colours. Replacing it with the inverse property — *every* coloured focus outline reads the token, with exceptions named one by one — found two more the first pass had no way to see. `file-upload.ts` used `outline-inverted`, and nothing in this repository defines `--color-inverted`. Tailwind emits `outline-color: var(--color-inverted)` for it, which resolves to nothing, so that focus ring was not merely the wrong colour — it had no colour of its own at all and fell back to the browser's. Same family as `outline-primary` and a degree worse. `toast.ts` coloured focus from `--ui-color-design-outline-content-divider`, a divider grey. A fourth accent doing focus duty, in a file the original grep never looked at. The spec is the change that matters here. It had a `>= 7` threshold over the eight themes using the token, which review showed says nothing when one component stops using it, and nothing at all about a colour arriving that is simply not on a banned list. An allowlist inverts that: unknown colours are the failure case rather than the gap. Widths (`(length:…)`) and the control-palette carve-out (`--b24ui-*`, `--ui-btn-*`) are filtered out by shape; the isAction red is an entry in `EXCEPTIONS` with its reason and #473 beside it, and a test asserts each exception still describes something, because one that does not is a standing licence. Also from review: stripping comments from the first `//` also strips from the one inside `https://`, so a real class written after a URL on the same line disappeared with it. No theme is written that way today, which is the point. Refs #191 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc
75 occurrences, and the arithmetic closes again: 57 `outline-inverted` plus 18 `outline-content-divider` against 75 of the focus token, with no line changing for any other reason. The 57 are worth a second look. `outline-inverted` resolved to nothing — `--color-inverted` is not defined anywhere in this repository — so those 57 class strings were rendering a focus ring with no colour of its own for as long as they have existed, and every snapshot recorded them as valid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc
3 tasks
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
2 participants
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.
Linked issue
Resolves #191
Follow-ups filed: #473 (the red token's dark value), #476 (the focus token cannot reach 3:1 in the edge contexts).
Type of change
revert(Scope): ...)Description
#191 asks whether b24ui wants a uniform focus model, having skipped upstream's version as a design regression. The maintainer scoped the answer to colour only: form stays per component, and a stroke that is the control's own border —
--b24ui-*,--ui-btn-*— stays with the component palette.Eleven focus outlines now read
--ui-color-design-outline-focused-stroke. Nine were found by hand; the last two were found by the guard, after review showed the guard was the wrong shape.What the measurement found
WCAG 2.2 SC 1.4.11 asks 3:1 of a focus indicator:
focus-visible:outline-*outline-primary→--color-primary#2fc6f6outline-inverted→--color-inverted--ui-color-accent-soft-element-blue#0056bf#262626--ui-color-design-outline-content-divider--ui-color-accent-main-primary#0075ff--ui-color-design-outline-focused-strokeoutline-invertedinfile-upload.tsis the one to pause on. Nothing in this repository defines--color-inverted, so Tailwind emittedoutline-color: var(--color-inverted), which resolves to nothing — 57 snapshot lines recorded a focus ring that had no colour of its own, and every one of them looked like a valid class.Disclosure 1 — the edge contexts get worse
Review measured what I had not:
So: a fix in the two contexts where the backdrop is known, a regression in the two where it is not. Accordion, Splitter and Table lose a visible focus ring in edge-light.
Kept anyway, with the maintainer's agreement, for three reasons. There is nothing to roll back to — one variable serves all four contexts, so reverting means restoring the four-way split, 1.99:1 in light and 2.22:1 in dark included. The old edge figures were luck rather than a property: 6.05:1 is measured against edge-light's default backdrop, and an edge theme exists precisely so the portal can replace it, often with a photograph — the same opaque blue over a dark image would be worse than the token is. And a translucent luminance shift degrades evenly instead of gambling.
Whether evenly-mediocre clears the bar is a question about the token's values, not about its use here, and 2.4:1 says it does not. #476.
Disclosure 2 — the central claim was wrong
The first version of this description, and of the
PORTING.mdrule, said the focus token is "the only one of the four defined in all four theme contexts". Two reviewers flagged it independently and they are right:accent-main-primaryandaccent-soft-element-blueare defined in all four files too.The real distinction is better:
#0075ff#1587fa#0075ff#1587fa#1587fa#1587fa#0154c8#0056bf#0056bf#0056bf--color-primary#2fc6f6— one value, no per-context override at allThe focus token changes kind — saturated in light and dark, a translucent inversion in the edge contexts. The others stay one blue. And
outline-primaryreads from the Tailwind theme block, which has no contexts.Corrected in
PORTING.mdand in the spec, with a note saying the earlier draft got it wrong.The two design-agnostic bits (issue checkbox 2)
Both taken:
a:focus-visible { outline-offset: 0 }inindex.css, andoverflow: hiddenon every frame of the accordion and collapsible height animations.On the second, review is right that every consumer already carries
overflow-hiddenunconditionally, so it changes nothing today. Kept because it costs nothing and makes the keyframes correct on their own terms rather than by the good behaviour of five call sites — but flagging it as redundant rather than claiming it as a fix.What is deliberately left
ringvsoutline, widths and offsets. Only colour is done, by decision.isActionlink's red focus — an entry in the spec'sEXCEPTIONSwith its reason and a11y(tokens):--ui-color-accent-soft-element-redhas no dark-context value, failing contrast for both text and focus #473 beside it.--b24ui-*and--ui-btn-*, filtered by shape.The guard, and why it changed shape
The first version was a denylist of the three colours I had found, with a
>= 7threshold over the themes using the token. Review showed that says nothing when a component stops using the token, and nothing at all about a colour arriving that is not on the list.Inverted: every coloured focus outline reads the token, exceptions named one at a time with a test that each exception still describes something. Unknown colours are now the failure case rather than the gap — and it immediately found
file-uploadandtoast, which the grep had never looked at.Also fixed there: comments were stripped from the first
//, which is also the one insidehttps://, so a real class after a URL on the same line vanished with it.Verification
Snapshots: 511 occurrences across eleven components in both projects, with the arithmetic closing exactly in each pass and zero lines changing for any other reason.
happy-dom neither resolves custom properties nor computes contrast, so none of this is visible to the suite — every one of those 511 lines was a valid class string before and after,
outline-invertedincluded.Checklist
No docs change: focus colour was never documented, confirmed against the theme pages and the component pages for the affected components.
Gate run locally with
CI=true:lint·typecheck·test.