From ecf611931bf8539ca974bc73ffcf952d9a139abc Mon Sep 17 00:00:00 2001
From: Roland <roland@learningselfreliance.com>
Date: Thu, 30 Jul 2026 19:46:10 -0600
Subject: [PATCH] ui: tags look like tags, and stay readable in every theme

Four things, three cosmetic and one a real legibility bug.

* Tags were square chips.  They now have a pointed left edge and an
  eyelet, like a tag tied to a garment -- the shape Semantic gave them,
  and the reason a tag reads as a tag rather than as a small button.  The
  point is a square rotated behind the left edge, sized so its diagonal
  equals the body height and its two edges meet the body at the corners.

  Only where something is actually a tag.  `Label` carries the count
  badges in the Tags table too -- file, zim, channel and domain counts --
  and a point on a number reads as an arrow, so the shape is behind a
  `tag` prop and `NameToTagLabel` opts in.

* They were smaller than what they replaced: 11.5px text with 3px of
  padding, against Semantic's 12px and ~7px.  Now 12.5px and 5px, which
  also keeps them a reasonable thing to tap.

* Wrapped rows of tags sat on the row above, so they carry a bottom
  margin, plus a left margin for the point to occupy.

* The text colour was unreadable in night and wrong in amber.

Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank.  Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured.  Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.

The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override.  There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.

`.wrolpi-label-white` and `-black` moved to the same variable.  Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.

`contrastingColor` was also picking the worse of its two options.  It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1.  Every
mid-tone blue, teal and purple a user might pick sat in that band.  It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.

Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name.  That is how a tag could be unreadable
without a single test noticing.  Two of these were verified red against
the inline `color`.

