diff --git a/app/cypress/support/component.js b/app/cypress/support/component.js index 9be6f32d..d27f01c5 100644 --- a/app/cypress/support/component.js +++ b/app/cypress/support/component.js @@ -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 -- 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( + + {mediaFilter ? : null} + {component} + , + 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', { diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js new file mode 100644 index 00000000..3e605237 --- /dev/null +++ b/app/src/components/ui/ui-layout.cy.js @@ -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( + , + {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( + , + ); + + /* + * 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( + /media/wrolpi} + 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( + Fetch} + />, + ); + + 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(, {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(poster, + {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( +
+ poster +
, + {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(poster, + {theme: 'night'}); + + cy.get('img').should(($img) => + expect(getComputedStyle($img[0]).filter).to.equal('none')); + }); +}); diff --git a/app/src/components/ui/ui.test.js b/app/src/components/ui/ui.test.js index 5fdce00a..e4bd104a 100644 --- a/app/src/components/ui/ui.test.js +++ b/app/src/components/ui/ui.test.js @@ -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( diff --git a/app/src/themes/tokens.css b/app/src/themes/tokens.css index 387c67a2..f16dda44 100644 --- a/app/src/themes/tokens.css +++ b/app/src/themes/tokens.css @@ -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; } diff --git a/app/src/themes/tokens.test.js b/app/src/themes/tokens.test.js index 4774bc25..b7f0719e 100644 --- a/app/src/themes/tokens.test.js +++ b/app/src/themes/tokens.test.js @@ -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', () => {