diff --git a/app/src/components/Nav.js b/app/src/components/Nav.js
index bc537cbc..7ea77209 100644
--- a/app/src/components/Nav.js
+++ b/app/src/components/Nav.js
@@ -208,7 +208,7 @@ export function NavBar() {
* user's colour, not taken from a single token. See themes/navColors.
*/
const navColors = useNavColors(navColor);
- const wrolpiIcon =
;
+ const wrolpiIcon = ;
const name = {NAME || wrolpiIcon};
const topNavText = wrolModeEnabled ? <>{name} : name;
diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js
index b42b711d..b64a7b87 100644
--- a/app/src/components/ui/ui-layout.cy.js
+++ b/app/src/components/ui/ui-layout.cy.js
@@ -2199,8 +2199,13 @@ describe('the navigation bar is legible on every colour a user can pick', () =>
* The inverse. Every case above would pass on the old fixed foreground in the two
* themes where `--btn-text` happens to be the right end of the palette, so without
* this the suite could go green on a revert. Night is where the difference is
- * largest: `--brown` takes `--white` (#ff8a8a) and `--red` keeps `--btn-text`, so
- * the bar's foreground is demonstrably not one value.
+ * largest: `--brown` takes `--white` and `--red` keeps `--btn-text`, so the bar's
+ * foreground is demonstrably not one value.
+ *
+ * `--btn-text` is READ from the theme rather than written out as a hex. The unit
+ * tests parse tokens.css precisely so a palette edit cannot leave a stale claim
+ * behind, and a literal here would be the one place that rule was broken -- it would
+ * either go stale or quietly stop testing anything if night's token moved.
*/
cy.mountUI(
@@ -2211,11 +2216,19 @@ describe('the navigation bar is legible on every colour a user can pick', () =>
cy.get('[data-nav-sample="red"] .wrolpi-navbar-link').then(($red) => {
const brown = getComputedStyle($brown[0]).color;
const red = getComputedStyle($red[0]).color;
+ const btnText = getComputedStyle(document.documentElement)
+ .getPropertyValue('--btn-text');
+
+ // The premise: the token resolved. `toRgb` throws on an unresolved value
+ // rather than letting the comparison below pass by comparing nothing.
+ expect(toRgb(btnText), 'night --btn-text resolved').to.match(/^rgb/);
expect(brown, 'brown and red bars take different foregrounds')
.to.not.equal(red);
// And the one that changed is the one that was unreadable.
expect(toRgb(brown), 'brown no longer takes --btn-text')
- .to.not.equal(toRgb('#0a0000'));
+ .to.not.equal(toRgb(btnText));
+ // While red, which was always legible, still does.
+ expect(toRgb(red), 'red still takes --btn-text').to.equal(toRgb(btnText));
});
});
});
diff --git a/app/src/themes/contrast.ts b/app/src/themes/contrast.ts
index 344e6ea9..0516ad69 100644
--- a/app/src/themes/contrast.ts
+++ b/app/src/themes/contrast.ts
@@ -8,14 +8,14 @@
* its existing callers are unaffected.
*/
-/** A hex colour as [r, g, b], accepting both `#rrggbb` and the shorthand `#rgb`. */
+/** A hex colour as [r, g, b], accepting `#rgb`, `#rrggbb`, and `#rrggbbaa`. */
export function hexToRGBArray(color: string): number[] | undefined {
let hex = (typeof color === 'string' ? color : '').trim().replace(/^#/, '');
- if (hex.length === 3) {
- // #abc means #aabbcc, not #0a0b0c.
+ if (hex.length === 3 || hex.length === 4) {
+ // #abc means #aabbcc, not #0a0b0c. A fourth digit is alpha; see parseColor.
hex = hex.split('').map(digit => digit + digit).join('');
}
- if (!/^[0-9a-fA-F]{6}$/.test(hex)) {
+ if (!/^[0-9a-fA-F]{6}([0-9a-fA-F]{2})?$/.test(hex)) {
return undefined;
}
const rgb: number[] = [];
@@ -23,6 +23,46 @@ export function hexToRGBArray(color: string): number[] | undefined {
return rgb;
}
+/**
+ * Any colour these measurements can actually read, as [r, g, b] — or undefined.
+ *
+ * Undefined matters as much as the value. `relativeLuminance` used to return 0 for
+ * everything it could not parse, which is indistinguishable from black: a token written as
+ * `rgb(90, 79, 168)` measured as pure black, so the "best" foreground was chosen against a
+ * colour the theme does not contain, and the wrong one was frozen into the bar's inline
+ * style. A caller that cannot tell "unreadable" from "black" cannot fall back, and the
+ * custom YAML themes ahead of us are exactly where a non-hex token will first appear.
+ *
+ * Alpha is deliberately dropped rather than rejected: WCAG contrast is defined on composited
+ * colour, and what a translucent value composites over is not knowable here. Every token in
+ * tokens.css is opaque, so this only arises for a hand-written theme, where measuring the
+ * colour itself is a better answer than refusing.
+ */
+export function parseColor(color: string): number[] | undefined {
+ const value = (typeof color === 'string' ? color : '').trim();
+ if (!value) return undefined;
+ if (value.startsWith('#')) return hexToRGBArray(value);
+
+ // `rgb(90 79 168 / 0.5)` and `rgba(90, 79, 168, 0.5)` are both current syntax.
+ const functional = value.match(/^rgba?\(([^)]*)\)$/i);
+ if (functional) {
+ const parts = functional[1].split(/[\s,/]+/).filter(Boolean).slice(0, 3);
+ if (parts.length !== 3) return undefined;
+ const channels = parts.map((part) => {
+ // A percentage is 0-100 of full scale; a bare number is already 0-255.
+ const numeric = parseFloat(part);
+ if (Number.isNaN(numeric)) return NaN;
+ return part.includes('%') ? (numeric / 100) * 255 : numeric;
+ });
+ if (channels.some(Number.isNaN)) return undefined;
+ return channels.map(channel => Math.min(255, Math.max(0, channel)));
+ }
+
+ // A named colour, `hsl()`, `color-mix()`, or an unresolved `var()`. Not measurable
+ // here, and saying so is the point.
+ return undefined;
+}
+
/**
* WCAG relative luminance: sRGB channels linearised first, then Rec. 709 weighted.
*
@@ -31,8 +71,11 @@ export function hexToRGBArray(color: string): number[] | undefined {
* the wrong text colour for them.
*/
export function relativeLuminance(color: string | number[]): number {
- const rgb = (typeof color === 'string') ? hexToRGBArray(color) : color;
+ const rgb = (typeof color === 'string') ? parseColor(color) : color;
if (!rgb) {
+ // Zero is black's luminance, so this is a lie -- but the signature has no way to
+ // say otherwise, and every caller that needs to distinguish the two asks
+ // `isMeasurable` first.
return 0;
}
const linear = rgb.map(value => {
@@ -48,6 +91,9 @@ export function contrastRatio(a: string | number[], b: string | number[]): numbe
return (luminances[0] + 0.05) / (luminances[1] + 0.05);
}
+/** Whether a colour can be measured at all, as distinct from measuring as black. */
+export const isMeasurable = (color: string): boolean => parseColor(color) !== undefined;
+
/**
* Whichever candidate actually reads best on `background`.
*
diff --git a/app/src/themes/navColors.test.js b/app/src/themes/navColors.test.js
index 4cad4dc5..acd69fa8 100644
--- a/app/src/themes/navColors.test.js
+++ b/app/src/themes/navColors.test.js
@@ -1,6 +1,6 @@
import fs from 'fs';
import path from 'path';
-import {contrastRatio} from './contrast';
+import {contrastRatio, isMeasurable} from './contrast';
import {defaultNavColor, navColorNames, navColorsFrom} from './navColors';
import {themeNames} from './names';
import {semanticUIColorMap} from '../components/Vars';
@@ -150,11 +150,11 @@ describe('the navbar foreground', () => {
describe('what measurement cannot fix', () => {
/*
- * Eleven of the forty-eight combinations still sit under 4.5:1, and no choice of
+ * Ten of the forty-eight combinations still sit under 4.5:1, and no choice of
* foreground will lift them, because the shortfall is in the BACKGROUND.
*
* A monochrome theme resolves all twelve colour names onto one red (or amber) ramp, and
- * seven of night's land in the middle of it -- #b03030 is 2.91:1 from night's darkest red
+ * six of night's land in the middle of it -- #b03030 is 3.27:1 from night's darkest red
* and 2.52:1 from its brightest, so the best available option is the one this picks and
* it is still short. There is no foreground in a one-hue palette that reads on a
* mid-tone of that same hue; the fix, if the residue matters, is to the palette.
@@ -242,3 +242,85 @@ describe('when the tokens cannot be read', () => {
.toEqual(paletteOf('light')[`--${defaultNavColor}`]);
});
});
+
+describe('a token this cannot measure', () => {
+ /*
+ * Every token in tokens.css is a hex, so this is about the custom YAML themes ahead of
+ * us -- the first place a palette will be written by hand, and the first place something
+ * other than a hex will appear.
+ *
+ * The failure it guards was silent and worse than doing nothing. An unparseable colour
+ * measured as luminance zero, indistinguishable from black, so the bar chose its
+ * foreground against a colour the theme does not contain and then froze the wrong hex
+ * into an inline style, where CSS could no longer correct it. Falling back leaves the
+ * bar painted by CSS: wrong about the foreground at worst, rather than about both.
+ */
+ const hexPalette = () => paletteOf('light');
+
+ /** The light palette with one token rewritten into some other notation. */
+ const readerWith = (overrides) => {
+ const palette = {...hexPalette(), ...overrides};
+ return (token) => palette[token] || '';
+ };
+
+ it('measures rgb() as readily as hex, since both are colours', () => {
+ // `--violet` is #5c4fa8 in light. The same colour, written the other way, must
+ // produce the same answer rather than falling back.
+ const asHex = navColorsFrom(readerFor('light'), 'violet');
+ const asRgb = navColorsFrom(readerWith({'--violet': 'rgb(92, 79, 168)'}), 'violet');
+
+ expect(asRgb.color).toEqual(asHex.color);
+ expect(asRgb.ratio).toBeCloseTo(asHex.ratio, 4);
+ });
+
+ it('handles the modern space-separated form too', () => {
+ const asHex = navColorsFrom(readerFor('light'), 'violet');
+ const asRgb = navColorsFrom(readerWith({'--violet': 'rgb(92 79 168 / 1)'}), 'violet');
+
+ expect(asRgb.ratio).toBeCloseTo(asHex.ratio, 4);
+ });
+
+ it('falls back rather than measuring a background it cannot read', () => {
+ // `color-mix` is the shape a hand-written theme is most likely to reach for, and
+ // resolving it needs a browser.
+ const colors = navColorsFrom(
+ readerWith({'--olive': 'color-mix(in srgb, #6f7a24 60%, white)'}), 'olive');
+
+ expect(colors).toEqual({
+ background: 'var(--olive)', color: 'var(--btn-text)', ratio: null,
+ });
+ });
+
+ it('falls back when no candidate foreground can be read', () => {
+ const colors = navColorsFrom(readerWith({
+ '--btn-text': 'hsl(0 0% 100%)', '--black': 'rebeccapurple', '--white': 'hsl(0 0% 0%)',
+ }), 'violet');
+
+ expect(colors.ratio).toBeNull();
+ });
+
+ it('still measures when only SOME candidates are unreadable', () => {
+ /*
+ * The narrow case, and the reason the unreadable ones are filtered out rather than
+ * the whole measurement abandoned. A theme that writes one token oddly should not
+ * lose the measurement for the other two.
+ */
+ const colors = navColorsFrom(
+ readerWith({'--white': 'oklch(0.9 0.1 20)'}), 'violet');
+
+ expect(colors.ratio).not.toBeNull();
+ expect([hexPalette()['--btn-text'], hexPalette()['--black']]).toContain(colors.color);
+ });
+
+ it('is a real constraint: an unreadable colour is NOT treated as black', () => {
+ /*
+ * Without this the fallback could be removed and every case above would still pass
+ * on `relativeLuminance` returning zero -- which is precisely the bug, because zero
+ * is a legitimate luminance and nothing downstream could tell the two apart.
+ */
+ expect(contrastRatio('color-mix(in srgb, red, blue)', '#ffffff'))
+ .toBeCloseTo(contrastRatio('#000000', '#ffffff'), 4);
+ expect(isMeasurable('color-mix(in srgb, red, blue)')).toBe(false);
+ expect(isMeasurable('#000000')).toBe(true);
+ });
+});
diff --git a/app/src/themes/navColors.ts b/app/src/themes/navColors.ts
index 63713182..eb53a5c8 100644
--- a/app/src/themes/navColors.ts
+++ b/app/src/themes/navColors.ts
@@ -1,5 +1,5 @@
import React from 'react';
-import {bestForeground, contrastRatio} from './contrast';
+import {bestForeground, contrastRatio, isMeasurable} from './contrast';
/*
@@ -23,8 +23,31 @@ import {bestForeground, contrastRatio} from './contrast';
* night's "white" is #ff8a8a, a bright red, and amber's is #ffc875.
*
* Measuring at runtime rather than shipping a paired `--on-red`/`--on-olive` token for every
- * colour also means the custom YAML themes still ahead of us get this for free: a theme
- * supplies a palette, and the correct foreground for each entry falls out of it.
+ * colour also means a theme only has to supply a palette: the correct foreground for each
+ * entry falls out of it, with nothing extra to declare and nothing to keep in step.
+ *
+ * What is NOT measured, and should be: the warning indicators. NavBar still picks their
+ * colour from `conflictingColors`, a hardcoded list of navbar names that swaps `yellow`/`red`
+ * for `null`/`black` on the five bars those hues would blend into. It is the same defect
+ * this replaced -- a name list standing in for a measurement -- and it is worse than the one
+ * fixed here rather than better: `--danger` and `--warning` measure 1.0-1.8:1 against the bar
+ * in EVERY theme, light and dark included, because a red icon on a red bar is a red icon on a
+ * red bar. It is left alone deliberately, because the fix is not a better colour. Those
+ * icons have to stay distinguishable from each other AND from the bar's own text, and in a
+ * monochrome theme there is no third brightness to spend, so severity has to stop being a
+ * hue -- a filled badge, or a shape. That is a redesign of how severity reads, not a
+ * substitution, and it is not this change.
+ *
+ * /theme-sample draws the indicators as they are, taking the measured bar colour, which is
+ * what the bar does when nothing is wrong. It does not stage a fake overheating Pi to
+ * exhibit a fault nobody has agreed how to fix.
+ *
+ * Two limits on the measuring that IS done here, both of which the custom YAML themes will
+ * meet before anything else does. A token has to be a colour these measurements can read -- hex or `rgb()`; anything
+ * else falls back to CSS rather than being measured (see navColorsFrom). And re-measurement
+ * is triggered by `data-theme` changing, which is how a theme is applied today; a future
+ * editor that patches custom properties in place without touching the attribute would leave
+ * the bar on the values it last measured (see useNavColors).
*/
/**
@@ -73,17 +96,28 @@ export function navColorsFrom(read: (name: string) => string, navColor: string):
*/
const candidates = [read('--btn-text'), read('--black'), read('--white')].filter(Boolean);
- if (!background || candidates.length === 0) {
+ const measurable = candidates.filter(isMeasurable);
+
+ if (!isMeasurable(background) || measurable.length === 0) {
/*
- * No stylesheet to read: jsdom, or a very early render. Fall back to the tokens
- * themselves so the bar is still painted by CSS and still changes with the theme --
- * this is the pre-measurement behaviour, and it is only ever wrong about the
- * foreground.
+ * Nothing readable to measure, so measure nothing and let CSS do its job.
+ *
+ * Two cases reach here. The ordinary one is that there is no stylesheet yet -- jsdom,
+ * or a very early render -- and the reader returns empty strings. The other is a
+ * token this cannot parse: a hand-written theme writing `rgb(90 79 168)` or
+ * `color-mix(...)` rather than a hex. That one used to be silent and worse than
+ * useless: an unparseable colour measured as luminance zero, indistinguishable from
+ * black, so the bar picked its foreground against a colour the theme does not contain
+ * and then froze the wrong hex into an inline style, where CSS could not correct it.
+ *
+ * Falling back to the tokens themselves is the pre-measurement behaviour: the bar is
+ * painted by CSS, follows the theme, and is only ever wrong about the foreground --
+ * which is a great deal better than being confidently wrong about both.
*/
return {background: `var(--${name})`, color: 'var(--btn-text)', ratio: null};
}
- const color = bestForeground(background, candidates) as string;
+ const color = bestForeground(background, measurable) as string;
return {background, color, ratio: contrastRatio(color, background)};
}
@@ -126,6 +160,13 @@ export function navBarStyle(colors: NavColors): React.CSSProperties {
*
* The attribute is also stamped by the inline script in index.html before React loads, and
* that is a change no context can report at all.
+ *
+ * This assumes token VALUES only change when `data-theme` does, which holds for every way a
+ * theme is applied today. It will not hold for a custom-theme editor that rewrites custom
+ * properties in place: the measured pair is a snapshot of hexes, so the bar would keep the
+ * old one until the theme name or the chosen colour changed. When that editor exists it
+ * should either re-stamp the attribute or announce the change for this to observe -- there
+ * is no event for "a custom property was assigned" to watch instead.
*/
export function useNavColors(navColor: string): NavColors {
const [colors, setColors] = React.useState(