Skip to content

fix(theme): derive button text colour from accent luminance - #6108

Open
YahyaZekry wants to merge 5 commits into
odysseus-dev:devfrom
YahyaZekry:fix/on-accent-contrast
Open

fix(theme): derive button text colour from accent luminance#6108
YahyaZekry wants to merge 5 commits into
odysseus-dev:devfrom
YahyaZekry:fix/on-accent-contrast

Conversation

@YahyaZekry

@YahyaZekry YahyaZekry commented Aug 18, 2026

Copy link
Copy Markdown

Summary

46 rules in static/style.css, plus 5 inline styles in admin.js, document.js, settings.js and compare/scoreboard.js, pair a background: var(--red) with a hardcoded color: #fff. Because --red is theme-configurable, six built-in themes currently render text on their own buttons below the 3:1 large-text floor — terminal is 1.37:1, ume 1.97, paper 2.24, forest 2.35, ocean 2.42, cute 2.68. This adds an --on-accent token, recomputed in applyColors() from --red's WCAG relative luminance (dark text when white would fall under 3:1, white otherwise), and points every hardcoded site at it. The threshold is deliberately 3:1 so only the six failing themes change appearance; the other ten keep white exactly as today.

Target branch

  • This PR targets dev, not main.

Linked Issue

Fixes #6109

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)
  • New feature (non-breaking — adds new behaviour)
  • Breaking change (changes or removes existing behaviour)
  • Refactor / cleanup (behaviour unchanged)
  • Documentation only
  • CI / tooling / configuration

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up) and verified the change works end-to-end. See the note under Testing about which tree was run.

How to Test

  1. docker compose up -d --build and open the app.
  2. Switch to the cute or terminal theme (Theme → Browse).
  3. Look at the Send button, "Open Email Settings" (.admin-btn-add), and any primary/confirm button. Before this change the label is white on a light accent and effectively unreadable; after it, the label is near-black.
  4. Switch to dark or claude — these are above the 3:1 threshold, so they should look exactly as before (white text, unchanged).
  5. To check the numbers directly, in the console:
    const cs = getComputedStyle(document.documentElement);
    cs.getPropertyValue('--red').trim();        // theme accent
    cs.getPropertyValue('--on-accent').trim();  // '#171717' on the 6 failing themes, '#fff' elsewhere

Measured contrast against --red, before → after: terminal 1.37 → 13.13, ume 1.97 → 9.11, paper 2.24 → 8.00, forest 2.35 → 7.64, ocean 2.42 → 7.40, cute 2.68 → 6.69. Worst case across all 16 themes goes from 1.37 to 3.03.

Scope note on the "ran the app" checkbox: I run this fix in my own fork, which is where I verified it end-to-end in the browser (measured .admin-btn-add at 10.68:1 live, up from ~1.68:1) and where it has been in daily use. I did not separately boot this exact branch off dev; the change is identical, but I'd rather state that precisely than overclaim. node --check passes on every changed JS file and the stylesheet's braces balance.

Visual / UI changes — REQUIRED if you touched anything that renders

  • Screenshot or short clip of the change in the running app, attached below.
  • Style match: no new colour values, font sizes or spacing units are introduced. The change removes hardcoded #fff literals in favour of a token derived from the existing --red, and adds one variable alongside the existing --bg/--fg/--panel/--border/--red set. No new classes, no emoji, no font or theme-system changes.
  • No new component patterns. No markup or components added — this only changes which colour existing rules resolve to.
  • I am not an LLM agent submitting a bulk PR. — left unchecked deliberately; see below.

Screenshots / clips

Before — white label on a light accent, unreadable:

before

After--on-accent resolves to near-black:

after

Same screen (Settings → Email), same build, only --on-accent differs.

