diff --git a/app/src/DashboardPage.js b/app/src/DashboardPage.js index f9f50679..2ff83051 100644 --- a/app/src/DashboardPage.js +++ b/app/src/DashboardPage.js @@ -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 - + {/* + * 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. + */} +
Status
@@ -335,7 +344,7 @@ function DashboardStatus() { - + diff --git a/app/src/DashboardPage.test.js b/app/src/DashboardPage.test.js index 20f71055..1d75414c 100644 --- a/app/src/DashboardPage.test.js +++ b/app/src/DashboardPage.test.js @@ -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( + + + + ); +} + +describe('the dashboard status panel links without looking like a link', () => { + /* + * The whole panel is wrapped in a 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'])); + }); +}); diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js index aba5421e..75920b40 100644 --- a/app/src/components/ui/ui-layout.cy.js +++ b/app/src/components/ui/ui-layout.cy.js @@ -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 ``, 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( + + +
Status
+ + + + +
+
+
, 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