diff --git a/app/src/Tags.js b/app/src/Tags.js index 3c0e1925..f7b59e09 100644 --- a/app/src/Tags.js +++ b/app/src/Tags.js @@ -336,9 +336,16 @@ function EditTagsModal() { Edit Tags
+ {/* + * `--label-text`, not `color`, for the same reason as the chip itself: + * inline `color` cannot be overridden, so in night this preview showed + * black text on a transparent outline over a near-black page. It is the + * preview of the colour being chosen, so being honest about how the tag + * will actually look matters here more than anywhere. + */} {tagName || 'Example Tag'} diff --git a/app/src/components/Common.js b/app/src/components/Common.js index e7bf603d..8332765d 100644 --- a/app/src/components/Common.js +++ b/app/src/components/Common.js @@ -1554,16 +1554,26 @@ export function contrastRatio(a, b) { return (luminances[0] + 0.05) / (luminances[1] + 0.05); } +/** + * A hex colour as [r, g, b], accepting both `#rrggbb` and the shorthand `#rgb`. + * + * 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 + * -- 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) { - if (color.length === 7) { - color = color.slice(1); + 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 (color.length !== 6) { + 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(color.substr(i * 2, 2), 16); + for (let i = 0; i <= 2; i++) rgb[i] = parseInt(hex.substr(i * 2, 2), 16); return rgb; } diff --git a/app/src/components/Common.test.js b/app/src/components/Common.test.js index f08ca07f..cac547b9 100644 --- a/app/src/components/Common.test.js +++ b/app/src/components/Common.test.js @@ -531,3 +531,32 @@ describe('contrastingColor', () => { expect(contrastRatio('#ffffff', '#ffffff')).toBeCloseTo(1, 5); }); }); + +describe('contrastingColor with shorthand hex', () => { + /* + * Tag colours reach the UI from the database, and a tags config is a file a user can edit + * by hand -- configs are the source of truth in WROLPi -- so a perfectly valid `#fff` can + * arrive here. `hexToRGBArray` used to reject anything but six digits, which left the + * luminance at zero: a near-white tag was treated as black and given light text. + */ + + it('reads a three-digit hex as the colour it is', () => { + expect(contrastRatio('#000000', '#fff')).toBeCloseTo(21, 1); + expect(contrastRatio('#000000', '#ffffff')).toBeCloseTo(contrastRatio('#000000', '#fff'), 5); + }); + + it('picks dark text for a near-white shorthand fill', () => { + expect(contrastingColor('#fff')).toBe(TAG_TEXT_DARK); + }); + + it('picks light text for a near-black shorthand fill', () => { + expect(contrastingColor('#000')).toBe(TAG_TEXT_LIGHT); + }); + + it('agrees with the six-digit form of the same colour', () => { + for (const [short, long] of [['#fff', '#ffffff'], ['#000', '#000000'], + ['#f00', '#ff0000'], ['#1b2', '#11bb22']]) { + expect(contrastingColor(short)).toBe(contrastingColor(long)); + } + }); +}); diff --git a/app/src/components/Flasher.js b/app/src/components/Flasher.js index fe125a69..ac1652d3 100644 --- a/app/src/components/Flasher.js +++ b/app/src/components/Flasher.js @@ -783,9 +783,12 @@ export function FlasherPage() { {humanFileSize(result.size)} {result.esp_chip + // `--label-text` rather than `color`: an inline + // `color` cannot be overridden, and night and + // amber replace the chip's fill with their own. ? {result.esp_chip} : —} diff --git a/app/src/components/ui/ui.test.js b/app/src/components/ui/ui.test.js index 62100fe5..25e09439 100644 --- a/app/src/components/ui/ui.test.js +++ b/app/src/components/ui/ui.test.js @@ -599,6 +599,65 @@ describe('Label as a tag', () => { }); }); +describe('nothing paints a label\'s text with an inline colour', () => { + /* + * A source guard, because this defect appeared at three separate call sites and fixing the + * first one did not fix the others. + * + * A label's text colour has to travel through `--label-text`. An inline `color` is a + * declaration no stylesheet rule can outrank, so night -- which turns a label into an + * outline over a near-black page -- keeps whatever was calculated for the fill it just + * discarded. A bright fill leaves black text on near-black: the tag reads as an empty + * outline, and nothing fails. + * + * The three were the tag chip, the tag edit preview (the very preview of the colour being + * chosen), and the Flasher's chip badges. The stylesheet comment explaining the night and + * amber override already named the Flasher, and it was still missed. + */ + + const sourceFiles = (directory) => fs.readdirSync(directory, {withFileTypes: true}) + .flatMap(entry => { + const full = path.join(directory, entry.name); + if (entry.isDirectory()) { + return entry.name === 'node_modules' ? [] : sourceFiles(full); + } + if (!/\.(js|tsx)$/.test(entry.name) || /\.test\.|\.cy\./.test(entry.name)) { + return []; + } + return [[full, fs.readFileSync(full, 'utf8')]]; + }); + + it('sets --label-text instead, everywhere', () => { + const src = path.join(__dirname, '..', '..'); + const offenders = []; + + for (const [file, source] of sourceFiles(src)) { + const stripped = source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, ''); + let from = 0; + while (true) { + const at = stripped.indexOf('wrolpi-label', from); + if (at === -1) break; + from = at + 1; + + // The style object belonging to this element, if it has one nearby. + const styleAt = stripped.indexOf('style={{', at); + if (styleAt === -1 || styleAt - at > 200) continue; + const styleEnd = stripped.indexOf('}}', styleAt); + if (styleEnd === -1) continue; + const styleObject = stripped.slice(styleAt, styleEnd); + + // A bare `color:` -- not `--label-color`, and not `backgroundColor` and friends, + // which are camelCase and so do not match a lowercase `color:`. + if (/(?:^|[^-\w])color\s*:/.test(styleObject)) { + offenders.push(`${path.relative(src, file)}: ${styleObject.slice(0, 90)}`); + } + } + } + + expect(offenders).toEqual([]); + }); +}); + describe('Table', () => { it('renders a Semantic-shaped compound table', () => { renderUI(