mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
ui: the corner icons opt out of the bar's foreground, so make them stop
Measuring the foreground against the bar is worth nothing if the icons ignore
it, and two of them did -- the search glyph and the hamburger came out blue on
every bar in every theme while the rest of the bar was correct.
Neither inherits. An anchor takes `a {color: var(--blue)}` from tokens.css,
which outranks an inherited value however the bar sets it; the search glyph is
an anchor. A Mantine ActionIcon paints `--ai-color`, its own themed blue, and
Mantine writes that variable INLINE on the element, so a stylesheet declaring it
loses every time -- the painted property has to be set instead. The status
icons that link to /admin are anchors too, and only looked right because they
carry an inline colour of their own, which still wins and is meant to.
The test that claimed to cover this read `querySelector('...svg')`: one element,
the first, which was a plain inheriting Icon. It asserted every icon and
measured the only one that was never broken. It now reads querySelectorAll,
asserts the count, and checks all four -- and reddens in all four themes with
the rule removed, naming --blue as the value it found.
The gallery sample was wrong in the same direction: it used an IconButton for
the search icon, so it showed the ActionIcon fault twice and hid the anchor
fault entirely. It now carries all three shapes the real bar puts in that
corner.
1115 jest / 62 suites, 177 in ui-layout.cy.js, clean npm run build.
This commit is contained in:
parent
24fcf978d9
commit
a9d12952a8
3 changed files with 72 additions and 10 deletions
|
|
@ -42,6 +42,35 @@
|
|||
background: rgba(0, 0, 0, 0.12);
|
||||
}
|
||||
|
||||
/*
|
||||
* Everything in the bar takes the bar's foreground.
|
||||
*
|
||||
* `color: inherit` on the links alone was not enough, because two of the things in the
|
||||
* corner do not inherit at all and both came out blue on every bar in every theme:
|
||||
*
|
||||
* - An anchor takes `a {color: var(--blue)}` from tokens.css, which outranks an inherited
|
||||
* value however the bar sets it. The search glyph is an anchor, and so is every status
|
||||
* icon that links to /admin -- those only looked right because they carry an inline
|
||||
* colour of their own, which still wins here and is meant to.
|
||||
* - A Mantine ActionIcon paints `--ai-color`, its own themed blue. The hamburger and the
|
||||
* theme toggle are ActionIcons. The variable is no use as a hook: Mantine writes it
|
||||
* INLINE on the element, so a stylesheet declaring it loses every time (the same trap
|
||||
* as `--group-justify` on Pagination). The painted property has to be set instead, and
|
||||
* `html` lifts this over Mantine's own single-class rule.
|
||||
*
|
||||
* Measuring the foreground against the bar is worth nothing if the icons opt out of it.
|
||||
*/
|
||||
.wrolpi-navbar a,
|
||||
html .wrolpi-navbar .mantine-ActionIcon-root {
|
||||
color: inherit;
|
||||
}
|
||||
|
||||
/* The bar's own hover, rather than a blue tint from Mantine's palette. */
|
||||
html .wrolpi-navbar .mantine-ActionIcon-root:hover {
|
||||
background: rgba(0, 0, 0, 0.12);
|
||||
color: inherit;
|
||||
}
|
||||
|
||||
.channel-edit {
|
||||
border-radius: 3px;
|
||||
padding: 0.75em;
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ import {
|
|||
import {semanticColorNames} from '../themes/mantine';
|
||||
import {contrastingColor} from './Common';
|
||||
import {navBarStyle, navColorNames, useNavColors} from '../themes/navColors';
|
||||
import {IconMenu2, IconSearch} from '@tabler/icons-react';
|
||||
import {IconMenu2} from '@tabler/icons-react';
|
||||
|
||||
/*
|
||||
* A gallery of every component in the library, in the current theme.
|
||||
|
|
@ -106,10 +106,23 @@ export const NavBarSample = ({color}) => {
|
|||
<span className='wrolpi-navbar-link'>Videos</span>
|
||||
<span className='wrolpi-navbar-link'>Map</span>
|
||||
<div className='wrolpi-navbar-right' style={{gap: '0.5em', paddingRight: '0.5em'}}>
|
||||
{/*
|
||||
* All three shapes the real bar puts in this corner, because they take
|
||||
* their colour by three different routes and only one of them was ever
|
||||
* right. A bare Icon inherits. An anchor does NOT -- `a {color:
|
||||
* var(--blue)}` in tokens.css outranks the inherited value, which is what
|
||||
* made the search icon blue on every bar in every theme. And a Mantine
|
||||
* ActionIcon paints its own themed colour, which is what made the
|
||||
* hamburger blue. The first version of this sample used an IconButton for
|
||||
* the search icon too, so it showed the second fault twice and hid the
|
||||
* first entirely.
|
||||
*/}
|
||||
<Icon name='warning sign' size='large' label='Drive health warning'/>
|
||||
<Icon name='hdd' size='large' label='Drive temperature warning'/>
|
||||
<Icon name='plug' size='large' label='API is not responding'/>
|
||||
<IconButton icon={IconSearch} label='Search' variant='subtle'/>
|
||||
{/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
|
||||
<a className='item' role='button' tabIndex={0} aria-label='Search'>
|
||||
<Icon name='search' size='large'/>
|
||||
</a>
|
||||
<IconButton icon={IconMenu2} label='Menu' variant='subtle'/>
|
||||
</div>
|
||||
</nav>
|
||||
|
|
|
|||
|
|
@ -2134,18 +2134,38 @@ describe('the navigation bar is legible on every colour a user can pick', () =>
|
|||
});
|
||||
});
|
||||
|
||||
it(`gives its icons the same foreground as its links in ${theme}`, () => {
|
||||
// The actual complaint was about the icons, not the words. They take `color`
|
||||
// by inheritance, which is a thing a stylesheet can quietly break.
|
||||
it(`gives EVERY icon in the corner the same foreground as its links in ${theme}`, () => {
|
||||
/*
|
||||
* The actual complaint was about the icons, not the words -- and about two of
|
||||
* them specifically, the search glyph and the hamburger, which came out blue on
|
||||
* every bar in every theme while the rest of the bar was correct.
|
||||
*
|
||||
* They inherit nothing. An anchor takes `a {color: var(--blue)}` from
|
||||
* tokens.css, which outranks the bar's inherited value; a Mantine ActionIcon
|
||||
* paints `--ai-color`, its own themed blue, and Mantine writes that variable
|
||||
* INLINE so no stylesheet variable can reach it. Three routes to a colour, of
|
||||
* which one worked.
|
||||
*
|
||||
* `querySelectorAll`, and the count asserted, because the version of this test
|
||||
* that shipped in the first pass read `querySelector('...svg')` -- one element,
|
||||
* the first, which was a plain inheriting Icon. It claimed every icon and
|
||||
* measured the only one that was never broken.
|
||||
*/
|
||||
barsFor(theme);
|
||||
|
||||
navColorNames.forEach((color) => {
|
||||
cy.get(`[data-nav-sample="${color}"]`).should(($sample) => {
|
||||
const link = $sample[0].querySelector('.wrolpi-navbar-link');
|
||||
const icon = $sample[0].querySelector('.wrolpi-navbar-right svg');
|
||||
expect(icon, `${theme}/${color} has an icon to measure`).to.not.be.null;
|
||||
expect(getComputedStyle(icon).color, `${theme}/${color} icon colour`)
|
||||
.to.equal(getComputedStyle(link).color);
|
||||
const expected = getComputedStyle(link).color;
|
||||
const icons = [...$sample[0].querySelectorAll('.wrolpi-navbar-right svg')];
|
||||
|
||||
// Two bare Icons, one inside an anchor, one inside an ActionIcon. If
|
||||
// this drops, the loop below is covering less than it says.
|
||||
expect(icons.length, `${theme}/${color} icons found`).to.equal(4);
|
||||
icons.forEach((icon, index) => {
|
||||
expect(getComputedStyle(icon).color, `${theme}/${color} icon ${index}`)
|
||||
.to.equal(expected);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue