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: the dashboard's status panel links without looking like a link

Browse source 
The whole panel is wrapped in a `<Link to='/admin/status'>`, so the block is one
anchor -- and `a` takes `color: var(--blue)` with an underline on hover from
tokens.css.  `.wrolpi-statistic-value` sets no color of its own, so the three
load figures inherited the link color: blue numbers on a panel where nothing
else is blue, underlining as a group whenever the pointer crossed any part of
it.  The Downloading figure below the divider had it too.

`no-link-underscore card-link` is the pair already used wherever a link IS the
element's own text rather than a link through it -- every card title and meta
line, via `cardLinkClass` in Common.js.  Reused rather than adding a third
class for the same idea.

The pointer cursor is untouched, which is the point: it comes from the UA
stylesheet for `a[href]`, and the affordance is the only reason to wrap a panel
in a link at all.  A test asserts it so a later change cannot take it away
while tidying the rest.

DashboardStatus is exported so its call site can be rendered directly; the
colors are asserted in cypress, since jsdom drops `color: var(--text)` as an
invalid declaration and cannot see this at all.

Each new assertion has a paired negative: the same panel WITHOUT the classes
comes out blue, and has no rule protecting it from an underline.  The first
draft of the underline test checked only that no matching hover rule was
`!important`, which was true before the fix as well -- it would have passed on
a panel that still underlined.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-08-05 19:25:56 -06:00
parent 3eb233cd7d
commit 36d087d489
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
3 changed files with 195 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

15
app/src/DashboardPage.js
View file

