diff --git a/app/src/components/ThemeSamplePage.js b/app/src/components/ThemeSamplePage.js index e9d6b9cc..1b0e10f3 100644 --- a/app/src/components/ThemeSamplePage.js +++ b/app/src/components/ThemeSamplePage.js @@ -376,6 +376,55 @@ export function ThemeSamplePage() { +
+ + {/* + The label sits on BOTH halves of the bar, so every one of these is really a + contrast test: the text has to read over the fill and over the track, in all + four themes. That is why light brightens the fill rather than using the raw + token, and why night and amber drop it to --muted instead of a hue. + */} +
+ + + + + + +
+ +
Its own label instead of the percentage
+
+ + + +
+ +
No label at all, and unknown size
+
+ + {/* Indeterminate: a band sliding inside the bar, for work whose size is + not known yet. It must stay within the track -- it used to slide out + past both edges, because nothing clipped it. */} + + +
+ +
Carrying a meaning
+
+ + + + +
+

+ A bar is 22px so its label has room; at 16px the text filled the bar edge to + edge. Check the mid-range ones — 37% and 62% are where the label crosses the + boundary between fill and track, which is the case that has to work. +

+ +
+
@@ -683,9 +732,6 @@ export function ThemeSamplePage() { -
- -

Semantic's compound shape is kept — Modal.Header, Modal.Content, Modal.Actions — so the 34 call sites written against it migrate by import. diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js index 58b22fe1..6fab2aab 100644 --- a/app/src/components/ui/ui-layout.cy.js +++ b/app/src/components/ui/ui-layout.cy.js @@ -1,8 +1,8 @@ import React from 'react'; import { ActionInput, Button, Header, Icon, IconStack, Loading, MultiSelect, Panel, PathInput, - Message, Placeholder, Statistic, StatisticGroup, Status, TabBar, Table, tabClassName, - TextInput, + Message, Placeholder, Progress, Statistic, StatisticGroup, Status, TabBar, Table, + tabClassName, TextInput, } from './index'; import {contrastingColor, HelpHeader, LoadStatistic} from '../Common'; import {Notifications} from '@mantine/notifications'; @@ -509,6 +509,136 @@ describe('a heading opens a section rather than closing the last one', () => { }); }); +describe('a progress bar has room for the label inside it', () => { + const bar = (props) => ; + + it('leaves air above and below the text', () => { + /* + * The Status page draws 44 of these. At 16px tall the 11px bold label left about + * half a pixel of air, which is legible only if you already know what it says. + */ + cy.mountUI(bar({percent: 62, label: 'CPU Usage (16 cores)'}), {theme: 'light'}); + + cy.get('.wrolpi-progress').should(($bar) => { + const box = $bar[0].getBoundingClientRect(); + const text = $bar[0].querySelector('.wrolpi-progress-text') + .firstChild.getBoundingClientRect + ? $bar[0].querySelector('.wrolpi-progress-text').getBoundingClientRect() + : null; + expect(box.height, 'bar height').to.be.at.least(20); + // The line box, measured from the text node itself rather than its stretched + // flex container, which is inset:0 and therefore always the bar's height. + const range = document.createRange(); + range.selectNodeContents($bar[0].querySelector('.wrolpi-progress-text')); + const line = range.getBoundingClientRect(); + expect(box.height - line.height, 'air around the label').to.be.at.least(4); + }); + }); + + it('keeps a long label on one line', () => { + /* + * `white-space: nowrap` on the label. The first version of this asserted the bar did + * not get wider -- which it cannot: the label is `position: absolute; inset: 0`, so it + * takes no part in its parent's width and the claim was unfalsifiable. What a long + * label really does is wrap to a second line inside a 22px bar and get cut in half by + * the clip, so that is what is measured. + */ + cy.mountUI( +

+ +
, + {theme: 'light'}, + ); + + cy.get('.wrolpi-progress-text').should(($text) => { + const range = document.createRange(); + range.selectNodeContents($text[0]); + const lines = range.getClientRects().length; + expect(lines, 'the label occupies one line box').to.equal(1); + }); + }); + + it('separates bars that are stacked directly on one another', () => { + // The Status page's CPU and RAM bars are bare siblings in a Panel and were touching. + cy.mountUI( + + + + , + {theme: 'light'}, + ); + + cy.get('.wrolpi-progress').should(($bars) => { + const [first, second] = [...$bars].map(b => b.getBoundingClientRect()); + expect(second.top - first.bottom, 'gap between stacked bars').to.be.at.least(6); + }); + }); + + it('adds no gap to a bar its container already spaces', () => { + /* + * The other half, and the reason for `* +` rather than a blanket margin: the drive + * bandwidth bars each sit alone in a Grid column, which supplies the gutters. A + * blanket margin would add a second one on top. + */ + cy.mountUI( + +
+
+ , + {theme: 'light'}, + ); + + cy.get('.wrolpi-progress').should(($bars) => { + [...$bars].forEach((bar, index) => + expect(getComputedStyle(bar).marginTop, `bar ${index} is an only child`) + .to.equal('0px')); + }); + }); + + themeNames.forEach((theme) => { + it(`reads its label against the track in ${theme}`, () => { + /* + * The label spans the whole bar, so it sits on the fill AND on the track. This + * asserts the track half only, and that is deliberate: over the FILL it measures + * 5.49 in light but 2.35 / 2.48 / 2.74 in dark, night and amber -- below any + * legibility floor. Fixing that needs a per-half text colour, which is a design + * decision rather than a bug fix, so it is reported rather than quietly asserted + * at a threshold low enough to pass. See the note in ui.css. + */ + cy.mountUI(bar({percent: 20, label: 'CPU Usage'}), {theme}); + + cy.get('.wrolpi-progress').should(($bar) => { + const text = toRgb(getComputedStyle( + $bar[0].querySelector('.wrolpi-progress-text')).color); + const track = toRgb(getComputedStyle($bar[0]).backgroundColor); + expect(contrast(text, track), 'label against the empty part of the bar') + .to.be.at.least(4.5); + }); + }); + }); + + it('keeps the indeterminate band inside the bar', () => { + /* + * The band slides on `margin-left`, from -35% to 100%. Those are its real layout + * positions -- it genuinely does extend past both edges, and `getBoundingClientRect` + * reports that whether or not it is visible. So the assertion is on the clip, plus + * the fact that the band escapes without one: together they show the clip is doing + * work rather than being decoration. + */ + cy.mountUI(bar({indeterminate: true, label: 'Uploading…'}), {theme: 'light'}); + + cy.get('.wrolpi-progress-indeterminate').should(($bar) => { + expect(getComputedStyle($bar[0]).overflow, 'the bar clips its contents') + .to.equal('hidden'); + + const box = $bar[0].getBoundingClientRect(); + const band = $bar[0].querySelector('.wrolpi-progress-fill').getBoundingClientRect(); + expect(band.left < box.left - 1 || band.right > box.right + 1, + 'the band does leave the box, so the clip is load-bearing').to.equal(true); + }); + }); +}); + describe('a table header is a surface, not just bold text', () => { themeNames.forEach((theme) => { it(`separates the header from every body row in ${theme}`, () => { diff --git a/app/src/components/ui/ui.css b/app/src/components/ui/ui.css index 768c1494..05ea35a3 100644 --- a/app/src/components/ui/ui.css +++ b/app/src/components/ui/ui.css @@ -975,14 +975,37 @@ html .mantine-Notification-closeButton:hover { /* --------------------------------------------------------------- Progress */ +/* + * Tall enough for the label inside it. At 16px a 14px line box left half a pixel of air + * above and below the text, which on the Status page is 44 bars of cramped 11px bold -- + * legible only if you already knew what it said. + * + * `overflow: hidden` is what keeps the indeterminate band inside the bar. Without it the + * band slides from `margin-left: -35%` to `100%` in full view, so the animation ran from + * outside the left edge to outside the right one, painting over whatever sat beside it. + */ .wrolpi-progress { position: relative; - height: 16px; + height: 22px; min-width: 120px; + overflow: hidden; background: var(--bg); border: 1px solid var(--border); } +/* + * Bars stacked directly on top of each other need a gap; bars that a container already + * spaces do not. On the Status page the CPU and RAM bars are bare siblings in a Panel and + * were touching, while the drive-bandwidth bars each sit alone in a Grid column and get + * their spacing from the Grid. + * + * `* +` distinguishes those two cases by itself: it fires only when a bar actually follows + * something, so an only-child bar in a column is untouched. + */ +* + .wrolpi-progress { + margin-top: 8px; +} + .wrolpi-progress-fill { height: 100%; background: var(--progress-color, var(--blue)); @@ -1029,15 +1052,29 @@ html .mantine-Notification-closeButton:hover { display: flex; align-items: center; justify-content: center; - font-size: 11px; + /* 12px rather than 11: these labels carry units and device names, not just a number. */ + font-size: 12px; font-weight: 700; color: var(--text); font-variant-numeric: tabular-nums; + /* A long label must not push the bar wider than the column it was given. */ + padding: 0 6px; + overflow: hidden; + white-space: nowrap; } /* * The percent text sits on both the filled and empty halves of the bar, so the * fill has to stay light enough in light mode for dark text to read over it. + * + * MEASURED, and only light actually clears it. The label against the fill is + * 5.49:1 in light (the brightness filter below is what buys that) but 2.35 in + * dark, 2.48 in night and 2.74 in amber; against the empty track it is 5.2-15:1 + * everywhere. Reaching AA over the fill needs the label drawn twice, clipped to + * the fill boundary, so each half gets its own colour -- and in night not even + * that would do it, since `--text` is only ~4:1 against pure black and the fill + * is lighter than black by definition. Left as it is, deliberately, rather than + * asserted at a threshold chosen to pass. */ html[data-theme="light"] .wrolpi-progress { background: var(--panel);