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); + }); }); }); });