rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-26 19:01:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

ui: three button-layout defects, each with a different cause

Browse source 
Reported together as "button groups", but they turned out to be three unrelated
things.

The file browser's footer buttons touched while the Channel Edit page's had
gaps.  Channel Edit lays its buttons out with Mantine's `Group`, which is a flex
row with a gap; the footer is hand-rolled and was a plain block, so its
inline-block buttons sat flush -- JSX drops whitespace-only lines, so there was
not even a space character between them.  The footer is now the flex row every
other button group already is, wrapping because it carries eight controls.

The Settings config table's Import and Save buttons had off-centre icons while
Restore in the same row looked right.  Restore is an IconButton -- an ActionIcon,
which centres a lone glyph by construction.  Import and Save are icon-only
Buttons, and an icon on a Button goes into Mantine's `leftSection`, which carries
a `margin-inline-end` to hold it off the label.  With no label that margin is
pure offset: measured at 8px left of centre.  An icon with no label is now
rendered as the button's content instead, so it centres.  `Children.toArray`
decides that, because it drops nulls and the file browser passes
`{label('Delete')}` -- null whenever the bar is too narrow for words, which is
exactly where the offset showed most.

The search field's clear button sat inside the field rather than attached to it:
the control is a bordered box with `padding: 0 8px`, leaving the button 9px short
of the edge and 2px short of the height.  Semantic welded it to the field --
measured on the QA Pi at zero gap and full height -- so it now stretches and
negative margins cancel the padding and the flex gap.  Negative margins rather
than dropping the control's padding, because that padding is what holds the
search icon and the text off the border; only this one child should escape it.

The `html` prefix on that rule is load-bearing: Mantine sets ActionIcon's height
from `--ai-size` on its own single class, and at equal specificity the winner is
whichever stylesheet loaded last.

Fourteen new cases.  One correction worth recording: the height assertion first
compared against the control's border box and failed on correct behaviour -- the
button fills the CONTENT box, and the 1px above and below it is the field's own
border, which it should sit inside rather than paint over.  Each fix planted
separately; the icon change reddens three, the footer one, the clear button one.
Every case is paired with an inverse -- a labelled button must keep its icon on
the left, and the field must still be padded around its other children.

1128 jest / 63 suites, 226 in ui-layout.cy.js, clean npm run build.  Confirmed
live: footer gaps 8px on one row, config glyphs at 0.0px from centre, clear
button 1px from the field edge at the field's full inner height.
This commit is contained in:
Roland 2026-08-02 17:13:28 -06:00 • committed by lrnselfreliance
parent 5778808b8e
commit feaca59ce5
4 changed files with 262 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

13
app/src/App.css
View file

@ -597,6 +597,19 @@ table.ratio-table td.min-width {
background-color: var(--panel);
border-top: 1px solid var(--border);
z-index: 100;
/*
* A row with gaps, like every other group of buttons in the app.
*
* This was a plain block, so the buttons -- inline-block elements with no whitespace
* between them, because JSX drops whitespace-only lines -- sat flush against each other
* in one unbroken slab. Everywhere else buttons are laid out with Mantine's `Group`,
* which is a flex row with a gap; the file browser's footer is hand-rolled and never got
* one. It wraps because the bar carries eight controls and a phone cannot hold them.
*/
display: flex;
align-items: center;
flex-wrap: wrap;
gap: 0.5em;
}
/* Drag selection visual feedback for FileBrowser */

30
app/src/components/ui/Button.tsx
View file

