From 149b40acbd81ea663852a670086a5c94da0d9161 Mon Sep 17 00:00:00 2001 From: Roland Date: Sun, 2 Aug 2026 14:12:39 -0600 Subject: [PATCH] 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. --- app/src/App.css | 29 +++++++++++++++++++++++ app/src/components/ThemeSamplePage.js | 19 ++++++++++++--- app/src/components/ui/ui-layout.cy.js | 34 +++++++++++++++++++++------ 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/app/src/App.css b/app/src/App.css index be8f7950..dde63798 100644 --- a/app/src/App.css +++ b/app/src/App.css @@ -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; diff --git a/app/src/components/ThemeSamplePage.js b/app/src/components/ThemeSamplePage.js index 43918a79..feed09d6 100644 --- a/app/src/components/ThemeSamplePage.js +++ b/app/src/components/ThemeSamplePage.js @@ -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}) => { Videos Map
+ {/* + * 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. + */} - - + {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */} + + +
diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js index a917fdf1..996a8ec5 100644 --- a/app/src/components/ui/ui-layout.cy.js +++ b/app/src/components/ui/ui-layout.cy.js @@ -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); + }); }); }); });