@ -292,7 +292,7 @@ export function Getters() {
}
function DashboardStatus() {
export function DashboardStatus() {
const {status} = useContext(StatusContext);
let percent = 0;
@ -319,7 +319,16 @@ function DashboardStatus() {
}
return <Panel>
<Link to='/admin/status'>
{/*
* The panel is a link, but it should not read as one.
*
* `a` takes `color: var(--blue)` and an underline on hover from tokens.css, and a
* statistic's value sets no color of its own -- so the three load figures inherited
* the link color, and crossing the panel underlined them together. The same pair
* every card title uses says "this text belongs to what contains it": the pointer
* cursor still comes from the UA stylesheet, which is the affordance that matters.
*/}
<Link to='/admin/status' className='no-link-underscore card-link'>
<Header as='h2'>Status</Header>
<CPUUsageProgress value={percent} label='CPU Usage'/>
@ -335,7 +344,7 @@ function DashboardStatus() {
<Divider style={{marginTop: '3em'}}/>
<Link to='/admin'>
<Link to='/admin' className='no-link-underscore card-link'>
<StatisticGroup>
<Statistic label='Downloading' value={pending_downloads}/>
</StatisticGroup>

64
app/src/DashboardPage.test.js
View file

@ -1,6 +1,6 @@
import React from 'react';
import {render, screen} from './test-utils';
import {Getters, FlagsMessages} from './DashboardPage';
import {DashboardStatus, Getters, FlagsMessages} from './DashboardPage';
import {SettingsContext, StatusContext} from './contexts/contexts';
// Replace DownloadMenu with a stub that records its props. The deep-link
@ -101,3 +101,65 @@ describe('DashboardPage no-drive-mounted banner', () => {
expect(screen.queryByText(/No drive mounted/i)).toBeNull();
});
});
function renderDashboardStatus() {
const status = {
cpu_stats: {percent: 12, cores: 4},
load_stats: {minute_1: 0.4, minute_5: 0.5, minute_15: 0.6},
downloads: {pending: 3, disabled: false},
nic_bandwidth_stats: {},
};
return render(
<StatusContext.Provider value={{status}}>
<DashboardStatus/>
</StatusContext.Provider>
);
}
describe('the dashboard status panel links without looking like a link', () => {
/*
* The whole panel is wrapped in a <Link> to /admin/status, which makes it an anchor --
* and `a` takes `color: var(--blue)` with an underline on hover from tokens.css. A
* statistic's value sets no color of its own, so the load figures inherited that: three
* blue numbers on a panel where nothing else is blue, underlining as a group when the
* pointer crossed any part of the panel.
*
* `no-link-underscore card-link` is the pair already used wherever a link IS the
* element's own text rather than a link through it -- every card title and meta line.
* The pointer cursor is untouched: it comes from the UA stylesheet for `a[href]`, and
* the affordance is the point of wrapping the panel at all.
*
* Colors are asserted in ui-layout.cy.js; jsdom does not resolve the custom property
* these classes rely on.
*/
it('dresses the status link as the panel\'s own text', () => {
const {container} = renderDashboardStatus();
const link = container.querySelector('a[href="/admin/status"]');
expect(link).toBeInTheDocument();
expect(link).toHaveClass('card-link');
expect(link).toHaveClass('no-link-underscore');
});
it('does the same for the downloads link beneath it', () => {
const {container} = renderDashboardStatus();
const link = container.querySelector('a[href="/admin"]');
expect(link).toBeInTheDocument();
expect(link).toHaveClass('card-link');
expect(link).toHaveClass('no-link-underscore');
});
it('leaves the load figures inside it, which is what went blue', () => {
// The premise for the two assertions above: this link really does wrap statistics
// whose value carries no color of its own, so it decides theirs.
const {container} = renderDashboardStatus();
const link = container.querySelector('a[href="/admin/status"]');
const values = [...link.querySelectorAll('.wrolpi-statistic-value')];
expect(values.length).toBeGreaterThanOrEqual(3);
expect(values.map(value => value.textContent)).toEqual(
expect.arrayContaining(['0.4', '0.5', '0.6']));
});
});

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

@ -2667,6 +2667,126 @@ describe('a card reads as titles, not as a wall of links', () => {
});
});
describe('a panel wrapped in a link does not read as a link', () => {
/*
* The dashboard's Status panel is wrapped in a `<Link to='/admin/status'>`, so the whole
* block is one anchor. `a` takes `color: var(--blue)` and an underline on hover from
* tokens.css, and `.wrolpi-statistic-value` sets no color of its own -- so the three load
* figures inherited the link color and underlined as a group whenever the pointer crossed
* any part of the panel. Blue numbers on a panel where nothing else is blue.
*
* The panel should still be clickable and still say so with the cursor. That is the
* whole reason to wrap it; it is only the link's LOOK that is wrong here.
*/
const mountLinkedPanel = (className, theme) => cy.mountUI(<MemoryRouter>
<Panel>
<a href='/admin/status' className={className} data-testid='panel-link'>
<Header as='h2'>Status</Header>
<StatisticGroup>
<Statistic label='1 Min. Load' value='0.4'/>
<Statistic label='5 Min. Load' value='0.5'/>
</StatisticGroup>
</a>
</Panel>
</MemoryRouter>, theme ? {theme} : undefined);
it('gives the figures the panel\'s text color rather than the link color', () => {
mountLinkedPanel('no-link-underscore card-link');
cy.get('[data-testid="panel-link"] .wrolpi-statistic-value').first().should(($value) => {
const root = getComputedStyle(document.documentElement);
const shown = toRgb(getComputedStyle($value[0]).color);
expect(shown, 'the load figure').to.deep.equal(toRgb(root.getPropertyValue('--text')));
expect(shown, 'and not the link color')
.to.not.deep.equal(toRgb(root.getPropertyValue('--blue')));
});
});
it('is a real constraint: the same panel without the classes IS blue', () => {
// Without this the assertion above would pass just as well if `--text` and `--blue`
// happened to resolve to the same value, or if the anchor were never styled at all.
mountLinkedPanel(undefined);
cy.get('[data-testid="panel-link"] .wrolpi-statistic-value').first().should(($value) => {
const root = getComputedStyle(document.documentElement);
expect(toRgb(getComputedStyle($value[0]).color), 'an unclassed link is blue')
.to.deep.equal(toRgb(root.getPropertyValue('--blue')));
});
});
/*
* `:hover` cannot be forced -- no event applies it and getComputedStyle will never
* report it -- so the live CSSOM is searched instead.
*
* The question is not "does a hover rule exist": `a:hover {text-decoration: underline}`
* in tokens.css matches this element either way, and asserting merely that it is not
* `!important` would pass with or without the fix. What decides it is whether some
* rule the element matches at rest sets `text-decoration: none` as `!important`, since
* that is what no later hover rule can outrank. That is `no-link-underscore`, and
* nothing else in the app declares it.
*/
const importantlyUndecorated = (element) => [...document.styleSheets]
.flatMap((sheet) => {
try {
return [...sheet.cssRules];
} catch (e) {
return []; // A cross-origin sheet; none of ours are.
}
})
.filter(rule => rule.selectorText
&& !rule.selectorText.includes(':hover')
&& rule.style.getPropertyPriority('text-decoration') === 'important'
&& rule.style.textDecoration.includes('none')
&& rule.selectorText.split(',').some((selector) => {
try {
return selector.trim() && element.matches(selector.trim());
} catch (e) {
return false;
}
}));
it('cannot be underlined, resting or hovered', () => {
mountLinkedPanel('no-link-underscore card-link');
cy.get('[data-testid="panel-link"]').should(($link) => {
expect(getComputedStyle($link[0]).textDecorationLine, 'at rest').to.equal('none');
expect(importantlyUndecorated($link[0]).map(rule => rule.selectorText),
'a rule no hover rule can outrank').to.not.be.empty;
});
});
it('is a real constraint: the same panel without the classes can be', () => {
mountLinkedPanel(undefined);
cy.get('[data-testid="panel-link"]').should(($link) => {
expect(importantlyUndecorated($link[0]).map(rule => rule.selectorText),
'nothing protects an unclassed link').to.be.empty;
});
});
it('keeps the pointer, so the panel still says it is clickable', () => {
mountLinkedPanel('no-link-underscore card-link');
cy.get('[data-testid="panel-link"]').should(($link) => {
expect(getComputedStyle($link[0]).cursor).to.equal('pointer');
});
});
themeNames.forEach((theme) => {
it(`keeps the figures legible on the panel in ${theme}`, () => {
mountLinkedPanel('no-link-underscore card-link', theme);
cy.get('[data-testid="panel-link"] .wrolpi-statistic-value').first().should(($value) => {
const panel = toRgb(getComputedStyle(document.documentElement)
.getPropertyValue('--panel'));
expect(contrast(toRgb(getComputedStyle($value[0]).color), panel),
`load figure on the panel in ${theme}`).to.be.greaterThan(4.5);
});
});
});
});
describe('a card keeps its meta with the title and its actions at the foot', () => {
/*
* A grid stretches every card to the tallest in its row. The meta line used to carry