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: a disabled button keeps its own colour

Browse source 
The file browser's footer is eight buttons, six of them disabled until something
is selected, so the toolbar a user meets on arriving at /files was an
undifferentiated grey row: Delete, Rename, Move, Ignore and Tag rendered as five
identical pixels.  Colour is how those are told apart at a glance.

Mantine repaints a disabled button flat grey -- background, text and border all
replaced with `--mantine-color-disabled`.  Semantic kept the colour and dropped
the whole control to `opacity: 0.45`; measured on the QA Pi, a disabled Delete
is still red at 0.45, Rename still yellow, Tag still violet.  That is the better
signal: it says "not available" without also saying "no longer the delete
button".

Mantine leaves the variables alone and overrides only the painted properties, so
the colour the call site asked for is still sitting in `--button-bg` and can
simply be reinstated.  Applies to every disabled Button and ActionIcon, not only
the file browser's -- it is one convention, and having two would be worse than
either.

A pre-existing test earned its keep here: the IconStack corner disc was pinned
to `--mantine-color-disabled` so it would follow a disabled button's grey, and
it reddened the moment the button stopped being grey.  That override is gone;
`--button-bg` is now correct in both states, so the general rule covers them.

Ten new cases.  Fill equality is checked against each button's own enabled twin
in all four themes, and separately across the five colours at once -- comparing
each against its twin would still pass if every pair were grey, which is the
actual bug.  Opacity is asserted from both sides, because keeping the colour
must not cost the affordance.  Removing the rule reddens eleven.

