From bbb267aa83d475952f85c6e1ce7b51492961dea8 Mon Sep 17 00:00:00 2001 From: Roland Date: Sun, 2 Aug 2026 14:07:19 -0600 Subject: [PATCH] ui: the navbar measures its foreground instead of assuming one The bar's background is the one surface a user picks -- twelve colour names, which each theme resolves to twelve of its own colours. Forty-eight backgrounds, and the links and status icons were drawn on all of them in a single token, `--btn-text`, which is near-black in three of the four themes. Night's `--brown` is #451212. A near-black glyph on it is 1.33:1. Nine of night's twelve colours and seven of amber's sat below 4.5:1, and twelve of the forty-eight below 3:1 -- icons the same colour as the bar behind them. So the foreground is measured against the background the theme actually resolved, choosing between that theme's own `--btn-text`, `--black` and `--white`. Its own, not literal ones: a white pixel in night mode undoes the dark adaptation the theme exists to protect, so night's "white" is #ff8a8a. Measuring at runtime rather than shipping an `--on-red` token per colour also means the custom YAML themes still ahead get this for free. Nothing is below 3:1 any more and the worst case is 3.27:1, up from 1.33:1. No combination is worse than it was. Ten of the forty-eight are still under 4.5:1 and no foreground will lift them: a monochrome theme resolves all twelve names onto one ramp, and a mid-tone of a hue cannot carry text of that same hue. That shortfall is in the palette, so /theme-sample now draws all twelve bars with the measured ratio printed beside each one, calling out the ten -- it is a decision to make while looking at them. The WCAG primitives move to themes/contrast so theme code can measure a colour without importing Common.js, which pulls in half the component library. Common.js forwards `contrastRatio` with a delegating function rather than a bare re-export: six specs mock it with `{...jest.requireActual('./Common')}`, the spread invokes every getter, and those spreads run re-entrantly while the module is still evaluating -- a re-export getter fires before its import binding exists and takes the whole suite down. Tested: navColors.test.js measures all forty-eight against the palette parsed out of tokens.css, and holds Nav.js to the call by source since neither bar is reachable from a test that could measure it. ui-layout.cy.js measures the painted result in a real browser in all four themes and holds it against the number the component reports. Reverting to the fixed foreground reddens four jest cases and three cypress ones. 1115 jest / 62 suites, 177 in ui-layout.cy.js, clean npm run build. --- app/src/App.css | 6 +- app/src/components/Common.js | 59 ++----- app/src/components/Nav.js | 34 ++-- app/src/components/ThemeSamplePage.js | 64 +++++++ app/src/components/ui/ui-layout.cy.js | 127 ++++++++++++++ app/src/themes/contrast.ts | 72 ++++++++ app/src/themes/navColors.test.js | 244 ++++++++++++++++++++++++++ app/src/themes/navColors.ts | 150 ++++++++++++++++ 8 files changed, 690 insertions(+), 66 deletions(-) create mode 100644 app/src/themes/contrast.ts create mode 100644 app/src/themes/navColors.test.js create mode 100644 app/src/themes/navColors.ts diff --git a/app/src/App.css b/app/src/App.css index 094e7696..be8f7950 100644 --- a/app/src/App.css +++ b/app/src/App.css @@ -1,6 +1,6 @@ -/* Nav bar: a hand-rolled flexbox bar (was a Semantic Menu). The user-chosen - background color and the `--btn-text` foreground are set inline per theme; - this stylesheet only supplies structure and hover affordances. */ +/* Nav bar: a hand-rolled flexbox bar (was a Semantic Menu). The user-chosen background + and the foreground measured against it are set inline (see themes/navColors); this + stylesheet only supplies structure and hover affordances. */ .wrolpi-navbar { display: flex; align-items: stretch; diff --git a/app/src/components/Common.js b/app/src/components/Common.js index 217a2235..afc3b8a7 100644 --- a/app/src/components/Common.js +++ b/app/src/components/Common.js @@ -29,6 +29,7 @@ import { import {useBluetooth, useDesktop, useHotspot, useSearchDirectories, useSearchOrder, useThrottle, useVnc, useWROLMode} from "../hooks/customHooks"; import {Media, SettingsContext, StatusContext, ThemeContext} from "../contexts/contexts"; import {themeChoices} from "../themes/names"; +import {contrastRatio as measureContrast} from "../themes/contrast"; import {FilePreviewContext} from "./FilePreview"; import _ from "lodash"; import {killDownloads, startDownloads, unlockCookies} from "../api"; @@ -1575,52 +1576,28 @@ export const toLocaleString = (num, locale = 'en-US') => { return num.toLocaleString(locale); } -/** - * WCAG relative luminance: sRGB channels linearised first, then Rec. 709 weighted. +/* + * The WCAG primitives now live in themes/contrast, so the theme code can measure a colour + * without importing this module -- Common.js pulls in half the component library, and + * `themes/` importing it is how an import cycle starts. Forwarded because a good many call + * sites and tests already take `contrastRatio` from here. * - * The linearisation is the part that matters. Weighting the gamma-encoded bytes directly -- - * which is what this did before -- is a rough approximation that misjudges mid-tone blues and - * purples badly enough to pick the wrong text colour for them. - */ -function relativeLuminance(color) { - const rgb = (typeof color === 'string') ? hexToRGBArray(color) : color; - if (!rgb) { - return 0; - } - const linear = rgb.map(value => { - const channel = value / 255; - return channel <= 0.03928 ? channel / 12.92 : Math.pow((channel + 0.055) / 1.055, 2.4); - }); - return (0.2126 * linear[0]) + (0.7152 * linear[1]) + (0.0722 * linear[2]); -} - -/** WCAG contrast ratio between two colours, from 1:1 (identical) to 21:1 (black on white). */ -export function contrastRatio(a, b) { - const luminances = [relativeLuminance(a), relativeLuminance(b)].sort((x, y) => y - x); - return (luminances[0] + 0.05) / (luminances[1] + 0.05); -} - -/** - * A hex colour as [r, g, b], accepting both `#rrggbb` and the shorthand `#rgb`. + * It is forwarded by a function that delegates rather than by `export {contrastRatio}`, and + * that is load-bearing. A bare re-export compiles to a getter that dereferences the imported + * module, and six specs mock this module with `{...jest.requireActual('./Common')}` -- a + * spread invokes every getter, and those spreads run re-entrantly while Common.js is still + * evaluating (Common -> customHooks -> Common, both mocked that way). The getter fires + * before the import binding is assigned, throws `Cannot read properties of undefined`, and + * takes the whole suite with it. A function is read by the spread but not called, so it + * dereferences nothing until evaluation has finished. * - * The shorthand matters because tag colours are not only chosen in the colour picker: they - * live in the database and in a config file, and a config is something a user edits by hand + * The hex parser accepting the `#rgb` shorthand matters and is not incidental: tag colours + * are not only chosen in the colour picker, they live in a config file a user edits by hand * -- configs are the source of truth in WROLPi. Rejecting `#fff` left the luminance at zero, * so a near-white tag was treated as black and handed light text. */ -function hexToRGBArray(color) { - let hex = (typeof color === 'string' ? color : '').replace(/^#/, ''); - if (hex.length === 3) { - // #abc means #aabbcc, not #0a0b0c. - hex = hex.split('').map(digit => digit + digit).join(''); - } - if (!/^[0-9a-fA-F]{6}$/.test(hex)) { - console.error('Invalid hex color: ' + color); - return; - } - let rgb = []; - for (let i = 0; i <= 2; i++) rgb[i] = parseInt(hex.substr(i * 2, 2), 16); - return rgb; +export function contrastRatio(a, b) { + return measureContrast(a, b); } export const TAG_TEXT_DARK = '#000000'; diff --git a/app/src/components/Nav.js b/app/src/components/Nav.js index 212b19df..d0a78733 100644 --- a/app/src/components/Nav.js +++ b/app/src/components/Nav.js @@ -19,6 +19,7 @@ import {useReorganizationStatus} from "../contexts/FileWorkerStatusContext"; import {SearchIconButton} from "./Search"; import {HELP_VIEWER_URI, NAME, semanticUIColorMap} from "./Vars"; import {useOverflowNav} from "../hooks/useOverflowNav"; +import {defaultNavColor, navBarStyle, useNavColors} from "../themes/navColors"; import _ from "lodash"; function updateFavicon(colorName) { @@ -141,7 +142,7 @@ function NavIconWrapper({children}) { function useNavColorSetting() { // Use localstorage to avoid flickering navbar color on startup. const {settings} = React.useContext(SettingsContext); - const [navColor, setNavColor] = useLocalStorage('nav_color', 'violet'); + const [navColor, setNavColor] = useLocalStorage('nav_color', defaultNavColor); React.useEffect(() => { if (!_.isEmpty(settings)) { @@ -163,7 +164,12 @@ export function NavBar() { const wrolModeEnabled = useWROLMode(); const {status} = React.useContext(StatusContext); const navColor = useNavColorSetting(); - const wrolpiIcon = WROLPi Home Icon; + /* + * The bar's foreground is measured against the background the theme resolved for the + * user's colour, not taken from a single token. See themes/navColors. + */ + const navColors = useNavColors(navColor); + const wrolpiIcon =WROLPi Home Icon; const name = {NAME || wrolpiIcon}; const topNavText = wrolModeEnabled ? <>{name}  : name; @@ -362,15 +368,7 @@ export function NavBar() { return <> -