mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
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.
This commit is contained in:
Roland
parent
d697265267
commit
ecf611931b
7 changed files with 301 additions and 11 deletions
|
|
@ -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', {
|
||||
|
|
|
|||
|
|
@ -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}) => {
|
||||
|
|
|
|||
115
app/src/Tags.test.js
Normal file
115
app/src/Tags.test.js
Normal file
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
@ -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) => {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue