rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-26 19:01:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

ui: two more labels were painting text with an inline colour, and shorthand hex

Browse source 
Both from review, and both correct.

* The tag edit preview and the Flasher's chip badges still wrote the
  calculated text colour to `color`.  That is the same defect the last
  commit fixed on the tag chip itself: an inline declaration no stylesheet
  rule can outrank, so night keeps the colour calculated for the fill it
  has just discarded and a bright fill leaves black text on near-black.

  The preview is the worse of the two -- it is the preview of the colour
  being chosen, so it was misrepresenting what the tag would look like at
  the moment the user picks it.  It now shows the tag shape as well.

  Fixing the first site did not fix these, and the stylesheet comment
  explaining the night and amber override already named the Flasher.  So
  there is a source guard now: no element carrying `wrolpi-label` may set
  `color` in its style object.  Verified against a planted violation,
  which it reports by file.

* `hexToRGBArray` rejected the shorthand `#fff`, which left the luminance
  at zero -- a near-white tag was read as black and handed light text.
  Reachable because tag colours are not only picked in the colour picker:
  they are stored in a config, and a config is a file a user edits by
  hand.  Three tests were red first, including one asserting the shorthand
  and the six-digit form of the same colour agree.
This commit is contained in:
Roland 2026-08-01 14:34:17 -06:00
parent d117d92a85
commit 672e7973f6
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
5 changed files with 115 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

11
app/src/Tags.js
View file

@ -336,9 +336,16 @@ function EditTagsModal() {
<Modal.Header>Edit Tags</Modal.Header>
<div id='editModalContent'>
<Group gap={6}>
{/*
* `--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.
*/}
<span
className='wrolpi-label'
style={{['--label-color']: tagColor, color: textColor}}
className='wrolpi-label wrolpi-tag'
style={{['--label-color']: tagColor, ['--label-text']: textColor}}
>
{tagName || 'Example Tag'}
</span>

18
app/src/components/Common.js
View file

@ -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;
}

29
app/src/components/Common.test.js
View file

@ -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));
}
});
});

5
app/src/components/Flasher.js
View file

@ -783,9 +783,12 @@ export function FlasherPage() {
<Table.Cell>{humanFileSize(result.size)}</Table.Cell>
<Table.Cell>
{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.
? <span className='wrolpi-label' style={{
'--label-color': chipColor(result.esp_chip),
color: chipTextColor(chipColor(result.esp_chip)),
'--label-text': chipTextColor(chipColor(result.esp_chip)),
}}>{result.esp_chip}</span>
: <span style={{opacity: 0.5}}>—</span>}
</Table.Cell>

59
app/src/components/ui/ui.test.js
View file

@ -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(<Table>