From 65d78cae3218e41d2e02f1073de61b5af12d459e Mon Sep 17 00:00:00 2001 From: Roland Date: Wed, 29 Jul 2026 19:57:06 -0600 Subject: [PATCH] ui: night mode actually filters media, and fix three picker icons Addresses the automated review on #583; all three were real. The night filter only applied to elements tagged `.media`, and nothing in the app carried that class - so selecting Night filtered the interface but left every video, thumbnail, PDF and map full-colour, which is precisely the thing the mode exists to prevent. The Settings copy promised otherwise, so it was also wrong. It now targets the element types instead of an opt-in class, because night mode has to fail safe: a library of thousands of files reaches the screen through many components, and one that forgot to opt in would undo half an hour of dark adaptation. `.media` still works for wrappers that must be filtered as a unit, and `.night-unfiltered` opts out. The filter is restated on `:fullscreen`, which a filter on an ancestor stops reaching. Verified on the videos page: all 25 media elements filtered. Three of the five theme choices rendered a fallback glyph and logged errors, because the icon map was built by grepping literal name='...' strings and the navigation picker passes its icon through a variable. Adds sun, moon and lightbulb, plus a test asserting every theme choice resolves to a real icon. The pre-paint script wrapped the `matchMedia` fallback in the same try as the storage read, so a device with storage blocked painted light even when it prefers dark - recreating the flash the script exists to prevent. Co-Authored-By: Claude Fable 5 --- app/public/index.html | 19 +++++++++++-------- app/src/components/ui/Icon.tsx | 6 ++++++ app/src/components/ui/ui.test.js | 10 ++++++++++ app/src/themes/tokens.css | 23 +++++++++++++++++++---- 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/app/public/index.html b/app/public/index.html index 89d4a7de..3f327e57 100644 --- a/app/public/index.html +++ b/app/public/index.html @@ -37,16 +37,19 @@ */ (function () { var theme = 'light'; + var saved = null; + // Only the storage read can throw (blocked cookies/storage). Keep the try + // around just that, so a device that prefers dark still gets dark. try { - var saved = localStorage.getItem('color-scheme'); - if (saved === 'light' || saved === 'dark' || saved === 'night' || saved === 'amber') { - theme = saved; - } else if (window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches) { - // `system`, unset, or written by an older version. - theme = 'dark'; - } + saved = localStorage.getItem('color-scheme'); } catch (e) { - // Storage blocked; fall back to light. + saved = null; + } + if (saved === 'light' || saved === 'dark' || saved === 'night' || saved === 'amber') { + theme = saved; + } else if (window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches) { + // `system`, unset, or written by an older version. + theme = 'dark'; } document.documentElement.setAttribute('data-theme', theme); })(); diff --git a/app/src/components/ui/Icon.tsx b/app/src/components/ui/Icon.tsx index 5dd4c9a9..dffd2c29 100644 --- a/app/src/components/ui/Icon.tsx +++ b/app/src/components/ui/Icon.tsx @@ -81,7 +81,11 @@ const semanticNames: Record = { 'heartbeat': 'IconHeartRateMonitor', 'history': 'IconHistory', 'image': 'IconPhoto', + 'lightbulb': 'IconBulb', + 'lightbulb outline': 'IconBulb', 'lightning': 'IconBolt', + 'moon': 'IconMoon', + 'moon outline': 'IconMoon', 'linkify': 'IconLink', 'list': 'IconList', 'lock': 'IconLock', @@ -108,6 +112,8 @@ const semanticNames: Record = { 'star': 'IconStarFilled', 'star outline': 'IconStar', 'stop': 'IconPlayerStop', + 'sun': 'IconSun', + 'sun outline': 'IconSun', 'sync': 'IconRefresh', 'tachometer alternate': 'IconGauge', 'tag': 'IconTag', diff --git a/app/src/components/ui/ui.test.js b/app/src/components/ui/ui.test.js index 1d1c1aa2..f84d91c4 100644 --- a/app/src/components/ui/ui.test.js +++ b/app/src/components/ui/ui.test.js @@ -5,6 +5,7 @@ import {MantineProvider} from '@mantine/core'; import {IconMoodSmile} from '@tabler/icons-react'; import {ThemeContext} from '../../contexts/contexts'; import {cssVariablesResolver, mantineTheme, semanticColorNames} from '../../themes/mantine'; +import {themeChoices} from '../../themes/names'; import { Button, Confirm, @@ -61,6 +62,15 @@ describe('Icon', () => { expect(labelled).not.toHaveAttribute('aria-hidden'); }); + it('maps every icon the theme choices ask for', () => { + // Regression: the map was built by grepping literal name='...' strings, so icons + // passed through a variable (the nav bar's picker) were missed and three of the + // five theme choices rendered a fallback glyph. + const unmapped = themeChoices.filter(choice => !resolveIconName(choice.icon)); + + expect(unmapped.map(c => `${c.text}: ${c.icon}`)).toEqual([]); + }); + it('reports an unknown name instead of failing silently', () => { const error = jest.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/app/src/themes/tokens.css b/app/src/themes/tokens.css index 2cc702d3..4f94da48 100644 --- a/app/src/themes/tokens.css +++ b/app/src/themes/tokens.css @@ -173,12 +173,27 @@ body { * luminance onto the red channel and drives green and blue to zero. Nothing * a user opens can break their dark adaptation. * - * Tag the media element (or its immediate wrapper) with `.media`. Never - * filter an app-level ancestor: a CSS filter creates a containing block, which - * breaks `position: fixed` descendants like modals and the nav bar. + * This targets the element types rather than an opt-in class on purpose: night + * mode has to fail safe. A library of thousands of files reaches the screen + * through many components, and one that forgot to opt in would undo half an + * hour of dark adaptation. `.media` stays available for a wrapper that needs + * filtering as a unit (a player with custom controls, say). + * + * Only leaf media is filtered, never an app-level ancestor: a CSS filter + * creates a containing block, which would break `position: fixed` descendants + * like modals and the nav bar. These elements have no such descendants — an + * iframe's contents are a separate document with their own viewport. * * Filter definitions live in (mounted once by App). */ -html[data-theme="night"] .media { +html[data-theme="night"] :is(img, video, canvas, iframe, embed, object, .media):not(.night-unfiltered) { + filter: url(#wrolpi-night-red); +} + +/* + * A filter on an ancestor stops applying once the browser promotes a video to + * native fullscreen, so the filter is restated on the fullscreened element. + */ +html[data-theme="night"] :fullscreen:not(.night-unfiltered) { filter: url(#wrolpi-night-red); }