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: route genuine success and failure through the roles

Browse source 
Sixteen call sites, chosen by reading each one rather than by grepping for a
hue.  Four sub-agents judged them independently against one rule: convert only
what genuinely means success or failure; identification, category and
decoration stay hue-named.

Converted:
  SMART drive health (all four branches -- PASS/WARN/FAIL/unknown)
  config validity: valid / invalid / empty
  "Failed to load preview"
  the two "Download error" triggers
  expired inventory items, desktop and mobile
  the refresh phases: complete / active / not reached
  a refresh Progress bar in its error state
  cookie status: Unlocked / Locked
  a chip-mismatch label on the flasher
  CPU load thresholds (LoadStatistic)
  the outcome of an API call in useAPIButton
  the gallery's "Danger zone" heading and its load readings

Left alone, deliberately:
  the file browser toolbar -- every action has its own colour to be told apart,
    and delete already uses role='danger' (Roland's ruling)
  diff add/remove in the config and inventory import previews.  Adding is not a
    success and removing is not a failure; in Overwrite mode the removals are
    exactly what the user asked for, and painting them danger would report their
    intended action as broken
  seasons in DatesSelector, the uploader star and favourite heart, TagIcon,
  downloader category buttons, "Recommended to Keep"

Three sites were wrong in a way worth naming.  Both "Download error" triggers
guard on the same condition and render the same modal, but one was `orange` and
the other `red` -- and `orange` is the single worst choice, byte-identical to
`--text` in night, so that error button was indistinguishable from an ordinary
control.  Config validity drew its valid state in `--violet`, which no user
parses as "valid".  And `LoadStatistic` -- the thing users actually watch on the
Status page -- rendered its warning threshold in that same invisible orange.

Also fixed a stale comment in Videos.js that claimed no token existed for these.

Two notes on the tests:

The first LoadStatistic test I wrote rendered the component and asserted the
inline colour.  jsdom REJECTS `color: var(--danger)` as an invalid declaration
and drops it, so every colour reads back as the empty string -- two of the four
assertions passed for that reason alone and would have kept passing with the
hues restored.  It is a source guard now, with the painted result asserted in
the browser instead.

That browser test then found something real: an uncoloured reading inherits
`--text`, which in night is 4.98:1 against `--warning`'s 4.91:1, so a machine
under load is marginally QUIETER than one that is fine.  Not fixed here.  The
only way up on a dark panel is lightness, and night's danger would desaturate
from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character for
loudness.  That is a palette decision, so the test documents it and asserts what
does hold rather than quietly making it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-08-01 18:08:34 -06:00
parent ec94a939fb
commit 675bd5d620
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
12 changed files with 133 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -1229,9 +1229,9 @@ export function LoadStatistic({label, value, cores, ...props}) {
const quarter = cores / 4;
let color;
if (cores && value >= (quarter * 3)) {
color = 'red';
color = 'danger';
} else if (cores && value >= (quarter * 2)) {
color = 'orange';
color = 'warning';
}
return <Statistic
label={label}
@ -1785,7 +1785,9 @@ export function useAPIButton(
// The result is shown as the icon rather than an animation: the outcome should be
// legible at a glance, and in night mode a moving element is the last thing wanted.
const outcomeIcon = showSuccess ? 'check' : showFailure ? 'close' : null;
const outcomeColor = showSuccess ? 'green' : showFailure ? 'red' : undefined;
// Roles are registered as Mantine colors, so these reach `color` the same way a hue
// would. This is the plainest success/failure in the app: the call worked, or it did not.
const outcomeColor = showSuccess ? 'success' : showFailure ? 'danger' : undefined;
if (outcomeColor) {
buttonArgs['color'] = outcomeColor;
}

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

@ -1,3 +1,5 @@
import fs from 'fs';
import path from 'path';
import React from 'react';
import {act, render, screen, waitFor} from '../test-utils';
import userEvent from '@testing-library/user-event';
@ -560,3 +562,46 @@ describe('contrastingColor with shorthand hex', () => {
}
});
});
describe('LoadStatistic ranks load by role, not by hue', () => {
/*
* A source guard, not a render test, and deliberately so: jsdom's CSS parser REJECTS
* `color: var(--danger)` as an invalid declaration and drops it, so every inline colour
* reads back as the empty string here. A render test would pass whatever the component
* did -- the first version of this asserted `''` for the below-threshold cases and would
* have gone on passing with the hues restored.
*
* What is painted is asserted in ui-layout.cy.js, in a browser, at the thresholds.
*/
it('names roles rather than hues at both thresholds', () => {
const source = fs.readFileSync(path.join(__dirname, 'Common.js'), 'utf8');
const helper = source.slice(
source.indexOf('export function LoadStatistic'),
source.indexOf('export function', source.indexOf('export function LoadStatistic') + 10));
expect(helper).toContain("color = 'danger'");
expect(helper).toContain("color = 'warning'");
expect(helper.match(/'(red|green|amber|yellow|orange)'/)).toBeNull();
});
});
describe('a drive\'s SMART assessment is a severity, not a colour', () => {
/*
* A source guard: `getHealthColor` is module-private, and exporting it only so a test
* can reach it would be shaping the code around the test. What matters is that the
* helper stays on the ramp -- all four branches or none, since a helper where PASS and
* FAIL rank but WARN is an unranked hue is worse than either choice.
*/
it('maps every branch to a role', () => {
const source = fs.readFileSync(
path.join(__dirname, 'admin', 'ControllerPage.js'), 'utf8');
const helper = source.slice(
source.indexOf('const getHealthColor'),
source.indexOf('};', source.indexOf('const getHealthColor')));
expect(helper).toContain("'var(--success)'");
expect(helper).toContain("'var(--warning)'");
expect(helper).toContain("'var(--danger)'");
expect(helper.match(/var\(--(red|green|amber|yellow|orange)\)/)).toBeNull();
});
});

