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: a browser layer for the library, and the .media reset that never applied

Browse source 
The PathInput overlap could not have been caught by jest -- jsdom computes no
layout -- so the only guard was a grep for one spelling of one mistake.  This
adds the layer that can actually see it, and it found a real bug immediately.

ui-layout.cy.js mounts library components in a real browser through a new
cy.mountUI(), which supplies Mantine's provider and a stamped data-theme and
nothing else.  That spartan setup is the point: these components read no
ambient state, so a spec can assert what jsdom cannot compute -- real geometry,
resolved custom properties, applied filters -- across all four themes.

It reproduces the leftSectionWidth='auto' overlap on purpose and asserts it
still overlaps, because a geometry check that has never gone red is
indistinguishable from one that cannot fail.

The bug it found: the rule resetting `filter` on leaf media inside a `.media`
wrapper scored (0,2,2) against the (0,3,1) rule it was correcting, so it lost
every time and never applied to anything.  Every image inside a filtered
wrapper has been through the luminance matrix twice since that rule landed --
markedly darker than its surroundings, which is exactly what it was written to
prevent.  tokens.test.js asserted only that the rule's text was present, and so
stayed green throughout; it now compares the two selectors' specificity, which
is computable without a browser and goes red on the old selector.

Three guards in ui.test.js keep the library mock-free by construction rather
than by habit: no jest.mock in its own tests, no fetch in any library file, and
no useContext outside the theme controls, whose job is to read the theme.  All
three were verified red against planted violations.

959 jest tests, 14 new browser tests, clean cache-busted build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-07-30 15:31:09 -06:00 • committed by lrnselfreliance
parent c534cbfe23
commit c100ffb378
5 changed files with 338 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

55
app/cypress/support/component.js
View file