1128 jest / 63 suites, 209 in ui-layout.cy.js, clean npm run build.
This commit is contained in:
Roland 2026-08-02 16:38:41 -06:00
parent 4d6a6d467d
commit bb65cd0c28
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
2 changed files with 157 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -1530,10 +1530,13 @@ describe('an icon stack carries the surface it was placed on', () => {
it('follows a filled button when it is disabled', () => {
/*
* The file browser's New Folder button is disabled in WROL mode, and Mantine swaps a
* disabled button's fill for --mantine-color-disabled. A disc pinned to `--blue`
* then becomes a blue chip on a grey surface -- the same defect as the original
* hole, inverted. The disc has to follow the button, not the button's enabled fill.
* The file browser's New Folder button is disabled in WROL mode. The disc has to
* follow the button into that state, whatever the state happens to look like -- and
* it has now looked like two different things. Mantine used to repaint a disabled
* button with `--mantine-color-disabled`, so a disc pinned to `--blue` became a blue
* chip on a grey surface; disabled buttons keep their own fill now, so the disc must
* be blue again. This caught the disc still pinned to the old grey the moment that
* changed, which is exactly what it is for.
*/
cy.mountUI(
<Button color='blue' disabled>
@ -2837,3 +2840,109 @@ describe('a toast does not cover the navigation bar', () => {
});
});
});
describe('a disabled button keeps its own colour', () => {
/*
* Mantine repaints a disabled button flat grey -- background, text and border all
* replaced with `--mantine-color-disabled`. Semantic kept the colour and dropped the
* whole control to `opacity: 0.45`, so a disabled Delete was still visibly the red one.
*
* The file browser is where this bites. Its footer is eight buttons, six of which are
* disabled until something is selected, so the toolbar a user meets on arriving at /files
* is an undifferentiated grey row -- Delete, Rename, Move, Ignore and Tag all identical.
* Colour is how those are told apart at a glance, and disabling them threw it away.
*
* Opacity is the better signal anyway: it says "not available" without also saying "no
* longer the delete button".
*
* Mantine leaves `--button-bg` intact on a disabled button -- it overrides the painted
* `background` rather than the variable -- so the colour the call site asked for is still
* there to be read back.
*/
const pairs = [
{color: 'red', label: 'Delete'},
{color: 'yellow', label: 'Rename'},
{color: 'teal', label: 'Move'},
{color: 'violet', label: 'Tag'},
{color: 'grey', label: 'Ignore'},
];
const mountPairs = (theme) => cy.mountUI(<div>
{pairs.map(({color, label}) => <span key={color}>
<Button color={color} data-testid={`on-${color}`}>{label}</Button>
<Button color={color} disabled data-testid={`off-${color}`}>{label}</Button>
</span>)}
</div>, {theme});
themeNames.forEach((theme) => {
it(`keeps every colour recognisable while disabled in ${theme}`, () => {
mountPairs(theme);
cy.get('[data-testid^="off-"]').should(($disabled) => {
expect($disabled, 'a disabled button per colour').to.have.length(pairs.length);
pairs.forEach(({color}) => {
const on = Cypress.$(`[data-testid="on-${color}"]`)[0];
const off = Cypress.$(`[data-testid="off-${color}"]`)[0];
expect(getComputedStyle(off).backgroundColor, `${theme}/${color} fill`)
.to.equal(getComputedStyle(on).backgroundColor);
});
});
});
it(`still marks them as unavailable in ${theme}`, () => {
// Keeping the colour must not cost the affordance: a disabled button that looks
// identical to an enabled one is a worse bug than a grey one.
mountPairs(theme);
cy.get('[data-testid="off-red"]').should(($off) => {
const disabled = parseFloat(getComputedStyle($off[0]).opacity);
const enabled = parseFloat(
getComputedStyle(Cypress.$('[data-testid="on-red"]')[0]).opacity);
expect(enabled, `${theme} enabled is solid`).to.equal(1);
expect(disabled, `${theme} disabled is faded`).to.be.lessThan(0.7);
expect(disabled, `${theme} disabled is still legible`).to.be.greaterThan(0.25);
});
});
});
it('tells two disabled buttons of different colours apart', () => {
/*
* The claim stated as the user meets it, and the inverse of the bug: five grey blobs
* were five EQUAL greys. Comparing each against its enabled twin above would still
* pass if every pair were grey, so this compares them against each other.
*/
mountPairs('light');
cy.get('[data-testid^="off-"]').should(($disabled) => {
const fills = [...$disabled].map(button => getComputedStyle(button).backgroundColor);
expect(new Set(fills).size, `five colours, distinct fills: ${fills.join(' ')}`)
.to.equal(pairs.length);
});
});
it('does the same for icon-only buttons', () => {
// The file browser's row actions are IconButtons, and they disable the same way.
cy.mountUI(<div>
{/* Filled, so there is a colour to lose. IconButton defaults to Mantine's
`default` variant, which is a white face, and comparing white with white
would prove nothing. */}
<IconButton icon='trash' label='Delete' color='red' variant='filled'
data-testid='icon-on'/>
<IconButton icon='trash' label='Delete' color='red' variant='filled' disabled
data-testid='icon-off'/>
</div>);
cy.get('[data-testid="icon-off"]').should(($off) => {
const on = Cypress.$('[data-testid="icon-on"]')[0];
expect(getComputedStyle($off[0]).backgroundColor, 'icon button keeps its fill')
.to.equal(getComputedStyle(on).backgroundColor);
expect(parseFloat(getComputedStyle($off[0]).opacity), 'and is faded')
.to.be.lessThan(0.7);
});
});
});

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

@ -54,7 +54,7 @@
/*
* A stack inside a button is the one case the call site should not have to describe,
* because the surface changes underneath it: Mantine paints a filled button from
* `--button-bg` but a disabled one from `--mantine-color-disabled`, whatever its variant.
* `--button-bg`, and the disc has to follow it into whatever state the button is in.
* The file browser's New Folder button is disabled in WROL mode, so a disc pinned to the
* enabled fill would be a blue chip on a grey button -- the original hole, inverted.
*
@ -70,10 +70,13 @@
background: var(--button-bg, var(--icon-stack-bg, var(--bg)));
}
.mantine-Button-root:disabled .wrolpi-icon-stack-corner,
.mantine-Button-root[data-disabled] .wrolpi-icon-stack-corner {
background: var(--mantine-color-disabled);
}
/*
* There is deliberately no `:disabled` override here any more. There used to be one pinning
* the disc to `--mantine-color-disabled`, because Mantine repainted a disabled button grey
* and a disc left on `--blue` became a blue chip on a grey surface. Disabled buttons now
* keep their own fill (see the disabled-button rule further down), so `--button-bg` is
* correct in both states and the rule above covers them.
*/
/* ---------------------------------------------------------------- Buttons */
/*
@ -1455,3 +1458,39 @@ html[data-theme="amber"] .wrolpi-th-sort:focus-visible {
.wrolpi-table-scroll {
overflow-x: auto;
}
/*
* A disabled button keeps its own colour.
*
* Mantine repaints one flat grey -- background, text and border all replaced with
* `--mantine-color-disabled`. Semantic kept the colour and dropped the whole control to
* `opacity: 0.45`, and that is the better signal: it says "not available" without also
* saying "no longer the delete button".
*
* The file browser is where the difference shows. Its footer is eight buttons, six of them
* disabled until something is selected, so the toolbar a user meets on arriving at /files was
* an undifferentiated grey row -- Delete, Rename, Move, Ignore and Tag all the same pixel.
* Colour is how those are told apart at a glance.
*
* Mantine leaves the variables alone and overrides only the painted properties, so the colour
* the call site asked for is still in `--button-bg` and can simply be reinstated. `--button-bd`
* is a whole border shorthand, not a colour, so it is set with `border`.
*
* The `html` prefix beats Mantine's own `.m_hash:disabled` at (0,2,0) without needing
* `!important`.
*/
html .mantine-Button-root:disabled,
html .mantine-Button-root[data-disabled] {
background: var(--button-bg);
color: var(--button-color);
border: var(--button-bd);
opacity: 0.45;
}
html .mantine-ActionIcon-root:disabled,
html .mantine-ActionIcon-root[data-disabled] {
background: var(--ai-bg);
color: var(--ai-color);
border: var(--ai-bd);
opacity: 0.45;
}