Captured on a custom theme whose --red is a light amber (#f2c14e), which puts it in the same failing band as the shipped cute / terminal / ume themes — it exercises the identical code path, just with an accent that makes the difference easy to see side by side. The --on-accent value is derived from --red alone, so behaviour is the same for any theme below the 3:1 threshold.


Disclosure. I use an LLM agent as part of my workflow, so I have not checked the "not an LLM agent" box — checking it would be untrue. Per CONTRIBUTING I opened #6109 first; this PR is the proposed implementation attached to it, not a bulk drop. It is one focused fix, hand-reviewed, with the reasoning and measurements above. If you'd rather discuss it on the issue and have me close this, say the word and I will.

46 rules in static/style.css (plus 5 inline styles in admin.js,
document.js, settings.js and compare/scoreboard.js) pair a
`background: var(--red)` with a hardcoded `color: #fff`. Because --red is
theme-configurable, several built-in themes render unreadable text on
their own buttons -- terminal is 1.37:1, ume 1.97, paper 2.24, forest
2.35, ocean 2.42, cute 2.68, all below the 3:1 large-text floor.

Adds an `--on-accent` token, recomputed in applyColors() from --red's
WCAG relative luminance: dark text when white would fall under 3:1,
white otherwise. Every hardcoded site now reads the token.

Contrast against --red, before -> after:

  terminal 1.37 -> 13.13    ume    1.97 -> 9.11
  paper    2.24 ->  8.00    forest 2.35 -> 7.64
  ocean    2.42 ->  7.40    cute   2.68 -> 6.69

Themes already at or above 3:1 are untouched, so the visual change is
limited to the six that were failing.

HSL lightness would not work here: #e06c75 and #f2c14e have nearly
identical HSL-L but 0.27 vs 0.56 relative luminance, because the WCAG
formula weights green (0.7152) far above red (0.2126) and blue (0.0722).

models.js's IMG badge is deliberately left alone -- its background is a
fixed purple fallback, not --red.
@github-actions github-actions Bot added needs work PR description incomplete — please update before review needs runtime validation Runtime validation not attested — tick the app-run box after running it, or state the gap needs visual evidence UI-sensitive change without an attested screenshot or clip from the running app labels Aug 18, 2026
@github-actions github-actions Bot added ready for review Description complete — ready for maintainer review and removed needs work PR description incomplete — please update before review needs runtime validation Runtime validation not attested — tick the app-run box after running it, or state the gap needs visual evidence UI-sensitive change without an attested screenshot or clip from the running app labels Aug 18, 2026
RaresKeY

This comment was marked as outdated.

Review feedback: --on-accent was derived from --red alone, but the send
button's background is independently configurable via --send-btn-bg /
--send-btn-hover, and those overrides are applied after the token is
computed. A dark --red with a light custom send button therefore kept
white text on a light surface -- e.g. --red #e06c75 with --send-btn-bg
#f2c14e left #fff at 1.68:1.

Each independently configurable accent surface now gets a foreground
derived from the background it actually renders, resolved after the
advanced overrides are applied:

  --on-accent                  <- --red            (unchanged consumers)
  --on-send-btn                <- resolved --send-btn-bg
  --on-send-btn-hover          <- resolved --send-btn-hover
  --on-send-btn-newchat-hover  <- that hover colour mixed 85% into --panel

The new-chat state is a third surface because its background is itself a
color-mix(); _mixSrgb() reproduces the CSS mix so it can be measured
rather than approximated. Repointed .send-btn, .send-btn:hover,
.send-btn.newchat-mode:hover and .ge-ai-command-run at the matching
token; no rule now pairs --on-accent with a send-button background.

Adds tests/test_theme_accent_foreground_js.py, which runs the derivation
helpers under Node with a dark accent plus a light send-button override
and asserts the two foregrounds differ, that reusing the accent's
foreground would drop below 3:1, and that every surface clears 3:1
against its own background. A second test scans style.css so the
mismatch cannot reappear.
@YahyaZekry

Copy link
Copy Markdown
Author

Good catch — you're right, and the reproduction is exact. --on-accent was computed at theme.js:278 from colors.red, but the advanced overrides are applied further down in the same function, so --send-btn-bg / --send-btn-hover could land on a completely different colour while the foreground still reflected --red. With --red: #e06c75 and --send-btn-bg: #f2c14e the token stayed #fff at 1.68:1, exactly as you describe.

Pushed 00147e9 taking the first option — each independently configurable surface is now a coordinated background/foreground pair, derived after the overrides resolve:

token derived from
--on-accent --red (only consumers whose background really is --red)
--on-send-btn resolved --send-btn-bg
--on-send-btn-hover resolved --send-btn-hover
--on-send-btn-newchat-hover that hover colour mixed 85% into --panel

The new-chat state needed its own token because its background is itself a color-mix() rather than a flat variable, so I added a small _mixSrgb() that reproduces the CSS mix — that surface is measured rather than assumed. .send-btn, .send-btn:hover, .send-btn.newchat-mode:hover and .ge-ai-command-run now each point at the token matching their own background.

On coverage: tests/test_theme_accent_foreground_js.py runs the derivation helpers under Node with precisely the case you named — dark accent, light send-button override — and asserts (a) the accent and send-button foregrounds disagree, (b) reusing the accent's foreground on the send button drops below 3:1, and (c) every surface clears 3:1 against the background it actually renders. A second test scans style.css for any rule pairing --on-accent with a send-button background, so the mismatch can't quietly return. Both pass.

You're also right that the console snippet in the description only proved --red; I've left it in place but it's no longer the check that matters — the test is.

@RaresKeY

Copy link
Copy Markdown
Member

@YahyaZekry can you refresh screenshots?

@RaresKeY
RaresKeY dismissed their stale review August 19, 2026 12:57

resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

White button text fails WCAG contrast on most built-in themes

2 participants