From 675bd5d62037c58335c101be5864a97f63025b91 Mon Sep 17 00:00:00 2001 From: Roland Date: Sat, 1 Aug 2026 18:08:34 -0600 Subject: [PATCH] ui: route genuine success and failure through the roles 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) --- app/src/components/Common.js | 8 +-- app/src/components/Common.test.js | 45 +++++++++++++++++ app/src/components/Files.js | 8 +-- app/src/components/Flasher.js | 2 +- app/src/components/ThemeSamplePage.js | 4 +- app/src/components/Videos.js | 11 ++-- app/src/components/admin/Configs.js | 4 +- app/src/components/admin/ControllerPage.js | 18 ++++--- app/src/components/admin/Downloads.js | 4 +- .../inventory/InventoryItemsMobile.js | 2 +- .../components/inventory/InventoryTable.js | 2 +- app/src/components/ui/ui-layout.cy.js | 50 +++++++++++++++++++ 12 files changed, 133 insertions(+), 25 deletions(-) diff --git a/app/src/components/Common.js b/app/src/components/Common.js index 8332765d..9c493765 100644 --- a/app/src/components/Common.js +++ b/app/src/components/Common.js @@ -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 { } }); }); + +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(); + }); +}); diff --git a/app/src/components/Files.js b/app/src/components/Files.js index 1b3644de..2b8bd23a 100644 --- a/app/src/components/Files.js +++ b/app/src/components/Files.js @@ -979,10 +979,12 @@ function RefreshStepItem({phase, isActive, isCompleted, isDisabled, description} return
+ {/* 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. */} + style={{color: isCompleted ? 'var(--success)' : isActive ? 'var(--info)' : 'var(--neutral)'}}/>
{phase.title}
{isActive || isCompleted ? description : ''}
@@ -1033,7 +1035,7 @@ function RefreshProgressBar({status, operation_total, operation_processed, opera // Show error state if (status === 'error' && error) { - return ; + return ; } let label = `${phaseLabel}`; diff --git a/app/src/components/Flasher.js b/app/src/components/Flasher.js index ac1652d3..295c2fcc 100644 --- a/app/src/components/Flasher.js +++ b/app/src/components/Flasher.js @@ -909,7 +909,7 @@ export function FlasherPage() { {chipMismatch && -