Paint the chosen operator with a colour the theme declares - #39
Merged
Merged
Conversation
The column-filter popup read two custom properties that are declared
nowhere — not in global.css, not in vscode-theme.css, not locally. An
undeclared property is not a missing colour, it is a silent one: every
var() falls back to the literal written beside it, nothing warns, and the
literal is a light-theme value by construction.
The one that matters is the pressed operator button, which is the popup's
whole state display — the only thing saying which comparison Apply is
about to write. It asked for --dex-bg-selected and so painted #cce4f7 in
every theme, under the near-white ink a dark theme gives it. Measured in
a real browser over the shipped dist/webview bundle, all four themes plus
forced-colors:
before after
Dark Modern 1.22 7.61
Light Modern 8.54 9.14
HC Black 1.31 10.41
HC White 11.09 12.33
Two of those failed AA and the worse of them is below the 2.13:1 that
issue #26 was filed over; all four pass now. It is a one-word fix,
because the repo already declares the token for a selected surface:
--dex-color-accent-bg, aliased onto --dex-selection-bg by
vscode-theme.css and pinned there by vscodeThemeTokens.test.ts. Its
light value is #cde4f7 — the invented name's fallback was that value with
one digit changed, which is the evidence the name was a slip and not a
design choice, and why this is not a decision about what colour the
button should be.
The second, --dex-font-family-mono, is a font rather than a colour, so it
cost legibility rather than contrast: the one line in the popup that
shows literal syntax to retype was the one line that ignored the font the
user types code in. The declared name is --dex-font-mono, which carries
editor.fontFamily.
Forced colors was never affected and still is not (21:1 in both
directions) — the pressed button's outline there comes from a
@media (forced-colors: active) block that overrides both backgrounds.
columnFilterTokens.test.ts now sweeps every var(--dex-*) this component
reads and demands that each one be declared somewhere, so the next
invented name fails on the name rather than in a theme. Two files are
knowingly outside its file list, named in its header with what each
reads; the rule there is to add a file once it is clean, not to add an
exemption to keep one in.
This was referenced Oct 3, 2026
Merged
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
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.
The filter popup a column header opens read two CSS custom properties that
are declared nowhere — not in
src/webview/components/styles/global.css, notin
src/webview/vscode-theme.css, not locally in the component. That is thebug class this repo already knows: an undeclared property is not a missing
colour, it is a silent one. Every
var()takes the literal written besideit as a fallback, nothing warns at build or at runtime, and the literal is a
light-theme value by construction. The Variable Editor had a whole family of
these (v1.32.0);
test/variableEditorTheme.test.tsis what stopped themcoming back there.
What it cost
.op-button[aria-pressed='true']—src/webview/components/dex-column-filter.ts:81The pressed operator button is the popup's entire state display: it is the one
element that says which comparison Apply is about to write. It asked for
--dex-bg-selectedand therefore painted#cce4f7in all four themes, underwhatever ink the theme gives it.
Measured in Chromium over the shipped
dist/webviewbundle with the real--vscode-*palette, popup open, pressed button against its own text:Two of the four failed AA, and the worse of them sits below the 2.13:1 that
#26 was filed over. All four pass now.
.writes-value—dex-column-filter.ts:114--dex-font-family-monois a font, not a colour, so it cost legibility ratherthan contrast: the hard-coded stack beside it always won, which made the one
line in the popup that shows literal syntax to retype the one line that
ignored the font the user types code in.
Why this is not a colour decision
The repo already declares the token for a selected surface:
--dex-color-accent-bg, whichvscode-theme.cssaliases onto--dex-selection-bgandtest/vscodeThemeTokens.test.ts:106pins there. Itslight value is
#cde4f7— the invented name's fallback was that same valuewith one hex digit changed. So the fix is to spell the existing token
correctly, not to choose a new colour; same for
--dex-font-mono, whichcarries the user's
editor.fontFamily.Qualification
Phase-3 browser run, before and after, four themes each, zero failed checks in
either direction (every
contrast()call returning a value, no console errors,no failed requests). Rows come from
hostRows()— a real.slddthrough thereal
buildRows().Controls that must not move, and did not: the unpressed operator buttons, the
popup title, the
writes:line, and both chip kinds in the filter bar allmeasure identically before and after. Forced colors is 21:1 in both
directions — it was never affected, because the existing
@media (forced-colors: active)block replaces both backgrounds and marks thepressed button with a
Highlightoutline. That block is untouched.One trap worth recording: the first version of the scenario measured
forced-colors and the plain chip after applying the filter, which closes the
popup — both came back
{error: "no element matching …"}in all four themesof both runs, which reads exactly like a clean check in a summary table. The
measurements above are from the corrected run.
The test
test/columnFilterTokens.test.tsis new; no existing test is modified.It sweeps every
var(--dex-*)this component reads and requires each to bedeclared in
global.css,vscode-theme.css, or the component itself — so thenext invented name fails on the name, in the unit suite, rather than in a
theme nobody ran. Two further assertions name the two sites directly, so the
reason survives a refactor of the sweep.
Two files are knowingly outside the sweep's file list and are named in its
header with what each reads and what it measured
(
dex-filter-bar.ts's--dex-bg-badge,dex-tree-table.ts's--dex-border-color-ultralight). The rule written there is to add a file onceit is clean, not to add an exemption to keep one in. Both are on the agenda:
the badge is benign (8.16–17.92:1, a neutral grey at 18% alpha), and the
ultralight border turns out to sit in CSS that cannot be reached, which is a
question about whether
table-styleshould exist rather than a colour bug.npm run verifygreen: 163 files, 3106 tests, leak check OK.