Skip to content

fix(a11y): meet WCAG AA contrast in the shipped default theme - #536

Merged
Aman-Mittal merged 1 commit into
apache:mainfrom
opensource-joe:fix/484-default-theme-contrast
Sep 10, 2026
Merged

fix(a11y): meet WCAG AA contrast in the shipped default theme#536
Aman-Mittal merged 1 commit into
apache:mainfrom
opensource-joe:fix/484-default-theme-contrast

Conversation

@opensource-joe

Copy link
Copy Markdown
Contributor

What and why

Six elements in the header and on the dashboard fail WCAG AA contrast in the shipped light theme. All six reproduce, and the fix completes a pattern already in the codebase rather than adding a new one.

Closes #484

Two things in the issue did not hold up when I checked them, and one changes the shape of the fix:

The values are not outside the branding system. The issue says they are hardcoded outside BRANDABLE_TOKENS and that a deployer cannot fix them. All six sites read var(--token) today, and all five distinct tokens (primary-color, primary-dark, error-color, warning-color, success-color) are already on the allow-list. branded-deployment.spec.ts asserts a deployment's palette reaches the chrome and passes. So there is no branding-coverage defect to fix.

One ratio is off. .system-info .value is given as 3.48:1 against #2c3e50. Its background is --hover-bg, rgba(44, 62, 80, 0.06), composited over --card-bg #ffffff, which resolves to #f2f3f5. Measured live it is 2.83:1. Looks like the rgba's own channels were used instead of compositing over the ancestor. The other five reproduce exactly.

What is actually going on is in the test suite. accessibility.spec.ts runs axe with wcag2aa, which includes color-contrast, and always has. Every scan is scoped to ion-card or an open modal, on /clients and /clients/create. The header is not inside an ion-card and /dashboard is never visited, so the rule has been running the whole time pointed somewhere it could not see the application chrome.

The fix

--primary-strong already carries the idea: its comment says --primary-color holds white at 3.15:1 and --primary-dark at 4.31:1, both short of AA. .tour-btn sets color: white on --primary-dark, which is exactly the case that comment warns about, so one of the six needs no new value at all.

Completing that pattern instead of patching six CSS sites:

  • Fills carrying white text, theme-independent, since a fill carries its own contrast: .tour-btn moves to --primary-strong; new --error-strong: #c0392b for .logout-btn.
  • Palette drawn as text on a surface, theme-scoped, because the requirement inverts: new --primary-text, --warning-text, --success-text. Light takes darkened values (#2471a3, reusing the --primary-strong value; #b45309, the value already proven for light in --guidance-highlight-color; #1e8449). Dark keeps the shipped palette, which already clears AA on #1e1e1e at 5.29, 7.60 and 7.93. Same reasoning already written down for --guidance-highlight-color.

One genuinely new hex across the change.

All four go on BRANDABLE_TOKENS, the three text ones on THEME_SCOPED, and error-strong on REQUIRES_WHITE_TEXT so an override of it gets the same 4.5:1 floor primary-strong gets. Without that last part a deployment that recolours the palette would find its own blue everywhere except the header title, which would introduce the coverage gap the issue was worried about rather than remove it.

Element Before After (light) After (dark) Needs
.app-title 3.15 5.30 5.29 4.5
.system-info .value 2.83 4.77 4.98 4.5
.tour-btn 4.30 5.30 5.30 4.5
.logout-btn 3.82 5.44 5.44 4.5
.widget-trend.highlight 2.19 5.02 7.60 4.5
.widget-value.healthy 2.10 4.72 7.93 3.0 (36px/700)

Happy to do this the other way if you would rather. The alternative is darkening the light-theme values of --primary-color, --warning-color and --success-color directly and adding no tokens. Smaller diff, but it recolours every accent, border, chart segment and ion-color-* that derives from them, including things that currently pass. I went with the tokens because it changes nothing that already works, but these become public API and it is your architecture, so say the word.

Verification

Mocked, on a dev server, Chromium at 1366x900. No real backend; this is presentational and none of it touches a request.

  • Negative control. With the new test in place and the colour changes reverted, it fails naming exactly those six with exactly the ratios above, and the other three a11y tests stay green. Restored: 4 passed.
  • Contrast measured live in both themes, computed foreground against the first opaque ancestor. Confirmed the theme-scoped tokens resolve to the bright values under [data-theme='dark']. Dark is verified by computed styles plus the WCAG formula; the suite scans the light theme only, as before.
  • Unit tests: 239 files, 1461 passed.
  • e2e mocked: accessibility.spec.ts and branded-deployment.spec.ts, 13 passed. The branded-deployment suite is the one that would catch a branding-coverage regression.
  • npm run lint and prettier --check src e2e clean.

About the new test

It needed a catch-all /api/v1/ mock, and that turned out to be the most important detail. Without one the dashboard widgets' requests fail, the app raises an error toast per failure, and the Ionic toast overlay covers the page. axe then reports color-contrast as incomplete for nearly every node, with "background color could not be determined because it is overlapped by another element", and returns an empty violations array. The test would have passed while looking at nothing.

So there is an expectScanWasNotBlind assertion: a green run with zero passes is the signature of that, and it now fails with a message naming the cause. The catch-all is registered first because Playwright matches routes in reverse registration order and a catch-all added last swallows the authentication and offices mocks.

app-sidebar is deliberately out of scope, with a comment saying why. It has two pre-existing blocking failures that are not this issue's: role-img-alt on around sixty ion-icon nav glyphs, the same Ionic behaviour already carried in CLIENT_LIST_BASELINE, and scrollable-region-focusable on the nav's scroll container. Baselining sixty selectors to make an unrelated area green seemed the wrong trade.

SHELL_BASELINE carries eleven role-img-alt entries for icons in the banner and dashboard cards, plus scrollable-region-focusable|main. That last one is a real keyboard-access finding on .content-area, unrelated to this change and true before it. Listed rather than hidden, and I am happy to open a separate issue for it and for the sidebar pair if useful.

Screenshots

Not added. The change is a set of measured contrast ratios rather than a layout change, and the numbers in the table are the evidence; a screenshot of slightly darker text is easy to misread as no change. Happy to add before/after captures if you would like them.

Checklist

  • I did not hand-edit generated files under src/app/api/.
  • New component or service code uses the adapter boundary in src/app/core/adapters/ instead of direct browser globals or imperative third-party APIs. No new component or service code; this is token definitions, six var() references and a test.
  • User-facing strings use translation keys. No new strings.
  • I added or updated tests appropriate to this change, or explained why tests were not needed.
  • UI workflow changes include suitable e2e coverage, including real-backend testing where relevant. Covered by the new mocked e2e scan; no workflow or request behaviour changes, so no real-backend testing.
  • Commits are signed.

Six elements in the header and on the dashboard fail AA contrast in the
light theme: the app title and system-info value at 3.15:1 and 2.83:1,
the Guide and Logout buttons at 4.30:1 and 3.82:1, and the dashboard
trend and health values at 2.19:1 and 2.10:1.

The palette is chosen for fills. A colour that reads well behind white
text is usually too light to read as words on white, which is why
--primary-strong already exists. Complete that pattern rather than
patching the six CSS sites: --error-strong for the remaining
white-on-fill case, and --primary-text, --warning-text and
--success-text for the palette drawn as text. The text tokens are
theme-scoped, because on the dark card the shipped values already clear
AA and darkening them fails.

All four are brandable, so a deployment that recolours the palette does
not find its blue everywhere except the header title, and --error-strong
carries the same white-text contrast floor as --primary-strong.

The suite has run color-contrast as part of wcag2aa the whole time, but
every scan is scoped to ion-card or an open modal on the client routes,
so it has never seen the chrome. Adds a scan of the banner and the
dashboard, which is what surfaces these six.

Closes apache#484
@Aman-Mittal
Aman-Mittal merged commit aeb667f into apache:main Sep 10, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Six shipped default-theme colours fail WCAG AA contrast, and they sit outside the branding token system that would catch them

2 participants