@ -65,9 +65,29 @@ export const resolveSize = <T, >(size: T): T =>
typeof size === 'string' ? ((semanticSizes[size] ?? size) as T) : size;
export const Button = forwardRef<HTMLButtonElement, ButtonProps>((
{role, icon, iconAfter, className, ...props}, ref
{role, icon, iconAfter, className, children, ...props}, ref
) => {
const fromRole = role ? roleProps[role] : undefined;
/*
* An icon with no label is the button's CONTENT, not a leading icon.
*
* Mantine gives `leftSection` a `margin-inline-end` to hold it off the label. With no
* label that margin is pure offset: the glyph sits about 8px left of the button's centre,
* which is what the import and save buttons on the Settings config table looked like next
* to a correctly centred Restore -- Restore is an IconButton, which centres a lone glyph
* by construction.
*
* `React.Children.toArray` drops null, undefined and booleans, which matters because the
* file browser's footer passes `{label('Delete')}` and that is null whenever the bar is
* too narrow for words. Those buttons are icon-only at exactly the widths where the
* offset is most visible.
*/
const labelled = React.Children.toArray(children).length > 0;
const soleIcon = !labelled && (icon || iconAfter) && !(icon && iconAfter)
? renderIcon(icon || iconAfter)
: undefined;
return <MButton
ref={ref}
/*
@ -81,11 +101,13 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>((
color={props.color ?? fromRole?.color}
variant={props.variant ?? fromRole?.variant}
className={[fromRole?.className, className].filter(Boolean).join(' ') || undefined}
leftSection={renderIcon(icon)}
rightSection={renderIcon(iconAfter)}
leftSection={soleIcon ? undefined : renderIcon(icon)}
rightSection={soleIcon ? undefined : renderIcon(iconAfter)}
{...props}
size={resolveSize(props.size)}
/>
>
{soleIcon ?? children}
</MButton>
});
Button.displayName = 'Button';

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

@ -3044,3 +3044,200 @@ describe('a disabled button keeps its own colour', () => {
});
});
});
describe('a button with an icon and no label centres the icon', () => {
/*
* Mantine's `leftSection` carries a `margin-inline-end` to hold the icon off the label.
* With no label that margin is pure offset, and the glyph sits about 8px left of the
* button's centre.
*
* The Settings config table is where it showed: Import and Save are icon-only Buttons and
* looked visibly off, while Restore in the same row looked right -- Restore is an
* IconButton, which centres a lone glyph by construction. Measured on the running app at
* -8px before the fix.
*/
it('puts the glyph in the middle when there is nothing else in the button', () => {
cy.mountUI(<Button icon='upload' aria-label='Import' data-testid='icon-only'/>);
cy.get('[data-testid="icon-only"]').should(($button) => {
const box = $button[0].getBoundingClientRect();
const glyph = $button[0].querySelector('svg').getBoundingClientRect();
const offset = (glyph.left + glyph.width / 2) - (box.left + box.width / 2);
expect(box.width, 'the button has width to be off-centre in')
.to.be.greaterThan(glyph.width + 8);
expect(offset, 'glyph centre vs button centre').to.be.closeTo(0, 1);
});
});
it('treats a null label as no label, which is how the file browser renders', () => {
/*
* The footer passes `{label('Delete')}`, and that is null whenever the bar is too
* narrow for words -- so those buttons are icon-only at exactly the widths where the
* offset is most visible. `React.Children.toArray` drops nulls; a plain truthiness
* check on `children` would not.
*/
cy.mountUI(<Button icon='trash' aria-label='Delete' data-testid='null-label'>
{null}
</Button>);
cy.get('[data-testid="null-label"]').should(($button) => {
const box = $button[0].getBoundingClientRect();
const glyph = $button[0].querySelector('svg').getBoundingClientRect();
expect((glyph.left + glyph.width / 2) - (box.left + box.width / 2),
'glyph centre vs button centre').to.be.closeTo(0, 1);
});
});
it('is a real constraint: a labelled button keeps its icon on the left', () => {
// The inverse. Centring the glyph must not apply when there IS a label, or every
// labelled button in the app loses its leading icon's position.
cy.mountUI(<Button icon='trash' data-testid='labelled'>Delete</Button>);
cy.get('[data-testid="labelled"]').should(($button) => {
const box = $button[0].getBoundingClientRect();
const glyph = $button[0].querySelector('svg').getBoundingClientRect();
expect((glyph.left + glyph.width / 2) - (box.left + box.width / 2),
'the icon leads the label').to.be.lessThan(-4);
expect($button[0].textContent, 'and the label is still there').to.contain('Delete');
});
});
it('centres a trailing-only icon too', () => {
// `rightSection` has the mirror-image margin.
cy.mountUI(<Button iconAfter='upload' aria-label='Send' data-testid='after-only'/>);
cy.get('[data-testid="after-only"]').should(($button) => {
const box = $button[0].getBoundingClientRect();
const glyph = $button[0].querySelector('svg').getBoundingClientRect();
expect((glyph.left + glyph.width / 2) - (box.left + box.width / 2))
.to.be.closeTo(0, 1);
});
});
});
describe('the file browser footer is a row of buttons, not a slab', () => {
/*
* The footer was a plain block, so its buttons -- inline-block elements with no whitespace
* between them, since JSX drops whitespace-only lines -- sat flush against each other in
* one unbroken bar. Every other group of buttons in the app is a Mantine `Group`, which
* is a flex row with a gap; this one is hand-rolled and never got one.
*
* The class carries the layout, so the class is what is mounted. FileBrowser itself
* needs the file API, drag selection and a router, none of which decides the spacing.
*/
it('leaves a gap between adjacent buttons', () => {
cy.mountUI(<div className='sticky-footer'>
<Button color='red' data-testid='a'>Delete</Button>
<Button color='yellow' data-testid='b'>Rename</Button>
<Button color='teal' data-testid='c'>Move</Button>
</div>);
cy.get('.sticky-footer').should(($footer) => {
const boxes = ['a', 'b', 'c']
.map(id => $footer[0].querySelector(`[data-testid="${id}"]`).getBoundingClientRect());
expect(boxes[1].left - boxes[0].right, 'gap between the first two')
.to.be.greaterThan(2);
expect(boxes[2].left - boxes[1].right, 'gap between the next two')
.to.be.greaterThan(2);
});
});
it('keeps them on one line and vertically aligned', () => {
// A flex row is only right if it behaves like one: the buttons share a baseline and
// do not stack while there is room for them.
cy.mountUI(<div className='sticky-footer'>
<Button data-testid='a'>Delete</Button>
<Button data-testid='b'>Rename</Button>
</div>);
cy.get('.sticky-footer').should(($footer) => {
const a = $footer[0].querySelector('[data-testid="a"]').getBoundingClientRect();
const b = $footer[0].querySelector('[data-testid="b"]').getBoundingClientRect();
expect(a.top, 'same row').to.be.closeTo(b.top, 1);
});
});
});
describe('the search field\'s clear button is attached to it', () => {
/*
* The control is a bordered box with `padding: 0 8px`, so the clear button sat 9px short
* of the right edge and 2px short of the full height -- a glyph dropped inside the input
* rather than a control of its own.
*
* Semantic rendered it as an action button welded to the field. Measured on the QA Pi,
* which still runs Semantic: zero gap from the input, zero to the outer edge, and the
* full height of the field.
*/
const mountField = () => cy.mountUI(<div style={{width: 420}}>
<SearchBox value='wind turbine' clearable onChange={() => {}}/>
</div>);
it('meets the right-hand edge of the field', () => {
mountField();
cy.get('.wrolpi-searchbox-clear').should(($clear) => {
const control = Cypress.$('.wrolpi-searchbox-control')[0].getBoundingClientRect();
const clear = $clear[0].getBoundingClientRect();
expect(control.right - clear.right, 'gap to the field edge').to.be.closeTo(0, 1.5);
});
});
it('fills the height of the field', () => {
/*
* Half the reason it read as "inside": a 36px control floating in a 38px box.
*
* Measured against the control's CONTENT box, not its border box. The field's own
* 1px border is the only thing left above and below the button, and the button
* should sit inside it rather than paint over it -- `clientHeight` is that box, and
* comparing against `getBoundingClientRect().height` would be demanding the button
* cover the field's border, which is a different and wrong design.
*/
mountField();
cy.get('.wrolpi-searchbox-clear').should(($clear) => {
const control = Cypress.$('.wrolpi-searchbox-control')[0];
const border = parseFloat(getComputedStyle(control).borderTopWidth);
expect(border, 'the field has a border to sit inside').to.be.greaterThan(0);
expect($clear[0].getBoundingClientRect().height, 'clear button height')
.to.be.closeTo(control.clientHeight, 1);
});
});
it('is divided from the text rather than merged into it', () => {
// Attached is not the same as indistinguishable: without a rule between them the
// glyph reads as part of the input again, just further right.
mountField();
cy.get('.wrolpi-searchbox-clear').should(($clear) => {
const style = getComputedStyle($clear[0]);
expect(parseFloat(style.borderLeftWidth), 'a divider on its left')
.to.be.greaterThan(0);
expect(style.borderLeftStyle).to.not.equal('none');
});
});
it('is a real constraint: the field itself is still padded', () => {
/*
* The negative margins are aimed at one child. If the control's padding had simply
* been removed instead, the search icon and the text would sit against the border
* too -- so this checks the thing that must NOT have moved.
*/
mountField();
cy.get('.wrolpi-searchbox-icon').should(($icon) => {
const control = Cypress.$('.wrolpi-searchbox-control')[0].getBoundingClientRect();
expect($icon[0].getBoundingClientRect().left - control.left, 'icon is inset')
.to.be.greaterThan(4);
});
});
});

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

@ -348,6 +348,32 @@ html[data-theme="amber"] .wrolpi-searchbox-control:focus-within {
flex: none;
}
/*
* The clear button is attached to the field, not floating inside it.
*
* The control is a bordered box with `padding: 0 8px`, so the button sat 9px short of the
* right edge and 2px short of the full height -- reading as a glyph dropped inside the input
* rather than as a control of its own. Semantic rendered this as an action button welded to
* the field: measured on the QA Pi, zero gap to the input, zero to the outer edge, and the
* full 44px height.
*
* Negative margins rather than removing the control's padding: the padding is what holds the
* search icon and the text off the border, and only this one child should escape it. The
* left margin cancels the flex gap as well, so the divider lands against the text the way
* the attached button did.
*
* `html` because Mantine sets ActionIcon's height from `--ai-size` on its own single class;
* at equal specificity the winner is whichever stylesheet loaded last, which is not a thing
* to rely on.
*/
html .wrolpi-searchbox-clear {
align-self: stretch;
height: auto;
margin-left: -8px;
margin-right: -8px;
border-left: 1px solid var(--border);
}
/*
* The suggestion list overlays the page rather than pushing it down — a search
* that reflows the results underneath it is unusable while typing.