@ -25,6 +25,7 @@ import {QueryProvider} from "../../src/hooks/customHooks";
import {TagsProvider} from "../../src/Tags";
import {MantineProvider} from "@mantine/core";
import {cssVariablesResolver, mantineTheme} from "../../src/themes/mantine";
import {MediaFilterDefs} from "../../src/themes/MediaFilterDefs";
import React from "react";
Cypress.on('uncaught:exception', (err, runnable) => {
@ -60,6 +61,60 @@ Cypress.Commands.add('mountWithRouter', (component, options) => {
);
});
/*
* Mount a component from src/components/ui with nothing but Mantine's provider and the
* theme stamped on <html> -- no router, no query client, no tag fixtures.
*
* That spartan setup is the whole point. These components take props and read no ambient
* state, so a real browser can render one in isolation, which is what lets a spec assert
* things jsdom cannot compute: whether two boxes overlap, whether text is clipped, whether
* a token actually resolved. The Settings page shipped five unreadable inputs because
* `padding-inline-start: auto` silently became zero, and no jsdom test could have seen it --
* getBoundingClientRect() returns zeros there.
*
* `theme` stamps data-theme the way ThemeProvider does; `mediaFilter` stamps
* data-media-filter and mounts the SVG filter definitions, since a filter referenced but
* never defined silently does nothing.
*/
Cypress.Commands.add('mountUI', (component, options = {}) => {
const {theme = 'light', mediaFilter, ...mountOptions} = options;
const html = document.documentElement;
html.dataset.theme = theme;
if (mediaFilter) html.dataset.mediaFilter = mediaFilter;
else delete html.dataset.mediaFilter;
return cy.mount(
<MantineProvider theme={mantineTheme} cssVariablesResolver={cssVariablesResolver}>
{mediaFilter ? <MediaFilterDefs/> : null}
{component}
</MantineProvider>,
mountOptions
);
});
/**
* Fail unless two elements are laid out side by side.
*
* Reads real geometry, so it catches an element painted over another regardless of how the
* overlap came about -- an invalid length, a collapsed flex row, a stale absolute position.
*/
Cypress.Commands.add('shouldNotOverlap', {prevSubject: false}, (firstSelector, secondSelector) => {
cy.get(firstSelector).then(($first) => {
cy.get(secondSelector).then(($second) => {
const a = $first[0].getBoundingClientRect();
const b = $second[0].getBoundingClientRect();
const overlaps = a.right > b.left + 0.5 && b.right > a.left + 0.5
&& a.bottom > b.top + 0.5 && b.bottom > a.top + 0.5;
expect(
overlaps,
`${firstSelector} [${a.left}, ${a.right}] must not overlap `
+ `${secondSelector} [${b.left}, ${b.right}]`,
).to.equal(false);
});
});
});
Cypress.Commands.add('mountWithTags', (component, options) => {
cy.fixture('tagsMock.json').then((mockTags) => {
cy.intercept('GET', '**/api/tag', {

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

@ -0,0 +1,175 @@
import React from 'react';
import {ActionInput, Button, PathInput, TextInput} from './index';
import {themeNames} from '../../themes/names';
/*
* Layout and theming assertions for the component library, run in a real browser.
*
* These belong here rather than in ui.test.js because jsdom parses CSS but never lays it
* out: getBoundingClientRect() returns zeros, getComputedStyle() will not resolve a custom
* property through the cascade, and no filter is ever applied. A jest test therefore
* cannot tell whether two boxes overlap, whether a token resolved, or whether media got
* tinted -- which is precisely how five unreadable inputs reached the Settings page.
*
* Keep this file to claims that need real layout. Behaviour that jsdom can judge belongs
* in ui.test.js, which runs in about two seconds against the whole library.
*/
describe('PathInput lays its prefix beside the value, never over it', () => {
// Every theme, because a theme is free to restyle borders and padding, and the fix has
// to hold in all four rather than in whichever one happened to be open.
themeNames.forEach((theme) => {
it(`keeps the prefix clear of the input in ${theme}`, () => {
cy.mountUI(
<PathInput
label='Archive Destination'
prefix='/media/wrolpi'
defaultValue='archive/%(domain)s'
/>,
{theme},
);
cy.shouldNotOverlap('.wrolpi-path-input-prefix', '.wrolpi-path-input-control input');
// Sharing one outline is the reason the pair reads as a single control; a gap
// would mean the flex row broke and they are two separate fields again.
cy.get('.wrolpi-path-input-prefix').then(($prefix) => {
cy.get('.wrolpi-path-input-control input').then(($input) => {
const prefix = $prefix[0].getBoundingClientRect();
const input = $input[0].getBoundingClientRect();
expect(input.left - prefix.right, 'gap between prefix and input')
.to.be.closeTo(0, 2);
});
});
});
});
it('shows the whole value, with room left to type', () => {
cy.mountUI(
<PathInput prefix='/media/wrolpi' defaultValue='archive/%(domain)s'/>,
);
/*
* scrollWidth well beyond clientWidth means the value is cut off -- the visible
* symptom when a prefix eats the input's usable width. Both are rounded to whole
* pixels from a fractional layout, so they disagree by a pixel on a field that fits
* perfectly; the tolerance is for that, and is far below the tens of pixels a real
* clip costs.
*/
cy.get('.wrolpi-path-input-control input').should(($input) => {
const el = $input[0];
expect(el.scrollWidth, 'value is not clipped').to.be.at.most(el.clientWidth + 2);
expect(el.clientWidth, 'input has usable width').to.be.greaterThan(80);
});
});
it('detects the overlap that started this, so the check has teeth', () => {
/*
* The original defect, reproduced deliberately: Mantine turns leftSectionWidth into
* --input-padding-inline-start, and `padding-inline-start: auto` is invalid, so it
* resolves to zero and the absolutely-positioned section prints over the value.
*
* Asserting that this DOES overlap proves shouldNotOverlap is measuring something
* real. A geometry check that has never once gone red is indistinguishable from a
* check that cannot fail.
*/
cy.mountUI(
<TextInput
leftSectionWidth='auto'
leftSection={<span className='probe-section'>/media/wrolpi</span>}
defaultValue='archive/%(domain)s'
/>,
);
cy.get('.probe-section').then(($section) => {
cy.get('input').then(($input) => {
const section = $section[0].getBoundingClientRect();
const input = $input[0].getBoundingClientRect();
expect(
section.right > input.left + 0.5,
'the historical bug still overlaps, so the geometry check is meaningful',
).to.equal(true);
});
});
});
});
describe('ActionInput', () => {
it('joins its button to the field without covering it', () => {
cy.mountUI(
<ActionInput
defaultValue='https://example.com'
action={<Button>Fetch</Button>}
/>,
);
cy.shouldNotOverlap('.wrolpi-action-input input', '.wrolpi-action-input button');
});
});
describe('theme tokens actually resolve', () => {
themeNames.forEach((theme) => {
it(`paints text and borders from ${theme}'s table`, () => {
cy.mountUI(<PathInput prefix='/media/wrolpi' defaultValue='x'/>, {theme});
/*
* A token that a theme forgot to define resolves to the empty string, and the
* property falls back to something inherited -- usually black text on a dark
* panel. jsdom cannot catch that; it does not resolve var() at all.
*/
cy.get('.wrolpi-path-input-control input').should(($input) => {
const colour = getComputedStyle($input[0]).color;
expect(colour, 'input text colour resolved').to.match(/^rgba?\(/);
// Only an rgba() alpha of zero is invisible. Matching a trailing ", 0)"
// loosely would flag amber's opaque rgb(255, 149, 0) for its blue channel.
expect(colour, 'input text is not transparent')
.not.to.match(/^rgba\(.*,\s*0(\.0+)?\)$/);
});
cy.get('.wrolpi-path-input-control').should(($control) => {
expect(getComputedStyle($control[0]).borderTopColor, 'border resolved')
.to.match(/^rgba?\(/);
});
});
});
});
describe('media filtering', () => {
it('tints a bare image in night', () => {
cy.mountUI(<img alt='poster' src='/logo.png' width='80' height='80'/>,
{theme: 'night', mediaFilter: 'night-red'});
cy.get('img').should(($img) =>
expect(getComputedStyle($img[0]).filter).to.contain('wrolpi-night-red'));
});
it('filters a .media wrapper once, not its contents twice', () => {
/*
* Two passes of a luminance-to-red matrix is not the same as one -- the second pass
* takes the luminance of an already-red pixel and the image comes out markedly
* darker than its surroundings. So a `.media` wrapper is filtered as a unit and the
* leaf inside it must come back `none`.
*/
cy.mountUI(
<div className='media'>
<img alt='poster' src='/logo.png' width='80' height='80'/>
</div>,
{theme: 'night', mediaFilter: 'night-red'},
);
cy.get('.media').should(($wrapper) =>
expect(getComputedStyle($wrapper[0]).filter).to.contain('wrolpi-night-red'));
cy.get('.media img').should(($img) =>
expect(getComputedStyle($img[0]).filter).to.equal('none'));
});
it('leaves media alone when the user has turned the filter off', () => {
// The attribute is absent rather than empty when filtering is off, so night with the
// setting disabled must match no rule at all.
cy.mountUI(<img alt='poster' src='/logo.png' width='80' height='80'/>,
{theme: 'night'});
cy.get('img').should(($img) =>
expect(getComputedStyle($img[0]).filter).to.equal('none'));
});
});

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

@ -366,7 +366,10 @@ describe('no input reserves space for a section it cannot measure', () => {
for (const entry of fs.readdirSync(dir, {withFileTypes: true})) {
const full = path.join(dir, entry.name);
if (entry.isDirectory()) walk(full);
else if (/\.(js|jsx|tsx)$/.test(entry.name) && !/\.test\./.test(entry.name)) {
// Specs are exempt: ui-layout.cy.js reproduces this exact mistake on
// purpose, to prove its overlap check can still detect it.
else if (/\.(js|jsx|tsx)$/.test(entry.name)
&& !/\.test\.|\.cy\./.test(entry.name)) {
// Comments are stripped first: the component that replaced this
// pattern documents it by name, and prose about a mistake is not the
// mistake.
@ -386,6 +389,56 @@ describe('no input reserves space for a section it cannot measure', () => {
});
});
describe('the library stays mockable-free', () => {
/*
* The whole reason these 80-odd tests need no setup is that a library component takes
* props and reaches for nothing else -- no module-level fetch, no API helper, no
* ambient state it has to be lied to about. Pages are the opposite: DomainEditPage
* needs seven jest.mock calls before it will render at all, which is precisely why the
* inputs on the Settings page had no test when they broke.
*
* That property is worth asserting rather than hoping for. The day a library
* component grows a fetch, the mock somebody has to add to keep this file passing
* trips the first check, and the import trips the second.
*/
const libraryFiles = () => fs.readdirSync(__dirname)
.filter(name => /\.(ts|tsx)$/.test(name) && !/\.test\./.test(name))
.map(name => [name, fs.readFileSync(path.join(__dirname, name), 'utf8')]);
it('needs no module mocked to render any of it', () => {
const source = fs.readFileSync(path.join(__dirname, 'ui.test.js'), 'utf8')
.replace(/\/\*[\s\S]*?\*\//g, '')
.replace(/^\s*\/\/.*$/gm, '');
expect(source).not.toMatch(/jest\.mock\s*\(/);
});
it('asks the network for nothing', () => {
const offenders = libraryFiles()
.filter(([, source]) => /\bfetch\s*\(|\bapiCall\b|\baxios\b/
.test(source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, '')))
.map(([name]) => name);
expect(offenders).toEqual([]);
});
it('reads ambient state only in the theme controls', () => {
/*
* ThemePicker and MediaFilterToggle are the deliberate exception -- a control whose
* entire job is to show and change the current theme has to know it. Their tests
* pass a real ThemeContext.Provider holding plain values, which is supplying data,
* not mocking a module. Everything else takes props.
*/
const offenders = libraryFiles()
.filter(([name]) => name !== 'ThemePicker.tsx')
.filter(([, source]) => /useContext\s*\(/.test(source))
.map(([name]) => name);
expect(offenders).toEqual([]);
});
});
describe('ActionInput', () => {
it('joins the input and its action into one control', () => {
const {container} = renderUI(

9
app/src/themes/tokens.css
View file

@ -261,8 +261,15 @@ html[data-media-filter="amber-mono"] :is(img, video, canvas, iframe, embed, obje
*
* The audio player's poster is the case that found this — the poster sits under
* custom control glyphs, so the wrapper is filtered to keep the two tinted together.
*
* The trailing `:not(.night-unfiltered)` is here for specificity, not for its meaning.
* The rule above scores (0,3,1) — an attribute, a `.media` inside `:is()`, and a `:not()`
* — so this reset had to reach three class-level components to win it; without the extra
* one it scored (0,2,2) and silently lost every time, which is exactly what it did until
* a browser test measured the leaf and found it filtered twice. jsdom cannot catch this:
* it never resolves `filter` at all.
*/
html[data-media-filter] .media :is(img, video, canvas, iframe, embed, object) {
html[data-media-filter] .media :is(img, video, canvas, iframe, embed, object):not(.night-unfiltered) {
filter: none;
}

51
app/src/themes/tokens.test.js
View file

@ -120,11 +120,52 @@ describe('media filter rules', () => {
});
it('does not filter leaf media twice inside a filtered wrapper', () => {
// A `.media` wrapper filters its subtree as a unit; the img/video inside it also
// matches the element-type rule, and two passes of the matrix take the luminance
// of an already-red pixel, leaving that image much darker than its surroundings.
expect(tokensCss).toMatch(
/html\[data-media-filter] \.media :is\([^)]*\)\s*{\s*filter:\s*none/);
/*
* A `.media` wrapper filters its subtree as a unit; the img/video inside it also
* matches the element-type rule, and two passes of the matrix take the luminance of
* an already-red pixel, leaving that image much darker than its surroundings.
*
* This used to assert only that the reset rule's text was present, and so it passed
* for as long as the rule existed -- including the whole time the rule lost on
* specificity and never applied to anything. Presence is not effect. What has to
* hold is that the reset outranks the rule it is correcting, which is computable
* from the selectors themselves. ui-layout.cy.js measures the resulting `filter` in
* a real browser, which is the only place the effect itself is observable.
*/
const specificity = (selector) => {
// Enough of the algorithm for these selectors: :is() and :not() contribute the
// highest specificity among their arguments.
let classes = 0, elements = 0;
const counted = selector.replace(/:(is|not)\(([^)]*)\)/g, (_, __, args) => {
const best = args.split(',')
.map(arg => specificity(arg.trim()))
.sort((a, b) => (b.classes - a.classes) || (b.elements - a.elements))[0];
classes += best.classes;
elements += best.elements;
return ' ';
});
classes += (counted.match(/\.[\w-]+|\[[^\]]*]|:[\w-]+/g) || []).length;
elements += (counted.match(/(^|[\s>+~])[a-z][\w-]*/gi) || []).length;
return {classes, elements};
};
const rank = (selector) => {
const {classes, elements} = specificity(selector);
return classes * 1000 + elements;
};
const selectorFor = (needle) => {
const match = tokensCss.split('}')
.map(block => block.split('{')[0].trim())
.filter(Boolean)
.find(selector => selector.includes(needle));
if (!match) throw new Error(`tokens.css has no rule matching ${needle}`);
return match.split('\n').pop().trim();
};
const reset = selectorFor('.media :is(img');
const filtered = selectorFor('[data-media-filter="night-red"] :is(img');
expect(rank(reset)).toBeGreaterThan(rank(filtered));
});
it('never filters an ancestor, only leaf media', () => {