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: test the last four components, and put the last four in the gallery

Browse source 
Closes the tail of the library: `Loader`, `Loading`, `Placeholder` and
`Checkbox` had no test of their own, and `IconStack`, `MediaFilterToggle`,
`MultiSelect` and `NumberInput` were not in the gallery, which the project
rule says every component must be.

Putting them in the gallery is what found the rest of this.

An IconStack's corner glyph paints a disc of the surface behind itself so
the two sets of strokes do not cross, and that surface was hardcoded to the
page background.  It is right in exactly one of the three places a stack
appears.  The nav bar showed a white disc on the red bar, and the file
browser's "New Folder" button showed one punched through the blue fill.  It
is now `--icon-stack-bg`, defaulting to the page: the file browser passes
the button's fill, and the nav bar sets it on the <nav> beside the colour it
already writes there, so it inherits -- the bar's colour is a user setting
and cannot be named by a token.

A MultiSelect's chosen options are Mantine Pills, and Pill is the one
component that ignores the radius theme: it writes `--pill-radius: rem(1000)`
onto the element, which zeroing `--mantine-radius-xl` cannot reach.  They
were lozenges on a page of square boxes.

Twelve jest tests and sixteen browser tests.  The browser half covers what
jsdom cannot judge: that a Skeleton is a different colour from the panel it
covers in all four themes (its greys are remapped to our tokens, and neither
`--border` nor `--bg` was picked with a panel behind it), that the loader
colour resolves through the token, and that the corner disc matches its
surface.  Every one was checked red against a planted regression first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-08-01 15:48:10 -06:00
parent b36f7beeb8
commit affe5869f5
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
7 changed files with 392 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

5
app/src/components/FileBrowser.js
View file

