diff --git a/app/src/components/ThemeSamplePage.js b/app/src/components/ThemeSamplePage.js index e022e63c..0ccb38e5 100644 --- a/app/src/components/ThemeSamplePage.js +++ b/app/src/components/ThemeSamplePage.js @@ -244,10 +244,25 @@ export function ThemeSamplePage() { })}>Show a toast +
+ + {['xs', 'sm', 'md', 'lg', 'xl'].map(size =>
+ + +
)} +
+

In night mode Delete drops its fill for a dashed red outline; dashed means destructive or failed, and nothing else uses it.

+

+ 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 + to its scale for buttons — sm is 22px against a button's 36px + — and the two components start from different defaults. +

diff --git a/app/src/components/ui/Button.tsx b/app/src/components/ui/Button.tsx index de512c4a..4e3e3e43 100644 --- a/app/src/components/ui/Button.tsx +++ b/app/src/components/ui/Button.tsx @@ -101,9 +101,16 @@ export interface IconButtonProps extends Omit { component?: any; } -/** A square, icon-only button. */ +/** + * A square, icon-only button, exactly as tall as a labelled `Button` beside it. + * + * The height needs BOTH halves of the fix. ui.css aligns ActionIcon's size scale with + * Button's, and this default aligns the starting point: Mantine defaults ActionIcon to `md` + * where it defaults Button to `sm`, so leaving it alone would trade a 28-vs-36 mismatch for + * a 42-vs-36 one. + */ export const IconButton = forwardRef(( - {role, icon, label, className, ...props}, ref + {role, icon, label, className, size = 'sm', ...props}, ref ) => { const fromRole = role ? roleProps[role] : undefined; return (( variant={props.variant ?? fromRole?.variant ?? 'default'} className={[fromRole?.className, className].filter(Boolean).join(' ') || undefined} {...props} - size={resolveSize(props.size)} + size={resolveSize(size)} > {renderIcon(icon)} diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js index cbbde7d9..143ecddd 100644 --- a/app/src/components/ui/ui-layout.cy.js +++ b/app/src/components/ui/ui-layout.cy.js @@ -1,6 +1,6 @@ import React from 'react'; import { - ActionInput, Button, Card, Header, Icon, IconStack, Loading, MultiSelect, Panel, PathInput, + ActionInput, Button, Card, Header, Icon, IconButton, IconStack, Loading, MultiSelect, Panel, PathInput, Message, Placeholder, Progress, Statistic, StatisticGroup, Status, TabBar, Table, tabClassName, TextInput, } from './index'; @@ -1863,3 +1863,56 @@ describe('a card keeps its meta with the title and its actions at the foot', () }); }); }); + +describe('a button and an icon button are the same height', () => { + /* + * Mantine gives ActionIcon a size scale of its own that is nothing like Button's -- + * `sm` is 22px against a button's 36px -- and defaults it to `md` where Button defaults + * to `sm`. So the Details/Open pair on an archive card came out 36px beside 28px, and + * every other toolbar that mixes the two was ragged in the same way. + * + * Mantine already ships the aligned scale as `--ai-size-input-*`, for an ActionIcon + * sitting beside an Input; those five values are identical to `--button-height-*`. The + * fix points one at the other, so this holds for a size we have never used as well as + * the ones we have. + * + * Heights, so it belongs here: jsdom returns zero for both and the comparison would + * pass on any pair of numbers at all. + */ + const sizes = [undefined, 'xs', 'sm', 'md', 'lg', 'xl']; + + sizes.forEach((size) => { + it(`matches at size ${size || '(default)'}`, () => { + cy.mountUI(
+ + +
); + + cy.get('button').should(($buttons) => { + expect($buttons.length).to.equal(2); + const [button, iconButton] = [...$buttons].map(el => el.getBoundingClientRect()); + // A zero-height pair would satisfy the equality below without meaning anything. + expect(button.height, 'the button has a height at all').to.be.greaterThan(10); + expect(iconButton.height, `icon button at size ${size || '(default)'}`) + .to.be.closeTo(button.height, 0.5); + // Square, which is the whole point of an icon button. + expect(iconButton.width, 'the icon button stays square') + .to.be.closeTo(iconButton.height, 0.5); + }); + }); + }); + + it('grows the icon button rather than shrinking the button', () => { + /* + * Both could be made equal by cutting the labelled button down to 28px, which would + * be the wrong fix -- a 28px target is below the 44px-ish guidance already stretched + * by having buttons this small, and every OTHER button on the page would still be 36. + * Pinned so a future "make them match" cannot resolve the other way. + */ + cy.mountUI(); + + cy.get('button').should(($el) => + expect($el[0].getBoundingClientRect().height, 'default icon button height') + .to.be.at.least(34)); + }); +}); diff --git a/app/src/components/ui/ui.css b/app/src/components/ui/ui.css index b9e1e93e..191dd82c 100644 --- a/app/src/components/ui/ui.css +++ b/app/src/components/ui/ui.css @@ -102,6 +102,28 @@ html[data-theme="night"] .wrolpi-button-danger:hover:not(:disabled) { /* The icon inside inherits currentColor, so it follows without another rule. */ +/* + * An icon button is the same height as a labelled one, so a group of them lines up. + * + * Mantine gives ActionIcon a size scale unrelated to Button's -- `sm` is 22px against a + * button's 36px -- which left the Details/Open pair on every archive card ragged, and any + * other toolbar that mixes the two. It already ships the aligned scale as + * `--ai-size-input-*`, for an ActionIcon standing beside an Input; those five values are + * identical to `--button-height-*`. Pointing one at the other fixes every size at once, + * including ones no call site uses yet. + * + * The `html` prefix is load-bearing. Mantine declares this scale on ActionIcon's own + * hashed class, which has the same specificity as `.mantine-ActionIcon-root` and would win + * on source order. + */ +html .mantine-ActionIcon-root { + --ai-size-xs: var(--ai-size-input-xs); + --ai-size-sm: var(--ai-size-input-sm); + --ai-size-md: var(--ai-size-input-md); + --ai-size-lg: var(--ai-size-input-lg); + --ai-size-xl: var(--ai-size-input-xl); +} + /* ----------------------------------------------------------- Theme picker */ .wrolpi-theme-picker {