2025-10-29 22:16:33 -06:00
|
|
|
import React from 'react';
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
import fs from 'fs';
|
|
|
|
|
import path from 'path';
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
import {act, render, screen, waitFor} from '../test-utils';
|
2025-10-29 22:16:33 -06:00
|
|
|
import userEvent from '@testing-library/user-event';
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
import {
|
ui: act on review -- corner chrome, a swallowed className, and the poster frame
Three real defects from the PR review, each reproduced before it was fixed.
The tag mark and a video's duration were pinned to the poster BAND, which
is the full width of the card, rather than to the painted image. Once the
height cap landed the two stopped being the same box: measured, the mark
sat 39px clear of a book cover, 56px clear of a small poster and 60px
clear of a vertical video, and the duration 55px clear. It went unnoticed
because on a 16:9 poster the image fills the band and the two pin to the
same place -- so the tests cover the height-bound ratios, and assert the
letterbox gap is really there before measuring into it.
CardPoster now centres a frame that shrink-wraps the image, and the frame
is what corner chrome is positioned against. VideoCard passes its
duration through a new `overlay` prop rather than wrapping CardPoster in a
box of its own.
The mark also went back INSIDE the link. Splitting it out made the
tagged corner a dead spot on an otherwise clickable poster.
CardLink and ExternalCardLink wrote `className='...' {...props}`, so a
caller's className replaced the pair instead of adding to it. Every
archive title passes `card-title-ellipsis`, so every archive title lost
`card-link` -- the one card type this PR migrates for layout was the one
missing the title treatment. Both compose now.
On the fourth comment, the breadth of the ActionIcon remap: it is
deliberate and the numbers are small. 59 IconButton call sites, 52 of
them at the default size, 7 at `xs` (one in Flasher, three in the
inventory schema editor). The one library control that stands beside an
input rather than a button is SearchBox's clear, and the remap is exactly
what that scale is for -- it now matches its field to the pixel, where
before it was 28px against a 36px input. Guarded by a test that fails at
42 vs 36 if the default is reverted.
1078 jest, 158 in ui-layout.cy.js, clean build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-02 11:35:49 -06:00
|
|
|
CardLink, contrastingColor, contrastRatio, DirectorySearch, ExternalCardLink, HelpHeader,
|
|
|
|
|
InfoHeader, loadRole,
|
ui: pages own the space between their blocks
Panels shared edges: two 1px borders meeting, which reads as one tall
surface with a rule across it rather than a seam between two.
Done as `margin-bottom` on the panel first, which is the anti-pattern, and
it failed in both of the ways that rule is known to fail before the reading
was done. It compounded with the gap of every Stack that already held
panels -- four of them, spacing those lists at 35.2px against the app's
17.6px -- and the `:last-child` reset meant to keep trailing space off the
bottom of a container silently did nothing, because Media wrapping makes a
panel the only child of its own wrapper. On Status that separated four
pairs out of five and left the first touching.
So the space belongs to whatever stacks the blocks. `.wrolpi-stack` is on
PageContainer, which every page uses, and on the three wrappers that stack
blocks somewhere the page's own stack cannot reach: Settings' panel wrapper,
the block below the video player, and the Zim and Map route wrappers, which
hold their tab bar outside PageContainer.
A gap rather than `> * + * { margin-top }`, which was tried and is quietly
broken here: Media injects `.fresnel-container { margin: 0; padding: 0 }` at
runtime, same specificity, later source order, so every wrapped block threw
the margin away. A child cannot zero its parent's gap. It also means
Media's hidden half contributes nothing, where the owl selector spaced
around something invisible.
With the page owning the spacing, the blocks that owned some of it had to
give it up, or their margin was added to the gap: the tab bar (33px against
17.6 everywhere else), the shared search control bar and paginator on
Videos/Archives/Docs/Files, the card grid, the file browser's filter row and
the inventory search. Two margins deliberately stay -- the file browser's
4em, which reserves room for the sticky buttons (62645831), and heading
margins, which bind a heading to the content below it.
Tests plant each of the three failures: the wrapped block, a child that
zeroes its own margin, and a hidden Media half. Verified in the browser on
sixteen routes and in all four themes -- every stacked pair 17.6px, the
map's 85vh box and the video player unchanged, no horizontal overflow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:40:27 -06:00
|
|
|
PageContainer,
|
ui: headings open their section, and keep their help icon on one line
Two things, and the second was a regression this migration caused.
A heading had `margin: 0 0 10px` -- no top margin at all -- so it sat flush
against whatever came before and read as part of it rather than as the start of
the next thing. On /more/statistics "Tags" touched the panel above it with a
0px gap. Now 22px above and 10px below, because a heading belongs to what
follows it.
`* + .wrolpi-header` rather than a blanket margin, for the same reason
StatisticGroup needed it: a heading that opens a Panel or a page is already
where it should be, and pushing it down just dents the top of its own container.
The help icon on every HelpHeader and InfoHeader had dropped to its own line --
"Root CA Certificate" in Settings is the clearest one. `.inline-header` set
`h1..h5 {display: inline-block}` so the heading and a sibling `<span>` shared a
row, which worked while these were a bare `<h2>` and stopped the moment they
gained a block wrapper: the `<label>`'s block child forced a break.
Fixed in the library rather than patched in App.css. `Header` takes an `after`
slot rendered inside the heading element, where the existing flex row already
handles alignment and spacing. Both wrappers pass their icon through it, the
`<label>` moves inside the heading so `for_` keeps associating the text with its
field (three call sites rely on it), and the `.inline-header` rules are deleted.
Fifteen call sites are fixed by fixing the two wrappers.
Tests: containment in jest, and the part jsdom cannot judge -- that the two
boxes actually share a line, and that the heading stays one row tall -- in a
browser. Planted both original shapes: the margin removed, a blanket margin
denting its container, and the control moved back outside the heading element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:41:58 -06:00
|
|
|
SearchResultsInput, TAG_TEXT_DARK, TAG_TEXT_LIGHT,
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
} from './Common';
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
import {healthRole} from './admin/ControllerPage';
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
// Mock debounce to make tests run faster
|
|
|
|
|
jest.mock('lodash/debounce', () => jest.fn(fn => {
|
|
|
|
|
fn.cancel = jest.fn();
|
|
|
|
|
return fn;
|
|
|
|
|
}));
|
|
|
|
|
|
|
|
|
|
// Mock useSearchDirectories hook to avoid async state updates that cause act() warnings
|
|
|
|
|
const mockSetDirectoryName = jest.fn();
|
|
|
|
|
let mockHookState = {
|
|
|
|
|
directoryName: '',
|
|
|
|
|
directories: [],
|
|
|
|
|
channelDirectories: [],
|
|
|
|
|
domainDirectories: [],
|
|
|
|
|
isDir: false,
|
|
|
|
|
loading: false,
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
jest.mock('../hooks/customHooks', () => ({
|
|
|
|
|
...jest.requireActual('../hooks/customHooks'),
|
|
|
|
|
useSearchDirectories: (value) => {
|
|
|
|
|
// Return current mock state - tests control state via setMockHookState
|
|
|
|
|
return {
|
|
|
|
|
...mockHookState,
|
|
|
|
|
setDirectoryName: (newValue) => {
|
|
|
|
|
mockHookState.directoryName = newValue;
|
|
|
|
|
mockSetDirectoryName(newValue);
|
|
|
|
|
},
|
|
|
|
|
};
|
|
|
|
|
},
|
|
|
|
|
}));
|
|
|
|
|
|
|
|
|
|
describe('DirectorySearch', () => {
|
|
|
|
|
const mockOnSelect = jest.fn();
|
|
|
|
|
|
|
|
|
|
const mockSearchResults = {
|
|
|
|
|
directories: [
|
|
|
|
|
{path: 'videos/nature'},
|
|
|
|
|
{path: 'videos/tech'}
|
|
|
|
|
],
|
|
|
|
|
channelDirectories: [
|
|
|
|
|
{path: 'videos/channels/news', name: 'News Channel'}
|
|
|
|
|
],
|
|
|
|
|
domainDirectories: [
|
|
|
|
|
{path: 'archive/example.com', domain: 'example.com'}
|
|
|
|
|
],
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
// Helper to reset mock hook state with specific values
|
|
|
|
|
const setMockHookState = (overrides = {}) => {
|
|
|
|
|
mockHookState = {
|
|
|
|
|
directoryName: '',
|
|
|
|
|
directories: [],
|
|
|
|
|
channelDirectories: [],
|
|
|
|
|
domainDirectories: [],
|
|
|
|
|
isDir: false,
|
|
|
|
|
loading: false,
|
|
|
|
|
...overrides,
|
|
|
|
|
};
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
beforeEach(() => {
|
|
|
|
|
jest.clearAllMocks();
|
|
|
|
|
// Reset mock hook state with default search results
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
isDir: false,
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
describe('Rendering', () => {
|
|
|
|
|
it('renders with placeholder text', () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
expect(input).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('shows initial value when provided', () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
directoryName: 'videos/test',
|
|
|
|
|
});
|
|
|
|
|
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value="videos/test"/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByDisplayValue('videos/test');
|
|
|
|
|
expect(input).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('applies disabled state correctly', () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value="" disabled/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
expect(input).toBeDisabled();
|
|
|
|
|
});
|
|
|
|
|
|
2026-06-24 15:23:32 -06:00
|
|
|
it('renders the dropdown without crashing when directoryName is null', async () => {
|
|
|
|
|
// An unset form field (e.g. the new Channel page) passes a null value. With
|
|
|
|
|
// isDir=false a "New Directory" result is built from directoryName; if that title is
|
ui: the last of Semantic UI, in name and in dead CSS
The library has been uninstalled for some time, but the codebase still read as
though it were there: identifiers named after it, rules written against its
class names, and about 180 comments explaining a rule in terms of a thing that
is no longer installed.
Renamed, keeping the vocabulary the app actually uses: semanticUIColorMap ->
navColorHexMap, semanticColorNames -> paletteColorNames, semanticNames ->
iconNameAliases, semanticSizes -> sizeAliases, and SemanticUIColors ->
NavColors in wrolpi/schema.py, which is what validates nav_color on the way in.
ColorToSemanticHexColor had no callers at all and is gone.
The baseline ratchet has done its job -- the list reached zero, so the JSON is
deleted and the guard keeps only the checks with teeth: no package, no import,
no `className="ui "`, no toast library. Its Theme.tsx check went with the
wrappers it was watching for. It also asserts its own walk found files, so a
clean result cannot mean it scanned nothing.
Deleted from App.css: every rule requiring `.ui`, `.inverted`, `.segment` or
`.modal`, none of which any component has set since the stylesheet stopped
loading, plus the form skeletons, ui-alerts and progress-text rules that
nothing renders. Two of them were doing harm rather than nothing: the drag
tints had a dark-theme variant under `.inverted` that stopped applying, leaving
15% of a fixed blue over a dark row, and now mix a token instead.
Two selectors naming markup that no longer exists were live bugs. The file
browser's filter had `.ui.input.file-browser-filter`, so it rendered at the
browser default width on a bar laid out to give it 34em -- now qualified by its
container, because `.wrolpi-action-input` sets `width: 100%` at the same
specificity and is loaded later. And SearchBox takes named props rather than a
rest spread, so the `id` DestinationForm hands it was dropped in silence and
the label it renders pointed at nothing; the download forms' Destination field
has had no associated label since the migration.
"Semantic roles" survives in four places, where it means what it says.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 19:15:57 -06:00
|
|
|
// null, the old Search crashed reading null.length when the menu opened.
|
2026-06-24 15:23:32 -06:00
|
|
|
setMockHookState({
|
|
|
|
|
directories: [],
|
|
|
|
|
channelDirectories: [],
|
|
|
|
|
domainDirectories: [],
|
|
|
|
|
isDir: false,
|
|
|
|
|
directoryName: null,
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value={null}/>);
|
|
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
// Opening the menu (as Tab/focus does in the browser) must not throw.
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
await waitFor(() => {
|
|
|
|
|
expect(screen.getByText(/New Directory/i)).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
2025-10-29 22:16:33 -06:00
|
|
|
it('displays with required indicator', () => {
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
render(
|
2025-12-06 14:49:46 -07:00
|
|
|
<DirectorySearch onSelect={mockOnSelect} value="" required/>
|
2025-10-29 22:16:33 -06:00
|
|
|
);
|
|
|
|
|
|
ui: the last of Semantic UI, in name and in dead CSS
The library has been uninstalled for some time, but the codebase still read as
though it were there: identifiers named after it, rules written against its
class names, and about 180 comments explaining a rule in terms of a thing that
is no longer installed.
Renamed, keeping the vocabulary the app actually uses: semanticUIColorMap ->
navColorHexMap, semanticColorNames -> paletteColorNames, semanticNames ->
iconNameAliases, semanticSizes -> sizeAliases, and SemanticUIColors ->
NavColors in wrolpi/schema.py, which is what validates nav_color on the way in.
ColorToSemanticHexColor had no callers at all and is gone.
The baseline ratchet has done its job -- the list reached zero, so the JSON is
deleted and the guard keeps only the checks with teeth: no package, no import,
no `className="ui "`, no toast library. Its Theme.tsx check went with the
wrappers it was watching for. It also asserts its own walk found files, so a
clean result cannot mean it scanned nothing.
Deleted from App.css: every rule requiring `.ui`, `.inverted`, `.segment` or
`.modal`, none of which any component has set since the stylesheet stopped
loading, plus the form skeletons, ui-alerts and progress-text rules that
nothing renders. Two of them were doing harm rather than nothing: the drag
tints had a dark-theme variant under `.inverted` that stopped applying, leaving
15% of a fixed blue over a dark row, and now mix a token instead.
Two selectors naming markup that no longer exists were live bugs. The file
browser's filter had `.ui.input.file-browser-filter`, so it rendered at the
browser default width on a bar laid out to give it 34em -- now qualified by its
container, because `.wrolpi-action-input` sets `width: 100%` at the same
specificity and is loaded later. And SearchBox takes named props rather than a
rest spread, so the `id` DestinationForm hands it was dropped in silence and
the label it renders pointed at nothing; the download forms' Destination field
has had no associated label since the migration.
"Semantic roles" survives in four places, where it means what it says.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 19:15:57 -06:00
|
|
|
// Unlike the old Search, the SearchBox forwards `required` straight
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
// onto the input.
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
expect(input).toBeRequired();
|
2025-10-29 22:16:33 -06:00
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
describe('Search Functionality', () => {
|
|
|
|
|
it('triggers setDirectoryName on value change', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
await userEvent.type(input, 'videos');
|
|
|
|
|
|
|
|
|
|
// Verify setDirectoryName was called (via mocked hook)
|
|
|
|
|
expect(mockSetDirectoryName).toHaveBeenCalled();
|
|
|
|
|
// Debounce is mocked, so each character triggers a call
|
|
|
|
|
// Verify it was called 6 times (one per character in "videos")
|
|
|
|
|
expect(mockSetDirectoryName.mock.calls.length).toBe(6);
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('shows loading indicator when loading state is true', () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
loading: true,
|
|
|
|
|
});
|
|
|
|
|
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
const {container} = render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
// SearchBox renders a Loader alongside the input while `loading` is true.
|
|
|
|
|
expect(container.querySelector('.wrolpi-searchbox-loading')).toBeInTheDocument();
|
2025-10-29 22:16:33 -06:00
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('hides loading indicator when loading state is false', () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
loading: false,
|
|
|
|
|
});
|
|
|
|
|
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
const {container} = render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
expect(container.querySelector('.wrolpi-searchbox-loading')).not.toBeInTheDocument();
|
2025-10-29 22:16:33 -06:00
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('displays categorized results', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
// Click to open dropdown
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
await waitFor(() => {
|
|
|
|
|
// Should show category names
|
|
|
|
|
expect(screen.getAllByText(/Directories/i).length).toBeGreaterThan(0);
|
|
|
|
|
expect(screen.getAllByText(/Channels/i).length).toBeGreaterThan(0);
|
|
|
|
|
expect(screen.getAllByText(/Domains/i).length).toBeGreaterThan(0);
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('shows "New Directory" when path doesn\'t exist (isDir=false)', async () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
directories: [],
|
|
|
|
|
channelDirectories: [],
|
|
|
|
|
domainDirectories: [],
|
|
|
|
|
isDir: false,
|
|
|
|
|
directoryName: 'new/path',
|
|
|
|
|
});
|
|
|
|
|
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
await waitFor(() => {
|
|
|
|
|
expect(screen.getByText(/New Directory/i)).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('hides "New Directory" when path exists (isDir=true)', async () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
directories: [{path: 'videos/nature/wildlife'}],
|
|
|
|
|
channelDirectories: [],
|
|
|
|
|
domainDirectories: [],
|
|
|
|
|
isDir: true,
|
|
|
|
|
directoryName: 'videos/nature',
|
|
|
|
|
});
|
|
|
|
|
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
// "New Directory" should not appear when is_dir=true
|
|
|
|
|
expect(screen.queryByText(/New Directory/i)).not.toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('debounces rapid typing (verifies setDirectoryName is called)', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
|
|
|
|
|
// Type rapidly
|
|
|
|
|
await userEvent.type(input, 'abc', {delay: 10});
|
|
|
|
|
|
|
|
|
|
// Verify setDirectoryName was called
|
|
|
|
|
expect(mockSetDirectoryName).toHaveBeenCalled();
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
describe('User Interactions', () => {
|
|
|
|
|
it('calls onSelect when result is clicked', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
await waitFor(() => {
|
|
|
|
|
expect(screen.getByText('videos/nature')).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
// Click on a result
|
|
|
|
|
const result = screen.getByText('videos/nature');
|
|
|
|
|
await userEvent.click(result);
|
|
|
|
|
|
|
|
|
|
expect(mockOnSelect).toHaveBeenCalledWith('videos/nature');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('commits typed value on blur when directoryName differs from value', async () => {
|
|
|
|
|
// Set up mock state where directoryName differs from the prop value
|
|
|
|
|
// This simulates what happens after user types in the input
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
directoryName: 'typed/path', // User has typed this
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
// Render with empty value prop (different from directoryName)
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
// The SearchBox wires `onBlur` straight to the input (there is no longer a
|
|
|
|
|
// wrapping Search component to blur instead).
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
// Blur the input - should trigger onBlur which calls onSelect
|
2025-10-29 22:16:33 -06:00
|
|
|
await act(async () => {
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
const {fireEvent} = require('../test-utils');
|
|
|
|
|
fireEvent.blur(input);
|
2025-10-29 22:16:33 -06:00
|
|
|
});
|
|
|
|
|
|
|
|
|
|
// Should call onSelect with directoryName from hook state
|
|
|
|
|
expect(mockOnSelect).toHaveBeenCalledWith('typed/path');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('does not call onSelect on blur if value unchanged', async () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
directoryName: 'existing/path',
|
|
|
|
|
});
|
|
|
|
|
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value="existing/path"/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByDisplayValue('existing/path');
|
|
|
|
|
|
|
|
|
|
// Blur without changing value
|
|
|
|
|
await act(async () => {
|
|
|
|
|
input.blur();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
// Should not call onSelect since value didn't change
|
|
|
|
|
expect(mockOnSelect).not.toHaveBeenCalled();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('disabled state prevents interactions', () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value="" disabled/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
|
|
|
|
|
// Input should be disabled
|
|
|
|
|
expect(input).toBeDisabled();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('handles rapid selection changes', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
await waitFor(() => {
|
|
|
|
|
expect(screen.getByText('videos/nature')).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
// Click multiple results in succession. Choosing a result closes the
|
|
|
|
|
// dropdown (as any combobox would) without moving focus off the input
|
|
|
|
|
// (the option is chosen on mousedown, with the default prevented), so
|
|
|
|
|
// pressing a key that reopens the list -- same as a real user -- is
|
|
|
|
|
// needed before the next selection.
|
2025-10-29 22:16:33 -06:00
|
|
|
await userEvent.click(screen.getByText('videos/nature'));
|
ui: Common.js and thirteen more files off Semantic
Phase 2, part 3. Baseline 63 -> 50.
Common.js is the one that mattered: 51 files import it, so its Semantic usage
was propping up the rest of the migration. Everything in it now comes from the
library -- Paginator, the two search inputs, DirectorySearch, TabLinks, the
toggles, the messages, APIButton, Breadcrumbs, SimpleAccordion, IframeViewer,
CardPoster, FileIcon, LoadStatistic, the theme menu, and the hotspot icons.
Also migrated: Placeholder, Apps, ArchivePreview, CbzViewer, AddToPlaylist,
TaggedDeleteConfirmModal, the collection edit/tag forms, and four inventory
components.
New in the library, all in the gallery and tested:
SearchBox Semantic's `<Search category>` with its grouped results shape
kept, because four call sites already build it. A real
combobox rather than a div that listens for clicks.
Pagination Semantic's prop names; onPageChange gets the page number.
TabBar the strip only -- routing stays with the caller.
IconStack Semantic's IconGroup, named once for assistive technology.
Table.HeaderCell sorted/onSort -- a real button carrying aria-sort.
Button/IconButton translate Semantic's size names.
That last one is defensive: unmigrated callers still pass size='tiny', and
Mantine drops a size it does not recognise without a word, so the button
quietly renders at the wrong size. Mapping here keeps the mixed state honest.
Three real bugs found and fixed on the way, none of them cosmetic:
* `fileSuffixIconName` returned fourteen icon names the map did not have, so
opening a folder with a PDF, a Word document or an archive in it logged an
error and drew a fallback glyph. All fourteen mapped, with a test that
walks the mapper's own return values, and another asserting every mapped
name resolves to a Tabler component that exists.
* `Statistic` relied on Semantic's `Statistic.Group` for its spacing, so on
the dashboard the load averages ran together into "1 MIN. LOAD5 MIN.
LOAD15 MIN. LOAD". It carries its own spacing now.
* Semantic's inverted stylesheet colors every descendant of an inverted
segment, so Toggle labels inherited #dddddd -- white pixels in night mode,
on the Settings page, which is exactly where someone adjusts their WROLPi
in the dark. Restated from tokens.
The baseline ratchet gained a third assertion: a migrated file may still lean
on Semantic's CSS classes without importing the library, which the import check
cannot see and which would collapse silently when Phase 4 deletes the
stylesheet. There is one such bridge today and it is now declared.
2026-07-30 10:00:48 -06:00
|
|
|
await userEvent.keyboard('{ArrowDown}');
|
|
|
|
|
await waitFor(() => {
|
|
|
|
|
expect(screen.getByText('videos/tech')).toBeInTheDocument();
|
|
|
|
|
});
|
2025-10-29 22:16:33 -06:00
|
|
|
await userEvent.click(screen.getByText('videos/tech'));
|
|
|
|
|
|
|
|
|
|
// Should call onSelect for each selection
|
|
|
|
|
expect(mockOnSelect).toHaveBeenCalledWith('videos/nature');
|
|
|
|
|
expect(mockOnSelect).toHaveBeenCalledWith('videos/tech');
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
describe('Hook Integration', () => {
|
|
|
|
|
it('calls setDirectoryName from hook on search change', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
await userEvent.type(input, 'archive');
|
|
|
|
|
|
|
|
|
|
expect(mockSetDirectoryName).toHaveBeenCalled();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('displays results from hook state', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
await waitFor(() => {
|
|
|
|
|
// Should display results from mock hook state
|
|
|
|
|
expect(screen.getByText('videos/nature')).toBeInTheDocument();
|
|
|
|
|
expect(screen.getByText('videos/tech')).toBeInTheDocument();
|
|
|
|
|
expect(screen.getByText('News Channel')).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
describe('Edge Cases', () => {
|
2025-12-05 18:26:36 -07:00
|
|
|
it('renders without error when directories are null (initial state)', async () => {
|
|
|
|
|
// Simulate the initial state before API call completes
|
|
|
|
|
setMockHookState({
|
|
|
|
|
directories: null,
|
|
|
|
|
channelDirectories: null,
|
|
|
|
|
domainDirectories: null,
|
|
|
|
|
isDir: false,
|
|
|
|
|
directoryName: '',
|
|
|
|
|
loading: true,
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
// This should not throw an error
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-12-05 18:26:36 -07:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
|
|
|
|
|
// Clicking input should not throw "Cannot read properties of null"
|
|
|
|
|
await userEvent.click(input);
|
|
|
|
|
|
|
|
|
|
// Component should still be rendered
|
|
|
|
|
expect(input).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
|
2025-10-29 22:16:33 -06:00
|
|
|
it('handles null/undefined initial value', () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
directoryName: '',
|
|
|
|
|
});
|
|
|
|
|
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value={null}/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
expect(input).toHaveValue('');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('clears results display when empty', async () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
directories: [],
|
|
|
|
|
channelDirectories: [],
|
|
|
|
|
domainDirectories: [],
|
|
|
|
|
isDir: false,
|
|
|
|
|
directoryName: '',
|
|
|
|
|
});
|
|
|
|
|
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
expect(input).toHaveValue('');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('maintains value when component remounts', () => {
|
|
|
|
|
setMockHookState({
|
|
|
|
|
...mockSearchResults,
|
|
|
|
|
directoryName: 'videos/test',
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
const {rerender} = render(
|
2025-12-06 14:49:46 -07:00
|
|
|
<DirectorySearch onSelect={mockOnSelect} value="videos/test"/>
|
2025-10-29 22:16:33 -06:00
|
|
|
);
|
|
|
|
|
|
|
|
|
|
expect(screen.getByDisplayValue('videos/test')).toBeInTheDocument();
|
|
|
|
|
|
|
|
|
|
// Remount with same value
|
2025-12-06 14:49:46 -07:00
|
|
|
rerender(<DirectorySearch onSelect={mockOnSelect} value="videos/test"/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
expect(screen.getByDisplayValue('videos/test')).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('handles special characters in path', async () => {
|
2025-12-06 14:49:46 -07:00
|
|
|
render(<DirectorySearch onSelect={mockOnSelect} value=""/>);
|
2025-10-29 22:16:33 -06:00
|
|
|
|
|
|
|
|
const input = screen.getByPlaceholderText(/search directory names/i);
|
|
|
|
|
|
|
|
|
|
// Type path with special characters
|
|
|
|
|
const specialPath = 'videos/test-folder_2024/v1.0';
|
|
|
|
|
await userEvent.type(input, specialPath);
|
|
|
|
|
|
|
|
|
|
expect(mockSetDirectoryName).toHaveBeenCalled();
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
});
|
ui: fix search suggestion selection, rendering, and clipping
Three findings from review and CI on the Common.js migration, all real.
Selecting a suggestion navigated nowhere. Semantic handed its callers
`{result}` and every caller destructures that shape, so passing the bare result
left `result.location` undefined -- the search suggestions and the Ctrl+K modal
both stopped navigating. The shape stays until the call sites can change
together. (Greptile P1.)
Suggestions lost their custom markup. Callers pass `resultRenderer` to draw a
tag as a chip rather than a line of text, and the migrated wrapper forwarded it
into a div's props instead of to SearchBox, so every suggestion fell back to the
generic title and description. SearchBox takes it now. (Greptile P2.)
The suggestion list clipped its last group. A cypress spec caught it: with
directories, channels and domains all present, the content runs past the 320px
cap and the final row was cut off. Eight grouped suggestions is an ordinary
result set, so the cap was simply too low; it is viewport-relative now and
still bounded.
Both regressions are pinned by tests -- one asserting the callback shape a
caller actually destructures, one asserting the custom renderer replaces the
default markup rather than wrapping it.
2026-07-30 11:28:17 -06:00
|
|
|
|
|
|
|
|
describe('SearchResultsInput', () => {
|
|
|
|
|
const results = {
|
|
|
|
|
tags: {name: 'Tags', results: [{type: 'tag', title: 'Cooking'}]},
|
|
|
|
|
videos: {name: 'Videos', results: [
|
|
|
|
|
{title: 'How To Sharpen An Axe', description: 'Wranglerstar', location: '/videos/1'},
|
|
|
|
|
]},
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
it('hands the selected suggestion to its caller as {result}', async () => {
|
|
|
|
|
/*
|
|
|
|
|
* Regression: the migration passed the bare result, but every caller destructures
|
|
|
|
|
* `{result}` (see Search.js), so `result.location` was undefined and clicking a
|
|
|
|
|
* search suggestion navigated nowhere.
|
|
|
|
|
*/
|
|
|
|
|
const handleResultSelect = jest.fn();
|
|
|
|
|
render(<SearchResultsInput searchStr='axe' onSubmit={jest.fn()} results={results}
|
|
|
|
|
handleResultSelect={handleResultSelect}/>);
|
|
|
|
|
|
|
|
|
|
await userEvent.click(screen.getByRole('combobox'));
|
|
|
|
|
await userEvent.click(screen.getByText('How To Sharpen An Axe'));
|
|
|
|
|
|
|
|
|
|
expect(handleResultSelect).toHaveBeenCalledWith(
|
|
|
|
|
{result: expect.objectContaining({location: '/videos/1'})});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('forwards resultRenderer so a caller keeps its own suggestion markup', async () => {
|
|
|
|
|
// Search.js renders a tag suggestion as a tag chip, not a line of text.
|
|
|
|
|
const resultRenderer = (result) => <span>rendered:{result.title}</span>;
|
|
|
|
|
render(<SearchResultsInput searchStr='cook' onSubmit={jest.fn()} results={results}
|
|
|
|
|
handleResultSelect={jest.fn()} resultRenderer={resultRenderer}/>);
|
|
|
|
|
|
|
|
|
|
await userEvent.click(screen.getByRole('combobox'));
|
|
|
|
|
|
|
|
|
|
expect(screen.getByText('rendered:Cooking')).toBeInTheDocument();
|
|
|
|
|
});
|
|
|
|
|
});
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
|
|
|
|
|
describe('contrastingColor', () => {
|
|
|
|
|
/*
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
* This decides the text color on every tag, against a fill the user picked, so it is the
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
* one place in the app where legibility is computed rather than designed.
|
|
|
|
|
*/
|
|
|
|
|
|
|
|
|
|
it('picks whichever of the two options actually reads better', () => {
|
ui: the last of Semantic UI, in name and in dead CSS
The library has been uninstalled for some time, but the codebase still read as
though it were there: identifiers named after it, rules written against its
class names, and about 180 comments explaining a rule in terms of a thing that
is no longer installed.
Renamed, keeping the vocabulary the app actually uses: semanticUIColorMap ->
navColorHexMap, semanticColorNames -> paletteColorNames, semanticNames ->
iconNameAliases, semanticSizes -> sizeAliases, and SemanticUIColors ->
NavColors in wrolpi/schema.py, which is what validates nav_color on the way in.
ColorToSemanticHexColor had no callers at all and is gone.
The baseline ratchet has done its job -- the list reached zero, so the JSON is
deleted and the guard keeps only the checks with teeth: no package, no import,
no `className="ui "`, no toast library. Its Theme.tsx check went with the
wrappers it was watching for. It also asserts its own walk found files, so a
clean result cannot mean it scanned nothing.
Deleted from App.css: every rule requiring `.ui`, `.inverted`, `.segment` or
`.modal`, none of which any component has set since the stylesheet stopped
loading, plus the form skeletons, ui-alerts and progress-text rules that
nothing renders. Two of them were doing harm rather than nothing: the drag
tints had a dark-theme variant under `.inverted` that stopped applying, leaving
15% of a fixed blue over a dark row, and now mix a token instead.
Two selectors naming markup that no longer exists were live bugs. The file
browser's filter had `.ui.input.file-browser-filter`, so it rendered at the
browser default width on a bar laid out to give it 34em -- now qualified by its
container, because `.wrolpi-action-input` sets `width: 100%` at the same
specificity and is loaded later. And SearchBox takes named props rather than a
rest spread, so the `id` DestinationForm hands it was dropped in silence and
the label it renders pointed at nothing; the download forms' Destination field
has had no associated label since the migration.
"Semantic roles" survives in four places, where it means what it says.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 19:15:57 -06:00
|
|
|
// The palette blue. The old threshold check chose light text here, at 2.9:1, when dark
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
// text on the same fill gives 5.3:1 -- the worse of the only two available answers.
|
|
|
|
|
// Every mid-tone blue, teal and purple a user might choose sat in that band.
|
|
|
|
|
const blue = '#2185d0';
|
|
|
|
|
|
|
|
|
|
const chosen = contrastingColor(blue);
|
|
|
|
|
const rejected = chosen === TAG_TEXT_DARK ? TAG_TEXT_LIGHT : TAG_TEXT_DARK;
|
|
|
|
|
|
|
|
|
|
expect(contrastRatio(chosen, blue)).toBeGreaterThan(contrastRatio(rejected, blue));
|
|
|
|
|
expect(chosen).toBe(TAG_TEXT_DARK);
|
|
|
|
|
});
|
|
|
|
|
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
it('never picks the worse option, for any color', () => {
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
// The property, rather than a list of examples: whatever it returns must be at least as
|
|
|
|
|
// legible as the alternative would have been.
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
const colors = [
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
'#000000', '#ffffff', '#f2f2f2', '#1b1c1d', '#2185d0', '#21ba45', '#db2828',
|
|
|
|
|
'#fbbd08', '#6435c9', '#a5673f', '#00b5ad', '#e03997', '#b5cc18', '#767676',
|
|
|
|
|
'#808080', '#7f7f7f',
|
|
|
|
|
];
|
|
|
|
|
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
for (const color of colors) {
|
|
|
|
|
const chosen = contrastingColor(color);
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
const rejected = chosen === TAG_TEXT_DARK ? TAG_TEXT_LIGHT : TAG_TEXT_DARK;
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
expect(contrastRatio(chosen, color))
|
|
|
|
|
.toBeGreaterThanOrEqual(contrastRatio(rejected, color));
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
}
|
|
|
|
|
});
|
|
|
|
|
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
it('returns one of the two text colors and nothing else', () => {
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
// It is written into `--label-text`; anything undefined leaves the tag unstyled.
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
for (const color of ['#000000', '#ffffff', '#2185d0']) {
|
|
|
|
|
expect([TAG_TEXT_DARK, TAG_TEXT_LIGHT]).toContain(contrastingColor(color));
|
ui: tags look like tags, and stay readable in every theme
Four things, three cosmetic and one a real legibility bug.
* Tags were square chips. They now have a pointed left edge and an
eyelet, like a tag tied to a garment -- the shape Semantic gave them,
and the reason a tag reads as a tag rather than as a small button. The
point is a square rotated behind the left edge, sized so its diagonal
equals the body height and its two edges meet the body at the corners.
Only where something is actually a tag. `Label` carries the count
badges in the Tags table too -- file, zim, channel and domain counts --
and a point on a number reads as an arrow, so the shape is behind a
`tag` prop and `NameToTagLabel` opts in.
* They were smaller than what they replaced: 11.5px text with 3px of
padding, against Semantic's 12px and ~7px. Now 12.5px and 5px, which
also keeps them a reasonable thing to tap.
* Wrapped rows of tags sat on the row above, so they carry a bottom
margin, plus a left margin for the point to occupy.
* The text colour was unreadable in night and wrong in amber.
Tags.js calculates black-or-light against the colour the user chose for
each tag, and wrote the answer to `color` inline -- a declaration no
stylesheet rule can outrank. Night turns a tag into an outline over a
near-black page, so a near-white tag kept the black text calculated for
its fill and was left legible only by its border: 1.1:1, measured. Amber
replaces every fill with its own bright orange and was painting `#dddddd`
on `#ff9500`, about 1.9:1, and a neutral grey in a theme whose whole
point is that no other hue reaches the eye.
The fix is to stop hard-coding `color` so the theme can decide: the
calculated colour goes into `--label-text`, and night and amber override
it alongside the fill they already override. There is no second contrast
function to write, which is what this looked like it needed -- once a
theme has discarded the user's colour there is nothing left to calculate
against, and the answer is just the theme's own token.
`.wrolpi-label-white` and `-black` moved to the same variable. Setting
`color` there outranked the base rule, so amber was painting `--white` on
`--text`: light orange on orange.
`contrastingColor` was also picking the worse of its two options. It
compared a brightness figure against a threshold, and the figure weighted
the gamma-encoded bytes without linearising them, so Semantic's blue
`#2185d0` got light text at 2.9:1 where dark text gives 5.3:1. Every
mid-tone blue, teal and purple a user might pick sat in that band. It
now measures the WCAG contrast ratio of both candidates and returns the
better one, which needs no threshold at all.
Tests
-----
`Tags.test.js` is new, and covers the chip nothing covered: every other
spec takes `NameToTagLabel` from `tagsContextFixture`, which stubs it as
a function returning the name. That is how a tag could be unreadable
without a single test noticing. Two of these were verified red against
the inline `color`.
`cy.contrastRatio()` measures text against the surface *actually* behind
it, walking up to the first ancestor that paints something -- a
transparent tag has no background of its own, so a naive check reads
rgba(0,0,0,0) and concludes anything is legible. Twelve cases, three
tag colours across four themes, at WCAG AA. Plus a test asserting the
old inline-`color` version still measures below 1.5:1, so the check is
provably able to fail.
The gallery had label swatches in theme colours and no user-coloured tag,
which is why none of this was visible in it. It has eleven now, spanning
the range where the black-or-light decision flips.
2026-07-30 19:46:10 -06:00
|
|
|
}
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('measures contrast on the WCAG scale', () => {
|
|
|
|
|
// Anchors the ratio itself, so "better" above is comparing real numbers.
|
|
|
|
|
expect(contrastRatio('#000000', '#ffffff')).toBeCloseTo(21, 1);
|
|
|
|
|
expect(contrastRatio('#ffffff', '#ffffff')).toBeCloseTo(1, 5);
|
|
|
|
|
});
|
|
|
|
|
});
|
2026-08-01 14:34:17 -06:00
|
|
|
|
|
|
|
|
describe('contrastingColor with shorthand hex', () => {
|
|
|
|
|
/*
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
* Tag colors reach the UI from the database, and a tags config is a file a user can edit
|
2026-08-01 14:34:17 -06:00
|
|
|
* by hand -- configs are the source of truth in WROLPi -- so a perfectly valid `#fff` can
|
|
|
|
|
* arrive here. `hexToRGBArray` used to reject anything but six digits, which left the
|
|
|
|
|
* luminance at zero: a near-white tag was treated as black and given light text.
|
|
|
|
|
*/
|
|
|
|
|
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
it('reads a three-digit hex as the color it is', () => {
|
2026-08-01 14:34:17 -06:00
|
|
|
expect(contrastRatio('#000000', '#fff')).toBeCloseTo(21, 1);
|
|
|
|
|
expect(contrastRatio('#000000', '#ffffff')).toBeCloseTo(contrastRatio('#000000', '#fff'), 5);
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('picks dark text for a near-white shorthand fill', () => {
|
|
|
|
|
expect(contrastingColor('#fff')).toBe(TAG_TEXT_DARK);
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('picks light text for a near-black shorthand fill', () => {
|
|
|
|
|
expect(contrastingColor('#000')).toBe(TAG_TEXT_LIGHT);
|
|
|
|
|
});
|
|
|
|
|
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
it('agrees with the six-digit form of the same color', () => {
|
2026-08-01 14:34:17 -06:00
|
|
|
for (const [short, long] of [['#fff', '#ffffff'], ['#000', '#000000'],
|
|
|
|
|
['#f00', '#ff0000'], ['#1b2', '#11bb22']]) {
|
|
|
|
|
expect(contrastingColor(short)).toBe(contrastingColor(long));
|
|
|
|
|
}
|
|
|
|
|
});
|
|
|
|
|
});
|
ui: route genuine success and failure through the roles
Sixteen call sites, chosen by reading each one rather than by grepping for a
hue. Four sub-agents judged them independently against one rule: convert only
what genuinely means success or failure; identification, category and
decoration stay hue-named.
Converted:
SMART drive health (all four branches -- PASS/WARN/FAIL/unknown)
config validity: valid / invalid / empty
"Failed to load preview"
the two "Download error" triggers
expired inventory items, desktop and mobile
the refresh phases: complete / active / not reached
a refresh Progress bar in its error state
cookie status: Unlocked / Locked
a chip-mismatch label on the flasher
CPU load thresholds (LoadStatistic)
the outcome of an API call in useAPIButton
the gallery's "Danger zone" heading and its load readings
Left alone, deliberately:
the file browser toolbar -- every action has its own colour to be told apart,
and delete already uses role='danger' (Roland's ruling)
diff add/remove in the config and inventory import previews. Adding is not a
success and removing is not a failure; in Overwrite mode the removals are
exactly what the user asked for, and painting them danger would report their
intended action as broken
seasons in DatesSelector, the uploader star and favourite heart, TagIcon,
downloader category buttons, "Recommended to Keep"
Three sites were wrong in a way worth naming. Both "Download error" triggers
guard on the same condition and render the same modal, but one was `orange` and
the other `red` -- and `orange` is the single worst choice, byte-identical to
`--text` in night, so that error button was indistinguishable from an ordinary
control. Config validity drew its valid state in `--violet`, which no user
parses as "valid". And `LoadStatistic` -- the thing users actually watch on the
Status page -- rendered its warning threshold in that same invisible orange.
Also fixed a stale comment in Videos.js that claimed no token existed for these.
Two notes on the tests:
The first LoadStatistic test I wrote rendered the component and asserted the
inline colour. jsdom REJECTS `color: var(--danger)` as an invalid declaration
and drops it, so every colour reads back as the empty string -- two of the four
assertions passed for that reason alone and would have kept passing with the
hues restored. It is a source guard now, with the painted result asserted in
the browser instead.
That browser test then found something real: an uncoloured reading inherits
`--text`, which in night is 4.98:1 against `--warning`'s 4.91:1, so a machine
under load is marginally QUIETER than one that is fine. Not fixed here. The
only way up on a dark panel is lightness, and night's danger would desaturate
from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character for
loudness. That is a palette decision, so the test documents it and asserts what
does hold rather than quietly making it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:08:34 -06:00
|
|
|
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
describe('loadRole', () => {
|
ui: route genuine success and failure through the roles
Sixteen call sites, chosen by reading each one rather than by grepping for a
hue. Four sub-agents judged them independently against one rule: convert only
what genuinely means success or failure; identification, category and
decoration stay hue-named.
Converted:
SMART drive health (all four branches -- PASS/WARN/FAIL/unknown)
config validity: valid / invalid / empty
"Failed to load preview"
the two "Download error" triggers
expired inventory items, desktop and mobile
the refresh phases: complete / active / not reached
a refresh Progress bar in its error state
cookie status: Unlocked / Locked
a chip-mismatch label on the flasher
CPU load thresholds (LoadStatistic)
the outcome of an API call in useAPIButton
the gallery's "Danger zone" heading and its load readings
Left alone, deliberately:
the file browser toolbar -- every action has its own colour to be told apart,
and delete already uses role='danger' (Roland's ruling)
diff add/remove in the config and inventory import previews. Adding is not a
success and removing is not a failure; in Overwrite mode the removals are
exactly what the user asked for, and painting them danger would report their
intended action as broken
seasons in DatesSelector, the uploader star and favourite heart, TagIcon,
downloader category buttons, "Recommended to Keep"
Three sites were wrong in a way worth naming. Both "Download error" triggers
guard on the same condition and render the same modal, but one was `orange` and
the other `red` -- and `orange` is the single worst choice, byte-identical to
`--text` in night, so that error button was indistinguishable from an ordinary
control. Config validity drew its valid state in `--violet`, which no user
parses as "valid". And `LoadStatistic` -- the thing users actually watch on the
Status page -- rendered its warning threshold in that same invisible orange.
Also fixed a stale comment in Videos.js that claimed no token existed for these.
Two notes on the tests:
The first LoadStatistic test I wrote rendered the component and asserted the
inline colour. jsdom REJECTS `color: var(--danger)` as an invalid declaration
and drops it, so every colour reads back as the empty string -- two of the four
assertions passed for that reason alone and would have kept passing with the
hues restored. It is a source guard now, with the painted result asserted in
the browser instead.
That browser test then found something real: an uncoloured reading inherits
`--text`, which in night is 4.98:1 against `--warning`'s 4.91:1, so a machine
under load is marginally QUIETER than one that is fine. Not fixed here. The
only way up on a dark panel is lightness, and night's danger would desaturate
from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character for
loudness. That is a palette decision, so the test documents it and asserts what
does hold rather than quietly making it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:08:34 -06:00
|
|
|
/*
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
* The branches, tested as branches. This used to be a source guard scraping the
|
|
|
|
|
* function body out of Common.js, because the obvious render test cannot work: jsdom
|
|
|
|
|
* REJECTS `color: var(--danger)` as an invalid declaration and drops it, so every inline
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
* color reads back as the empty string and the assertions passed for that reason alone.
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
* Pulling the mapping out as a pure function is the honest fix.
|
ui: route genuine success and failure through the roles
Sixteen call sites, chosen by reading each one rather than by grepping for a
hue. Four sub-agents judged them independently against one rule: convert only
what genuinely means success or failure; identification, category and
decoration stay hue-named.
Converted:
SMART drive health (all four branches -- PASS/WARN/FAIL/unknown)
config validity: valid / invalid / empty
"Failed to load preview"
the two "Download error" triggers
expired inventory items, desktop and mobile
the refresh phases: complete / active / not reached
a refresh Progress bar in its error state
cookie status: Unlocked / Locked
a chip-mismatch label on the flasher
CPU load thresholds (LoadStatistic)
the outcome of an API call in useAPIButton
the gallery's "Danger zone" heading and its load readings
Left alone, deliberately:
the file browser toolbar -- every action has its own colour to be told apart,
and delete already uses role='danger' (Roland's ruling)
diff add/remove in the config and inventory import previews. Adding is not a
success and removing is not a failure; in Overwrite mode the removals are
exactly what the user asked for, and painting them danger would report their
intended action as broken
seasons in DatesSelector, the uploader star and favourite heart, TagIcon,
downloader category buttons, "Recommended to Keep"
Three sites were wrong in a way worth naming. Both "Download error" triggers
guard on the same condition and render the same modal, but one was `orange` and
the other `red` -- and `orange` is the single worst choice, byte-identical to
`--text` in night, so that error button was indistinguishable from an ordinary
control. Config validity drew its valid state in `--violet`, which no user
parses as "valid". And `LoadStatistic` -- the thing users actually watch on the
Status page -- rendered its warning threshold in that same invisible orange.
Also fixed a stale comment in Videos.js that claimed no token existed for these.
Two notes on the tests:
The first LoadStatistic test I wrote rendered the component and asserted the
inline colour. jsdom REJECTS `color: var(--danger)` as an invalid declaration
and drops it, so every colour reads back as the empty string -- two of the four
assertions passed for that reason alone and would have kept passing with the
hues restored. It is a source guard now, with the painted result asserted in
the browser instead.
That browser test then found something real: an uncoloured reading inherits
`--text`, which in night is 4.98:1 against `--warning`'s 4.91:1, so a machine
under load is marginally QUIETER than one that is fine. Not fixed here. The
only way up on a dark panel is lightness, and night's danger would desaturate
from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character for
loudness. That is a palette decision, so the test documents it and asserts what
does hold rather than quietly making it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:08:34 -06:00
|
|
|
*
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
* Thresholds are halves and three quarters of the core count, so each is asserted either
|
|
|
|
|
* side of its boundary rather than at one comfortable value.
|
ui: route genuine success and failure through the roles
Sixteen call sites, chosen by reading each one rather than by grepping for a
hue. Four sub-agents judged them independently against one rule: convert only
what genuinely means success or failure; identification, category and
decoration stay hue-named.
Converted:
SMART drive health (all four branches -- PASS/WARN/FAIL/unknown)
config validity: valid / invalid / empty
"Failed to load preview"
the two "Download error" triggers
expired inventory items, desktop and mobile
the refresh phases: complete / active / not reached
a refresh Progress bar in its error state
cookie status: Unlocked / Locked
a chip-mismatch label on the flasher
CPU load thresholds (LoadStatistic)
the outcome of an API call in useAPIButton
the gallery's "Danger zone" heading and its load readings
Left alone, deliberately:
the file browser toolbar -- every action has its own colour to be told apart,
and delete already uses role='danger' (Roland's ruling)
diff add/remove in the config and inventory import previews. Adding is not a
success and removing is not a failure; in Overwrite mode the removals are
exactly what the user asked for, and painting them danger would report their
intended action as broken
seasons in DatesSelector, the uploader star and favourite heart, TagIcon,
downloader category buttons, "Recommended to Keep"
Three sites were wrong in a way worth naming. Both "Download error" triggers
guard on the same condition and render the same modal, but one was `orange` and
the other `red` -- and `orange` is the single worst choice, byte-identical to
`--text` in night, so that error button was indistinguishable from an ordinary
control. Config validity drew its valid state in `--violet`, which no user
parses as "valid". And `LoadStatistic` -- the thing users actually watch on the
Status page -- rendered its warning threshold in that same invisible orange.
Also fixed a stale comment in Videos.js that claimed no token existed for these.
Two notes on the tests:
The first LoadStatistic test I wrote rendered the component and asserted the
inline colour. jsdom REJECTS `color: var(--danger)` as an invalid declaration
and drops it, so every colour reads back as the empty string -- two of the four
assertions passed for that reason alone and would have kept passing with the
hues restored. It is a source guard now, with the painted result asserted in
the browser instead.
That browser test then found something real: an uncoloured reading inherits
`--text`, which in night is 4.98:1 against `--warning`'s 4.91:1, so a machine
under load is marginally QUIETER than one that is fine. Not fixed here. The
only way up on a dark panel is lightness, and night's danger would desaturate
from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character for
loudness. That is a palette decision, so the test documents it and asserts what
does hold rather than quietly making it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:08:34 -06:00
|
|
|
*/
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
it('says nothing about a load below half the cores', () => {
|
|
|
|
|
expect(loadRole(1.9, 4)).toBeUndefined();
|
|
|
|
|
expect(loadRole(0, 4)).toBeUndefined();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('warns from half the cores', () => {
|
|
|
|
|
expect(loadRole(2.0, 4)).toBe('warning');
|
|
|
|
|
expect(loadRole(2.9, 4)).toBe('warning');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('calls it danger from three quarters', () => {
|
|
|
|
|
expect(loadRole(3.0, 4)).toBe('danger');
|
|
|
|
|
expect(loadRole(99, 4)).toBe('danger');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('says nothing when the core count is unknown', () => {
|
|
|
|
|
// Both thresholds are guarded on `cores`. Without it every load would read as fine,
|
|
|
|
|
// which is the safe direction, but worth pinning so it stays deliberate.
|
|
|
|
|
expect(loadRole(12, undefined)).toBeUndefined();
|
|
|
|
|
expect(loadRole(12, 0)).toBeUndefined();
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('never returns a hue', () => {
|
|
|
|
|
// The regression this exists for: `orange` is byte-identical to `--text` in night,
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
// so a warning load rendered as an ordinary uncolored number.
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
[loadRole(1, 4), loadRole(2.5, 4), loadRole(4, 4)].filter(Boolean).forEach(role =>
|
|
|
|
|
expect(role).not.toMatch(/^(red|green|amber|yellow|orange|blue)$/));
|
ui: route genuine success and failure through the roles
Sixteen call sites, chosen by reading each one rather than by grepping for a
hue. Four sub-agents judged them independently against one rule: convert only
what genuinely means success or failure; identification, category and
decoration stay hue-named.
Converted:
SMART drive health (all four branches -- PASS/WARN/FAIL/unknown)
config validity: valid / invalid / empty
"Failed to load preview"
the two "Download error" triggers
expired inventory items, desktop and mobile
the refresh phases: complete / active / not reached
a refresh Progress bar in its error state
cookie status: Unlocked / Locked
a chip-mismatch label on the flasher
CPU load thresholds (LoadStatistic)
the outcome of an API call in useAPIButton
the gallery's "Danger zone" heading and its load readings
Left alone, deliberately:
the file browser toolbar -- every action has its own colour to be told apart,
and delete already uses role='danger' (Roland's ruling)
diff add/remove in the config and inventory import previews. Adding is not a
success and removing is not a failure; in Overwrite mode the removals are
exactly what the user asked for, and painting them danger would report their
intended action as broken
seasons in DatesSelector, the uploader star and favourite heart, TagIcon,
downloader category buttons, "Recommended to Keep"
Three sites were wrong in a way worth naming. Both "Download error" triggers
guard on the same condition and render the same modal, but one was `orange` and
the other `red` -- and `orange` is the single worst choice, byte-identical to
`--text` in night, so that error button was indistinguishable from an ordinary
control. Config validity drew its valid state in `--violet`, which no user
parses as "valid". And `LoadStatistic` -- the thing users actually watch on the
Status page -- rendered its warning threshold in that same invisible orange.
Also fixed a stale comment in Videos.js that claimed no token existed for these.
Two notes on the tests:
The first LoadStatistic test I wrote rendered the component and asserted the
inline colour. jsdom REJECTS `color: var(--danger)` as an invalid declaration
and drops it, so every colour reads back as the empty string -- two of the four
assertions passed for that reason alone and would have kept passing with the
hues restored. It is a source guard now, with the painted result asserted in
the browser instead.
That browser test then found something real: an uncoloured reading inherits
`--text`, which in night is 4.98:1 against `--warning`'s 4.91:1, so a machine
under load is marginally QUIETER than one that is fine. Not fixed here. The
only way up on a dark panel is lightness, and night's danger would desaturate
from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character for
loudness. That is a palette decision, so the test documents it and asserts what
does hold rather than quietly making it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:08:34 -06:00
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the
thresholds, and cross-referenced from Common.test.js as the place the painted
result is checked. It mounted a hand-built `<Statistic color='warning'/>`.
So it proved that role tokens resolve distinctly -- which the roles suite two
blocks up already covers -- and would have stayed green with LoadStatistic
reverted to `orange`/`red`. The same shape as the column-border test in #609:
a claim that outran its measurement. It mounts the real helper at the real
thresholds now, and the reversion fails it.
The source guards are gone. They scraped function bodies out of files with
indexOf slices, covered two helpers, and never executed a branch. `loadRole`
and `healthRole` are pure exported functions instead, tested as mappings:
either side of both load thresholds, every SMART assessment, lower case (the
value comes from smartctl), and the case that matters most -- an unreadable
drive must report as unknown, never as healthy.
That is also the better answer to the jsdom problem that produced the guards.
jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read
back off a rendered component; extracting it is the honest fix rather than
asserting on source text.
And the gallery was itself half-migrated: 15 Min. Load became `danger` while
5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader
to go and check night and amber, where orange is byte-identical to --text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:17:18 -06:00
|
|
|
describe('healthRole', () => {
|
|
|
|
|
it('maps every SMART assessment to a severity', () => {
|
|
|
|
|
expect(healthRole('PASS')).toBe('success');
|
|
|
|
|
expect(healthRole('WARN')).toBe('warning');
|
|
|
|
|
expect(healthRole('FAIL')).toBe('danger');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('is not case sensitive, because the value comes from smartctl', () => {
|
|
|
|
|
expect(healthRole('pass')).toBe('success');
|
|
|
|
|
expect(healthRole('Fail')).toBe('danger');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('treats a missing or unrecognised assessment as unknown, not as healthy', () => {
|
|
|
|
|
// Reporting an unreadable drive as PASS would be the dangerous direction.
|
|
|
|
|
[undefined, null, '', 'SOMETHING_ELSE'].forEach(value =>
|
|
|
|
|
expect(healthRole(value)).toBe('neutral'));
|
ui: route genuine success and failure through the roles
Sixteen call sites, chosen by reading each one rather than by grepping for a
hue. Four sub-agents judged them independently against one rule: convert only
what genuinely means success or failure; identification, category and
decoration stay hue-named.
Converted:
SMART drive health (all four branches -- PASS/WARN/FAIL/unknown)
config validity: valid / invalid / empty
"Failed to load preview"
the two "Download error" triggers
expired inventory items, desktop and mobile
the refresh phases: complete / active / not reached
a refresh Progress bar in its error state
cookie status: Unlocked / Locked
a chip-mismatch label on the flasher
CPU load thresholds (LoadStatistic)
the outcome of an API call in useAPIButton
the gallery's "Danger zone" heading and its load readings
Left alone, deliberately:
the file browser toolbar -- every action has its own colour to be told apart,
and delete already uses role='danger' (Roland's ruling)
diff add/remove in the config and inventory import previews. Adding is not a
success and removing is not a failure; in Overwrite mode the removals are
exactly what the user asked for, and painting them danger would report their
intended action as broken
seasons in DatesSelector, the uploader star and favourite heart, TagIcon,
downloader category buttons, "Recommended to Keep"
Three sites were wrong in a way worth naming. Both "Download error" triggers
guard on the same condition and render the same modal, but one was `orange` and
the other `red` -- and `orange` is the single worst choice, byte-identical to
`--text` in night, so that error button was indistinguishable from an ordinary
control. Config validity drew its valid state in `--violet`, which no user
parses as "valid". And `LoadStatistic` -- the thing users actually watch on the
Status page -- rendered its warning threshold in that same invisible orange.
Also fixed a stale comment in Videos.js that claimed no token existed for these.
Two notes on the tests:
The first LoadStatistic test I wrote rendered the component and asserted the
inline colour. jsdom REJECTS `color: var(--danger)` as an invalid declaration
and drops it, so every colour reads back as the empty string -- two of the four
assertions passed for that reason alone and would have kept passing with the
hues restored. It is a source guard now, with the painted result asserted in
the browser instead.
That browser test then found something real: an uncoloured reading inherits
`--text`, which in night is 4.98:1 against `--warning`'s 4.91:1, so a machine
under load is marginally QUIETER than one that is fine. Not fixed here. The
only way up on a dark panel is lightness, and night's danger would desaturate
from #ff3e3e to about #ff5d5d -- trading the theme's "red only" character for
loudness. That is a palette decision, so the test documents it and asserts what
does hold rather than quietly making it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:08:34 -06:00
|
|
|
});
|
|
|
|
|
});
|
ui: headings open their section, and keep their help icon on one line
Two things, and the second was a regression this migration caused.
A heading had `margin: 0 0 10px` -- no top margin at all -- so it sat flush
against whatever came before and read as part of it rather than as the start of
the next thing. On /more/statistics "Tags" touched the panel above it with a
0px gap. Now 22px above and 10px below, because a heading belongs to what
follows it.
`* + .wrolpi-header` rather than a blanket margin, for the same reason
StatisticGroup needed it: a heading that opens a Panel or a page is already
where it should be, and pushing it down just dents the top of its own container.
The help icon on every HelpHeader and InfoHeader had dropped to its own line --
"Root CA Certificate" in Settings is the clearest one. `.inline-header` set
`h1..h5 {display: inline-block}` so the heading and a sibling `<span>` shared a
row, which worked while these were a bare `<h2>` and stopped the moment they
gained a block wrapper: the `<label>`'s block child forced a break.
Fixed in the library rather than patched in App.css. `Header` takes an `after`
slot rendered inside the heading element, where the existing flex row already
handles alignment and spacing. Both wrappers pass their icon through it, the
`<label>` moves inside the heading so `for_` keeps associating the text with its
field (three call sites rely on it), and the `.inline-header` rules are deleted.
Fifteen call sites are fixed by fixing the two wrappers.
Tests: containment in jest, and the part jsdom cannot judge -- that the two
boxes actually share a line, and that the heading stays one row tall -- in a
browser. Planted both original shapes: the margin removed, a blanket margin
denting its container, and the control moved back outside the heading element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:41:58 -06:00
|
|
|
|
test: act on review — and one of the seven turned out to be undemonstrable
Six real fixes and one honest retraction.
The control is now a SIBLING of the heading inside a flex row, not a child of
it. Inside the `<h3>` it joined the heading's accessible name, so screen reader
heading navigation announced "Root CA Certificate Help". The row is what keeps
them on one line, and it is only rendered when there is something to sit beside,
so the markup for the other 46 call sites is unchanged.
`style` now lands on the wrapper. It used to reach the outer div; going through
Header put it on the inner heading, where a margin sets space INSIDE the wrapper
and only worked at all through margin collapse -- which stops the moment the
wrapper gains padding or a border. Status's "Drive Bandwidth" is the live case:
verified 16px on the block, 0 on the h2. Text properties still arrive, because
colour and text-align inherit; the `color` PROP stays on the heading.
A `<label>` only when `for_` names a field. Several call sites pass an
InfoHeader as the `label` prop of InputForm/ToggleForm, which wraps it in a
label of its own -- an unconditional one nested labels, which is invalid and
makes clicking the text ambiguous. Verified: zero `label label` on
/videos/settings, where "Video Resolutions" keeps its label and "Video File
Format" no longer has one.
The help controls carried `iconStyle: {margin: '0.5em'}`, which separated a
free-floating sibling and, inside the row, added to the gap on every side.
Zeroed; the row's gap is the spacing.
The browser test mounted a plain Button in an `after` slot rather than a real
HelpHeader, so the default icon chrome above was exactly what it could not see.
It mounts the real wrapper now. Two of my assertions were also too weak to
catch what they claimed: the gap was measured to `.wrolpi-header-after`, a flex
container whose own rect does not move when a child gains a margin, so it had to
measure the button; and the squeeze case used a title of ordinary words, which
simply wraps and never puts the row under pressure.
Renamed the suites -- "keeps the icon on its own line" read as the opposite of
the fix.
The retraction: `flex-shrink: 0` is declared but has NO test. Three attempts
could not find a width where removing it changes what is painted -- the row's
only other item is a heading, and its automatic minimum size absorbs the
pressure; the help control is an ActionIcon with a min-width it will not go
below either. The declaration stays as a statement of intent, with a comment
saying it is unproven, rather than a green test that does not discriminate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 19:00:32 -06:00
|
|
|
describe('a heading keeps its help icon on the same row as the text', () => {
|
ui: headings open their section, and keep their help icon on one line
Two things, and the second was a regression this migration caused.
A heading had `margin: 0 0 10px` -- no top margin at all -- so it sat flush
against whatever came before and read as part of it rather than as the start of
the next thing. On /more/statistics "Tags" touched the panel above it with a
0px gap. Now 22px above and 10px below, because a heading belongs to what
follows it.
`* + .wrolpi-header` rather than a blanket margin, for the same reason
StatisticGroup needed it: a heading that opens a Panel or a page is already
where it should be, and pushing it down just dents the top of its own container.
The help icon on every HelpHeader and InfoHeader had dropped to its own line --
"Root CA Certificate" in Settings is the clearest one. `.inline-header` set
`h1..h5 {display: inline-block}` so the heading and a sibling `<span>` shared a
row, which worked while these were a bare `<h2>` and stopped the moment they
gained a block wrapper: the `<label>`'s block child forced a break.
Fixed in the library rather than patched in App.css. `Header` takes an `after`
slot rendered inside the heading element, where the existing flex row already
handles alignment and spacing. Both wrappers pass their icon through it, the
`<label>` moves inside the heading so `for_` keeps associating the text with its
field (three call sites rely on it), and the `.inline-header` rules are deleted.
Fifteen call sites are fixed by fixing the two wrappers.
Tests: containment in jest, and the part jsdom cannot judge -- that the two
boxes actually share a line, and that the heading stays one row tall -- in a
browser. Planted both original shapes: the margin removed, a blanket margin
denting its container, and the control moved back outside the heading element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:41:58 -06:00
|
|
|
/*
|
|
|
|
|
* `HelpHeader` and `InfoHeader` put their icon in a sibling `<span>` beside a `<label>`,
|
|
|
|
|
* held on one line by `.inline-header h1..h5 {display: inline-block}` in App.css. That
|
|
|
|
|
* worked while the heading was a bare `<h2>`; the moment it gained a block wrapper the
|
|
|
|
|
* label's block child forced a break, and every help icon in the app dropped below its
|
|
|
|
|
* own heading -- visible on "Root CA Certificate" in Settings.
|
|
|
|
|
*
|
|
|
|
|
* They pass the icon through the heading's `after` slot now. Containment is the claim
|
|
|
|
|
* jsdom can judge; that the two actually share a line is asserted in ui-layout.cy.js,
|
|
|
|
|
* since jsdom has no layout at all.
|
|
|
|
|
*/
|
test: act on review — and one of the seven turned out to be undemonstrable
Six real fixes and one honest retraction.
The control is now a SIBLING of the heading inside a flex row, not a child of
it. Inside the `<h3>` it joined the heading's accessible name, so screen reader
heading navigation announced "Root CA Certificate Help". The row is what keeps
them on one line, and it is only rendered when there is something to sit beside,
so the markup for the other 46 call sites is unchanged.
`style` now lands on the wrapper. It used to reach the outer div; going through
Header put it on the inner heading, where a margin sets space INSIDE the wrapper
and only worked at all through margin collapse -- which stops the moment the
wrapper gains padding or a border. Status's "Drive Bandwidth" is the live case:
verified 16px on the block, 0 on the h2. Text properties still arrive, because
colour and text-align inherit; the `color` PROP stays on the heading.
A `<label>` only when `for_` names a field. Several call sites pass an
InfoHeader as the `label` prop of InputForm/ToggleForm, which wraps it in a
label of its own -- an unconditional one nested labels, which is invalid and
makes clicking the text ambiguous. Verified: zero `label label` on
/videos/settings, where "Video Resolutions" keeps its label and "Video File
Format" no longer has one.
The help controls carried `iconStyle: {margin: '0.5em'}`, which separated a
free-floating sibling and, inside the row, added to the gap on every side.
Zeroed; the row's gap is the spacing.
The browser test mounted a plain Button in an `after` slot rather than a real
HelpHeader, so the default icon chrome above was exactly what it could not see.
It mounts the real wrapper now. Two of my assertions were also too weak to
catch what they claimed: the gap was measured to `.wrolpi-header-after`, a flex
container whose own rect does not move when a child gains a margin, so it had to
measure the button; and the squeeze case used a title of ordinary words, which
simply wraps and never puts the row under pressure.
Renamed the suites -- "keeps the icon on its own line" read as the opposite of
the fix.
The retraction: `flex-shrink: 0` is declared but has NO test. Three attempts
could not find a width where removing it changes what is painted -- the row's
only other item is a heading, and its automatic minimum size absorbs the
pressure; the help control is an ActionIcon with a min-width it will not go
below either. The declaration stays as a statement of intent, with a comment
saying it is unproven, rather than a green test that does not discriminate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 19:00:32 -06:00
|
|
|
it("renders the help control in the heading's row", () => {
|
|
|
|
|
const {container} = render(
|
|
|
|
|
<HelpHeader headerSize='h3' headerContent='Root CA Certificate'
|
|
|
|
|
helpPath='/system/certificates/'/>);
|
|
|
|
|
|
|
|
|
|
expect(container.querySelector('.wrolpi-header-row'))
|
|
|
|
|
.toContainElement(screen.getByRole('button', {name: 'Help'}));
|
|
|
|
|
// And not inside the heading itself, which would put it in the accessible name.
|
|
|
|
|
expect(screen.getByRole('heading')).toHaveAccessibleName('Root CA Certificate');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it("renders the info popup in the heading's row", () => {
|
|
|
|
|
const {container} = render(
|
|
|
|
|
<InfoHeader headerSize='h4' headerContent='Video Resolutions'
|
|
|
|
|
popupContent='Highest available is tried first.'/>);
|
ui: headings open their section, and keep their help icon on one line
Two things, and the second was a regression this migration caused.
A heading had `margin: 0 0 10px` -- no top margin at all -- so it sat flush
against whatever came before and read as part of it rather than as the start of
the next thing. On /more/statistics "Tags" touched the panel above it with a
0px gap. Now 22px above and 10px below, because a heading belongs to what
follows it.
`* + .wrolpi-header` rather than a blanket margin, for the same reason
StatisticGroup needed it: a heading that opens a Panel or a page is already
where it should be, and pushing it down just dents the top of its own container.
The help icon on every HelpHeader and InfoHeader had dropped to its own line --
"Root CA Certificate" in Settings is the clearest one. `.inline-header` set
`h1..h5 {display: inline-block}` so the heading and a sibling `<span>` shared a
row, which worked while these were a bare `<h2>` and stopped the moment they
gained a block wrapper: the `<label>`'s block child forced a break.
Fixed in the library rather than patched in App.css. `Header` takes an `after`
slot rendered inside the heading element, where the existing flex row already
handles alignment and spacing. Both wrappers pass their icon through it, the
`<label>` moves inside the heading so `for_` keeps associating the text with its
field (three call sites rely on it), and the `.inline-header` rules are deleted.
Fifteen call sites are fixed by fixing the two wrappers.
Tests: containment in jest, and the part jsdom cannot judge -- that the two
boxes actually share a line, and that the heading stays one row tall -- in a
browser. Planted both original shapes: the margin removed, a blanket margin
denting its container, and the control moved back outside the heading element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:41:58 -06:00
|
|
|
|
test: act on review — and one of the seven turned out to be undemonstrable
Six real fixes and one honest retraction.
The control is now a SIBLING of the heading inside a flex row, not a child of
it. Inside the `<h3>` it joined the heading's accessible name, so screen reader
heading navigation announced "Root CA Certificate Help". The row is what keeps
them on one line, and it is only rendered when there is something to sit beside,
so the markup for the other 46 call sites is unchanged.
`style` now lands on the wrapper. It used to reach the outer div; going through
Header put it on the inner heading, where a margin sets space INSIDE the wrapper
and only worked at all through margin collapse -- which stops the moment the
wrapper gains padding or a border. Status's "Drive Bandwidth" is the live case:
verified 16px on the block, 0 on the h2. Text properties still arrive, because
colour and text-align inherit; the `color` PROP stays on the heading.
A `<label>` only when `for_` names a field. Several call sites pass an
InfoHeader as the `label` prop of InputForm/ToggleForm, which wraps it in a
label of its own -- an unconditional one nested labels, which is invalid and
makes clicking the text ambiguous. Verified: zero `label label` on
/videos/settings, where "Video Resolutions" keeps its label and "Video File
Format" no longer has one.
The help controls carried `iconStyle: {margin: '0.5em'}`, which separated a
free-floating sibling and, inside the row, added to the gap on every side.
Zeroed; the row's gap is the spacing.
The browser test mounted a plain Button in an `after` slot rather than a real
HelpHeader, so the default icon chrome above was exactly what it could not see.
It mounts the real wrapper now. Two of my assertions were also too weak to
catch what they claimed: the gap was measured to `.wrolpi-header-after`, a flex
container whose own rect does not move when a child gains a margin, so it had to
measure the button; and the squeeze case used a title of ordinary words, which
simply wraps and never puts the row under pressure.
Renamed the suites -- "keeps the icon on its own line" read as the opposite of
the fix.
The retraction: `flex-shrink: 0` is declared but has NO test. Three attempts
could not find a width where removing it changes what is painted -- the row's
only other item is a heading, and its automatic minimum size absorbs the
pressure; the help control is an ActionIcon with a min-width it will not go
below either. The declaration stays as a statement of intent, with a comment
saying it is unproven, rather than a green test that does not discriminate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 19:00:32 -06:00
|
|
|
expect(container.querySelector('.wrolpi-header-after svg')).not.toBeNull();
|
ui: headings open their section, and keep their help icon on one line
Two things, and the second was a regression this migration caused.
A heading had `margin: 0 0 10px` -- no top margin at all -- so it sat flush
against whatever came before and read as part of it rather than as the start of
the next thing. On /more/statistics "Tags" touched the panel above it with a
0px gap. Now 22px above and 10px below, because a heading belongs to what
follows it.
`* + .wrolpi-header` rather than a blanket margin, for the same reason
StatisticGroup needed it: a heading that opens a Panel or a page is already
where it should be, and pushing it down just dents the top of its own container.
The help icon on every HelpHeader and InfoHeader had dropped to its own line --
"Root CA Certificate" in Settings is the clearest one. `.inline-header` set
`h1..h5 {display: inline-block}` so the heading and a sibling `<span>` shared a
row, which worked while these were a bare `<h2>` and stopped the moment they
gained a block wrapper: the `<label>`'s block child forced a break.
Fixed in the library rather than patched in App.css. `Header` takes an `after`
slot rendered inside the heading element, where the existing flex row already
handles alignment and spacing. Both wrappers pass their icon through it, the
`<label>` moves inside the heading so `for_` keeps associating the text with its
field (three call sites rely on it), and the `.inline-header` rules are deleted.
Fifteen call sites are fixed by fixing the two wrappers.
Tests: containment in jest, and the part jsdom cannot judge -- that the two
boxes actually share a line, and that the heading stays one row tall -- in a
browser. Planted both original shapes: the margin removed, a blanket margin
denting its container, and the control moved back outside the heading element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:41:58 -06:00
|
|
|
});
|
|
|
|
|
|
test: act on review — and one of the seven turned out to be undemonstrable
Six real fixes and one honest retraction.
The control is now a SIBLING of the heading inside a flex row, not a child of
it. Inside the `<h3>` it joined the heading's accessible name, so screen reader
heading navigation announced "Root CA Certificate Help". The row is what keeps
them on one line, and it is only rendered when there is something to sit beside,
so the markup for the other 46 call sites is unchanged.
`style` now lands on the wrapper. It used to reach the outer div; going through
Header put it on the inner heading, where a margin sets space INSIDE the wrapper
and only worked at all through margin collapse -- which stops the moment the
wrapper gains padding or a border. Status's "Drive Bandwidth" is the live case:
verified 16px on the block, 0 on the h2. Text properties still arrive, because
colour and text-align inherit; the `color` PROP stays on the heading.
A `<label>` only when `for_` names a field. Several call sites pass an
InfoHeader as the `label` prop of InputForm/ToggleForm, which wraps it in a
label of its own -- an unconditional one nested labels, which is invalid and
makes clicking the text ambiguous. Verified: zero `label label` on
/videos/settings, where "Video Resolutions" keeps its label and "Video File
Format" no longer has one.
The help controls carried `iconStyle: {margin: '0.5em'}`, which separated a
free-floating sibling and, inside the row, added to the gap on every side.
Zeroed; the row's gap is the spacing.
The browser test mounted a plain Button in an `after` slot rather than a real
HelpHeader, so the default icon chrome above was exactly what it could not see.
It mounts the real wrapper now. Two of my assertions were also too weak to
catch what they claimed: the gap was measured to `.wrolpi-header-after`, a flex
container whose own rect does not move when a child gains a margin, so it had to
measure the button; and the squeeze case used a title of ordinary words, which
simply wraps and never puts the row under pressure.
Renamed the suites -- "keeps the icon on its own line" read as the opposite of
the fix.
The retraction: `flex-shrink: 0` is declared but has NO test. Three attempts
could not find a width where removing it changes what is painted -- the row's
only other item is a heading, and its automatic minimum size absorbs the
pressure; the help control is an ActionIcon with a min-width it will not go
below either. The declaration stays as a statement of intent, with a comment
saying it is unproven, rather than a green test that does not discriminate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 19:00:32 -06:00
|
|
|
it('renders no label at all when there is no field to name', () => {
|
|
|
|
|
/*
|
|
|
|
|
* Several call sites pass an InfoHeader as the `label` prop of InputForm/ToggleForm,
|
|
|
|
|
* which wraps it in a `<label>` of its own. An unconditional label here nested one
|
|
|
|
|
* inside another -- invalid HTML, and it makes clicking the text ambiguous.
|
|
|
|
|
*/
|
|
|
|
|
const {container} = render(
|
|
|
|
|
<InfoHeader headerSize='h5' headerContent='Video File Format' popupContent='...'/>);
|
ui: headings open their section, and keep their help icon on one line
Two things, and the second was a regression this migration caused.
A heading had `margin: 0 0 10px` -- no top margin at all -- so it sat flush
against whatever came before and read as part of it rather than as the start of
the next thing. On /more/statistics "Tags" touched the panel above it with a
0px gap. Now 22px above and 10px below, because a heading belongs to what
follows it.
`* + .wrolpi-header` rather than a blanket margin, for the same reason
StatisticGroup needed it: a heading that opens a Panel or a page is already
where it should be, and pushing it down just dents the top of its own container.
The help icon on every HelpHeader and InfoHeader had dropped to its own line --
"Root CA Certificate" in Settings is the clearest one. `.inline-header` set
`h1..h5 {display: inline-block}` so the heading and a sibling `<span>` shared a
row, which worked while these were a bare `<h2>` and stopped the moment they
gained a block wrapper: the `<label>`'s block child forced a break.
Fixed in the library rather than patched in App.css. `Header` takes an `after`
slot rendered inside the heading element, where the existing flex row already
handles alignment and spacing. Both wrappers pass their icon through it, the
`<label>` moves inside the heading so `for_` keeps associating the text with its
field (three call sites rely on it), and the `.inline-header` rules are deleted.
Fifteen call sites are fixed by fixing the two wrappers.
Tests: containment in jest, and the part jsdom cannot judge -- that the two
boxes actually share a line, and that the heading stays one row tall -- in a
browser. Planted both original shapes: the margin removed, a blanket margin
denting its container, and the control moved back outside the heading element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:41:58 -06:00
|
|
|
|
test: act on review — and one of the seven turned out to be undemonstrable
Six real fixes and one honest retraction.
The control is now a SIBLING of the heading inside a flex row, not a child of
it. Inside the `<h3>` it joined the heading's accessible name, so screen reader
heading navigation announced "Root CA Certificate Help". The row is what keeps
them on one line, and it is only rendered when there is something to sit beside,
so the markup for the other 46 call sites is unchanged.
`style` now lands on the wrapper. It used to reach the outer div; going through
Header put it on the inner heading, where a margin sets space INSIDE the wrapper
and only worked at all through margin collapse -- which stops the moment the
wrapper gains padding or a border. Status's "Drive Bandwidth" is the live case:
verified 16px on the block, 0 on the h2. Text properties still arrive, because
colour and text-align inherit; the `color` PROP stays on the heading.
A `<label>` only when `for_` names a field. Several call sites pass an
InfoHeader as the `label` prop of InputForm/ToggleForm, which wraps it in a
label of its own -- an unconditional one nested labels, which is invalid and
makes clicking the text ambiguous. Verified: zero `label label` on
/videos/settings, where "Video Resolutions" keeps its label and "Video File
Format" no longer has one.
The help controls carried `iconStyle: {margin: '0.5em'}`, which separated a
free-floating sibling and, inside the row, added to the gap on every side.
Zeroed; the row's gap is the spacing.
The browser test mounted a plain Button in an `after` slot rather than a real
HelpHeader, so the default icon chrome above was exactly what it could not see.
It mounts the real wrapper now. Two of my assertions were also too weak to
catch what they claimed: the gap was measured to `.wrolpi-header-after`, a flex
container whose own rect does not move when a child gains a margin, so it had to
measure the button; and the squeeze case used a title of ordinary words, which
simply wraps and never puts the row under pressure.
Renamed the suites -- "keeps the icon on its own line" read as the opposite of
the fix.
The retraction: `flex-shrink: 0` is declared but has NO test. Three attempts
could not find a width where removing it changes what is painted -- the row's
only other item is a heading, and its automatic minimum size absorbs the
pressure; the help control is an ActionIcon with a min-width it will not go
below either. The declaration stays as a statement of intent, with a comment
saying it is unproven, rather than a green test that does not discriminate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 19:00:32 -06:00
|
|
|
expect(container.querySelector('label')).toBeNull();
|
|
|
|
|
expect(screen.getByRole('heading', {name: /Video File Format/})).toBeInTheDocument();
|
ui: headings open their section, and keep their help icon on one line
Two things, and the second was a regression this migration caused.
A heading had `margin: 0 0 10px` -- no top margin at all -- so it sat flush
against whatever came before and read as part of it rather than as the start of
the next thing. On /more/statistics "Tags" touched the panel above it with a
0px gap. Now 22px above and 10px below, because a heading belongs to what
follows it.
`* + .wrolpi-header` rather than a blanket margin, for the same reason
StatisticGroup needed it: a heading that opens a Panel or a page is already
where it should be, and pushing it down just dents the top of its own container.
The help icon on every HelpHeader and InfoHeader had dropped to its own line --
"Root CA Certificate" in Settings is the clearest one. `.inline-header` set
`h1..h5 {display: inline-block}` so the heading and a sibling `<span>` shared a
row, which worked while these were a bare `<h2>` and stopped the moment they
gained a block wrapper: the `<label>`'s block child forced a break.
Fixed in the library rather than patched in App.css. `Header` takes an `after`
slot rendered inside the heading element, where the existing flex row already
handles alignment and spacing. Both wrappers pass their icon through it, the
`<label>` moves inside the heading so `for_` keeps associating the text with its
field (three call sites rely on it), and the `.inline-header` rules are deleted.
Fifteen call sites are fixed by fixing the two wrappers.
Tests: containment in jest, and the part jsdom cannot judge -- that the two
boxes actually share a line, and that the heading stays one row tall -- in a
browser. Planted both original shapes: the margin removed, a blanket margin
denting its container, and the control moved back outside the heading element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:41:58 -06:00
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('keeps the label association, which is what `for_` is for', () => {
|
|
|
|
|
// Three call sites use it to tie the heading text to the field below it.
|
|
|
|
|
render(<InfoHeader headerSize='h4' headerContent='Video Resolutions'
|
|
|
|
|
for_='video_resolutions_input' popupContent='...'/>);
|
|
|
|
|
|
|
|
|
|
expect(screen.getByText(/Video Resolutions/).closest('label'))
|
|
|
|
|
.toHaveAttribute('for', 'video_resolutions_input');
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('leaves no .inline-header behind', () => {
|
|
|
|
|
// The class and its stylesheet rules are gone; a stray one would be dead markup
|
|
|
|
|
// relying on CSS that no longer exists.
|
|
|
|
|
const {container} = render(<HelpHeader headerContent='Root CA Certificate' helpPath='/x/'/>);
|
|
|
|
|
|
|
|
|
|
expect(container.querySelector('.inline-header')).toBeNull();
|
|
|
|
|
});
|
|
|
|
|
});
|
ui: act on review -- corner chrome, a swallowed className, and the poster frame
Three real defects from the PR review, each reproduced before it was fixed.
The tag mark and a video's duration were pinned to the poster BAND, which
is the full width of the card, rather than to the painted image. Once the
height cap landed the two stopped being the same box: measured, the mark
sat 39px clear of a book cover, 56px clear of a small poster and 60px
clear of a vertical video, and the duration 55px clear. It went unnoticed
because on a 16:9 poster the image fills the band and the two pin to the
same place -- so the tests cover the height-bound ratios, and assert the
letterbox gap is really there before measuring into it.
CardPoster now centres a frame that shrink-wraps the image, and the frame
is what corner chrome is positioned against. VideoCard passes its
duration through a new `overlay` prop rather than wrapping CardPoster in a
box of its own.
The mark also went back INSIDE the link. Splitting it out made the
tagged corner a dead spot on an otherwise clickable poster.
CardLink and ExternalCardLink wrote `className='...' {...props}`, so a
caller's className replaced the pair instead of adding to it. Every
archive title passes `card-title-ellipsis`, so every archive title lost
`card-link` -- the one card type this PR migrates for layout was the one
missing the title treatment. Both compose now.
On the fourth comment, the breadth of the ActionIcon remap: it is
deliberate and the numbers are small. 59 IconButton call sites, 52 of
them at the default size, 7 at `xs` (one in Flasher, three in the
inventory schema editor). The one library control that stands beside an
input rather than a button is SearchBox's clear, and the remap is exactly
what that scale is for -- it now matches its field to the pixel, where
before it was 28px against a 36px input. Guarded by a test that fails at
42 vs 36 if the default is reverted.
1078 jest, 158 in ui-layout.cy.js, clean build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-02 11:35:49 -06:00
|
|
|
|
|
|
|
|
describe('card links', () => {
|
|
|
|
|
/*
|
|
|
|
|
* `.card-link` is what makes a card's title read as the card's text rather than as a
|
style: spell it "color" everywhere
283 occurrences across 23 files, all of them mine and all of them in the
theme/UI work: comments, test names, and a handful of local identifiers
(`colours`, `roleColours`, `awaitColoured`) plus one test fixture tag named
'Uncoloured'.
Nothing was required to keep the other spelling. Checked before replacing: no
CSS custom property or class name contains it, no library prop or API key does,
and every string holding it is a test name or our own copy. The renamed
identifiers were checked for collisions with an existing `color`/`colors` in the
same file first -- there were none.
Verified as a pure rename: 262 lines changed, and normalising the spelling on
every removed line reproduces the added line exactly, for all 262. So the diff
contains no other edit hiding in it.
1134 jest / 64 suites, 271 cypress component tests with the same 12 pre-existing
Semantic-era failures in Tags/Channels/Download/Inventory, clean npm run build.
2026-08-02 18:02:44 -06:00
|
|
|
* link, so a title that misses it is the one thing on the card in the wrong color.
|
ui: act on review -- corner chrome, a swallowed className, and the poster frame
Three real defects from the PR review, each reproduced before it was fixed.
The tag mark and a video's duration were pinned to the poster BAND, which
is the full width of the card, rather than to the painted image. Once the
height cap landed the two stopped being the same box: measured, the mark
sat 39px clear of a book cover, 56px clear of a small poster and 60px
clear of a vertical video, and the duration 55px clear. It went unnoticed
because on a 16:9 poster the image fills the band and the two pin to the
same place -- so the tests cover the height-bound ratios, and assert the
letterbox gap is really there before measuring into it.
CardPoster now centres a frame that shrink-wraps the image, and the frame
is what corner chrome is positioned against. VideoCard passes its
duration through a new `overlay` prop rather than wrapping CardPoster in a
box of its own.
The mark also went back INSIDE the link. Splitting it out made the
tagged corner a dead spot on an otherwise clickable poster.
CardLink and ExternalCardLink wrote `className='...' {...props}`, so a
caller's className replaced the pair instead of adding to it. Every
archive title passes `card-title-ellipsis`, so every archive title lost
`card-link` -- the one card type this PR migrates for layout was the one
missing the title treatment. Both compose now.
On the fourth comment, the breadth of the ActionIcon remap: it is
deliberate and the numbers are small. 59 IconButton call sites, 52 of
them at the default size, 7 at `xs` (one in Flasher, three in the
inventory schema editor). The one library control that stands beside an
input rather than a button is SearchBox's clear, and the remap is exactly
what that scale is for -- it now matches its field to the pixel, where
before it was 28px against a 36px input. Guarded by a test that fails at
42 vs 36 if the default is reverted.
1078 jest, 158 in ui-layout.cy.js, clean build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-02 11:35:49 -06:00
|
|
|
* Both helpers set the class and then spread the caller's props over it, so any call
|
|
|
|
|
* site passing a className of its own -- which is every archive title, via
|
|
|
|
|
* `card-title-ellipsis` -- silently replaced the class instead of adding to it.
|
|
|
|
|
*/
|
|
|
|
|
it.each([
|
|
|
|
|
['CardLink', <CardLink to='/archives/1' className='card-title-ellipsis'>Title</CardLink>],
|
|
|
|
|
['ExternalCardLink',
|
|
|
|
|
<ExternalCardLink to='http://x/a.html' className='card-title-ellipsis'>Title</ExternalCardLink>],
|
|
|
|
|
])('%s keeps its own classes when the call site adds one', (_name, element) => {
|
|
|
|
|
render(element);
|
|
|
|
|
|
|
|
|
|
const link = screen.getByText('Title');
|
|
|
|
|
expect(link).toHaveClass('card-link');
|
|
|
|
|
expect(link).toHaveClass('no-link-underscore');
|
|
|
|
|
expect(link).toHaveClass('card-title-ellipsis');
|
|
|
|
|
});
|
|
|
|
|
});
|
ui: pages own the space between their blocks
Panels shared edges: two 1px borders meeting, which reads as one tall
surface with a rule across it rather than a seam between two.
Done as `margin-bottom` on the panel first, which is the anti-pattern, and
it failed in both of the ways that rule is known to fail before the reading
was done. It compounded with the gap of every Stack that already held
panels -- four of them, spacing those lists at 35.2px against the app's
17.6px -- and the `:last-child` reset meant to keep trailing space off the
bottom of a container silently did nothing, because Media wrapping makes a
panel the only child of its own wrapper. On Status that separated four
pairs out of five and left the first touching.
So the space belongs to whatever stacks the blocks. `.wrolpi-stack` is on
PageContainer, which every page uses, and on the three wrappers that stack
blocks somewhere the page's own stack cannot reach: Settings' panel wrapper,
the block below the video player, and the Zim and Map route wrappers, which
hold their tab bar outside PageContainer.
A gap rather than `> * + * { margin-top }`, which was tried and is quietly
broken here: Media injects `.fresnel-container { margin: 0; padding: 0 }` at
runtime, same specificity, later source order, so every wrapped block threw
the margin away. A child cannot zero its parent's gap. It also means
Media's hidden half contributes nothing, where the owl selector spaced
around something invisible.
With the page owning the spacing, the blocks that owned some of it had to
give it up, or their margin was added to the gap: the tab bar (33px against
17.6 everywhere else), the shared search control bar and paginator on
Videos/Archives/Docs/Files, the card grid, the file browser's filter row and
the inventory search. Two margins deliberately stay -- the file browser's
4em, which reserves room for the sticky buttons (62645831), and heading
margins, which bind a heading to the content below it.
Tests plant each of the three failures: the wrapped block, a child that
zeroes its own margin, and a hidden Media half. Verified in the browser on
sixteen routes and in all four themes -- every stacked pair 17.6px, the
map's 85vh box and the video player unchanged, no horizontal overflow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:40:27 -06:00
|
|
|
|
|
|
|
|
describe('PageContainer', () => {
|
|
|
|
|
/*
|
|
|
|
|
* The wrapper that decides how far apart a page's blocks sit. The spacing itself is a
|
|
|
|
|
* stylesheet rule and is measured in ui-layout.cy.js, where there is a layout engine; what
|
|
|
|
|
* this asserts is that the rule is REACHED -- the class has to be on both of the responsive
|
|
|
|
|
* branches, or the spacing silently applies on one viewport and not the other.
|
|
|
|
|
*/
|
|
|
|
|
it('marks both of its viewport branches as a stack', () => {
|
|
|
|
|
const {container} = render(<PageContainer><div>Body</div></PageContainer>);
|
|
|
|
|
|
|
|
|
|
const stacks = container.querySelectorAll('.wrolpi-stack');
|
|
|
|
|
// Mobile and tablet-and-up, both rendered; `Media` hides one with CSS.
|
|
|
|
|
expect(stacks).toHaveLength(2);
|
|
|
|
|
stacks.forEach(stack => expect(stack.textContent).toBe('Body'));
|
|
|
|
|
});
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
it('is never rendered by a page whose route already provides page chrome', () => {
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
/*
|
|
|
|
|
* `PageContainer` carries `margin-top: 1em` and `padding: 1em`, so nesting one inside
|
|
|
|
|
* another applies both twice: the page is pushed 1em further down and inset 1em further
|
|
|
|
|
* in on all four sides than every other page in the app. `/archives/settings` did this
|
|
|
|
|
* -- `ArchiveRoute` wraps its `<Routes>` in one, and `ArchiveSettingsPage` wrapped itself
|
|
|
|
|
* in another -- and read as "more margin than /archives/domains", which is exactly what
|
|
|
|
|
* it was.
|
|
|
|
|
*
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
* Nothing about a doubled margin throws, so this is a source scan. It finds each router
|
|
|
|
|
* whose enclosing `return` provides page chrome, takes the components it routes to, and
|
|
|
|
|
* checks the ones declared in the same file -- which is where a route and its pages
|
|
|
|
|
* normally live together.
|
|
|
|
|
*
|
|
|
|
|
* "Chrome" is `PageContainer` OR a `wrolpi-stack` wrapper, not just the former. Keying on
|
|
|
|
|
* `PageContainer` alone missed `MapRoute` and `ZimRoute`, which wrap their own content in a
|
|
|
|
|
* stack instead -- and three pages under them (`ManageMap`, `MapPins`, `ManageZim`) mounted
|
|
|
|
|
* a `PageContainer` each, so they sat a further 1em down and 1em in from the tab bar than
|
|
|
|
|
* the viewers beside them. The same defect as Archive, in a shape the first scan could not
|
|
|
|
|
* see.
|
|
|
|
|
*
|
|
|
|
|
* Known limit: a layout that routes through `<Outlet/>` has its `<Route>` list in App.js,
|
|
|
|
|
* naming components from other files, so the page cannot be resolved from one file's text.
|
|
|
|
|
* That is `VideosTabLayout`, whose pages are clean today but are not covered here.
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
*/
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
const src = SRC;
|
|
|
|
|
const files = () => sourceFiles();
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
|
|
|
|
|
const offenders = [];
|
|
|
|
|
for (const file of files(src)) {
|
|
|
|
|
const text = fs.readFileSync(file, 'utf8');
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
for (const block of chromedRouterBlocks(text)) {
|
|
|
|
|
for (const name of routedComponents(block)) {
|
|
|
|
|
if (bodyOf(text, name).includes('<PageContainer>')) {
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
offenders.push(`${path.relative(src, file)}: ${name}`);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/*
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
* The premise: a scan that found no routers, or resolved no page components, would report
|
|
|
|
|
* no offenders and read exactly like a clean codebase.
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
*/
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
const blocks = files(src).flatMap(file => chromedRouterBlocks(fs.readFileSync(file, 'utf8')));
|
|
|
|
|
expect(blocks.length).toBeGreaterThan(3);
|
|
|
|
|
expect(blocks.flatMap(routedComponents).length).toBeGreaterThan(8);
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
|
|
|
|
|
expect(offenders).toEqual([]);
|
|
|
|
|
});
|
ui: pages own the space between their blocks
Panels shared edges: two 1px borders meeting, which reads as one tall
surface with a rule across it rather than a seam between two.
Done as `margin-bottom` on the panel first, which is the anti-pattern, and
it failed in both of the ways that rule is known to fail before the reading
was done. It compounded with the gap of every Stack that already held
panels -- four of them, spacing those lists at 35.2px against the app's
17.6px -- and the `:last-child` reset meant to keep trailing space off the
bottom of a container silently did nothing, because Media wrapping makes a
panel the only child of its own wrapper. On Status that separated four
pairs out of five and left the first touching.
So the space belongs to whatever stacks the blocks. `.wrolpi-stack` is on
PageContainer, which every page uses, and on the three wrappers that stack
blocks somewhere the page's own stack cannot reach: Settings' panel wrapper,
the block below the video player, and the Zim and Map route wrappers, which
hold their tab bar outside PageContainer.
A gap rather than `> * + * { margin-top }`, which was tried and is quietly
broken here: Media injects `.fresnel-container { margin: 0; padding: 0 }` at
runtime, same specificity, later source order, so every wrapped block threw
the margin away. A child cannot zero its parent's gap. It also means
Media's hidden half contributes nothing, where the owl selector spaced
around something invisible.
With the page owning the spacing, the blocks that owned some of it had to
give it up, or their margin was added to the gap: the tab bar (33px against
17.6 everywhere else), the shared search control bar and paginator on
Videos/Archives/Docs/Files, the card grid, the file browser's filter row and
the inventory search. Two margins deliberately stay -- the file browser's
4em, which reserves room for the sticky buttons (62645831), and heading
margins, which bind a heading to the content below it.
Tests plant each of the three failures: the wrapped block, a child that
zeroes its own margin, and a hidden Media half. Verified in the browser on
sixteen routes and in all four themes -- every stacked pair 17.6px, the
map's 85vh box and the video player unchanged, no horizontal overflow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:40:27 -06:00
|
|
|
});
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
describe('a row of buttons', () => {
|
|
|
|
|
/*
|
ui: the last of Semantic UI, in name and in dead CSS
The library has been uninstalled for some time, but the codebase still read as
though it were there: identifiers named after it, rules written against its
class names, and about 180 comments explaining a rule in terms of a thing that
is no longer installed.
Renamed, keeping the vocabulary the app actually uses: semanticUIColorMap ->
navColorHexMap, semanticColorNames -> paletteColorNames, semanticNames ->
iconNameAliases, semanticSizes -> sizeAliases, and SemanticUIColors ->
NavColors in wrolpi/schema.py, which is what validates nav_color on the way in.
ColorToSemanticHexColor had no callers at all and is gone.
The baseline ratchet has done its job -- the list reached zero, so the JSON is
deleted and the guard keeps only the checks with teeth: no package, no import,
no `className="ui "`, no toast library. Its Theme.tsx check went with the
wrappers it was watching for. It also asserts its own walk found files, so a
clean result cannot mean it scanned nothing.
Deleted from App.css: every rule requiring `.ui`, `.inverted`, `.segment` or
`.modal`, none of which any component has set since the stylesheet stopped
loading, plus the form skeletons, ui-alerts and progress-text rules that
nothing renders. Two of them were doing harm rather than nothing: the drag
tints had a dark-theme variant under `.inverted` that stopped applying, leaving
15% of a fixed blue over a dark row, and now mix a token instead.
Two selectors naming markup that no longer exists were live bugs. The file
browser's filter had `.ui.input.file-browser-filter`, so it rendered at the
browser default width on a bar laid out to give it 34em -- now qualified by its
container, because `.wrolpi-action-input` sets `width: 100%` at the same
specificity and is loaded later. And SearchBox takes named props rather than a
rest spread, so the `id` DestinationForm hands it was dropped in silence and
the label it renders pointed at nothing; the download forms' Destination field
has had no associated label since the migration.
"Semantic roles" survives in four places, where it means what it says.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 19:15:57 -06:00
|
|
|
* Buttons used to bring `margin: 0 .25em .25em 0` with them, so a page could list buttons as
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
* bare siblings and they arrived both spaced and, when the row wrapped, separated. Nothing
|
|
|
|
|
* replaced that margin. The archive page's six actions shared edges, and two of them carried
|
|
|
|
|
* `marginTop: 0.5em` -- put there to keep a wrapped row from touching vertically, which instead
|
|
|
|
|
* made those two sit lower than the rest, because `vertical-align: middle` aligns the margin box.
|
|
|
|
|
*
|
|
|
|
|
* `.wrolpi-button-row` is a flex row with a gap, so it covers both axes at once and no button
|
|
|
|
|
* needs a margin of its own. ui-layout.cy.js measures that the row really spaces and aligns
|
|
|
|
|
* what it holds; these two scans are about the row being REACHED and left alone -- neither of
|
|
|
|
|
* which throws, and both of which is how the app got here.
|
|
|
|
|
*/
|
|
|
|
|
|
|
|
|
|
/*
|
|
|
|
|
* `=>` is blanked before any tag scanning. An arrow function in a prop -- `onClick={() =>
|
|
|
|
|
* handleDelete(false)}`, which most of these buttons have -- puts a `>` inside an opening tag,
|
|
|
|
|
* and a scanner reading `>` as "tag ends here" splits one button into two bogus tags. Two
|
|
|
|
|
* characters for two, so every index still points where it did in the original text.
|
|
|
|
|
*/
|
|
|
|
|
const scannable = (text) => text.replace(/=>/g, '=~');
|
|
|
|
|
|
|
|
|
|
const TAG = /<(\/?)([A-Za-z][\w.]*)([^>]*?)(\/?)>/g;
|
|
|
|
|
const tagsIn = (text) => [...scannable(text).matchAll(TAG)].map(match => ({
|
|
|
|
|
start: match.index,
|
|
|
|
|
end: match.index + match[0].length,
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
// The name was not captured until a test needed to accept `Group` and `Modal.Actions` as
|
|
|
|
|
// rows: `ROW_COMPONENTS.has(undefined)` is false for every tag, so the list of components
|
|
|
|
|
// that lay out their own children silently accepted none of them.
|
|
|
|
|
name: match[2],
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
attributes: match[3],
|
|
|
|
|
closing: match[1] === '/',
|
|
|
|
|
selfClosing: match[4] === '/',
|
|
|
|
|
}));
|
|
|
|
|
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
/**
|
|
|
|
|
* The text inside the element opened by `open`, found by walking forward to its MATCHING close.
|
|
|
|
|
*
|
|
|
|
|
* Written as "slice to the first `</div>`" first, which is true of every row in the app today
|
|
|
|
|
* and is a false negative waiting to happen: one nested div -- an error-trigger wrapper, a
|
|
|
|
|
* format-picker span, the Cancel/Save pair the collection form now groups -- truncates the body,
|
|
|
|
|
* and every button after it goes unscanned.
|
|
|
|
|
*/
|
|
|
|
|
const bodyOfElement = (text, tags, open) => {
|
|
|
|
|
let depth = 0;
|
|
|
|
|
for (const tag of tags) {
|
|
|
|
|
if (tag.start <= open.start || tag.selfClosing) continue;
|
|
|
|
|
if (tag.closing) {
|
|
|
|
|
if (depth === 0) return text.slice(open.end, tag.start);
|
|
|
|
|
depth -= 1;
|
|
|
|
|
} else {
|
|
|
|
|
depth += 1;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
return text.slice(open.end); // Unclosed; scan what is left rather than nothing.
|
|
|
|
|
};
|
|
|
|
|
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
/**
|
|
|
|
|
* The tag that encloses `index`: walk the tags before it backwards, letting a close push the
|
|
|
|
|
* depth up and an open bring it back down, and stop at the first open that is still unclosed.
|
|
|
|
|
*/
|
|
|
|
|
const enclosingTag = (tags, index) => {
|
|
|
|
|
let depth = 0;
|
|
|
|
|
for (const tag of [...tags].filter(tag => tag.end <= index).reverse()) {
|
|
|
|
|
if (tag.selfClosing) continue;
|
|
|
|
|
if (tag.closing) depth += 1;
|
|
|
|
|
else if (depth === 0) return tag;
|
|
|
|
|
else depth -= 1;
|
|
|
|
|
}
|
|
|
|
|
return null;
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Every `const <name> = ...` declared at any indentation, as button consts are.
|
|
|
|
|
*
|
|
|
|
|
* Every one, not the first: Archive.js declares a `deleteButton` in the archive page and
|
|
|
|
|
* another in the domain edit page, and a resolver that stopped at the first found the clean one
|
|
|
|
|
* and reported the page with the margin on it as clean too.
|
|
|
|
|
*/
|
|
|
|
|
const localBodiesOf = (text, name) => {
|
|
|
|
|
const pattern = new RegExp(`^[ \\t]*(?:const|let)\\s+${name}\\s*=`, 'gm');
|
|
|
|
|
return [...text.matchAll(pattern)].map(found => {
|
|
|
|
|
const after = text.slice(found.index + found[0].length);
|
|
|
|
|
const next = after.search(/\n[ \t]*(?:const|let|function|return|\})[\s(]/);
|
|
|
|
|
return next === -1 ? after : after.slice(0, next);
|
|
|
|
|
});
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
/** Every `const actionButtons = <>...</>` fragment in `text`: a button row under another name. */
|
|
|
|
|
const actionFragments = (text) => {
|
|
|
|
|
const bodies = [];
|
|
|
|
|
const pattern = /^[ \t]*const\s+actionButtons\s*=\s*<>/gm;
|
|
|
|
|
for (const found of text.matchAll(pattern)) {
|
|
|
|
|
const from = found.index + found[0].length;
|
|
|
|
|
const end = text.indexOf('</>', from);
|
|
|
|
|
if (end !== -1) bodies.push(text.slice(from, end));
|
|
|
|
|
}
|
|
|
|
|
return bodies;
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
const VERTICAL_MARGIN = /margin(?:Top|Bottom)\s*:|margin\s*:/;
|
|
|
|
|
|
|
|
|
|
/*
|
|
|
|
|
* The classes that lay buttons out in a row. A row is a row whatever it is called -- the file
|
|
|
|
|
* preview's toolbar had one before this, at a tighter gap because its buttons are icons -- so
|
|
|
|
|
* the check below is "is this one of them", and the test after it is what keeps the list honest
|
|
|
|
|
* by reading each one out of the stylesheet and confirming it really is a flex row with a gap.
|
|
|
|
|
*/
|
|
|
|
|
const ROW_CLASSES = [
|
|
|
|
|
'wrolpi-button-row', 'wrolpi-modal-actions', 'wrolpi-confirm-actions', 'preview-toolbar-group',
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
// Two that predate `.wrolpi-button-row` and are already correct: the file browser's sticky
|
|
|
|
|
// footer, and its filter row. Both are checked against the stylesheet below like the rest.
|
|
|
|
|
'sticky-footer', 'file-browser-filter-container',
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
];
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
|
|
|
|
|
/*
|
|
|
|
|
* Components that lay their own children out in a row. A `Group` or a `ButtonGroup` is a row by
|
|
|
|
|
* construction, so a caller putting buttons in one has already answered the question.
|
|
|
|
|
*/
|
|
|
|
|
const ROW_COMPONENTS = new Set(['Group', 'ButtonGroup', 'Row', 'Stack', 'Modal.Actions']);
|
|
|
|
|
|
|
|
|
|
/*
|
|
|
|
|
* An inline flex or grid with a gap of its own -- an anonymous row. Several call sites write one
|
|
|
|
|
* rather than reach for a class, and they are not wrong; a scan that only knew about class names
|
|
|
|
|
* would report every one of them.
|
|
|
|
|
*/
|
|
|
|
|
const INLINE_ROW = /display:\s*['"]?(?:flex|grid)/;
|
|
|
|
|
const isInlineRow = (attributes, text) => {
|
|
|
|
|
if (INLINE_ROW.test(attributes) && attributes.includes('gap')) return true;
|
|
|
|
|
/*
|
|
|
|
|
* `style={someStyle}` -- the style is a const somewhere above, so the attribute itself says
|
|
|
|
|
* nothing. Resolving it matters: the seasons row on the dates selector is a four-column
|
|
|
|
|
* grid with a gap, declared exactly that way, and it would otherwise be reported forever.
|
|
|
|
|
*/
|
|
|
|
|
const named = /style=\{(\w+)\}/.exec(attributes);
|
|
|
|
|
if (!named) return false;
|
|
|
|
|
const declared = new RegExp(`\\b${named[1]}\\s*=\\s*(\\{[^;]*?\\});`).exec(text);
|
|
|
|
|
return !!declared && INLINE_ROW.test(declared[1]) && declared[1].includes('gap');
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
const isRow = (tag, text = '') => !!tag && (
|
|
|
|
|
ROW_COMPONENTS.has(tag.name)
|
|
|
|
|
|| ROW_CLASSES.some(name => tag.attributes.includes(name))
|
|
|
|
|
|| isInlineRow(tag.attributes, text));
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
|
|
|
|
|
it('lays every row out as a flex row with a gap', () => {
|
|
|
|
|
// Otherwise the list above is just a list of names the scans below happen to accept.
|
|
|
|
|
const stylesheets = ['components/ui/ui.css', 'App.css']
|
|
|
|
|
.map(name => fs.readFileSync(path.join(SRC, name), 'utf8')).join('\n');
|
|
|
|
|
|
|
|
|
|
ROW_CLASSES.forEach(name => {
|
|
|
|
|
const found = new RegExp(`\\.${name}\\s*\\{([^}]*)\\}`).exec(stylesheets);
|
|
|
|
|
// Named in the failure rather than passed to `expect`, which takes no message.
|
|
|
|
|
expect(found ? name : `${name} is not declared`).toBe(name);
|
|
|
|
|
expect(found[1]).toMatch(/display:\s*flex/);
|
|
|
|
|
expect(found[1]).toMatch(/gap:/);
|
|
|
|
|
});
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('never stamps a vertical margin on a button the row already spaces', () => {
|
|
|
|
|
/*
|
|
|
|
|
* The margin and the gap add up. Nine buttons across the three collection edit pages had
|
|
|
|
|
* `marginTop: 1em` each and sat 1em below the Save button beside them; the archive page's
|
|
|
|
|
* Update and Generate Screenshot had 0.5em and sat half that below their own neighbours.
|
|
|
|
|
*/
|
|
|
|
|
const offenders = [];
|
|
|
|
|
let rows = 0;
|
|
|
|
|
let resolved = 0;
|
|
|
|
|
|
|
|
|
|
for (const file of sourceFiles()) {
|
|
|
|
|
const text = fs.readFileSync(file, 'utf8');
|
|
|
|
|
const where = path.relative(SRC, file);
|
|
|
|
|
|
|
|
|
|
const blocks = actionFragments(text);
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
const tags = tagsIn(text);
|
|
|
|
|
for (const tag of tags) {
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
if (!tag.closing && tag.attributes.includes('wrolpi-button-row')) {
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
blocks.push(bodyOfElement(text, tags, tag));
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
rows += blocks.length;
|
|
|
|
|
|
|
|
|
|
for (const block of blocks) {
|
|
|
|
|
// A button declared inline in the row, style and all.
|
|
|
|
|
if (VERTICAL_MARGIN.test(block)) offenders.push(`${where}: inline in the row`);
|
|
|
|
|
// ...or interpolated by name, which is how most of them are written.
|
|
|
|
|
for (const name of new Set([...block.matchAll(/\{(\w*[Bb]utton\w*)\}/g)]
|
|
|
|
|
.map(match => match[1]))) {
|
|
|
|
|
for (const body of localBodiesOf(text, name)) {
|
|
|
|
|
resolved += 1;
|
|
|
|
|
if (VERTICAL_MARGIN.test(body)) offenders.push(`${where}: ${name}`);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// A scan that found no rows, or resolved none of their buttons, reports a clean app.
|
|
|
|
|
expect(rows).toBeGreaterThan(3);
|
|
|
|
|
expect(resolved).toBeGreaterThan(8);
|
|
|
|
|
|
|
|
|
|
expect(offenders).toEqual([]);
|
|
|
|
|
});
|
|
|
|
|
|
|
|
|
|
it('puts every row of buttons in a row', () => {
|
|
|
|
|
/*
|
|
|
|
|
* The other half: a page can have margin-free buttons and still list them as bare siblings,
|
|
|
|
|
* which is the touching this fixes. Every `{actionButtons}` is rendered somewhere, and the
|
|
|
|
|
* element it is rendered into is what has to carry the gap -- on the doc page that was a
|
|
|
|
|
* plain `<div style={{marginTop: '1em'}}>`, spacing the row from what was above it while
|
|
|
|
|
* leaving the buttons inside it edge to edge.
|
|
|
|
|
*/
|
|
|
|
|
const offenders = [];
|
|
|
|
|
let sites = 0;
|
|
|
|
|
|
|
|
|
|
for (const file of sourceFiles()) {
|
|
|
|
|
const text = fs.readFileSync(file, 'utf8');
|
|
|
|
|
if (!text.includes('{actionButtons}')) continue;
|
|
|
|
|
const tags = tagsIn(text);
|
|
|
|
|
|
|
|
|
|
for (const found of text.matchAll(/\{actionButtons\}/g)) {
|
|
|
|
|
// `actionButtons={actionButtons}` is the prop being handed down, not a render.
|
|
|
|
|
if (text.slice(found.index - 14, found.index) === 'actionButtons=') continue;
|
|
|
|
|
sites += 1;
|
|
|
|
|
const parent = enclosingTag(tags, found.index);
|
|
|
|
|
if (!isRow(parent)) {
|
|
|
|
|
offenders.push(`${path.relative(SRC, file)}: ${
|
|
|
|
|
parent ? parent.attributes.trim().slice(0, 40) : 'no enclosing tag'}`);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/*
|
|
|
|
|
* Three: the shared collection edit form, which three pages hand a fragment to, and the doc
|
|
|
|
|
* page's own two responsive branches. A scan that counted the `actionButtons={...}` props
|
|
|
|
|
* instead would reach eight and never look at a render site at all.
|
|
|
|
|
*/
|
|
|
|
|
expect(sites).toBe(3);
|
|
|
|
|
|
|
|
|
|
expect(offenders).toEqual([]);
|
|
|
|
|
});
|
|
|
|
|
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
it('never lists interpolated buttons as bare siblings', () => {
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
/*
|
|
|
|
|
* The touching itself, and the reported defect: the archive page interpolated six buttons
|
|
|
|
|
* one after another straight into its Panel, so every pair shared an edge.
|
|
|
|
|
*
|
|
|
|
|
* A run of adjacent button interpolations is the shape to look for. Runs inside an
|
|
|
|
|
* `actionButtons` fragment are blanked first -- a fragment is spread into its parent's row
|
|
|
|
|
* and has no business being one itself, which the previous test is what checks. Blanked
|
|
|
|
|
* rather than skipped so every index still points where it did.
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
*
|
|
|
|
|
* The sibling test below covers the other way a row is written; this one only sees `{foo}`.
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
*/
|
|
|
|
|
const offenders = [];
|
|
|
|
|
let runs = 0;
|
|
|
|
|
|
|
|
|
|
for (const file of sourceFiles()) {
|
|
|
|
|
let text = fs.readFileSync(file, 'utf8');
|
|
|
|
|
for (const fragment of actionFragments(text)) {
|
|
|
|
|
text = text.replace(fragment, ' '.repeat(fragment.length));
|
|
|
|
|
}
|
|
|
|
|
const tags = tagsIn(text);
|
|
|
|
|
|
|
|
|
|
for (const found of text.matchAll(/\{\w*[Bb]utton\w*\}(?:\s*\{\w*[Bb]utton\w*\})+/g)) {
|
|
|
|
|
runs += 1;
|
|
|
|
|
const parent = enclosingTag(tags, found.index);
|
|
|
|
|
if (!isRow(parent)) {
|
|
|
|
|
offenders.push(`${path.relative(SRC, file)}: ${found[0].replace(/\s+/g, '')}`);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/*
|
|
|
|
|
* Four: the archive page's actions, the file preview's toolbar (which had a row of its own
|
|
|
|
|
* already), the downloads table's edit/restart pair, and the doc page's format picker
|
|
|
|
|
* beside its actions. Asserted exactly, because a run that stopped being recognised would
|
|
|
|
|
* leave this reporting a clean app.
|
|
|
|
|
*/
|
|
|
|
|
expect(runs).toBe(4);
|
|
|
|
|
|
|
|
|
|
expect(offenders).toEqual([]);
|
|
|
|
|
});
|
ui: a Back button is a row of one, not a stretched flex child
Reported: the Back button fills the page width on the domain and channel edit
pages. It fills it on all nine pages that have one, and this is mine from the
last PR.
`.wrolpi-stack` is a flex column at `align-items: stretch`, deliberately -- a
panel was full-width as a block element and has to stay that way. A BARE button
as a direct child of that stack is stretched too, and every page's Back button
was one. On the domain, channel and playlist edit pages it also pushed the
button beside it onto its own line, where the two had been inline.
Wrapping each in a `.wrolpi-button-row` fixes both without touching the stack:
the row is the stretched child, and it lays its buttons out at their own width.
The three pages that have a second button get one row holding both, which is how
they read before.
Also the Videos, Archives and Documents select bars, whose Tag/Delete/Invert/
Clear were four touching buttons in a bare div. They are inline elements rather
than interpolations, which is the shape the bare-siblings scan does not see.
Guarded twice: the row keeps its buttons their own width inside a stack while
itself stretching (both halves, since a row that refused to stretch would pass a
test that only looked at the button), and every `<BackButton/>` in the app is
inside a row.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:47:33 -06:00
|
|
|
|
review: the scan the first two commits needed
Five findings, all valid. The substantive one: the video page's Download /
Delete / Refresh / AddToPlaylist strip was still four bare buttons in a `<p>` --
the direct analogue of the archive row this PR started with -- and the scan that
was supposed to catch exactly that reported the app clean.
It only ever saw `{fooButton}` interpolations, and most of the app writes its
buttons out in place. Scanning for adjacent button ELEMENTS finds twelve more
rows in the same state: reorder icons in the playlist and field-schema tables,
both service tables on the controller page, its hotspot Save/QR pair and its
Restart/Shutdown pair, the downloads footer, the one-time pad's Generate/Cheat
Sheet, and the ration calculator's CSV/Print. All rowed.
Two holes had to be closed before that scan could be trusted, because a scan
that reports noise is worse than one with a stated limit:
- `tagsIn` never captured the tag NAME, so `ROW_COMPONENTS.has(undefined)` was
false for every tag and the list of components that lay out their own
children -- Group, ButtonGroup, Modal.Actions -- silently accepted none of
them. 56 false positives, every modal footer in the app among them.
- A row whose style is a const (`style={seasonRowStyle}`) says nothing in its
own attributes. Resolved now; the dates selector's seasons row is a
four-column grid with a gap and was reported three times.
Also from review: the row body is found by walking to the MATCHING close rather
than the first `</div>`, which one nested div would have truncated -- and the
collection form now nests one, so this was about to matter. Cancel and Save
travel together in a row of their own pushed to the end; `margin-left: auto` on
Save alone was right for a row that could not wrap, and would now drop Save onto
a second line by itself with Cancel left among the action buttons. The doc
page's comment no longer implies the picker's 1em separation was not a change.
One more found on the way: the file browser's filter row is flex with no gap, and
its collapse-all button carried `margin-left: 0.5em` for it. The gap is the
container's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 17:06:56 -06:00
|
|
|
it('never lists written-out buttons as bare siblings either', () => {
|
|
|
|
|
/*
|
|
|
|
|
* The same claim for the other way a row is written. The interpolation scan above sees
|
|
|
|
|
* `{fooButton}` and nothing else, and most of the app writes its buttons out in place --
|
|
|
|
|
* which is how the video page's Download/Delete/Refresh/AddToPlaylist strip, the direct
|
|
|
|
|
* analogue of the archive row this PR started with, went on touching after that scan
|
|
|
|
|
* reported the app clean. Twelve more rows were in the same state, in table cells, on the
|
|
|
|
|
* controller page, and under the one-time-pad and ration calculators.
|
|
|
|
|
*
|
|
|
|
|
* A "row" is deliberately broad here: the class, a Group or ButtonGroup, or an inline
|
|
|
|
|
* flex/grid with a gap, including one whose style is a const declared above. The point is
|
|
|
|
|
* whether the buttons are spaced by something, not whether they use this PR's class.
|
|
|
|
|
*/
|
|
|
|
|
const offenders = [];
|
|
|
|
|
let runs = 0;
|
|
|
|
|
|
|
|
|
|
for (const file of sourceFiles()) {
|
|
|
|
|
let text = fs.readFileSync(file, 'utf8');
|
|
|
|
|
// A fragment is spread into its parent's row; the earlier test checks that parent.
|
|
|
|
|
for (const fragment of actionFragments(text)) {
|
|
|
|
|
text = text.replace(fragment, ' '.repeat(fragment.length));
|
|
|
|
|
}
|
|
|
|
|
const tags = tagsIn(text);
|
|
|
|
|
|
|
|
|
|
// A button element ending, then whitespace, then another button beginning.
|
|
|
|
|
const runPattern = /(?:<\/\w*Button>|<\w*Button\b[^>]*?\/>)\s*<\w*Button\b/g;
|
|
|
|
|
for (const match of scannable(text).matchAll(runPattern)) {
|
|
|
|
|
// Anchor on the SECOND button's `<`: at the first one's closing tag the pair is
|
|
|
|
|
// still open, and the walk would report the first button as its own parent.
|
|
|
|
|
const at = match.index + match[0].lastIndexOf('<');
|
|
|
|
|
runs += 1;
|
|
|
|
|
if (!isRow(enclosingTag(tags, at), text)) {
|
|
|
|
|
offenders.push(`${path.relative(SRC, file)}:${
|
|
|
|
|
text.slice(0, at).split('\n').length}`);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/*
|
|
|
|
|
* Around eighty, which is why this is a scan and not a list. Asserted loosely at both ends
|
|
|
|
|
* -- tight enough that a pattern matching nothing shows up, loose enough that adding a
|
|
|
|
|
* button to an existing row is not a test failure.
|
|
|
|
|
*/
|
|
|
|
|
expect(runs).toBeGreaterThan(60);
|
|
|
|
|
expect(runs).toBeLessThan(200);
|
|
|
|
|
|
|
|
|
|
expect(offenders).toEqual([]);
|
|
|
|
|
});
|
|
|
|
|
|
ui: a Back button is a row of one, not a stretched flex child
Reported: the Back button fills the page width on the domain and channel edit
pages. It fills it on all nine pages that have one, and this is mine from the
last PR.
`.wrolpi-stack` is a flex column at `align-items: stretch`, deliberately -- a
panel was full-width as a block element and has to stay that way. A BARE button
as a direct child of that stack is stretched too, and every page's Back button
was one. On the domain, channel and playlist edit pages it also pushed the
button beside it onto its own line, where the two had been inline.
Wrapping each in a `.wrolpi-button-row` fixes both without touching the stack:
the row is the stretched child, and it lays its buttons out at their own width.
The three pages that have a second button get one row holding both, which is how
they read before.
Also the Videos, Archives and Documents select bars, whose Tag/Delete/Invert/
Clear were four touching buttons in a bare div. They are inline elements rather
than interpolations, which is the shape the bare-siblings scan does not see.
Guarded twice: the row keeps its buttons their own width inside a stack while
itself stretching (both halves, since a row that refused to stretch would pass a
test that only looked at the button), and every `<BackButton/>` in the app is
inside a row.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:47:33 -06:00
|
|
|
it('puts every Back button in a row, so the page stack cannot stretch it', () => {
|
|
|
|
|
/*
|
|
|
|
|
* A page's blocks are stacked by `.wrolpi-stack`, which is a flex column at
|
|
|
|
|
* `align-items: stretch` -- deliberately, because a panel was full-width as a block
|
|
|
|
|
* element and has to stay that way. A BARE button as a direct child of that stack is
|
|
|
|
|
* stretched too, and every page's Back button was one: full-page-width, and on the domain,
|
|
|
|
|
* channel and playlist edit pages the button beside it was pushed onto its own line where
|
|
|
|
|
* the two had been inline before.
|
|
|
|
|
*
|
|
|
|
|
* A row fixes it without touching the stack: the row is the stretched child, and it lays
|
|
|
|
|
* its buttons out at their own width. Nine sites, so this scan rather than nine
|
|
|
|
|
* assertions -- and a scan because a stretched button is still a working button.
|
|
|
|
|
*/
|
|
|
|
|
const offenders = [];
|
|
|
|
|
let found = 0;
|
|
|
|
|
|
|
|
|
|
for (const file of sourceFiles()) {
|
|
|
|
|
const text = fs.readFileSync(file, 'utf8');
|
|
|
|
|
const tags = tagsIn(text);
|
|
|
|
|
|
|
|
|
|
for (const match of text.matchAll(/<BackButton\b/g)) {
|
|
|
|
|
found += 1;
|
|
|
|
|
if (!isRow(enclosingTag(tags, match.index))) {
|
|
|
|
|
offenders.push(`${path.relative(SRC, file)}:${
|
|
|
|
|
text.slice(0, match.index).split('\n').length}`);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// Nine renders. Common.js declares it, and a declaration is not a `<BackButton`.
|
|
|
|
|
expect(found).toBe(9);
|
|
|
|
|
|
|
|
|
|
expect(offenders).toEqual([]);
|
|
|
|
|
});
|
ui: a row of buttons is spaced by the row
The archive page's six actions shared edges, and Update sat lower than the
rest. Both come from the same thing: Semantic's Button carried
`margin: 0 .25em .25em 0`, so a page could list buttons as bare siblings and
they arrived spaced -- and separated from the line below when the row wrapped.
Nothing replaced that margin.
Two of those six answered with `marginTop: 0.5em`, which is the wrong axis: a
Button is `display: inline-flex; vertical-align: middle`, and `vertical-align`
is measured on the MARGIN box, so half of any top margin shows up as the
visible button dropping. It was covering the wrapped-row case in the only axis
a margin can, and paying for it in the other one.
`.wrolpi-button-row` is a flex row with a gap, so both axes are covered at once
and no button needs a margin of its own -- the same rule as `.wrolpi-stack`,
along the other axis. Applied to the five rows in the app that had no gap: the
archive page, the shared collection edit form (which three pages hand action
buttons to, and where nine buttons plus Cancel carried `margin-top: 1em` each
while Save carried none), the doc page's two responsive branches, and the
downloads table's edit/restart pair.
`.action-button-spacing` goes with them; it was that same margin, on Cancel.
Guarded four ways. ui-layout.cy.js measures that the row really spaces what it
holds, wraps rather than overflows, centres its buttons against a taller item
beside them, and counts a confirmed APIButton as one item rather than two --
that one returns a fragment holding a `<Confirm/>`, which would open a hole in
the row if it did not portal. Common.test.js scans for the two ways this comes
back: a button in a row stamping a vertical margin, and a row of buttons that is
not a row. Neither throws, which is how the app got here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:39:03 -06:00
|
|
|
});
|
|
|
|
|
|
|
|
|
|
/** The app's own source, minus its tests: what a source scan is allowed to read. */
|
|
|
|
|
const SRC = path.join(__dirname, '..');
|
|
|
|
|
const sourceFiles = (dir = SRC) => fs.readdirSync(dir, {withFileTypes: true}).flatMap(entry => {
|
|
|
|
|
const full = path.join(dir, entry.name);
|
|
|
|
|
if (entry.isDirectory()) return sourceFiles(full);
|
|
|
|
|
if (!/\.jsx?$/.test(entry.name)) return [];
|
|
|
|
|
if (/\.(test|cy)\.jsx?$/.test(entry.name)) return [];
|
|
|
|
|
return [full];
|
|
|
|
|
});
|
|
|
|
|
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
/**
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
* Every `<Routes>...</Routes>` in `text` whose enclosing `return` provides page chrome -- a
|
|
|
|
|
* `PageContainer` or a `wrolpi-stack` wrapper. Those are the routers that have already spaced and
|
|
|
|
|
* inset the page, so a page under one must not do it again.
|
|
|
|
|
*
|
|
|
|
|
* All of them, and located from the router rather than from the chrome: keying on the first
|
|
|
|
|
* `<PageContainer>` in a file missed this entirely in Archive.js, where the first one belongs to
|
|
|
|
|
* the offending page and holds no router at all.
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
*/
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
const chromedRouterBlocks = (text) => {
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
const blocks = [];
|
|
|
|
|
let from = 0;
|
|
|
|
|
for (;;) {
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
const start = text.indexOf('<Routes>', from);
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
if (start === -1) return blocks;
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
const end = text.indexOf('</Routes>', start);
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
if (end === -1) return blocks;
|
|
|
|
|
from = end + 1;
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
|
|
|
|
|
const returned = text.lastIndexOf('return', start);
|
|
|
|
|
if (returned === -1) continue;
|
|
|
|
|
const chrome = text.slice(returned, start);
|
|
|
|
|
if (chrome.includes('<PageContainer>') || chrome.includes('wrolpi-stack')) {
|
|
|
|
|
blocks.push(text.slice(start, end));
|
|
|
|
|
}
|
ui: stop /archives/settings wrapping itself in a second PageContainer
`ArchiveRoute` wraps its `<Routes>` in a PageContainer, and
`ArchiveSettingsPage` wrapped itself in another. PageContainer carries
`margin-top: 1em` and `padding: 1em`, so that page sat 1em lower and 1em
further in on all four sides than /archives/domains beside it -- which is
exactly what "seems to have more margin" was. It is a fragment now, so its
panels are blocks of the route's own stack.
Guarded by a source scan rather than a render, since a doubled margin throws
nothing: it takes each PageContainer that wraps a router, reads the
components it routes to, and fails if one of them renders a PageContainer of
its own.
The first version of that scan passed against the live bug. It looked at
only the first `<PageContainer>` in a file, and in Archive.js that is the
offender itself, whose block holds no `<Routes>` -- so the file was skipped
before the route wrapper 400 lines below was reached. It reads every
occurrence now, and the premise assertions count both the wrappers found and
the routes within them.
Two more top-level blocks gave up margins that were being added to the
page's gap, both headers "matching" each other's pattern: the domain filter
row (35.2px against 17.6px) and the channel filter row.
/archives/settings and /archives/domains now measure identically -- tab bar
bottom at 136.8px, first block 17.6px below it, content inset 17.6px -- as
do the statistics and settings pages of every other tab group.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 15:53:04 -06:00
|
|
|
}
|
|
|
|
|
};
|
review: finish the sweep the first pass claimed
The review was right that completeness was not done. Every finding held up.
Three more pages mounted a `PageContainer` inside a route that already
provides page chrome -- `ManageMap`, `MapPins`, `ManageZim` -- so they sat a
further 1em down and 1em in from the tab bar than the viewers beside them.
The same defect as ArchiveSettings, in a shape the scan could not see: it
keyed on `PageContainer` route wrappers, and Map and Zim wrap their content
in a `wrolpi-stack` div instead. It now locates routers first and asks
whether the enclosing return provides chrome either way, resolves
`element={<ErrorBoundary><Page/></ErrorBoundary>}` as well as the bare form,
and knows about `class` components -- `ManageZim` is one, and a scan that
only knew functions reported it clean.
Five top-level blocks still added their own margin to the stack's gap: the
dashboard search row (52.8px against 17.6px, on the most-looked-at page in
the app), the Playlists filter row, both Docs filter rows, and the Map pins
filter. The extension page's hint panel had the same compound against a
Mantine Stack's gap.
ControllerPage held its four panel sections in a plain wrapper, so the
admin page's stack saw one child and the panels kept sharing edges -- the
exact hole Settings had.
The dashboard was reported clean in the first pass and was not. The sweep
measured wrapper-to-wrapper gaps, and that row's margin is INSIDE a Media
wrapper, so it never appeared: the measurement, not the page, was at fault.
Cypress gap assertions now derive from the stack's own `row-gap` instead of a
12-28px range that assumed the default interface scale and would have
false-failed a correct layout at a larger one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:09:12 -06:00
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* The page components a router block routes to. Any capitalised tag that is not routing furniture,
|
|
|
|
|
* so `element={<ErrorBoundary><Page/></ErrorBoundary>}` yields `Page` as well as the plain
|
|
|
|
|
* `element={<Page/>}` form.
|
|
|
|
|
*/
|
|
|
|
|
const FURNITURE = new Set(['Route', 'Routes', 'ErrorBoundary', 'Suspense', 'Navigate', 'Outlet']);
|
|
|
|
|
const routedComponents = (block) => [...new Set(
|
|
|
|
|
[...block.matchAll(/<([A-Z]\w*)/g)].map(m => m[1]).filter(name => !FURNITURE.has(name)))];
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* The source of the component named `name`, bounded by the next top-level declaration. `class` is
|
|
|
|
|
* in both patterns because `ManageZim` is one, and a scan that only knew about functions would
|
|
|
|
|
* have reported it clean.
|
|
|
|
|
*/
|
|
|
|
|
const bodyOf = (text, name) => {
|
|
|
|
|
const declaration = new RegExp(`^(?:export\\s+)?(?:function|const|class)\\s+${name}\\b`, 'm');
|
|
|
|
|
const found = declaration.exec(text);
|
|
|
|
|
if (!found) return ''; // Declared elsewhere; out of this scan's reach.
|
|
|
|
|
const after = text.slice(found.index + 1);
|
|
|
|
|
const next = after.search(/^(?:export\s+)?(?:function|const|class)\s/m);
|
|
|
|
|
return next === -1 ? after : after.slice(0, next);
|
|
|
|
|
};
|