mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04: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>
This commit is contained in:
Roland
parent
bb14bf4074
commit
5ce4f2ebf3
12 changed files with 464 additions and 44 deletions
|
|
@ -462,10 +462,6 @@ table.ratio-table td.min-width {
|
|||
color: red;
|
||||
}
|
||||
|
||||
.action-button-spacing {
|
||||
margin-top: 1em;
|
||||
}
|
||||
|
||||
/* Form loading skeleton */
|
||||
.form-skeleton-field {
|
||||
margin-bottom: 1.5em;
|
||||
|
|
|
|||
|
|
@ -174,7 +174,6 @@ function ArchivePage() {
|
|||
confirmButton='Update'
|
||||
onClick={localUpdateArchive}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '0.5em'}}
|
||||
>
|
||||
Update
|
||||
</APIButton>;
|
||||
|
|
@ -193,7 +192,6 @@ function ArchivePage() {
|
|||
confirmButton='Generate'
|
||||
onClick={localGenerateScreenshot}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '0.5em'}}
|
||||
>
|
||||
Generate Screenshot
|
||||
</APIButton> : null;
|
||||
|
|
@ -275,12 +273,14 @@ function ArchivePage() {
|
|||
</div>
|
||||
</div>
|
||||
|
||||
{singlefileButton}
|
||||
{readButton}
|
||||
{updateButton}
|
||||
{deleteButton}
|
||||
{generateScreenshotButton}
|
||||
<AddToPlaylistButton fileGroupId={archiveFile.id}/>
|
||||
<div className='wrolpi-button-row'>
|
||||
{singlefileButton}
|
||||
{readButton}
|
||||
{updateButton}
|
||||
{deleteButton}
|
||||
{generateScreenshotButton}
|
||||
<AddToPlaylistButton fileGroupId={archiveFile.id}/>
|
||||
</div>
|
||||
</Panel>
|
||||
|
||||
<Panel>
|
||||
|
|
@ -648,7 +648,6 @@ export function DomainEditPage() {
|
|||
confirmHeader='Delete Domain?'
|
||||
onClick={handleDelete}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '1em'}}
|
||||
>Delete</APIButton>;
|
||||
|
||||
const refreshButton = domain?.directory ? (
|
||||
|
|
@ -657,7 +656,6 @@ export function DomainEditPage() {
|
|||
size='small'
|
||||
onClick={handleRefreshDomain}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '1em'}}
|
||||
>Refresh</APIButton>
|
||||
) : null;
|
||||
|
||||
|
|
@ -666,7 +664,6 @@ export function DomainEditPage() {
|
|||
size='small'
|
||||
onClick={() => setTagEditModalOpen(true)}
|
||||
color='violet'
|
||||
style={{marginTop: '1em'}}
|
||||
>Tag</Button>;
|
||||
|
||||
const reorganizeButton = (
|
||||
|
|
@ -675,7 +672,6 @@ export function DomainEditPage() {
|
|||
size='small'
|
||||
onClick={() => setReorganizeModalOpen(true)}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '1em'}}
|
||||
>Reorganize Files</APIButton>
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -173,7 +173,6 @@ export function ChannelEditPage() {
|
|||
confirmHeader='Delete Channel?'
|
||||
onClick={handleDelete}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '1em'}}
|
||||
>Delete</APIButton>;
|
||||
|
||||
const refreshButton = <APIButton
|
||||
|
|
@ -181,7 +180,6 @@ export function ChannelEditPage() {
|
|||
size='small'
|
||||
onClick={handleRefreshChannel}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '1em'}}
|
||||
>Refresh</APIButton>;
|
||||
|
||||
const tagButton = <Button
|
||||
|
|
@ -189,7 +187,6 @@ export function ChannelEditPage() {
|
|||
size='small'
|
||||
onClick={() => setTagEditModalOpen(true)}
|
||||
role='primary'
|
||||
style={{marginTop: '1em'}}
|
||||
>Tag</Button>;
|
||||
|
||||
const reorganizeButton = (
|
||||
|
|
@ -198,7 +195,6 @@ export function ChannelEditPage() {
|
|||
size='small'
|
||||
onClick={() => setReorganizeModalOpen(true)}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '1em'}}
|
||||
>Reorganize Files</APIButton>
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -752,14 +752,8 @@ describe('PageContainer', () => {
|
|||
* 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.
|
||||
*/
|
||||
const src = path.join(__dirname, '..');
|
||||
const files = (dir) => fs.readdirSync(dir, {withFileTypes: true}).flatMap(entry => {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) return files(full);
|
||||
if (!/\.jsx?$/.test(entry.name)) return [];
|
||||
if (/\.(test|cy)\.jsx?$/.test(entry.name)) return [];
|
||||
return [full];
|
||||
});
|
||||
const src = SRC;
|
||||
const files = () => sourceFiles();
|
||||
|
||||
const offenders = [];
|
||||
for (const file of files(src)) {
|
||||
|
|
@ -785,6 +779,241 @@ describe('PageContainer', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('a row of buttons', () => {
|
||||
/*
|
||||
* Semantic's Button brought `margin: 0 .25em .25em 0` with it, so a page could list buttons as
|
||||
* 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,
|
||||
attributes: match[3],
|
||||
closing: match[1] === '/',
|
||||
selfClosing: match[4] === '/',
|
||||
}));
|
||||
|
||||
/**
|
||||
* 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',
|
||||
];
|
||||
const isRow = (tag) => !!tag && ROW_CLASSES.some(name => tag.attributes.includes(name));
|
||||
|
||||
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);
|
||||
for (const tag of tagsIn(text)) {
|
||||
if (!tag.closing && tag.attributes.includes('wrolpi-button-row')) {
|
||||
// A row holds buttons, not more divs, so its first `</div>` is its own.
|
||||
const closes = text.indexOf('</div>', tag.end);
|
||||
blocks.push(closes === -1 ? '' : text.slice(tag.end, closes));
|
||||
}
|
||||
}
|
||||
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([]);
|
||||
});
|
||||
|
||||
it('never lists buttons as bare siblings', () => {
|
||||
/*
|
||||
* 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.
|
||||
*/
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
|
||||
/** 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];
|
||||
});
|
||||
|
||||
/**
|
||||
* 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
|
||||
|
|
|
|||
|
|
@ -403,13 +403,16 @@ function DocPage() {
|
|||
return <>
|
||||
<Media at='mobile'>
|
||||
{formatButtons && <div style={{marginTop: '1em'}}>{formatButtons}</div>}
|
||||
<div style={{marginTop: '1em'}}>{actionButtons}</div>
|
||||
<div className='wrolpi-button-row' style={{marginTop: '1em'}}>
|
||||
{actionButtons}
|
||||
</div>
|
||||
</Media>
|
||||
<Media greaterThanOrEqual='tablet'>
|
||||
<div style={{marginTop: '1em'}}>
|
||||
{formatButtons && <span style={{marginRight: '1em', display: 'inline-block'}}>
|
||||
{formatButtons}
|
||||
</span>}
|
||||
{/* The row's own `margin-top` separates it from what is above; the gap
|
||||
inside it separates the format picker from the actions, so the span
|
||||
that used to hold them apart with `margin-right` is gone. */}
|
||||
<div className='wrolpi-button-row' style={{marginTop: '1em'}}>
|
||||
{formatButtons}
|
||||
{actionButtons}
|
||||
</div>
|
||||
</Media>
|
||||
|
|
|
|||
|
|
@ -479,7 +479,6 @@ export function PlaylistEditPage() {
|
|||
confirmHeader='Delete Playlist?'
|
||||
onClick={handleDelete}
|
||||
obeyWROLMode={true}
|
||||
style={{marginTop: '1em'}}
|
||||
>Delete</APIButton>;
|
||||
|
||||
const tagButton = <Button
|
||||
|
|
@ -488,7 +487,6 @@ export function PlaylistEditPage() {
|
|||
onClick={() => setTagModalOpen(true)}
|
||||
role='primary'
|
||||
disabled={!editable}
|
||||
style={{marginTop: '1em'}}
|
||||
>Tag</Button>;
|
||||
|
||||
const actionButtons = <>
|
||||
|
|
|
|||
|
|
@ -332,10 +332,31 @@ export function ThemeSamplePage() {
|
|||
</div>)}
|
||||
</Row>
|
||||
</div>
|
||||
<div style={{marginTop: 12}}>
|
||||
<Header as='h5'>A row of actions</Header>
|
||||
{/* `.wrolpi-button-row` is how a page lists the actions on a thing -- the
|
||||
archive page's View/Read/Update/Delete, the edit pages' action row. It
|
||||
wraps, so the last button drops to a second line at this width instead of
|
||||
overflowing, and the row gap keeps the two lines apart. */}
|
||||
<div className='wrolpi-button-row' style={{maxWidth: 320}}>
|
||||
<Button color='violet'>View</Button>
|
||||
<Button color='blue'>Read</Button>
|
||||
<Button role='save'>Update</Button>
|
||||
<Button role='danger'>Delete</Button>
|
||||
<Button color='yellow'>Generate Screenshot</Button>
|
||||
</div>
|
||||
</div>
|
||||
<p style={{fontSize: '0.75rem', color: 'var(--muted)', marginBottom: 0}}>
|
||||
In night mode Delete drops its fill for a dashed red outline; dashed means
|
||||
destructive or failed, and nothing else uses it.
|
||||
</p>
|
||||
<p style={{fontSize: '0.75rem', color: 'var(--muted)', marginBottom: 0}}>
|
||||
Buttons in a row are spaced by the row, never by margins of their own. Semantic's
|
||||
Button brought its own margin and nothing replaced it, so pages that listed
|
||||
buttons as bare siblings had them sharing edges — and the ones that answered with
|
||||
<code>margin-top</code> got a button sitting lower than its neighbours instead,
|
||||
because <code>vertical-align: middle</code> is measured on the margin box.
|
||||
</p>
|
||||
<p style={{fontSize: '0.75rem', color: 'var(--muted)', marginBottom: 0}}>
|
||||
An icon button is exactly as tall as a labelled one at every size, so a
|
||||
group of them lines up. Mantine's own scale for icon buttons is unrelated
|
||||
|
|
|
|||
|
|
@ -329,10 +329,14 @@ function RecurringDownloadRow({download, fetchDownloads, onDelete}) {
|
|||
{download.progress && <DownloadProgressModal progress={download.progress} url={download.url}/>}
|
||||
</Table.Cell>
|
||||
<Table.Cell>{next}</Table.Cell>
|
||||
<Table.Cell style={{textAlign: 'right'}}>
|
||||
{error && !download.progress && errorTrigger}
|
||||
{editButton}
|
||||
{restartButton}
|
||||
<Table.Cell>
|
||||
{/* `justify-content` rather than the cell's `text-align: right`, which a flex row
|
||||
does not answer to. These two are icon buttons and were sharing an edge. */}
|
||||
<div className='wrolpi-button-row' style={{justifyContent: 'flex-end'}}>
|
||||
{error && !download.progress && errorTrigger}
|
||||
{editButton}
|
||||
{restartButton}
|
||||
</div>
|
||||
</Table.Cell>
|
||||
{errorModal}
|
||||
{editModal}
|
||||
|
|
|
|||
|
|
@ -60,14 +60,21 @@ export function CollectionEditForm({
|
|||
|
||||
{appliedTagName && <div><SingleTag name={appliedTagName}/></div>}
|
||||
|
||||
<div style={{display: 'flex', alignItems: 'center', gap: 10}}>
|
||||
{/*
|
||||
* The shared row. It was already a flex row at this gap, but it did not wrap, so
|
||||
* the pages that hand it four action buttons ran out of width on a phone -- which
|
||||
* is why each of those buttons, and the Cancel below, carried `margin-top: 1em`.
|
||||
* That margin is what made them sit lower than Save, `vertical-align: middle`
|
||||
* being measured on the margin box. Wrapping plus a row gap covers the case the
|
||||
* margin was for, in the axis it actually happens in.
|
||||
*/}
|
||||
<div className='wrolpi-button-row'>
|
||||
{actionButtons}
|
||||
{onCancel && <Button
|
||||
type='button'
|
||||
role='cancel'
|
||||
onClick={onCancel}
|
||||
disabled={form.disabled}
|
||||
className="action-button-spacing"
|
||||
>
|
||||
Cancel
|
||||
</Button>}
|
||||
|
|
|
|||
|
|
@ -253,18 +253,28 @@ describe('CollectionEditForm', () => {
|
|||
});
|
||||
|
||||
describe('CSS Classes', () => {
|
||||
it('uses action-button-spacing class for cancel button', () => {
|
||||
it('puts its actions in a button row, and leaves their margins alone', () => {
|
||||
/*
|
||||
* `action-button-spacing` was `margin-top: 1em` on Cancel alone, matching the same
|
||||
* margin the three calling pages stamped on each of their action buttons -- so every
|
||||
* button in this row sat 1em below the Save beside it, which has none. The row spaces
|
||||
* and wraps them now, and nothing in it carries a margin of its own.
|
||||
*/
|
||||
const form = createTestForm(mockCollection);
|
||||
const mockOnCancel = jest.fn();
|
||||
|
||||
render(
|
||||
const {container} = render(
|
||||
<CollectionEditForm form={form} onCancel={mockOnCancel}>
|
||||
<div>Form content</div>
|
||||
</CollectionEditForm>
|
||||
);
|
||||
|
||||
const cancelButton = screen.getByRole('button', {name: /cancel/i});
|
||||
expect(cancelButton).toHaveClass('action-button-spacing');
|
||||
const row = container.querySelector('.wrolpi-button-row');
|
||||
expect(row).toBeInTheDocument();
|
||||
expect(row).toContainElement(cancelButton);
|
||||
expect(row).toContainElement(screen.getByRole('button', {name: /save/i}));
|
||||
expect(cancelButton).not.toHaveClass('action-button-spacing');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -277,6 +277,136 @@ describe('a page stacks its own blocks', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('a row of buttons is spaced and aligned by the row', () => {
|
||||
/*
|
||||
* Semantic's Button carried `margin: 0 .25em .25em 0` of its own, so a page could list
|
||||
* buttons as bare siblings and they arrived spaced. Nothing replaced that margin, and
|
||||
* five pages still list them that way -- the archive page's six actions all shared edges.
|
||||
*
|
||||
* Two of those six also carried `marginTop: 0.5em`, which is what made "Update" sit lower
|
||||
* than its neighbours: a Button is `display: inline-flex; vertical-align: middle`, and
|
||||
* `vertical-align` aligns the MARGIN box, so half of any top margin pushes the visible
|
||||
* button down. The margin was there to keep a wrapped row from touching vertically --
|
||||
* the right thing, solved in the wrong axis. A flex row with `gap` covers both axes and
|
||||
* needs no per-button margin, so the alignment comes back for free.
|
||||
*/
|
||||
|
||||
const rowButtons = ($row) => [...$row[0].querySelectorAll('button')];
|
||||
|
||||
it('puts a gap between the buttons, horizontally and when wrapped', () => {
|
||||
cy.mountUI(<div className='wrolpi-button-row'>
|
||||
<Button>View</Button>
|
||||
<Button>Read</Button>
|
||||
<Button>Update</Button>
|
||||
</div>);
|
||||
|
||||
cy.get('.wrolpi-button-row').should(($row) => {
|
||||
const row = $row[0];
|
||||
const styles = getComputedStyle(row);
|
||||
const declared = parseFloat(styles.columnGap);
|
||||
expect(declared, 'the row declares a gap').to.be.greaterThan(0);
|
||||
expect(parseFloat(styles.rowGap), 'and the same one between wrapped lines')
|
||||
.to.be.closeTo(declared, 0.5);
|
||||
expect(styles.flexWrap, 'the row wraps rather than overflowing').to.equal('wrap');
|
||||
|
||||
/*
|
||||
* Narrowed to exactly two buttons' worth, measured rather than guessed. A hardcoded
|
||||
* 150px was a hidden assertion about both the font and the interface scale, and at 1.1
|
||||
* it fitted one button, not the two the assertions below are about.
|
||||
*/
|
||||
const widths = rowButtons($row).map(el => el.getBoundingClientRect().width);
|
||||
expect(widths, 'three buttons measured').to.have.length(3);
|
||||
row.style.width = `${widths[0] + declared + widths[1] + 1}px`;
|
||||
|
||||
const rects = rowButtons($row).map(el => el.getBoundingClientRect());
|
||||
const top = Math.min(...rects.map(rect => rect.top));
|
||||
const first = rects.filter(rect => Math.round(rect.top) === Math.round(top));
|
||||
expect(first, 'two buttons on the first line').to.have.length(2);
|
||||
expect(first[1].left - first[0].right, 'gap along the line').to.be.closeTo(declared, 0.5);
|
||||
|
||||
// And the third dropped to a second line, held off the first by the same gap.
|
||||
const wrapped = rects.find(rect => Math.round(rect.top) !== Math.round(top));
|
||||
expect(wrapped, 'the third button wrapped').to.not.be.undefined;
|
||||
expect(wrapped.top - first[0].bottom, 'gap between the lines')
|
||||
.to.be.closeTo(declared, 0.5);
|
||||
});
|
||||
});
|
||||
|
||||
it('centres its buttons against a taller item beside them', () => {
|
||||
/*
|
||||
* A row is not always all buttons: the doc page puts its format picker beside its actions,
|
||||
* and the downloads table an error trigger beside its two icon buttons. Whatever is
|
||||
* tallest sets the line height, and `align-items: center` is what keeps the buttons reading
|
||||
* as part of that line rather than hanging from the top of it.
|
||||
*
|
||||
* Written as tops first, which was a test with no teeth -- `stretch` stretches the WRAPPER
|
||||
* and leaves the button at the top of it, so the tops agreed either way. Centres are what
|
||||
* the property actually decides.
|
||||
*
|
||||
* The row is also mixed in the way the archive page's is: a button inside an `<a>`, a bare
|
||||
* one, and an APIButton.
|
||||
*/
|
||||
cy.mountUI(<div className='wrolpi-button-row'>
|
||||
<div style={{height: '80px', width: '40px'}}>Tall</div>
|
||||
<a href='/media/archive.html'><Button>View</Button></a>
|
||||
<Button>Read</Button>
|
||||
<APIButton onClick={() => Promise.resolve()}>Update</APIButton>
|
||||
</div>);
|
||||
|
||||
cy.get('.wrolpi-button-row').should(($row) => {
|
||||
const row = $row[0].getBoundingClientRect();
|
||||
expect(row.height, 'the tall item set the line height').to.be.at.least(80);
|
||||
|
||||
const centers = rowButtons($row).map((el) => {
|
||||
const rect = el.getBoundingClientRect();
|
||||
return rect.top + rect.height / 2;
|
||||
});
|
||||
expect(centers, 'three buttons measured').to.have.length(3);
|
||||
const middle = row.top + row.height / 2;
|
||||
centers.forEach(center => expect(center, 'centred in the line').to.be.closeTo(middle, 1));
|
||||
});
|
||||
});
|
||||
|
||||
it('counts a confirmed action as one item in the row, not two', () => {
|
||||
/*
|
||||
* `APIButton` with `confirmContent` returns a fragment -- the button AND a `<Confirm/>`.
|
||||
* In a flex row those flatten into siblings, so a Confirm that rendered anything in
|
||||
* place would take a gap of its own and open a hole between two buttons. It portals,
|
||||
* and this is what says so; three of the archive page's six actions are confirmed.
|
||||
*/
|
||||
cy.mountUI(<div className='wrolpi-button-row'>
|
||||
<Button>Plain</Button>
|
||||
<APIButton confirmContent='Sure?' onClick={() => Promise.resolve()}>Delete</APIButton>
|
||||
<Button>Also plain</Button>
|
||||
</div>);
|
||||
|
||||
cy.get('.wrolpi-button-row').should(($row) => {
|
||||
const declared = parseFloat(getComputedStyle($row[0]).columnGap);
|
||||
const rects = rowButtons($row).map(el => el.getBoundingClientRect());
|
||||
expect(rects, 'three buttons measured').to.have.length(3);
|
||||
rects.slice(1).forEach((rect, index) => expect(
|
||||
rect.left - rects[index].right, 'gap either side of the confirmed action',
|
||||
).to.be.closeTo(declared, 0.5));
|
||||
});
|
||||
});
|
||||
|
||||
it('grows its gap with the interface scale', () => {
|
||||
cy.mountUI(<div className='wrolpi-button-row'><Button>One</Button><Button>Two</Button></div>);
|
||||
|
||||
cy.get('.wrolpi-button-row').should(($row) => {
|
||||
const before = parseFloat(getComputedStyle($row[0]).columnGap);
|
||||
const root = Cypress.$('html')[0];
|
||||
const scale = parseFloat(getComputedStyle(root).getPropertyValue('--ui-scale')) || 1;
|
||||
root.style.setProperty('--ui-scale', String(scale * 2));
|
||||
const after = parseFloat(getComputedStyle($row[0]).columnGap);
|
||||
root.style.removeProperty('--ui-scale');
|
||||
|
||||
expect(before, 'the gap exists at all').to.be.greaterThan(0);
|
||||
expect(after / before, 'and doubles with the scale').to.be.closeTo(2, 0.05);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
/* `--panel` is authored as a hex; computed backgrounds come back as rgb(). */
|
||||
const hexToRgb = (hex) => {
|
||||
const value = hex.replace('#', '');
|
||||
|
|
|
|||
|
|
@ -1306,6 +1306,36 @@ html[data-theme="amber"] .wrolpi-progress-fill {
|
|||
gap: 1rem;
|
||||
}
|
||||
|
||||
/*
|
||||
* The same idea along the other axis: a row of buttons, spaced by the row.
|
||||
*
|
||||
* Semantic's Button carried `margin: 0 .25em .25em 0`, so a page could list buttons as bare
|
||||
* siblings and they arrived both spaced and, when the row ran out of width, separated from the line
|
||||
* below. Nothing replaced that margin -- the archive page's six actions shared edges, and the
|
||||
* downloads table's two icon buttons did too.
|
||||
*
|
||||
* Two of the archive page's six had `marginTop: 0.5em` instead, which is where the "Update sits
|
||||
* lower than the rest" came from: a Button is `display: inline-flex; vertical-align: middle`, and
|
||||
* `vertical-align` aligns the MARGIN box, so half of any top margin shows up as the visible button
|
||||
* dropping. The margin was solving the wrapped-row case in the only axis a margin can, and paying
|
||||
* for it in the other one. A gap covers both at once, so no button needs a margin of its own; the
|
||||
* scans in Common.test.js are what keep them from growing back.
|
||||
*
|
||||
* 0.625rem matches `.wrolpi-modal-actions`, which is the same row in a modal footer.
|
||||
*/
|
||||
.wrolpi-button-row {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
/*
|
||||
* Centred, not stretched. Rows here are not all buttons -- the doc page puts its format
|
||||
* picker beside its actions, and the downloads table its error trigger -- and a taller item
|
||||
* makes the line taller than a button. `stretch` would leave the buttons hanging from the
|
||||
* top of it; centring keeps a row of unequal things reading as one line.
|
||||
*/
|
||||
align-items: center;
|
||||
gap: 0.625rem;
|
||||
}
|
||||
|
||||
/* ----------------------------------------------------------------- Panels */
|
||||
|
||||
.wrolpi-panel {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue