From 4a6a71f3b8ae5fdfd6611c983075f90d6d0f3500 Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 7 May 2026 10:25:18 +0100 Subject: [PATCH 1/2] fix(7606): sync theme-color meta with client-side dark-mode switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #7636 emits server-side from settings.skinVariants. That covers operators who hard-code a dark toolbar in settings.json, but not the runtime path: pad.ts auto-flips the toolbar to super-dark when enableDarkMode is on, the browser reports prefers-color-scheme: dark, and no localStorage white-mode override is set, plus the user can flip it via #options-darkmode. Both paths run skinVariants.updateSkinVariantsClasses(), which until now never touched the meta — so dark-mode users kept the light #ffffff baseline and saw a white address bar above a dark toolbar (stffen on #7606 after 2.7.3). Push the toolbar-color lookup into updateSkinVariantsClasses so the meta tracks every class change: the auto-switch on init, the user toggle, and the skinVariants builder. Mirrors the CSS-source-order table from src/node/utils/SkinColors.ts (last matching *-toolbar token wins). When no meta is present (non-colibris skin, server omits it) the helper is a no-op. Adds Playwright coverage for both paths under colorScheme: 'light' (manual toggle) and 'dark' (auto-switch on dark-OS clients — the case stffen reported). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/static/js/skin_variants.ts | 30 +++++++++++++ .../specs/theme_color_dark_mode.spec.ts | 42 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts diff --git a/src/static/js/skin_variants.ts b/src/static/js/skin_variants.ts index a10074384a8..99187d8135f 100644 --- a/src/static/js/skin_variants.ts +++ b/src/static/js/skin_variants.ts @@ -4,6 +4,34 @@ const containers = ['editor', 'background', 'toolbar']; const colors = ['super-light', 'light', 'dark', 'super-dark']; +// Mirrors src/node/utils/SkinColors.ts: toolbar variants in CSS source order +// from src/static/skins/colibris/src/pad-variants.css. The last matching token +// wins, so iterate in source order. +const TOOLBAR_COLORS_IN_CSS_ORDER: Array<[string, string]> = [ + ['super-light-toolbar', '#ffffff'], + ['light-toolbar', '#f2f3f4'], + ['super-dark-toolbar', '#485365'], + ['dark-toolbar', '#576273'], +]; + +// Keep in sync with the toolbar the user actually +// sees. The server emits a baseline derived from settings.skinVariants, but +// pad.ts may flip the toolbar to super-dark on first paint (enableDarkMode +// + prefers-color-scheme:dark + no localStorage white-mode override) and +// the user can toggle via #options-darkmode. Without this, dark-mode users +// keep the light meta and see a white address bar above a dark toolbar +// (issue #7606 follow-up). +const updateThemeColorMeta = (newClasses: string[]) => { + const meta = document.querySelector('meta[name="theme-color"]'); + if (!meta) return; + const tokens = new Set(newClasses.join(' ').split(/\s+/).filter(Boolean)); + let color = '#ffffff'; + for (const [variant, c] of TOOLBAR_COLORS_IN_CSS_ORDER) { + if (tokens.has(variant)) color = c; + } + meta.setAttribute('content', color); +}; + // add corresponding classes when config change const updateSkinVariantsClasses = (newClasses) => { const domsToUpdate = [ @@ -21,6 +49,8 @@ const updateSkinVariantsClasses = (newClasses) => { domsToUpdate.forEach((el) => { el.removeClass('full-width-editor'); }); domsToUpdate.forEach((el) => { el.addClass(newClasses.join(' ')); }); + + updateThemeColorMeta(newClasses); }; diff --git a/src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts b/src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts new file mode 100644 index 00000000000..442d4705b0f --- /dev/null +++ b/src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts @@ -0,0 +1,42 @@ +import {expect, test} from '@playwright/test'; +import {goToNewPad} from '../helper/padHelper'; + +const themeColor = (page: import('@playwright/test').Page) => + page.locator('meta[name="theme-color"]').getAttribute('content'); + +test.describe('light color scheme', () => { + test.use({colorScheme: 'light'}); + + test('theme-color meta tracks the dark-mode toggle', async ({page}) => { + await goToNewPad(page); + // Server emits the light baseline derived from settings.skinVariants. + expect(await themeColor(page)).toBe('#ffffff'); + + await page.locator('button[data-l10n-id="pad.toolbar.settings.title"]').click(); + await expect(page.locator('#theme-toggle-row')).toBeVisible(); + + // Colibris styles the native checkbox via a sibling label; click the label + // so the toggle fires the real change event the production code listens on. + await page.locator('label[for="options-darkmode"]').click(); + // pad.ts forces super-dark-toolbar (#485365) regardless of the configured + // light skinVariants, so the meta must follow the client-applied class. + await expect.poll(() => themeColor(page)).toBe('#485365'); + + await page.locator('label[for="options-darkmode"]').click(); + await expect.poll(() => themeColor(page)).toBe('#ffffff'); + }); +}); + +test.describe('dark color scheme', () => { + test.use({colorScheme: 'dark'}); + + test('theme-color meta follows the auto dark-mode switch on dark-OS clients', + async ({page}) => { + await goToNewPad(page); + // pad.ts auto-switches to super-dark-toolbar when enableDarkMode is on, + // matchMedia(prefers-color-scheme:dark) matches, and no localStorage + // white-mode override is set. The meta must follow the applied class — + // this is the case stffen reported on issue #7606. + await expect.poll(() => themeColor(page)).toBe('#485365'); + }); +}); From 1b724c559bf11faf868112f1a0afb03f99f2454d Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 7 May 2026 10:37:43 +0100 Subject: [PATCH 2/2] fix(theme-color): action Qodo PR review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (1) Bug — duplicated toolbar→color table: extract the CSS-source-order mapping and the default-color constant into src/static/js/skin_toolbar_colors, re-imported by both src/node/utils/SkinColors.ts (server, EJS template helper) and src/static/js/skin_variants.ts (client, runtime updates). Lives under static/js so the browser bundle can resolve it; server-side imports of static/js modules already exist (Changeset, AttributeMap, ImportHtml, hooks). One source of truth means a future palette change can no longer silently desync the server-rendered baseline meta from the client updates, which was the exact regression that brought us here. (2) Rule violation — 4-space continuation indentation in the new Playwright spec: re-indent the themeColor helper and the multiline test(...) call to the repo's 2-space rule (.editorconfig). Existing backend coverage (configuredToolbarColor unit tests + the specialpages server-render checks for both pad and timeslider) still passes against the refactored helper, so it's regression-locked end to end. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/node/utils/SkinColors.ts | 30 ++++-------------- src/static/js/skin_toolbar_colors.ts | 31 +++++++++++++++++++ src/static/js/skin_variants.ts | 24 +++++--------- .../specs/theme_color_dark_mode.spec.ts | 22 ++++++------- 4 files changed, 55 insertions(+), 52 deletions(-) create mode 100644 src/static/js/skin_toolbar_colors.ts diff --git a/src/node/utils/SkinColors.ts b/src/node/utils/SkinColors.ts index c599b4c3816..74b9bedbcfe 100644 --- a/src/node/utils/SkinColors.ts +++ b/src/node/utils/SkinColors.ts @@ -1,34 +1,16 @@ 'use strict'; -// Toolbar background colors that the colibris skin variants resolve to. -// Mirrors --bg-color in src/static/skins/colibris/src/pad-variants.css. Only -// the colibris skin has a known mapping; for any other skin we cannot derive -// the toolbar color server-side and emit no theme-color meta. -// -// Order matters: when skinVariants contains multiple *-toolbar tokens the -// CSS cascade picks the rule defined last in pad-variants.css, so iterate in -// source order and let the last matching token win. -const TOOLBAR_COLORS_IN_CSS_ORDER: Array<[string, string]> = [ - ['super-light-toolbar', '#ffffff'], - ['light-toolbar', '#f2f3f4'], - ['super-dark-toolbar', '#485365'], - ['dark-toolbar', '#576273'], -]; - -const COLIBRIS_DEFAULT_TOOLBAR_COLOR = '#ffffff'; +import {toolbarColorForTokens} from '../../static/js/skin_toolbar_colors'; // The toolbar color the user actually sees on first paint, derived from the -// configured skin and skinVariants. Returns null when the skin is unknown so -// callers can omit the meta rather than emit a misleading value. +// configured skin and skinVariants. Only the colibris skin has a known +// mapping (see src/static/js/skin_toolbar_colors). For any other skin we +// cannot derive the toolbar color server-side and return null so callers can +// omit the meta rather than emit a misleading value. export const configuredToolbarColor = ( skinName: string | undefined | null, skinVariants: string | undefined | null, ): string | null => { if (skinName !== 'colibris') return null; - const tokens = new Set((skinVariants || '').split(/\s+/).filter(Boolean)); - let color: string | null = null; - for (const [variant, c] of TOOLBAR_COLORS_IN_CSS_ORDER) { - if (tokens.has(variant)) color = c; - } - return color || COLIBRIS_DEFAULT_TOOLBAR_COLOR; + return toolbarColorForTokens((skinVariants || '').split(/\s+/).filter(Boolean)); }; diff --git a/src/static/js/skin_toolbar_colors.ts b/src/static/js/skin_toolbar_colors.ts new file mode 100644 index 00000000000..793b6f4b3de --- /dev/null +++ b/src/static/js/skin_toolbar_colors.ts @@ -0,0 +1,31 @@ +'use strict'; + +// Toolbar background colors that the colibris skin variants resolve to. +// Mirrors --bg-color in src/static/skins/colibris/src/pad-variants.css. Lives +// here (under static/js/) so both the browser bundle (skin_variants.ts) and +// the server-side EJS helper (node/utils/SkinColors.ts) can import it without +// duplication — a drift between client and server tables would silently +// reintroduce the "address bar disagrees with toolbar" bug. +// +// Order matters: when skinVariants contains multiple *-toolbar tokens the +// CSS cascade picks the rule defined last in pad-variants.css, so iterate in +// source order and let the last matching token win. +export const TOOLBAR_COLORS_IN_CSS_ORDER: ReadonlyArray = [ + ['super-light-toolbar', '#ffffff'], + ['light-toolbar', '#f2f3f4'], + ['super-dark-toolbar', '#485365'], + ['dark-toolbar', '#576273'], +]; + +export const COLIBRIS_DEFAULT_TOOLBAR_COLOR = '#ffffff'; + +// Resolve the toolbar color for a set of skin-variant tokens. Pure data: no +// DOM, no Node APIs — safe to call from both server and client. +export const toolbarColorForTokens = (tokens: Iterable): string => { + const set = new Set(tokens); + let color = COLIBRIS_DEFAULT_TOOLBAR_COLOR; + for (const [variant, c] of TOOLBAR_COLORS_IN_CSS_ORDER) { + if (set.has(variant)) color = c; + } + return color; +}; diff --git a/src/static/js/skin_variants.ts b/src/static/js/skin_variants.ts index 99187d8135f..99d6af3b6ec 100644 --- a/src/static/js/skin_variants.ts +++ b/src/static/js/skin_variants.ts @@ -1,35 +1,25 @@ // @ts-nocheck 'use strict'; +import {toolbarColorForTokens} from './skin_toolbar_colors'; + const containers = ['editor', 'background', 'toolbar']; const colors = ['super-light', 'light', 'dark', 'super-dark']; -// Mirrors src/node/utils/SkinColors.ts: toolbar variants in CSS source order -// from src/static/skins/colibris/src/pad-variants.css. The last matching token -// wins, so iterate in source order. -const TOOLBAR_COLORS_IN_CSS_ORDER: Array<[string, string]> = [ - ['super-light-toolbar', '#ffffff'], - ['light-toolbar', '#f2f3f4'], - ['super-dark-toolbar', '#485365'], - ['dark-toolbar', '#576273'], -]; - // Keep in sync with the toolbar the user actually // sees. The server emits a baseline derived from settings.skinVariants, but // pad.ts may flip the toolbar to super-dark on first paint (enableDarkMode // + prefers-color-scheme:dark + no localStorage white-mode override) and // the user can toggle via #options-darkmode. Without this, dark-mode users // keep the light meta and see a white address bar above a dark toolbar -// (issue #7606 follow-up). +// (issue #7606 follow-up). Color resolution lives in skin_toolbar_colors so +// the server-rendered baseline and the client updates share one source of +// truth — Qodo flagged the prior duplicated table as a drift hazard. const updateThemeColorMeta = (newClasses: string[]) => { const meta = document.querySelector('meta[name="theme-color"]'); if (!meta) return; - const tokens = new Set(newClasses.join(' ').split(/\s+/).filter(Boolean)); - let color = '#ffffff'; - for (const [variant, c] of TOOLBAR_COLORS_IN_CSS_ORDER) { - if (tokens.has(variant)) color = c; - } - meta.setAttribute('content', color); + meta.setAttribute('content', + toolbarColorForTokens(newClasses.join(' ').split(/\s+/).filter(Boolean))); }; // add corresponding classes when config change diff --git a/src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts b/src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts index 442d4705b0f..0393b913bde 100644 --- a/src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts +++ b/src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts @@ -1,8 +1,8 @@ -import {expect, test} from '@playwright/test'; +import {expect, test, Page} from '@playwright/test'; import {goToNewPad} from '../helper/padHelper'; -const themeColor = (page: import('@playwright/test').Page) => - page.locator('meta[name="theme-color"]').getAttribute('content'); +const themeColor = (page: Page) => + page.locator('meta[name="theme-color"]').getAttribute('content'); test.describe('light color scheme', () => { test.use({colorScheme: 'light'}); @@ -31,12 +31,12 @@ test.describe('dark color scheme', () => { test.use({colorScheme: 'dark'}); test('theme-color meta follows the auto dark-mode switch on dark-OS clients', - async ({page}) => { - await goToNewPad(page); - // pad.ts auto-switches to super-dark-toolbar when enableDarkMode is on, - // matchMedia(prefers-color-scheme:dark) matches, and no localStorage - // white-mode override is set. The meta must follow the applied class — - // this is the case stffen reported on issue #7606. - await expect.poll(() => themeColor(page)).toBe('#485365'); - }); + async ({page}) => { + await goToNewPad(page); + // pad.ts auto-switches to super-dark-toolbar when enableDarkMode is on, + // matchMedia(prefers-color-scheme:dark) matches, and no localStorage + // white-mode override is set. The meta must follow the applied class — + // this is the case stffen reported on issue #7606. + await expect.poll(() => themeColor(page)).toBe('#485365'); + }); });