@ -437,7 +437,10 @@ export function FileBrowser() {
disabled={wrolModeEnabled}
style={{paddingLeft: '1em', paddingRight: showLabels ? '1em' : '0.8em'}}
>
<IconStack corner={<Icon name='add'/>} label='New folder'>
{/* On a filled button, so the corner disc must be the button's fill and not
the page background, which would read as a hole punched in the button. */}
<IconStack corner={<Icon name='add'/>} label='New folder'
style={{'--icon-stack-bg': 'var(--blue)'}}>
<Icon name='folder'/>
</IconStack>
{showLabels ? <>&nbsp;New Folder</> : null}

18
app/src/components/Nav.js
View file

@ -363,7 +363,14 @@ export function NavBar() {
return <>
<Media between={['mobile', 'tablet']}>
<nav className='wrolpi-navbar' id='global_navbar'
style={{background: `var(--${navColor})`, color: 'var(--btn-text)'}}>
style={{
background: `var(--${navColor})`,
color: 'var(--btn-text)',
// The hotspot glyph stacks two icons, and the corner one paints a disc
// of its surface behind itself. The bar's colour is the user's choice,
// so it has to be handed down rather than looked up from a token.
'--icon-stack-bg': `var(--${navColor})`,
}}>
{homeLink}
<div className='wrolpi-navbar-right'>
{icons}
@ -394,7 +401,14 @@ function DesktopNav({navColor, homeLink, icons}) {
return (
<div ref={containerRef}>
<nav className='wrolpi-navbar' id='global_navbar'
style={{background: `var(--${navColor})`, color: 'var(--btn-text)'}}>
style={{
background: `var(--${navColor})`,
color: 'var(--btn-text)',
// The hotspot glyph stacks two icons, and the corner one paints a disc
// of its surface behind itself. The bar's colour is the user's choice,
// so it has to be handed down rather than looked up from a token.
'--icon-stack-bg': `var(--${navColor})`,
}}>
<span ref={homeRef} style={{display: 'contents'}}>{homeLink}</span>
{(isReady ? visibleLinks : allLinks).map((link, idx) => (
<span

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

@ -12,7 +12,11 @@ import {
Label,
Loader,
Loading,
IconStack,
MediaFilterToggle,
Message,
MultiSelect,
NumberInput,
Modal,
Pagination,
Panel,
@ -132,11 +136,24 @@ export function ThemeSamplePage() {
Night filters to red unless you turn it off; amber tints to match only if
you ask. Light and dark offer no filter, so both images look the same here.
<br/><br/>
Use the toggle in the theme picker above to see the difference. An
element can opt out with <code>night-unfiltered</code> — that is what
the right-hand copy does, which is why it is behind a click: an
unfiltered image is a bright patch, and night mode should not spring
one on you.
Use the toggle below to see the difference. An element can opt out
with <code>night-unfiltered</code> — that is what the right-hand copy
does, which is why it is behind a click: an unfiltered image is a
bright patch, and night mode should not spring one on you.
</p>
</div>
{/*
The same control the theme picker renders, and the same state: flipping
either one moves both. It renders nothing at all in light and dark, which
is why it appears to be missing there.
*/}
<div style={{marginTop: 14}}>
<MediaFilterToggle/>
<p style={{fontSize: 12, color: 'var(--muted)', margin: '8px 0 0'}}>
<code>MediaFilterToggle</code>, the control the theme picker ends with.
It is per theme — turning it off for night leaves amber's alone — and it
renders nothing for a theme that offers no filter, so light and dark
show only this caption.
</p>
</div>
</Panel>
@ -383,6 +400,22 @@ export function ThemeSamplePage() {
data={['Videos', 'Archive', 'File', 'Scrape']}/>
<TextInput label='Destination' defaultValue='videos/'/>
</div>
<div style={{marginTop: 12, display: 'grid', gridTemplateColumns: '2fr 1fr', gap: 12}}>
{/*
MultiSelect with a value already set, because the pills it renders for
chosen options are its own surface and they were the part most likely to
come out neutral grey. Open it to check the dropdown too.
*/}
<MultiSelect
label='Tags'
description='Applied to everything this download creates.'
data={['Preserve', 'Homestead', 'Radio', 'Medical', 'Reference']}
defaultValue={['Preserve', 'Radio']}
clearable
/>
<NumberInput label='Depth' description='Follow links this far.'
defaultValue={2} min={1} max={4}/>
</div>
<div style={{marginTop: 12}}>
<Textarea label='Notes' placeholder='Notes about this download…' minRows={2}/>
</div>
@ -521,6 +554,32 @@ export function ThemeSamplePage() {
</span>
</Row>
</div>
{/*
These are on a Panel, so each one says so. The default is the page
background, which is right for the nav bar and wrong everywhere else.
*/}
<div style={{marginTop: 18}}>
<Row>
<IconStack corner={<Icon name='question' size={12}/>} label='WiFi status unknown'
style={{'--icon-stack-bg': 'var(--panel)'}}>
<Icon name='wifi' size='large'/>
</IconStack>
<IconStack corner={<Icon name='add' size={12}/>} label='New folder'
style={{'--icon-stack-bg': 'var(--panel)'}}>
<Icon name='folder' size='large'/>
</IconStack>
<IconStack corner={<Icon name='x' size={12}/>} label='Download failed'
style={{'--icon-stack-bg': 'var(--panel)'}}>
<Icon name='download' size='large'/>
</IconStack>
<span style={{fontSize: 12, color: 'var(--muted)', maxWidth: 400}}>
<code>IconStack</code> composes two glyphs into one symbol. The corner
glyph paints the surface behind itself so the two sets of strokes do not
cross, which means it has to be told what that surface is:{' '}
<code>--icon-stack-bg</code>, set to the panel here.
</span>
</Row>
</div>
</Panel>
</Section>

17
app/src/components/ui/Icon.tsx
View file

@ -156,6 +156,12 @@ export interface IconStackProps {
corner: React.ReactNode;
/** Accessible name for the pair; the two glyphs mean one thing together. */
label: string;
/**
* The surface behind the stack, as `{'--icon-stack-bg': 'var(--blue)'}`. Needed
* wherever the stack is not on the page background — a filled button, a panel.
*/
style?: React.CSSProperties;
className?: string;
}
/**
@ -163,10 +169,15 @@ export interface IconStackProps {
* say. Replaces Semantic's IconGroup.
*
* The corner glyph gets a background matching the surface so it stays legible
* over the icon beneath it.
* over the icon beneath it; see `--icon-stack-bg` in ui.css.
*/
export function IconStack({children, corner, label}: IconStackProps) {
return <span className='wrolpi-icon-stack' role='img' aria-label={label}>
export function IconStack({children, corner, label, style, className}: IconStackProps) {
return <span
className={['wrolpi-icon-stack', className].filter(Boolean).join(' ')}
style={style}
role='img'
aria-label={label}
>
{children}
<span className='wrolpi-icon-stack-corner' aria-hidden='true'>{corner}</span>
</span>

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

@ -1,6 +1,7 @@
import React from 'react';
import {
ActionInput, Button, Header, PathInput, Statistic, StatisticGroup, TabBar, tabClassName, TextInput,
ActionInput, Button, Header, Icon, IconStack, Loading, MultiSelect, Panel, PathInput,
Placeholder, Statistic, StatisticGroup, TabBar, tabClassName, TextInput,
} from './index';
import {contrastingColor} from '../Common';
import {Notifications} from '@mantine/notifications';
@ -619,6 +620,164 @@ describe('a toast is readable in every theme', () => {
});
});
/*
* The colour a box actually shows: its own background, or the first pseudo-element that
* paints one. Mantine's Skeleton draws itself entirely from ::before and ::after, so
* reading the root alone reports `rgba(0, 0, 0, 0)` and any test built on it is vacuous.
*/
const paintedBackground = (el) => {
for (const pseudo of [null, '::before', '::after']) {
const colour = getComputedStyle(el, pseudo).backgroundColor;
if (colour && !/^rgba\(.*,\s*0(\.0+)?\)$/.test(colour) && colour !== 'transparent') {
return colour;
}
}
return null;
};
describe('a loading placeholder is visible on the surface it covers', () => {
themeNames.forEach((theme) => {
it(`separates the skeleton from the panel in ${theme}`, () => {
/*
* A Skeleton is Mantine's, and Mantine draws it from --mantine-color-gray-1 or
* --mantine-color-dark-4. Those are remapped to our tokens in themes/mantine.ts,
* which is exactly what makes this worth checking: `dark-4` is `--border` and
* `gray-1` is `--bg`, and neither was chosen with a panel behind it in mind. If
* the bars come out the same colour as the panel there is no placeholder at all,
* only an empty box, and nothing in jsdom can see that.
*/
cy.mountUI(<Panel><Placeholder lines={3}/></Panel>, {theme});
cy.get('.mantine-Skeleton-root').first().should(($line) => {
const line = paintedBackground($line[0]);
const panel = paintedBackground($line[0].closest('.wrolpi-panel'));
expect(line, 'the skeleton paints something').to.not.equal(null);
expect(line, 'the skeleton is not the panel it sits on').to.not.equal(panel);
});
});
});
it('keeps the last line short so the block reads as text', () => {
// The jest test asserts the declared width; this asserts it survived layout.
cy.mountUI(<Panel><Placeholder lines={3}/></Panel>, {theme: 'light'});
cy.get('.mantine-Skeleton-root').should(($lines) => {
const widths = [...$lines].map(line => line.getBoundingClientRect().width);
expect(widths[2], 'last line').to.be.lessThan(widths[0] * 0.75);
expect(widths[0], 'first two lines are the same length').to.be.closeTo(widths[1], 1);
});
});
});
describe('a spinner is visible in every theme', () => {
themeNames.forEach((theme) => {
it(`resolves the loader colour in ${theme}`, () => {
// `color='var(--blue)'` reaches Mantine as a string it does not understand and
// passes straight through to CSS. If the token were misspelled the property
// would resolve to nothing and the spinner would paint in currentColor -- or,
// for the segmented variants, not at all.
cy.mountUI(<Loading>Loading backups…</Loading>, {theme});
cy.get('.mantine-Loader-root').should(($loader) => {
const blue = getComputedStyle(document.documentElement)
.getPropertyValue('--blue').trim();
const resolved = getComputedStyle($loader[0])
.getPropertyValue('--loader-color').trim();
expect(resolved, '--loader-color resolved through the token')
.to.be.oneOf([blue, hexToRgb(blue)]);
});
cy.get('.mantine-Loader-root').should('be.visible');
});
});
});
describe('an icon stack carries the surface it was placed on', () => {
/*
* The corner glyph paints a disc behind itself so the two sets of strokes do not cross.
* That disc was hardcoded to the page background, which is right in the nav bar and
* wrong inside the file browser's filled blue button, where it read as a hole punched
* through the button. It is now `--icon-stack-bg`, and the call site says.
*/
themeNames.forEach((theme) => {
it(`matches a filled button in ${theme}`, () => {
cy.mountUI(
<Button color='blue'>
<IconStack corner={<Icon name='add'/>} label='New folder'
style={{'--icon-stack-bg': 'var(--blue)'}}>
<Icon name='folder'/>
</IconStack>
</Button>,
{theme},
);
cy.get('.wrolpi-icon-stack-corner').should(($corner) => {
const disc = getComputedStyle($corner[0]).backgroundColor;
const button = getComputedStyle($corner[0].closest('button')).backgroundColor;
expect(disc, 'the disc is the button fill').to.equal(button);
});
});
});
it('takes the surface from an ancestor, which is how the nav bar sets it', () => {
/*
* The nav bar's colour is a user setting written inline on the <nav>, so the stack
* inside it cannot name a token. It sets --icon-stack-bg on the bar itself and lets
* it inherit -- and this is the test for that, because setting the property on the
* stack (as the file browser does) would pass even if inheritance were broken.
*/
cy.mountUI(
<div style={{background: 'var(--red)', '--icon-stack-bg': 'var(--red)', padding: 20}}>
<IconStack corner={<Icon name='question'/>} label='WiFi status unknown'>
<Icon name='wifi'/>
</IconStack>
</div>,
{theme: 'light'},
);
cy.get('.wrolpi-icon-stack-corner').should(($corner) => {
const red = getComputedStyle(document.documentElement).getPropertyValue('--red').trim();
expect(getComputedStyle($corner[0]).backgroundColor).to.equal(hexToRgb(red));
});
});
it('defaults to the page background when nothing says otherwise', () => {
// The other half of the claim: the default still has to be right for the nav bar,
// which is where every other stack in the app lives.
cy.mountUI(
<IconStack corner={<Icon name='question'/>} label='WiFi status unknown'>
<Icon name='wifi'/>
</IconStack>,
{theme: 'night'},
);
cy.get('.wrolpi-icon-stack-corner').should(($corner) => {
const background = getComputedStyle(document.documentElement)
.getPropertyValue('--bg').trim();
expect(getComputedStyle($corner[0]).backgroundColor).to.equal(hexToRgb(background));
});
});
});
describe('corners stay hard', () => {
it('squares off the pills a MultiSelect renders for its chosen options', () => {
/*
* The design has no rounded corners, and mantine.ts zeroes every radius variable so
* a component that asks for `radius="md"` still gets none. Pill escapes that: it
* writes `--pill-radius: rem(1000)` onto the element, which outranks the theme, and
* the tags in a MultiSelect came out as lozenges among a page of square boxes.
*/
cy.mountUI(
<MultiSelect data={['Preserve', 'Radio']} defaultValue={['Preserve']} label='Tags'/>,
{theme: 'light'},
);
cy.get('[class*="mantine-Pill-root"]').should(($pill) => {
expect(getComputedStyle($pill[0]).borderRadius).to.equal('0px');
});
});
});
describe('theme tokens actually resolve', () => {
themeNames.forEach((theme) => {
it(`paints text and borders from ${theme}'s table`, () => {

18
app/src/components/ui/ui.css
View file

@ -35,13 +35,18 @@
/*
* The corner glyph sits on the main one, so it carries the surface color behind
* it — without that the two sets of strokes cross and neither reads.
*
* Which surface that is depends on where the stack was placed, and only the call
* site knows: the file browser puts one inside a filled blue button, where a disc
* of the page background is a hole punched in the button. `--icon-stack-bg`
* defaults to the page, and a call site anywhere else sets it.
*/
.wrolpi-icon-stack-corner {
position: absolute;
right: -3px;
bottom: -3px;
display: flex;
background: var(--bg);
background: var(--icon-stack-bg, var(--bg));
border-radius: 50%;
padding: 1px;
}
@ -357,6 +362,17 @@ html[data-theme="amber"] .wrolpi-searchbox-control:focus-within {
}
/* ------------------------------------------------------------- Form fields */
/*
* A Pill is what MultiSelect renders for each chosen option, and it is the one place
* Mantine does not honour the radius theme: `Pill` writes `--pill-radius: rem(1000)`
* on the element itself, which no amount of zeroing `--mantine-radius-xl` can reach.
* Everything else in the library has hard corners, so these do too.
*/
.mantine-Pill-root {
--pill-radius: 0;
border-radius: 0;
}
/*
* `hooks/useForm.js` renders its own <label> and native <input> — it always did, with
* Semantic's Form.Input only wrapping them for field styling. These rules supply that

117
app/src/components/ui/ui.test.js
View file

@ -13,10 +13,14 @@ import {
Button,
Card,
CardGroup,
Checkbox,
Confirm,
Header,
Icon,
IconStack,
Loader,
Loading,
Placeholder,
Pagination,
SearchBox,
TabBar,
@ -880,6 +884,119 @@ describe('Toggle', () => {
});
});
describe('Checkbox', () => {
it('reports and changes its state', async () => {
const onChange = jest.fn();
renderUI(<Checkbox label='Download comments' checked={false} onChange={onChange}/>);
const box = screen.getByRole('checkbox', {name: 'Download comments'});
expect(box).not.toBeChecked();
await userEvent.click(box);
expect(onChange).toHaveBeenCalled();
});
it('points Mantine\'s own variables at tokens', () => {
/*
* The only reason this wrapper exists. Mantine draws the box and the tick from
* --checkbox-color and --checkbox-icon-color; left alone they are Mantine's blue and
* a literal white, and a white tick is exactly the pixel night mode forbids.
*/
const {container} = renderUI(<Checkbox label='Download comments'/>);
const root = container.querySelector('.mantine-Checkbox-root');
expect(root.style.getPropertyValue('--checkbox-color')).toBe('var(--blue)');
expect(root.style.getPropertyValue('--checkbox-icon-color')).toBe('var(--btn-text)');
});
it('keeps those variables when the caller brings a style of its own', () => {
// `style={style}` instead of `style={{...checkboxStyles, ...style}}` would drop the
// tokens for any call site that sets so much as a margin, and only in that one place.
const {container} = renderUI(<Checkbox label='Download comments' style={{marginTop: 4}}/>);
const root = container.querySelector('.mantine-Checkbox-root');
expect(root.style.marginTop).toBe('4px');
expect(root.style.getPropertyValue('--checkbox-color')).toBe('var(--blue)');
});
});
describe('Loader', () => {
it('has an accessible name, because a spinner with no name is silence', () => {
renderUI(<Loader/>);
expect(screen.getByLabelText('Loading')).toBeInTheDocument();
});
it('says what is being waited on when the caller knows', () => {
renderUI(<Loader label='Fetching channels'/>);
expect(screen.getByLabelText('Fetching channels')).toBeInTheDocument();
});
it('takes its colour from a token rather than a fixed value', () => {
// A hex here would be one colour in all four themes, and a blue one in night mode.
const {container} = renderUI(<Loader/>);
expect(container.querySelector('.mantine-Loader-root').style.getPropertyValue('--loader-color'))
.toBe('var(--blue)');
});
});
describe('Loading', () => {
it('names the spinner after the caption, so the wait is announced', () => {
renderUI(<Loading>Loading backups…</Loading>);
expect(screen.getByLabelText('Loading backups…')).toBeInTheDocument();
});
it('falls back to a generic name when the caption is not text', () => {
// aria-label takes a string; handing it a React element yields "[object Object]".
renderUI(<Loading><strong>Loading backups…</strong></Loading>);
expect(screen.getByLabelText('Loading')).toBeInTheDocument();
});
it('renders no caption when there is nothing to say', () => {
const {container} = renderUI(<Loading/>);
expect(container.querySelector('.mantine-Loader-root')).toBeInTheDocument();
// An empty caption div would still take its line-height, moving the spinner off centre.
expect(container.querySelectorAll('div').length).toBe(1);
});
});
describe('Placeholder', () => {
const lineWidths = (container) =>
[...container.querySelectorAll('.mantine-Skeleton-root')]
.map(line => line.style.getPropertyValue('--skeleton-width'));
it('stands in for three lines by default', () => {
const {container} = renderUI(<Placeholder/>);
expect(lineWidths(container)).toHaveLength(3);
});
it('honours the number of lines asked for', () => {
const {container} = renderUI(<Placeholder lines={5}/>);
expect(lineWidths(container)).toHaveLength(5);
});
it('ends ragged, so it reads as text rather than a block', () => {
const {container} = renderUI(<Placeholder lines={4}/>);
expect(lineWidths(container)).toEqual(['100%', '100%', '100%', '60%']);
});
it('leaves a single line short as well', () => {
// lines={1} makes the first line the last one; an off-by-one here would render one
// full-width bar, which is a block and not a line of text.
const {container} = renderUI(<Placeholder lines={1}/>);
expect(lineWidths(container)).toEqual(['60%']);
});
});
describe('ThemePicker', () => {
const renderPicker = (themeContext = {}) => {
const setTheme = jest.fn();