8
app/src/components/Files.js
View file

@ -979,10 +979,12 @@ function RefreshStepItem({phase, isActive, isCompleted, isDisabled, description}
return <div style={{
display: 'flex', alignItems: 'center', gap: 8, flex: '1 1 160px', padding: '0.5em 0.75em',
opacity: isDisabled ? 0.5 : 1,
borderBottom: isActive ? '2px solid var(--blue)' : '2px solid transparent',
borderBottom: isActive ? '2px solid var(--info)' : '2px solid transparent',
}}>
{/* The same three roles Status maps complete/active/pending onto. Not the Status
component itself: these rows carry a per-phase glyph, which is the point of them. */}
<Icon name={phase.icon} size='large'
style={{color: isCompleted ? 'var(--green)' : isActive ? 'var(--blue)' : 'var(--muted)'}}/>
style={{color: isCompleted ? 'var(--success)' : isActive ? 'var(--info)' : 'var(--neutral)'}}/>
<div>
<div style={{fontWeight: 600, fontSize: 13}}>{phase.title}</div>
<div style={{fontSize: 12, color: 'var(--muted)'}}>{isActive || isCompleted ? description : ''}</div>
@ -1033,7 +1035,7 @@ function RefreshProgressBar({status, operation_total, operation_processed, opera
// Show error state
if (status === 'error' && error) {
return <Progress percent={100} color='red' label={`Error: ${error}`}/>;
return <Progress percent={100} color='danger' label={`Error: ${error}`}/>;
}
let label = `${phaseLabel}`;

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

@ -909,7 +909,7 @@ export function FlasherPage() {
</span>
{chipMismatch &&
<span style={{marginLeft: '0.5em'}}>
<Label color='red' icon='warning sign'>
<Label color='warning' icon='warning sign'>
Built for {chipIdName(fileChipId)}, not{' '}
{chipIdName(deviceChipId)}
</Label>

4
app/src/components/ThemeSamplePage.js
View file

@ -197,7 +197,7 @@ export function ThemeSamplePage() {
<StatisticGroup>
<Statistic value='0.4' label='1 Min. Load'/>
<Statistic value='2.6' label='5 Min. Load' color='orange'/>
<Statistic value='4.1' label='15 Min. Load' color='red'/>
<Statistic value='4.1' label='15 Min. Load' color='danger'/>
<Statistic value='48' label='Temp C°'/>
</StatisticGroup>
</Section>
@ -700,7 +700,7 @@ export function ThemeSamplePage() {
<Section label='Danger zone'>
<Panel danger>
<h2 style={{color: 'var(--red)', fontSize: 14, margin: '0 0 4px'}}>Danger zone</h2>
<h2 style={{color: 'var(--danger)', fontSize: 14, margin: '0 0 4px'}}>Danger zone</h2>
<p style={{color: 'var(--muted)', fontSize: 13, margin: '0 0 12px'}}>
These actions are destructive and cannot be undone.
</p>

11
app/src/components/Videos.js
View file

@ -614,13 +614,16 @@ function CookiesSettingsSection() {
fetchStatus();
};
// Status is encoded by color like any other status label; no filled "success/warning"
// words map directly to a token, so the wrapper picks the token color explicitly.
// The two states that mean something take roles; the two that are just "nothing to
// report" stay --muted, which is dimmer than --neutral in night and amber and so keeps
// the least important states at the bottom.
const getStatusLabel = () => {
if (loading) return <span style={{color: 'var(--muted)'}}>Loading...</span>;
if (!status.cookies_exist) return <span style={{color: 'var(--muted)'}}>No cookies stored</span>;
if (status.cookies_unlocked) return <span style={{color: 'var(--green)'}}>Unlocked</span>;
return <span style={{color: 'var(--amber)'}}>Locked</span>;
if (status.cookies_unlocked) return <span style={{color: 'var(--success)'}}>Unlocked</span>;
// Locked is not an error the user made, but cookie-requiring downloads fail until
// somebody enters the password after a reboot -- that is attention, not failure.
return <span style={{color: 'var(--warning)'}}>Locked</span>;
};
return <Panel>

4
app/src/components/admin/Configs.js
View file

@ -7,7 +7,7 @@ function WarningIcon({ok}) {
// null is empty, true is valid, false is invalid.
const content = ok === true ? 'Valid and can be imported.' : ok === false ? 'Invalid, can be overwritten.' : 'Empty config, can be overwritten.';
return <Tooltip label={content}>
<span style={{color: ok === true ? 'var(--violet)' : ok === false ? 'var(--red)' : 'var(--muted)'}}>
<span style={{color: ok === true ? 'var(--success)' : ok === false ? 'var(--danger)' : 'var(--neutral)'}}>
<Icon name={ok === false ? 'close' : 'check'} label={content}/>
</span>
</Tooltip>
@ -82,7 +82,7 @@ function BackupDateRow({date, previews, previewsLoaded, onSelect}) {
const noChanges = <span style={{color: 'var(--muted)', fontStyle: 'italic'}}>No changes</span>;
const previewFailed = <Tooltip label='Failed to load preview'>
<span style={{color: 'var(--red)'}}><Icon name='warning sign' label='Failed to load preview'/></span>
<span style={{color: 'var(--danger)'}}><Icon name='warning sign' label='Failed to load preview'/></span>
</Tooltip>;
let mergeCell;

18
app/src/components/admin/ControllerPage.js
View file

@ -606,12 +606,16 @@ function ServicesSection() {
// Helper to get health status color from SMART assessment
const getHealthColor = (assessment) => {
if (!assessment) return 'var(--muted)';
// Roles, not hues: a drive's assessment is a severity and has to rank as one. All
// four branches together -- a helper where PASS and FAIL rank but WARN is an
// unranked hue is worse than either choice, and in night `--amber` is byte-identical
// to `--yellow` so WARN would stop sitting between them.
if (!assessment) return 'var(--neutral)';
const upper = assessment.toUpperCase();
if (upper === 'PASS') return 'var(--green)';
if (upper === 'WARN') return 'var(--amber)';
if (upper === 'FAIL') return 'var(--red)';
return 'var(--muted)';
if (upper === 'PASS') return 'var(--success)';
if (upper === 'WARN') return 'var(--warning)';
if (upper === 'FAIL') return 'var(--danger)';
return 'var(--neutral)';
};
// Helper to format power-on hours
@ -829,7 +833,9 @@ function MountModal({disk, open, onClose, onMount}) {
<p>
Found <strong>{entries}</strong> ({formatBytes(shadowed.size_bytes)}) at the mount target.
</p>
<p style={{color: 'var(--red)'}}>
{/* A caution about a consequence, not a failure -- the destructive
weight is already carried by the confirm button below. */}
<p style={{color: 'var(--warning)'}}>
Those files will continue to consume space on the underlying filesystem
(typically the SD card on a Raspberry Pi) and can fill it up.
</p>

4
app/src/components/admin/Downloads.js
View file

@ -238,7 +238,7 @@ function RecurringDownloadRow({download, fetchDownloads, onDelete}) {
</Modal.Actions>
</Modal>;
const errorTrigger = <IconButton icon='exclamation triangle' label='Download error' color='orange'
const errorTrigger = <IconButton icon='exclamation triangle' label='Download error' color='danger'
onClick={handleErrorOpen}/>;
const hideEdit = downloader === Downloaders.MapCatalog || downloader === Downloaders.MapExtract;
@ -453,7 +453,7 @@ function OnceDownloadRow({download, fetchDownloads, isSelected, onSelect}) {
if (error && !download.progress) {
completedAtCell = (
<IconButton icon='exclamation triangle' label='Download error' color='red'
<IconButton icon='exclamation triangle' label='Download error' color='danger'
onClick={() => setErrorModalOpen(true)}/>
)
errorModal = <Modal

2
app/src/components/inventory/InventoryItemsMobile.js
View file

@ -49,7 +49,7 @@ export function InventoryItemsMobile({fields, items}) {
{columns.map((f, idx) => <Table.Cell key={f.key}>
{idx === 0 && expired &&
<Icon name='warning sign' label='Expired'
style={{color: 'var(--red)', marginRight: '0.4em'}}/>}
style={{color: 'var(--danger)', marginRight: '0.4em'}}/>}
{formatValue(item, f)}
</Table.Cell>)}
</Table.Row>;

2
app/src/components/inventory/InventoryTable.js
View file

@ -235,7 +235,7 @@ export function InventoryTable({slug, fields, items, locations, catalog, search,
<Table.Cell>
<Checkbox checked={selected.has(item.id)} onChange={() => toggleSelected(item.id)}/>
{expired && <Icon name='warning sign' label='Expired'
style={{color: 'var(--red)', marginLeft: '0.4em'}}/>}
style={{color: 'var(--danger)', marginLeft: '0.4em'}}/>}
</Table.Cell>
{sortedFields.map(f => <Table.Cell key={f.key}>
{editing

50
app/src/components/ui/ui-layout.cy.js
View file

@ -333,6 +333,56 @@ describe('semantic roles read as severity in every theme', () => {
});
});
describe('a load reading ranks itself in the monochrome themes', () => {
/*
* LoadStatistic is the Status page's CPU load, and the clearest payoff of the roles
* outside the library: it used to be `orange` above half the cores and `red` above three
* quarters, and in night `--orange` is byte-identical to `--text`, so a machine under
* load rendered its warning as an ordinary uncoloured number.
*
* Only a browser can see this. jsdom rejects `color: var(--warning)` as invalid and
* drops it, so the inline colour reads back empty whatever the component did.
*/
monochromeThemes.forEach((theme) => {
it(`separates fine, warning and danger loads in ${theme}`, () => {
cy.mountUI(
<Panel>
<Statistic value='1.9' label='fine' cores={4} color={undefined}/>
<Statistic value='2.6' label='warning' color='warning'/>
<Statistic value='4.1' label='danger' color='danger'/>
</Panel>,
{theme},
);
cy.get('.wrolpi-statistic-value').should(($values) => {
const [fine, warning, danger] = [...$values]
.map(el => toRgb(getComputedStyle(el).color));
const panel = toRgb(getComputedStyle(
Cypress.$('.wrolpi-panel')[0]).backgroundColor);
expect(new Set([fine, warning, danger]).size, 'three distinct readings').to.equal(3);
expect(contrast(danger, panel), 'danger is louder than warning')
.to.be.greaterThan(contrast(warning, panel));
expect(contrast(danger, panel), 'danger is louder than a plain reading')
.to.be.greaterThan(contrast(fine, panel));
/*
* NOT asserted: that warning outranks a plain reading. It does not, and that
* is a live question rather than an oversight -- an uncoloured value inherits
* `--text`, which in night is 4.98:1 against 4.91:1 for `--warning`. So a
* machine under load is marginally QUIETER than one that is fine.
*
* Lifting warning above text is a palette change, not a call-site one: on a
* dark panel the only way up is lightness, and night's danger would desaturate
* from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character
* for loudness. Worth a deliberate decision, so it is flagged rather than
* quietly made here.
*/
});
});
});
});
describe('a table header is a surface, not just bold text', () => {
themeNames.forEach((theme) => {
it(`separates the header from every body row in ${theme}`, () => {