`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible.  Twelve cases, three
tag colours across four themes, at WCAG AA.  Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.

The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it.  It has eleven now, spanning
the range where the black-or-light decision flips.
---
 app/cypress/support/component.js      |  42 ++++++++++
 app/src/Tags.js                       |  14 +++-
 app/src/Tags.test.js                  | 115 ++++++++++++++++++++++++++
 app/src/components/Common.js          |  41 ++++++++-
 app/src/components/Common.test.js     |  54 +++++++++++-
 app/src/components/ThemeSamplePage.js |  33 ++++++++
 app/src/components/ui/Feedback.tsx    |  13 ++-
 7 files changed, 301 insertions(+), 11 deletions(-)
 create mode 100644 app/src/Tags.test.js

diff --git a/app/cypress/support/component.js b/app/cypress/support/component.js
index dcb3f7b8..910ae720 100644
--- a/app/cypress/support/component.js
+++ b/app/cypress/support/component.js
@@ -116,6 +116,48 @@ Cypress.Commands.add('shouldNotOverlap', {prevSubject: false}, (firstSelector, s
     });
 });
 
+/**
+ * Contrast ratio between an element's text and the surface actually behind it, per WCAG.
+ *
+ * "The surface actually behind it" is the point.  A transparent element -- which is what a
+ * tag becomes in night mode -- has no background of its own to compare against, so a naive
+ * check reads `rgba(0,0,0,0)` and concludes anything is legible.  This walks up to the first
+ * ancestor that paints something, which is what the eye sees.
+ */
+const parseRgb = (value) => {
+    const parts = (value.match(/[\d.]+/g) || []).map(Number);
+    return {r: parts[0], g: parts[1], b: parts[2], a: parts.length > 3 ? parts[3] : 1};
+};
+
+const relativeLuminance = ({r, g, b}) => {
+    const channel = (v) => {
+        const s = v / 255;
+        return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4);
+    };
+    return 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b);
+};
+
+const effectiveBackground = (el) => {
+    let node = el;
+    while (node && node !== document.documentElement) {
+        const bg = parseRgb(getComputedStyle(node).backgroundColor);
+        if (bg.a > 0) return bg;
+        node = node.parentElement;
+    }
+    return parseRgb(getComputedStyle(document.documentElement).backgroundColor);
+};
+
+Cypress.Commands.add('contrastRatio', {prevSubject: false}, (selector) => {
+    return cy.get(selector).then(($el) => {
+        const el = $el[0];
+        const text = relativeLuminance(parseRgb(getComputedStyle(el).color));
+        const behind = relativeLuminance(effectiveBackground(el));
+        const lighter = Math.max(text, behind);
+        const darker = Math.min(text, behind);
+        return (lighter + 0.05) / (darker + 0.05);
+    });
+});
+
 Cypress.Commands.add('mountWithTags', (component, options) => {
     cy.fixture('tagsMock.json').then((mockTags) => {
         cy.intercept('GET', '**/api/tag', {
diff --git a/app/src/Tags.js b/app/src/Tags.js
index 585feac9..3c0e1925 100644
--- a/app/src/Tags.js
+++ b/app/src/Tags.js
@@ -111,18 +111,26 @@ export function useTags() {
         if (tag !== null && tag !== undefined) {
             const tagColor = tag['color'] || DEFAULT_TAG_COLOR;
             const textColor = contrastingColor(tagColor);
-            const className = ['wrolpi-label', props.onClick ? 'clickable' : ''].filter(Boolean).join(' ');
+            const className = ['wrolpi-label', 'wrolpi-tag', props.onClick ? 'clickable' : '']
+                .filter(Boolean).join(' ');
             return <span
                 {...props} // onClick passed here.
                 className={className}
-                style={{...props['style'], ['--label-color']: tagColor, color: textColor}}
+                /*
+                 * The contrasting text colour goes in a custom property, not in `color`.  As
+                 * `color` it was an inline declaration, which no stylesheet rule can outrank --
+                 * so night, where a tag becomes an outline over a near-black page, still got
+                 * the black text calculated for a bright tag's fill.  The tag was legible only
+                 * by its border.  A custom property lets each theme decide instead.
+                 */
+                style={{...props['style'], ['--label-color']: tagColor, ['--label-text']: textColor}}
             >
                 {name}
             </span>;
         }
 
         // No tags have been fetched.
-        return <Label>{name}</Label>;
+        return <Label tag>{name}</Label>;
     }
 
     const TagsGroup = ({tagNames, onClick}) => {
diff --git a/app/src/Tags.test.js b/app/src/Tags.test.js
new file mode 100644
index 00000000..ba4c108b
--- /dev/null
+++ b/app/src/Tags.test.js
@@ -0,0 +1,115 @@
+import React from 'react';
+import {screen, waitFor} from '@testing-library/react';
+import {render} from './test-utils';
+import {queryContextFixture} from './test-fixtures';
+import {QueryContext} from './contexts/contexts';
+import {useTags} from './Tags';
+
+/*
+ * Tests for the real tag chip.
+ *
+ * Everything else in the suite takes `NameToTagLabel` from `tagsContextFixture`, which stubs
+ * it as a function returning the name -- so nothing exercised the component that actually
+ * paints a tag.  That is how a tag came to be unreadable in night mode without a single test
+ * noticing: the colours are decided here, in the one place in the app that paints with a
+ * value no theme chose.
+ */
+
+jest.mock('./api', () => ({
+    getTags: jest.fn(),
+    getRecentTags: jest.fn(),
+    saveTag: jest.fn(),
+    deleteTag: jest.fn(),
+}));
+
+import {getTags, getRecentTags} from './api';
+
+const BRIGHT = '#ffe066';   // contrastingColor -> black
+const DARK = '#1a1a2e';     // contrastingColor -> light
+
+// A harness that renders the real NameToTagLabel out of the real hook.
+const TagProbe = ({name}) => {
+    const {NameToTagLabel} = useTags();
+    return <NameToTagLabel name={name}/>;
+};
+
+const renderTag = (name, tags) => render(
+    <QueryContext.Provider value={queryContextFixture()}>
+        <TagProbe name={name}/>
+    </QueryContext.Provider>
+);
+
+const tagOf = (name) => screen.getByText(name);
+
+/*
+ * Wait for the fetched colours to arrive.  Before they do, `NameToTagLabel` falls back to a
+ * plain `<Label tag>` -- which also carries `wrolpi-tag`, so waiting on the class alone
+ * passes against the fallback and asserts nothing about the real chip.  Wait for the user's
+ * colour instead.
+ */
+const awaitColoured = async (name, color) => {
+    await waitFor(() =>
+        expect(tagOf(name).style.getPropertyValue('--label-color')).toBe(color));
+    return tagOf(name);
+};
+
+describe('a tag chip', () => {
+    beforeEach(() => {
+        getTags.mockResolvedValue({
+            tags: [
+                {id: 1, name: 'Water', color: BRIGHT},
+                {id: 2, name: 'Archived', color: DARK},
+                {id: 3, name: 'Uncoloured', color: null},
+            ],
+        });
+        getRecentTags.mockResolvedValue({tags: []});
+    });
+
+    it('carries the tag shape, not just the chip shape', async () => {
+        renderTag('Water');
+
+        const tag = await awaitColoured('Water', BRIGHT);
+        expect(tag).toHaveClass('wrolpi-tag');
+        expect(tag).toHaveClass('wrolpi-label');
+    });
+
+    it('puts the calculated text colour in a variable, never in `color`', async () => {
+        /*
+         * This is the whole defect.  As an inline `color` declaration nothing in a stylesheet
+         * could outrank it, so night -- which turns a tag into an outline over a near-black
+         * page -- was still painting the black text that had been calculated for a bright
+         * yellow fill.  The tag was legible only by its border.
+         */
+        renderTag('Water');
+
+        const tag = await awaitColoured('Water', BRIGHT);
+        expect(tag.style.getPropertyValue('--label-text')).toBe('#000000');
+        expect(tag.style.color).toBe('');
+    });
+
+    it('calculates light text for a dark tag', async () => {
+        renderTag('Archived');
+
+        const tag = await awaitColoured('Archived', DARK);
+        const text = tag.style.getPropertyValue('--label-text');
+        // Non-empty first: an absent variable would satisfy "not black" for free.
+        expect(text).toBeTruthy();
+        expect(text).not.toBe('#000000');
+    });
+
+    it('falls back to a default colour rather than leaving the fill unset', async () => {
+        renderTag('Uncoloured');
+
+        // The tag exists with no colour of its own, so the default stands in.
+        await waitFor(() =>
+            expect(tagOf('Uncoloured').style.getPropertyValue('--label-color')).toBeTruthy());
+    });
+
+    it('still shows the name before any tags have been fetched', async () => {
+        // The colours are not known yet; the word matters more than the colour.
+        getTags.mockResolvedValue({tags: []});
+        renderTag('Water');
+
+        expect(await screen.findByText('Water')).toBeInTheDocument();
+    });
+});
diff --git a/app/src/components/Common.js b/app/src/components/Common.js
index ed62c693..e7bf603d 100644
--- a/app/src/components/Common.js
+++ b/app/src/components/Common.js
@@ -1529,9 +1529,29 @@ export const toLocaleString = (num, locale = 'en-US') => {
     return num.toLocaleString(locale);
 }
 
-function luma(color) {
-    let rgb = (typeof color === 'string') ? hexToRGBArray(color) : color;
-    return (0.2126 * rgb[0]) + (0.7152 * rgb[1]) + (0.0722 * rgb[2]); // SMPTE C, Rec. 709 weightings
+/**
+ * WCAG relative luminance: sRGB channels linearised first, then Rec. 709 weighted.
+ *
+ * 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);
 }
 
 function hexToRGBArray(color) {
@@ -1547,8 +1567,21 @@ function hexToRGBArray(color) {
     return rgb;
 }
 
+export const TAG_TEXT_DARK = '#000000';
+export const TAG_TEXT_LIGHT = '#dddddd';
+
+/**
+ * Dark or light text, whichever a user's chosen tag colour actually reads better against.
+ *
+ * Measures both instead of comparing a brightness figure to a threshold.  The threshold
+ * version put light text on Semantic's blue (`#2185d0`) at 2.9:1 when dark text on the same
+ * fill gives 5.3:1 -- it had picked the worse of the only two options.  Every mid-tone blue,
+ * teal and purple a user might choose sat in that band.
+ */
 export function contrastingColor(color) {
-    return (luma(color) >= 120) ? '#000000' : '#dddddd';
+    return contrastRatio(TAG_TEXT_DARK, color) >= contrastRatio(TAG_TEXT_LIGHT, color)
+        ? TAG_TEXT_DARK
+        : TAG_TEXT_LIGHT;
 }
 
 export const encodeMediaPath = (path) => {
diff --git a/app/src/components/Common.test.js b/app/src/components/Common.test.js
index a3c01a04..f08ca07f 100644
--- a/app/src/components/Common.test.js
+++ b/app/src/components/Common.test.js
@@ -1,7 +1,9 @@
 import React from 'react';
 import {act, render, screen, waitFor} from '../test-utils';
 import userEvent from '@testing-library/user-event';
-import {DirectorySearch, SearchResultsInput} from './Common';
+import {
+    contrastingColor, contrastRatio, DirectorySearch, SearchResultsInput, TAG_TEXT_DARK, TAG_TEXT_LIGHT,
+} from './Common';
 
 // Mock debounce to make tests run faster
 jest.mock('lodash/debounce', () => jest.fn(fn => {
@@ -479,3 +481,53 @@ describe('SearchResultsInput', () => {
         expect(screen.getByText('rendered:Cooking')).toBeInTheDocument();
     });
 });
+
+describe('contrastingColor', () => {
+    /*
+     * This decides the text colour on every tag, against a fill the user picked, so it is the
+     * one place in the app where legibility is computed rather than designed.
+     */
+
+    it('picks whichever of the two options actually reads better', () => {
+        // Semantic's blue.  The old threshold check chose light text here, at 2.9:1, when dark
+        // text on the same fill gives 5.3:1 -- the worse of the only two available answers.
+        // Every mid-tone blue, teal and purple a user might choose sat in that band.
+        const blue = '#2185d0';
+
+        const chosen = contrastingColor(blue);
+        const rejected = chosen === TAG_TEXT_DARK ? TAG_TEXT_LIGHT : TAG_TEXT_DARK;
+
+        expect(contrastRatio(chosen, blue)).toBeGreaterThan(contrastRatio(rejected, blue));
+        expect(chosen).toBe(TAG_TEXT_DARK);
+    });
+
+    it('never picks the worse option, for any colour', () => {
+        // The property, rather than a list of examples: whatever it returns must be at least as
+        // legible as the alternative would have been.
+        const colours = [
+            '#000000', '#ffffff', '#f2f2f2', '#1b1c1d', '#2185d0', '#21ba45', '#db2828',
+            '#fbbd08', '#6435c9', '#a5673f', '#00b5ad', '#e03997', '#b5cc18', '#767676',
+            '#808080', '#7f7f7f',
+        ];
+
+        for (const colour of colours) {
+            const chosen = contrastingColor(colour);
+            const rejected = chosen === TAG_TEXT_DARK ? TAG_TEXT_LIGHT : TAG_TEXT_DARK;
+            expect(contrastRatio(chosen, colour))
+                .toBeGreaterThanOrEqual(contrastRatio(rejected, colour));
+        }
+    });
+
+    it('returns one of the two text colours and nothing else', () => {
+        // It is written into `--label-text`; anything undefined leaves the tag unstyled.
+        for (const colour of ['#000000', '#ffffff', '#2185d0']) {
+            expect([TAG_TEXT_DARK, TAG_TEXT_LIGHT]).toContain(contrastingColor(colour));
+        }
+    });
+
+    it('measures contrast on the WCAG scale', () => {
+        // Anchors the ratio itself, so "better" above is comparing real numbers.
+        expect(contrastRatio('#000000', '#ffffff')).toBeCloseTo(21, 1);
+        expect(contrastRatio('#ffffff', '#ffffff')).toBeCloseTo(1, 5);
+    });
+});
diff --git a/app/src/components/ThemeSamplePage.js b/app/src/components/ThemeSamplePage.js
index d191974d..730186e4 100644
--- a/app/src/components/ThemeSamplePage.js
+++ b/app/src/components/ThemeSamplePage.js
@@ -36,6 +36,7 @@ import {
     toast,
 } from './ui';
 import {semanticColorNames} from '../themes/mantine';
+import {contrastingColor} from './Common';
 
 /*
  * A gallery of every component in the library, in the current theme.
@@ -389,6 +390,38 @@ export function ThemeSamplePage() {
             </Panel>
         </Section>
 
+        <Section label='Tags'>
+            <Panel>
+                {/*
+                  * Tag colours are the user's, stored per tag, so these are raw hex values --
+                  * the only place in the app that paints with something no theme chose.  That
+                  * is exactly why they belong here: the gallery had label swatches in theme
+                  * colours and no user-coloured tag, so nobody saw that night was calculating
+                  * the text colour against a fill it had already thrown away.
+                  *
+                  * Deliberately spans the range: near-white and near-black fills, where the
+                  * black-or-light text decision flips, plus enough tags to wrap a row.
+                  */}
+                <Row>
+                    {[
+                        ['Water', '#2185d0'], ['Food', '#21ba45'], ['Medical', '#db2828'],
+                        ['Shelter', '#a5673f'], ['Power', '#fbbd08'], ['Comms', '#6435c9'],
+                        ['Reference', '#f2f2f2'], ['Archived', '#1b1c1d'],
+                        ['Seeds', '#b5cc18'], ['Tools', '#00b5ad'], ['Navigation', '#e03997'],
+                    ].map(([name, color]) => <span
+                        key={name}
+                        className='wrolpi-label wrolpi-tag'
+                        style={{'--label-color': color, '--label-text': contrastingColor(color)}}
+                    >{name}</span>)}
+                </Row>
+                <p style={{marginTop: 14, fontSize: 13, color: 'var(--muted)'}}>
+                    Every tag must be readable in all four themes. In night and amber the
+                    user's colour is discarded, so all tags look alike — that is the same
+                    trade the labels above make, and it is deliberate.
+                </p>
+            </Panel>
+        </Section>
+
         <Section label='Icons'>
             <Panel>
                 <Row>
diff --git a/app/src/components/ui/Feedback.tsx b/app/src/components/ui/Feedback.tsx
index 9ba5c043..1b91faf3 100644
--- a/app/src/components/ui/Feedback.tsx
+++ b/app/src/components/ui/Feedback.tsx
@@ -63,16 +63,23 @@ export function Message({kind = 'info', title, children, icon, onDismiss, classN
 export interface LabelProps {
     color?: SemanticColorName | 'black' | 'white';
     icon?: string | React.ComponentType<any>;
+    /**
+     * Draw it as a physical tag — pointed left edge and an eyelet — rather than a plain
+     * chip.  Reserved for things that actually are tags: the same component also carries
+     * the count badges in the Tags table, and a point on a number reads as an arrow.
+     */
+    tag?: boolean;
     children?: React.ReactNode;
     onClick?: React.MouseEventHandler<HTMLSpanElement>;
     className?: string;
 }
 
-/** A tag/chip.  Filled in light and dark; an outline in night, where a filled
+/** A chip.  Filled in light, dark and amber; an outline in night, where a filled
  *  patch would be a bright surface. */
-export function Label({color = 'grey', icon, children, onClick, className}: LabelProps) {
+export function Label({color = 'grey', icon, tag, children, onClick, className}: LabelProps) {
     return <span
-        className={['wrolpi-label', `wrolpi-label-${color}`, className].filter(Boolean).join(' ')}
+        className={['wrolpi-label', `wrolpi-label-${color}`, tag ? 'wrolpi-tag' : '', className]
+            .filter(Boolean).join(' ')}
         style={{['--label-color' as string]: `var(--${color})`, cursor: onClick ? 'pointer' : undefined}}
         onClick={onClick}
     >