mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-17 16:47:05 +00:00
fix(7606): sync theme-color meta with client-side dark-mode switch (#7690)
* fix(7606): sync theme-color meta with client-side dark-mode switch PR #7636 emits <meta name="theme-color"> 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) <noreply@anthropic.com> * fix(theme-color): action Qodo PR review (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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
fe9727b31e
commit
ab0cff4d95
4 changed files with 99 additions and 24 deletions
|
|
@ -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));
|
||||
};
|
||||
|
|
|
|||
31
src/static/js/skin_toolbar_colors.ts
Normal file
31
src/static/js/skin_toolbar_colors.ts
Normal file
|
|
@ -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<readonly [string, string]> = [
|
||||
['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>): 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;
|
||||
};
|
||||
|
|
@ -1,9 +1,27 @@
|
|||
// @ts-nocheck
|
||||
'use strict';
|
||||
|
||||
import {toolbarColorForTokens} from './skin_toolbar_colors';
|
||||
|
||||
const containers = ['editor', 'background', 'toolbar'];
|
||||
const colors = ['super-light', 'light', 'dark', 'super-dark'];
|
||||
|
||||
// Keep <meta name="theme-color"> 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). 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;
|
||||
meta.setAttribute('content',
|
||||
toolbarColorForTokens(newClasses.join(' ').split(/\s+/).filter(Boolean)));
|
||||
};
|
||||
|
||||
// add corresponding classes when config change
|
||||
const updateSkinVariantsClasses = (newClasses) => {
|
||||
const domsToUpdate = [
|
||||
|
|
@ -21,6 +39,8 @@ const updateSkinVariantsClasses = (newClasses) => {
|
|||
domsToUpdate.forEach((el) => { el.removeClass('full-width-editor'); });
|
||||
|
||||
domsToUpdate.forEach((el) => { el.addClass(newClasses.join(' ')); });
|
||||
|
||||
updateThemeColorMeta(newClasses);
|
||||
};
|
||||
|
||||
|
||||
|
|
|
|||
42
src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts
Normal file
42
src/tests/frontend-new/specs/theme_color_dark_mode.spec.ts
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
import {expect, test, Page} from '@playwright/test';
|
||||
import {goToNewPad} from '../helper/padHelper';
|
||||
|
||||
const themeColor = (page: 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');
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue