opensource-joe opened a new pull request, #536: URL: https://github.com/apache/fineract-backoffice-ui/pull/536
## 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 - [x] I did not hand-edit generated files under `src/app/api/`. - [x] 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. - [x] User-facing strings use translation keys. No new strings. - [x] I added or updated tests appropriate to this change, or explained why tests were not needed. - [x] 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. - [x] Commits are signed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
