From 8b3658aef89ece32790ebb1fc05cffb7333a6354 Mon Sep 17 00:00:00 2001 From: Francesco Bigiarini Date: Wed, 7 Oct 2026 10:59:18 +0200 Subject: [PATCH 1/5] Add a custom theme: a background and a primary colour of the contributor's own The prototype's fourth theme (#560). Under Custom, the General tab asks for two colours, typed as hex or picked with the system's picker behind the swatch, and the design system builds every other colour from them, as it builds the dark theme from its one seed. The prototype calls the second colour the accent; the design system's name for it, primary, is used. A custom theme is a scheme too: main gives Electron light or dark by whether the background reads better with black text or with white, so the native form controls and the window's chrome match the page, and makes and paints the window in the background chosen. The window reads the settings above the design system's provider now, since the theme is one of them, and the terminal re-reads its colours whenever the theme as painted changes, by a name that changes with it. A colour that is not one is refused in main's words and the field goes back to what is kept; three hex digits are kept as six. The dialog says when the design system could not reach the contrast it wants between two of the colours it built, and keeps the colours all the same. Co-Authored-By: Claude Fable 5.1 --- docs/guide/settings.md | 2 +- src/main.js | 46 ++++--- src/renderer/components/app-theme.jsx | 61 ++++++--- src/renderer/components/settings-dialog.jsx | 91 +++++++++++-- src/renderer/hooks/use-site-terminal.jsx | 8 +- src/renderer/index.jsx | 19 ++- src/renderer/settings-view.cjs | 8 +- src/renderer/shell.css | 42 ++++++ src/settings.cjs | 28 +++- src/theme.cjs | 136 +++++++++++++++----- src/trac-view.js | 9 +- tests/e2e/journeys/settings.spec.js | 73 ++++++++++- tests/unit/ipc-wiring.test.cjs | 43 ++++++- tests/unit/settings-view.test.cjs | 5 +- tests/unit/settings.test.cjs | 25 +++- tests/unit/theme.test.cjs | 66 ++++++++-- tests/unit/trac-view.test.cjs | 11 +- 17 files changed, 539 insertions(+), 134 deletions(-) diff --git a/docs/guide/settings.md b/docs/guide/settings.md index e4031670..5d915481 100644 --- a/docs/guide/settings.md +++ b/docs/guide/settings.md @@ -4,7 +4,7 @@ The app's settings are in one dialog, opened from the cog at the bottom right of ## General -- **Theme** — light, dark, or whatever your operating system is set to, which is the default. A change applies as you make it, to the whole window; nothing running is touched. +- **Theme** — light, dark, whatever your operating system is set to, which is the default, or custom: a background and a primary colour of your own, typed as hex or picked from the swatch, from which the app builds every other colour. A change applies as you make it, to the whole window; nothing running is touched. The dialog says if some text may be hard to read on the colours you chose; they are kept all the same. - **Language** — which language the app is shown in: your system's language, which is the default, or one of the languages the app has a translation for. A change applies after a relaunch, which the dialog offers; relaunching stops running servers and builds, as quitting does. Translations come from [translate.wordpress.org](https://translate.wordpress.org/projects/meta/contributor-toolkit/) and ship with the app once they are mostly complete, so the list grows from release to release. - **Start the server when I open a site** and **Start the build watch when I open a site** — off unless you turn them on. On, opening a site that is set up starts its [development server](./running-the-site), its build watch, or both, as it would if you pressed Start; on WordPress Core the server's start brings the watch with it. Nothing starts for a site whose setup, update or deletion is under way, and nothing already running is started again. - **When I quit, running servers and build watches** — quitting always stops them, as it always has. **Stop them, and start them again next time** remembers which sites had a server or a watch running and starts them again when the app next opens, whichever site it opens on. diff --git a/src/main.js b/src/main.js index b81608a2..9272ef0c 100644 --- a/src/main.js +++ b/src/main.js @@ -93,7 +93,7 @@ const { addFilter } = require('@wordpress/hooks'); const { mergeInProgressError, mergeCheckFailedError } = require('./renderer/merge-in-progress.cjs'); const { parseHandle } = require('./wporg-handle.cjs'); const { SETTINGS, readSettings, acceptSetting } = require('./settings.cjs'); -const { windowBackground } = require('./theme.cjs'); +const { resolveTheme, nativeThemeSource, THEME_KEYS } = require('./theme.cjs'); const { parseEventName, buildProvenanceHeader, handoffFilename } = require('./patch-provenance.cjs'); const { describeRefused } = require('./safe-log'); const { detectEditors, matchDetectedEditor, openSiteInEditor, REFUSAL_REASONS } = require('./editor-launch'); @@ -564,7 +564,7 @@ function createWindow() { icon: process.platform === 'linux' ? path.join(__dirname, '..', 'build', 'icon.png') : undefined, // The colour of the theme the window is made in (#560), so that a dark // window is not white for the moment before its page has painted. - backgroundColor: windowBackground(nativeTheme.shouldUseDarkColors), + backgroundColor: currentTheme().background, webPreferences: { preload: path.join(__dirname, 'preload.js'), contextIsolation: true, @@ -715,28 +715,41 @@ function localeReply() { } // The theme (#560), given to Electron. `nativeTheme` is the one place the -// choice is made: Chromium answers the page's `prefers-color-scheme` from it, -// and paints the window's chrome and the native form controls to match, so -// the page only has to follow what it is told, as it would the operating -// system's. Read from the store before the window is made, so the window is -// made in it; a store that cannot be read leaves the system's theme, with a -// line in the log, as it leaves the system's language. +// scheme is decided: Chromium answers the page's `prefers-color-scheme` from +// it, and paints the window's chrome and the native form controls to match, +// so the page only has to follow what it is told, as it would the operating +// system's. A custom theme is given as the scheme its background comes to. +// Read from the store before the window is made, so the window is made in +// it; a store that cannot be read leaves the system's theme, with a line in +// the log, as it leaves the system's language. +// +// `themeSettings` is what the theme was last applied from, for the colour a +// window is made with: the system's theme until the store is read. +let themeSettings = { theme: 'system' }; +function applyTheme(settings) { + themeSettings = settings; + nativeTheme.themeSource = nativeThemeSource(settings); + paintWindowForTheme(); +} +function currentTheme() { + return resolveTheme({ ...themeSettings, systemDark: nativeTheme.shouldUseDarkColors }); +} async function applyStoredTheme() { try { - nativeTheme.themeSource = readSettings((await getStore()).get('preferences')).theme; + applyTheme(readSettings((await getStore()).get('preferences'))); } catch (e) { logError('theme', `the settings could not be read, so the theme is the system's: ${String(e && e.message ? e.message : e)}`); + // A deep link can have opened the window while the store was read. + paintWindowForTheme(); } - // A deep link can have opened the window while the store was read. - paintWindowForTheme(); } // The colour the window was made with shows wherever the page has not // painted yet (a live resize, a reload), so it is given again whenever the -// theme is: by the setting, here and in `settings:set`, and by Electron's +// theme is: by the setting, through `applyTheme`, and by Electron's // `updated`, which is how the system's theme reaches it under 'system'. function paintWindowForTheme() { - if (mainWindow && !mainWindow.isDestroyed()) mainWindow.setBackgroundColor(windowBackground(nativeTheme.shouldUseDarkColors)); + if (mainWindow && !mainWindow.isDestroyed()) mainWindow.setBackgroundColor(currentTheme().background); } // The languages the settings offer: what the build ships, read once. @@ -2438,7 +2451,7 @@ ipcMain.handle('trac:list-attachments', async (_e, sitePath) => { if (projectTypeForSite(meta).workItem.provider !== 'trac') return { ok: true, status: 'not-trac', items: [] }; const ticketId = meta.tracTicket; if (!ticketId) return { ok: true, status: 'no-ticket', items: [] }; - const result = await openAndScrape(ticketId); + const result = await openAndScrape(ticketId, { backgroundColor: currentTheme().background }); return { ok: true, ...result }; } catch (e) { logError('trac:list-attachments', String(e && e.stack ? e.stack : e)); @@ -3580,10 +3593,7 @@ ipcMain.handle('settings:set', async (_e, key, value) => { // The theme applies at once (#560): Electron tells the page, and the // window's own colour is set here rather than left to Electron's // `updated`, which is not promised for a change to 'system'. - if (key === 'theme') { - nativeTheme.themeSource = settings.theme; - paintWindowForTheme(); - } + if (THEME_KEYS.includes(key)) applyTheme(settings); return { ok: true, settings }; }); diff --git a/src/renderer/components/app-theme.jsx b/src/renderer/components/app-theme.jsx index 33d233cc..4471e1ad 100644 --- a/src/renderer/components/app-theme.jsx +++ b/src/renderer/components/app-theme.jsx @@ -1,17 +1,18 @@ -import { createContext, useContext, useEffect, useState } from 'react'; +import { createContext, useContext, useEffect, useMemo, useState } from 'react'; import { ThemeProvider } from '@wordpress/theme'; -import { themeColorSeeds } from '../../theme.cjs'; +import { resolveTheme } from '../../theme.cjs'; // What Chromium says of the window's colour scheme. It says it from the theme // main gave Electron (#560), or from the operating system under 'system', so -// this is the one thing the window reads: not the setting, which would be a -// second answer to the same question. +// the scheme is read here and not decided again from the setting: for the +// three named themes it is the answer, and for a custom one the setting's +// colours are what is painted, in the scheme main decided from them. const DARK_SCHEME = '(prefers-color-scheme: dark)'; -// Whether the window is in the dark scheme, for whatever paints with values -// rather than with a stylesheet (the terminal) and so has to be told when it -// changes. -const DarkSchemeContext = createContext(false); +// The theme as painted, for whatever paints with values rather than with a +// stylesheet (the terminal) and so has to be told when it changes: a name +// that changes with it, and what the design system said of the colours. +const ThemeContext = createContext({ key: 'light', warnings: [] }); function usePrefersDark() { const [dark, setDark] = useState(() => window.matchMedia(DARK_SCHEME).matches); @@ -30,18 +31,42 @@ function usePrefersDark() { // it overrides on the document rather than on its own wrapper, which is what // reaches a modal or a popover: those are portalled to `body`, outside this // tree. In the light scheme it is given no colour, so the tokens stylesheet's -// values stand as they ship; in the dark scheme it is given the dark seed and -// builds every colour token from it, for its own components, for the older -// ones through the variables they read, and for the app's own styles. -export function AppTheme({ children }) { - const dark = usePrefersDark(); +// values stand as they ship; in the dark scheme it is given the dark seed, and +// under a custom theme the two colours chosen, and builds every colour token +// from them, for its own components, for the older ones through the variables +// they read, and for the app's own styles. +// +// `settings` is what main holds, or null until it has answered: until then +// the window is painted for the scheme alone, which for a custom theme is +// the standard theme of its scheme for the moment before the colours arrive. +export function AppTheme({ settings, children }) { + const prefersDark = usePrefersDark(); + const [warnings, setWarnings] = useState([]); + let theme = prefersDark ? 'dark' : 'light'; + if (settings) theme = settings.theme; + const customBackground = settings ? settings.customBackground : undefined; + const customPrimary = settings ? settings.customPrimary : undefined; + const resolved = useMemo( + () => resolveTheme({ theme, customBackground, customPrimary, systemDark: prefersDark }), + [theme, customBackground, customPrimary, prefersDark] + ); + const value = useMemo(() => ({ key: resolved.key, warnings }), [resolved.key, warnings]); return ( - - {children} - + + {children} + ); } -export function useDarkScheme() { - return useContext(DarkSchemeContext); +// A name for the theme as painted, which changes whenever what is painted +// does. +export function useThemeKey() { + return useContext(ThemeContext).key; +} + +// What the design system said of the colours it was given: a contrast it +// could not reach is one, so that a custom theme can say when its text may +// be hard to read. +export function useThemeWarnings() { + return useContext(ThemeContext).warnings; } diff --git a/src/renderer/components/settings-dialog.jsx b/src/renderer/components/settings-dialog.jsx index 6767d3ed..357a1ca9 100644 --- a/src/renderer/components/settings-dialog.jsx +++ b/src/renderer/components/settings-dialog.jsx @@ -1,11 +1,12 @@ -import { useEffect, useId, useMemo, useState } from 'react'; +import { useEffect, useId, useMemo, useRef, useState } from 'react'; // The segmented control the design has for a choice of a few. The design // system has no other, and documents this one under these names: it is // stable in use and has not been given its final export yet. // eslint-disable-next-line @wordpress/no-unsafe-wp-apis -- see above. import { __experimentalToggleGroupControl as ToggleGroupControl, __experimentalToggleGroupControlOption as ToggleGroupControlOption } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; import { Button, Dialog, InputControl, Notice, SelectControl, Stack, SwitchControl, Tabs, Text } from '@wordpress/ui'; +import { useThemeWarnings } from './app-theme.jsx'; import { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, SYSTEM_LANGUAGE } from '../settings-view.cjs'; import { FolderField } from './folder-field.jsx'; @@ -81,16 +82,75 @@ function LanguageControl({ settings, loaded, onChange }) { ); } -// The window's theme (#560): light, dark, or the operating system's. Main -// gives the choice to Electron, and the window follows what Chromium then -// says of the colour scheme, so the change is on screen as the control is -// pressed. +// One colour of the custom theme (#560): typed as hex, or picked with the +// system's picker, which is the swatch before the field, showing the colour +// kept. What is typed is kept when the field is left or Enter is pressed, +// and main says what it accepts, so a colour that is not one is refused in +// main's words and the field goes back to what is kept. The picker's choice +// is kept when the picker is closed, not as it is dragged: each keep is a +// write to the store, and the field shows the colour under the pointer +// meanwhile. +function ColorField({ label, value, disabled, onKeep }) { + const [draft, setDraft] = useState(value); + useEffect(() => setDraft(value), [value]); + // A refusal leaves what is kept as it was, so nothing above changes the + // draft back: it is put back here. + const keep = async (text) => { + const result = await onKeep(text); + if (!result?.ok) setDraft(value); + }; + const picker = useRef(null); + useEffect(() => { + const input = picker.current; + if (!input) return undefined; + const picked = () => keep(input.value); + input.addEventListener('change', picked); + return () => input.removeEventListener('change', picked); + }); + const commit = () => { if (draft !== value) keep(draft); }; + return ( + setDraft(event.currentTarget.value)} + /> + } + onChange={(event) => setDraft(event.currentTarget.value)} + onBlur={commit} + onKeyDown={(event) => { if (event.key === 'Enter') { event.preventDefault(); commit(); } }} + /> + ); +} + +// The window's theme (#560): light, dark, the operating system's, or custom, +// a background and a primary colour of the contributor's own, from which +// the design system builds every other colour. Main gives the choice to +// Electron, and the window follows what Chromium then says of the colour +// scheme and what main holds, so the change is on screen as the control is +// pressed. Under custom, the design system says when a colour it built +// cannot be read on another, and the control passes that on. function ThemeControl({ settings, onChange }) { const [error, setError] = useState(''); - const keep = async (value) => { - const result = await onChange('theme', value); + const warnings = useThemeWarnings(); + const keep = async (key, value) => { + const result = await onChange(key, value); setError(result?.ok ? '' : (result?.error || __('Could not keep that.'))); + return result; }; + const custom = settings?.theme === 'custom'; return ( <> { if (value) keep(value); }} + onChange={(value) => { if (value) keep('theme', value); }} > {themeItems().map((item) => ( ))} + {custom ? ( +
+ keep('customBackground', value)} /> + keep('customPrimary', value)} /> +
+ ) : null} + {custom && warnings.length ? ( + + {__('Some text may be hard to read with these colours. They are kept as they are; choose others if it is.')} + + ) : null} {error ? ( {error} diff --git a/src/renderer/hooks/use-site-terminal.jsx b/src/renderer/hooks/use-site-terminal.jsx index 3e6c2be3..1b03fc21 100644 --- a/src/renderer/hooks/use-site-terminal.jsx +++ b/src/renderer/hooks/use-site-terminal.jsx @@ -2,7 +2,7 @@ import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react import { Terminal } from '@xterm/xterm'; import { terminalFont, terminalTheme, tokenName, TERMINAL_READABILITY } from '../terminal-theme.cjs'; import { terminalGrid } from '../tray.cjs'; -import { useDarkScheme } from '../components/app-theme.jsx'; +import { useThemeKey } from '../components/app-theme.jsx'; // What the terminal is painted with, read off the design system's tokens // where the terminal stands (#557). The terminal takes its colours and its @@ -59,7 +59,7 @@ const TERMINAL_INSTALL_ALIASES = ['npm install', 'npm i', 'install']; // None of them depends on the three runners, which may change as often as // they like. export function useSiteTerminal({ allowedScripts, runInstall, runScript, killCurrent, shown }) { - const dark = useDarkScheme(); + const themeKey = useThemeKey(); // Read through a ref by the terminal's command handlers rather than closed // over: the xterm instance is created by an effect that depends on // `printHelp`, so a new array identity here would otherwise dispose and @@ -384,7 +384,7 @@ export function useSiteTerminal({ allowedScripts, runInstall, runScript, killCur fitTerminal(); }); - // Painted again when the window's scheme changes (#560). The terminal was + // Painted again when the window's theme changes (#560). The terminal was // given its colours as values when it opened, and a change to the tokens // does not reach a value; so they are read again, after the provider has // put the new tokens on the document, which it does in a layout effect, @@ -393,7 +393,7 @@ export function useSiteTerminal({ allowedScripts, runInstall, runScript, killCur const term = terminalRef.current; if (!term || !term.element || !container) return; term.options.theme = readTerminalLook(container).theme; - }, [dark, container]); + }, [themeKey, container]); // Fitted again whenever its element changes size: the tray dragged, the // window resized, and the element coming back on screen, which is a change diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index 3acb12e4..fcdcc8cd 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -116,7 +116,7 @@ const SILENT = ''; const FEEDBACK_FORM_URL = 'https://docs.google.com/forms/d/e/1FAIpQLScnMxicyDxZO2OoaS5ela8FArYWjCyLfC3hxRBBRSF7XLPzKg/viewform'; -function App() { +function App({ settingsState }) { const { sites, siteMeta, refresh, setSiteMeta, applySetup } = useSites(); // The confirmation queue for the whole window. It lives here, above every // SiteRow, because only one row is visible at a time and a per-row toast would @@ -132,9 +132,10 @@ function App() { // machine has is a fact about the machine, not about a site. const detectedApplications = useDetectedEditors(); const wporg = useContributorProvenance(); - // The app's settings (#559), and the dialog they are changed in. The menu - // asks for the dialog too, over a subscription the whole window holds. - const { settings, loaded: loadedSettings, php: phpVersions, change: changeSetting } = useSettings(); + // The app's settings (#559), read once above the theme, which is one of + // them, and the dialog they are changed in. The menu asks for the dialog + // too, over a subscription the whole window holds. + const { settings, loaded: loadedSettings, php: phpVersions, change: changeSetting } = settingsState; // The PHP a server starts on: the one set where the bundle has it, and // the fallback where it does not, decided where the dialog decides it. const startingPhp = settings ? phpVersionChoice({ versions: phpVersions?.versions, fallback: phpVersions?.fallback, stored: settings.phpVersion }).value : null; @@ -2406,8 +2407,14 @@ async function loadLocale() { } // Under the design system's provider, in the theme the window is in (#560): -// see app-theme.jsx. +// see app-theme.jsx. The settings are read here, above the provider, since +// the theme is one of them; the app is handed what was read. +function Root() { + const settingsState = useSettings(); + return ; +} + loadLocale().then(() => { const root = createRoot(document.getElementById('root')); - root.render(); + root.render(); }); diff --git a/src/renderer/settings-view.cjs b/src/renderer/settings-view.cjs index af6e4394..08ff9953 100644 --- a/src/renderer/settings-view.cjs +++ b/src/renderer/settings-view.cjs @@ -124,8 +124,9 @@ function quitItems() { } /** - * The entries of the theme control (#560): light, dark, or the operating - * system's. The prototype's fourth, a custom pair of colours, is not offered. + * The entries of the theme control (#560): light, dark, the operating + * system's, or custom, a background and a primary colour of the + * contributor's own. * * @return {Array<{value: string, label: string}>} */ @@ -133,7 +134,8 @@ function themeItems() { return [ { value: 'light', label: _x('Light', 'the window’s theme') }, { value: 'dark', label: _x('Dark', 'the window’s theme') }, - { value: 'system', label: _x('System', 'the window’s theme: the operating system’s') } + { value: 'system', label: _x('System', 'the window’s theme: the operating system’s') }, + { value: 'custom', label: _x('Custom', 'the window’s theme: two colours of one’s own') } ]; } diff --git a/src/renderer/shell.css b/src/renderer/shell.css index ff0db516..154f7f41 100644 --- a/src/renderer/shell.css +++ b/src/renderer/shell.css @@ -1307,6 +1307,48 @@ body { overflow-wrap: anywhere; } +/* The custom theme's two colours (#560), side by side, each typed as hex in + the terminal's type after a swatch of the colour kept. The swatch is the + system's colour input itself, which draws the colour it holds, with the + browser's padding and border around that colour taken off so it fills + the swatch; pressing it opens the picker. */ +.theme-colors { + display: grid; + grid-template-columns: 1fr 1fr; + gap: var(--wpds-dimension-gap-xl); +} + +.theme-colors > * { + min-width: 0; +} + +.color-field input[type="text"] { + font-family: var(--wpds-typography-font-family-mono); +} + +.color-swatch { + display: block; + box-sizing: border-box; + width: var(--wpds-dimension-size-sm); + height: var(--wpds-dimension-size-sm); + margin-inline-start: var(--wpds-dimension-gap-xs); + padding: 0; + overflow: hidden; + border: var(--wpds-border-width-xs) solid var(--wpds-color-stroke-surface-neutral); + border-radius: var(--wpds-border-radius-sm); + background: none; + cursor: pointer; +} + +.color-swatch::-webkit-color-swatch-wrapper { + padding: 0; +} + +.color-swatch::-webkit-color-swatch { + border: 0; + border-radius: 0; +} + /* The settings dialog (#559): its tabs under its title, and each tab's sections one under another. */ .settings-dialog .settings-panel { diff --git a/src/settings.cjs b/src/settings.cjs index d42d21f7..c4782785 100644 --- a/src/settings.cjs +++ b/src/settings.cjs @@ -18,11 +18,20 @@ */ const { __ } = require('@wordpress/i18n'); -const { THEMES } = require('./theme.cjs'); +const { THEMES, normalizeHexColor, isHexColor, LIGHT_BACKGROUND, PRIMARY } = require('./theme.cjs'); // What the quit setting can be. const QUIT_BEHAVIOURS = ['stop', 'restart']; +// A colour of the custom theme (#560): kept as `#rrggbb`, from three or six +// hex digits as typed; nothing for the fallback. +function acceptColor(value) { + if (value === null || value === undefined || value === '') return { ok: true, value: null }; + const color = normalizeHexColor(value); + if (!color) return { ok: false, error: __('Choose a colour as six hex digits, like #3858e9.') }; + return { ok: true, value: color }; +} + // A switch: on or off, and nothing for the fallback. function acceptSwitch(value) { if (value === null || value === undefined) return { ok: true, value: null }; @@ -80,17 +89,23 @@ const SETTINGS = { return { ok: true, value }; } }, - // The window's theme (#560): light, dark, or the operating system's, - // which is the fallback. Main applies it to Electron's native theme, and - // the window follows what Chromium then says of the colour scheme. + // The window's theme (#560): light, dark, the operating system's, which + // is the fallback, or custom, the two colours below. Main applies it to + // Electron's native theme, and the window follows what Chromium then + // says of the colour scheme, and the colours. theme: { fallback: 'system', accept(value) { if (value === null || value === undefined || value === '') return { ok: true, value: null }; - if (!THEMES.includes(value)) return { ok: false, error: __('Choose light, dark, or your system’s theme.') }; + if (!THEMES.includes(value)) return { ok: false, error: __('Choose light, dark, your system’s theme, or custom.') }; return { ok: true, value }; } }, + // The custom theme's colours: the background its surfaces are built + // from, and the primary colour; the design system builds the rest. They + // start as the light theme's. + customBackground: { fallback: LIGHT_BACKGROUND, accept: acceptColor }, + customPrimary: { fallback: PRIMARY, accept: acceptColor }, // The folder new sites are made in, each in a subfolder of its own. Unset, // the create-site dialog asks for one every time, as it did before. The // path is kept as the system's dialog gave it: a folder's name can end in @@ -124,6 +139,7 @@ function readSettings(preferences = {}) { const stored = preferences && typeof preferences === 'object' ? preferences : {}; const text = (key) => (typeof stored[key] === 'string' && stored[key] ? stored[key] : SETTINGS[key].fallback); const flag = (key) => (typeof stored[key] === 'boolean' ? stored[key] : SETTINGS[key].fallback); + const color = (key) => (isHexColor(stored[key]) ? stored[key] : SETTINGS[key].fallback); return { locale: text('locale'), phpVersion: text('phpVersion'), @@ -133,6 +149,8 @@ function readSettings(preferences = {}) { autoStartWatch: flag('autoStartWatch'), quitBehavior: QUIT_BEHAVIOURS.includes(stored.quitBehavior) ? stored.quitBehavior : SETTINGS.quitBehavior.fallback, theme: THEMES.includes(stored.theme) ? stored.theme : SETTINGS.theme.fallback, + customBackground: color('customBackground'), + customPrimary: color('customPrimary'), newSiteLocation: text('newSiteLocation') }; } diff --git a/src/theme.cjs b/src/theme.cjs index e1ffaa19..5c10e15f 100644 --- a/src/theme.cjs +++ b/src/theme.cjs @@ -1,53 +1,129 @@ 'use strict'; /** - * The window's theme (#560): light, dark, or whatever the operating system - * is set to, and what each side of the app does with the answer. + * The window's theme (#560): light, dark, whatever the operating system is + * set to, or a pair of colours of the contributor's own, and what each side + * of the app does with the answer. * * Main gives the choice to Electron's `nativeTheme`, which is the one place - * the choice has to be made: Chromium then answers `prefers-color-scheme` in - * the window from it, and paints the window's own chrome and the native form - * controls to match. The window only reads that answer, and seeds the design - * system's colours from it. So there is one source of truth, and a change - * reaches the window as the operating system's would. + * the scheme has to be decided: Chromium then answers `prefers-color-scheme` + * in the window from it, and paints the window's own chrome and the native + * form controls to match. The window reads that answer and the setting, and + * seeds the design system's colours from them. A custom theme is a scheme + * too: light or dark by whether its background reads better with black text + * or with white, so that the native controls match the page. * - * Pure, and shared by main and the window: the dark seed is both the colour - * the design system builds its dark ramp from and the colour a window is - * made with, so that nothing white shows before the page has painted. + * Pure, and shared by main and the window: the colour a theme's surfaces are + * built from is also the colour a window is made with, so that nothing white + * shows before the page has painted. */ -// What the theme setting can be. 'system' follows the operating system. -const THEMES = ['light', 'dark', 'system']; +// What the theme setting can be. 'system' follows the operating system; +// 'custom' is the two colours below. +const THEMES = ['light', 'dark', 'system', 'custom']; + +// The settings that make the theme: a change to any of them is a change of +// theme. +const THEME_KEYS = ['theme', 'customBackground', 'customPrimary']; // The design system's own seeds, as the prototype has them: the colour the -// dark theme's surfaces are built from, and the colour the light theme's -// are. In the light theme the window passes no seed at all, since the tokens -// stylesheet already holds the light values and generating them again would -// move them by a hair; the light seed is the window's background only. +// dark theme's surfaces are built from, the colour the light theme's are, +// and the brand colour both are built around, which a custom theme starts +// from. In the light theme the window passes no seed at all, since the +// tokens stylesheet already holds the light values and generating them again +// would move them by a hair; the light seed is the window's background only. const DARK_BACKGROUND = '#1e1e1e'; const LIGHT_BACKGROUND = '#fcfcfc'; +const PRIMARY = '#3858e9'; + +/** + * A colour as the settings keep one, `#rrggbb` in lowercase, from what was + * typed: three or six hex digits, with or without the `#`, in either case. + * + * @param {*} text + * @return {?string} The colour, or null for anything that is not one. + */ +function normalizeHexColor(text) { + if (typeof text !== 'string') return null; + const digits = text.trim().replace(/^#/, '').toLowerCase(); + if (/^[0-9a-f]{6}$/.test(digits)) return `#${digits}`; + if (/^[0-9a-f]{3}$/.test(digits)) return `#${digits.split('').map((d) => d + d).join('')}`; + return null; +} + +/** + * Whether a value is a colour as the settings keep one. + * + * @param {*} value + * @return {boolean} + */ +function isHexColor(value) { + return typeof value === 'string' && /^#[0-9a-f]{6}$/.test(value); +} + +// The relative luminance of a colour, as WCAG defines it and as the design +// system measures contrast: 0 for black, 1 for white. +function luminance(hex) { + const channel = (at) => { + const c = parseInt(hex.slice(at, at + 2), 16) / 255; + return c <= 0.03928 ? c / 12.92 : ((c + 0.055) / 1.055) ** 2.4; + }; + return 0.2126 * channel(1) + 0.7152 * channel(3) + 0.0722 * channel(5); +} + +/** + * Whether a background is a dark one: white text reads better on it than + * black does. That is the point at which the native controls, which Chromium + * paints for the scheme, should turn dark with the page. + * + * @param {string} hex `#rrggbb`. + * @return {boolean} + */ +function isDarkColor(hex) { + const l = luminance(hex); + return (1.05) / (l + 0.05) > (l + 0.05) / 0.05; +} /** - * What the design system's provider is given for the scheme the window is - * in: a seed to build the dark ramp from, or nothing, which is the light - * theme as the tokens stylesheet ships it. + * What the theme setting comes to, for the window and for main. * - * @param {boolean} dark Whether the window is in the dark scheme. - * @return {Object} The provider's `color` prop. + * @param {Object} root0 + * @param {string} root0.theme One of THEMES. + * @param {string} [root0.customBackground] The custom theme's background, `#rrggbb`. + * @param {string} [root0.customPrimary] The custom theme's primary colour, `#rrggbb`. + * @param {boolean} [root0.systemDark] Whether the operating system is in dark mode, for 'system'. + * @return {{scheme: ('light'|'dark'), seeds: Object, background: string, key: string}} + * The scheme the window is in; what the design system's provider is given + * as its `color`; the colour a window is made with; and a name for the + * theme as painted, which changes when any of that does. */ -function themeColorSeeds(dark) { - return dark ? { background: DARK_BACKGROUND } : {}; +function resolveTheme({ theme, customBackground, customPrimary, systemDark = false }) { + if (theme === 'custom') { + const background = isHexColor(customBackground) ? customBackground : LIGHT_BACKGROUND; + const primary = isHexColor(customPrimary) ? customPrimary : PRIMARY; + return { + scheme: isDarkColor(background) ? 'dark' : 'light', + seeds: { background, primary }, + background, + key: `custom:${background}:${primary}` + }; + } + const dark = theme === 'dark' || (theme === 'system' && systemDark); + return dark + ? { scheme: 'dark', seeds: { background: DARK_BACKGROUND }, background: DARK_BACKGROUND, key: 'dark' } + : { scheme: 'light', seeds: {}, background: LIGHT_BACKGROUND, key: 'light' }; } /** - * The colour a window is made with, so that the frame is not white for the - * moment before the page paints a dark one. + * What main gives Electron's `nativeTheme.themeSource` for the setting: + * the setting itself for the three it knows, and for a custom theme the + * scheme its background comes to. * - * @param {boolean} dark Whether the window is in the dark scheme. - * @return {string} `#rrggbb`. + * @param {Object} settings The theme settings, as `readSettings` answers them. + * @return {('light'|'dark'|'system')} */ -function windowBackground(dark) { - return dark ? DARK_BACKGROUND : LIGHT_BACKGROUND; +function nativeThemeSource(settings) { + return settings.theme === 'custom' ? resolveTheme(settings).scheme : settings.theme; } -module.exports = { THEMES, themeColorSeeds, windowBackground, DARK_BACKGROUND, LIGHT_BACKGROUND }; +module.exports = { THEMES, THEME_KEYS, resolveTheme, nativeThemeSource, normalizeHexColor, isHexColor, isDarkColor, DARK_BACKGROUND, LIGHT_BACKGROUND, PRIMARY }; diff --git a/src/trac-view.js b/src/trac-view.js index dead9967..8ee25a0b 100644 --- a/src/trac-view.js +++ b/src/trac-view.js @@ -18,8 +18,8 @@ * engine (#11), which defends against path traversal. */ -const { BrowserWindow, nativeTheme, session } = require('electron'); -const { windowBackground } = require('./theme.cjs'); +const { BrowserWindow, session } = require('electron'); +const { LIGHT_BACKGROUND } = require('./theme.cjs'); const { parseAttachments, secureTracUrl } = require('./trac-attachments.cjs'); const { parseTicketInfo } = require('./trac-ticket-info.cjs'); const { httpGet } = require('./github-prs'); @@ -82,7 +82,8 @@ function pinToTrac(wc) { * * @param {number|string} ticketId * @param {Object} [deps] - * @param {number} [deps.readyTimeoutMs] Override for tests. + * @param {number} [deps.readyTimeoutMs] Override for tests. + * @param {string} [deps.backgroundColor] The colour of the app's theme (#560), which main holds. * @return {Promise<{status: string, items: Array, error?: string}>} */ async function openAndScrape(ticketId, deps = {}) { @@ -98,7 +99,7 @@ async function openAndScrape(ticketId, deps = {}) { title: `Trac #${id}`, // In the app's theme (#560), so the frame is not white where Trac's // page does not paint; Trac's own page is Trac's. - backgroundColor: windowBackground(nativeTheme.shouldUseDarkColors), + backgroundColor: deps.backgroundColor || LIGHT_BACKGROUND, webPreferences: { contextIsolation: true, nodeIntegration: false, diff --git a/tests/e2e/journeys/settings.spec.js b/tests/e2e/journeys/settings.spec.js index 22095b74..3cefd02e 100644 --- a/tests/e2e/journeys/settings.spec.js +++ b/tests/e2e/journeys/settings.spec.js @@ -20,7 +20,7 @@ const { test, expect } = require( '../helpers/app.cjs' ); const ui = require( '../helpers/ui.cjs' ); const { makeSite } = require( '../helpers/git-site.cjs' ); const { pseudoLocalize } = require( '../../../src/renderer/pseudo-locale.cjs' ); -const { DARK_BACKGROUND, LIGHT_BACKGROUND } = require( '../../../src/theme.cjs' ); +const { DARK_BACKGROUND, LIGHT_BACKGROUND, PRIMARY } = require( '../../../src/theme.cjs' ); const openFromMenu = ( app ) => app.evaluate( ( { Menu } ) => Menu.getApplicationMenu().getMenuItemById( 'settings' ).click() ); @@ -328,3 +328,74 @@ test( 'the theme set in the settings is the one the window is painted in, and a await ui.settingsButton( again.page ).click(); await expect( ui.settingsDialog( again.page ).getByRole( 'radio', { name: 'Dark', exact: true } ) ).toBeChecked(); } ); + +test( 'a custom theme is built from the two colours chosen, is kept, and a colour that is not one is refused (#560)', async ( { session } ) => { + const site = await makeSite( session ); + const { app, page } = await session.start( site.settings, { colorScheme: null } ); + const themeSource = () => app.evaluate( ( { nativeTheme } ) => nativeTheme.themeSource ); + const windowColour = ( electronApp ) => electronApp.evaluate( ( { BrowserWindow } ) => BrowserWindow.getAllWindows()[ 0 ].getBackgroundColor().toLowerCase() ); + const bodyColour = ( window ) => window.evaluate( () => window.getComputedStyle( document.body ).backgroundColor ); + const tokenColour = ( token ) => ui.tokenColour( page, token ); + const stored = ( key ) => session.readSettings().preferences?.[ key ]; + + await ui.settingsButton( page ).click(); + const dialog = ui.settingsDialog( page ); + const themes = dialog.getByRole( 'radiogroup', { name: 'Theme', exact: true } ); + const background = dialog.getByLabel( 'Background', { exact: true } ); + const primary = dialog.getByLabel( 'Primary', { exact: true } ); + + // INVARIANT — the colours are asked for under Custom alone, and start as + // the light theme's. + await expect( background ).toHaveCount( 0 ); + await themes.getByRole( 'radio', { name: 'Custom', exact: true } ).click(); + await expect.poll( () => stored( 'theme' ) ).toBe( 'custom' ); + await expect( background ).toHaveValue( LIGHT_BACKGROUND ); + await expect( primary ).toHaveValue( PRIMARY ); + await expect( dialog.getByLabel( 'Background colour picker', { exact: true } ) ).toHaveValue( LIGHT_BACKGROUND ); + + // INVARIANT — a dark background typed and entered is kept as the settings + // keep a colour, Electron is told the scheme it comes to, and the page + // and the window are painted in it at once. + await background.fill( '#102030' ); + await background.press( 'Enter' ); + await expect.poll( () => stored( 'customBackground' ) ).toBe( '#102030' ); + await expect.poll( themeSource ).toBe( 'dark' ); + await expect.poll( () => bodyColour( page ) ).toBe( 'rgb(16, 32, 48)' ); + expect( await bodyColour( page ) ).toBe( await tokenColour( 'var(--wpds-color-background-surface-neutral)' ) ); + await expect.poll( () => windowColour( app ) ).toBe( '#102030' ); + + // INVARIANT — the primary colour, in three digits, is kept in six, and + // the field shows it as kept; the brand surface is built from it. + await primary.fill( 'F80' ); + await primary.press( 'Tab' ); + await expect.poll( () => stored( 'customPrimary' ) ).toBe( '#ff8800' ); + await expect( primary ).toHaveValue( '#ff8800' ); + await expect.poll( () => tokenColour( 'var(--wpds-color-background-interactive-brand-strong)' ) ).toBe( 'rgb(255, 136, 0)' ); + + // INVARIANT — a colour that is not one is refused in main's words, and + // what is kept stays, in the store and in the field. + await background.fill( 'navy' ); + await background.press( 'Enter' ); + await expect( dialog.getByRole( 'alert' ) ).toHaveText( 'Choose a colour as six hex digits, like #3858e9.' ); + await expect( background ).toHaveValue( '#102030' ); + expect( stored( 'customBackground' ) ).toBe( '#102030' ); + + // INVARIANT — a light background makes a light theme again, with the + // native controls to match. + await background.fill( '#fff8e1' ); + await background.press( 'Enter' ); + await expect.poll( themeSource ).toBe( 'light' ); + await expect.poll( () => bodyColour( page ) ).toBe( 'rgb(255, 248, 225)' ); + + // INVARIANT — started again, the theme is the custom one, the window is + // made in its background, and the fields show the colours kept. + const again = await session.restart(); + expect( await again.app.evaluate( ( { nativeTheme } ) => nativeTheme.themeSource ) ).toBe( 'light' ); + expect( await windowColour( again.app ) ).toBe( '#fff8e1' ); + await expect.poll( () => bodyColour( again.page ) ).toBe( 'rgb(255, 248, 225)' ); + await ui.settingsButton( again.page ).click(); + const kept = ui.settingsDialog( again.page ); + await expect( kept.getByRole( 'radio', { name: 'Custom', exact: true } ) ).toBeChecked(); + await expect( kept.getByLabel( 'Background', { exact: true } ) ).toHaveValue( '#fff8e1' ); + await expect( kept.getByLabel( 'Primary', { exact: true } ) ).toHaveValue( '#ff8800' ); +} ); diff --git a/tests/unit/ipc-wiring.test.cjs b/tests/unit/ipc-wiring.test.cjs index 3482d019..32559013 100644 --- a/tests/unit/ipc-wiring.test.cjs +++ b/tests/unit/ipc-wiring.test.cjs @@ -1887,7 +1887,7 @@ test('settings:set asks the disk whether the folder is there', async (t) => { const settings = fakeSettingsStore(); const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs } }); - assert.deepEqual(await main.invoke('settings:set', 'newSiteLocation', folder), { ok: true, settings: { locale: null, phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', newSiteLocation: folder } }); + assert.deepEqual(await main.invoke('settings:set', 'newSiteLocation', folder), { ok: true, settings: { locale: null, phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', customBackground: '#fcfcfc', customPrimary: '#3858e9', newSiteLocation: folder } }); const gone = await main.invoke('settings:set', 'newSiteLocation', path.join(folder, 'gone')); assert.equal(gone.ok, false); assert.equal(settings.values.preferences.newSiteLocation, folder); @@ -1919,7 +1919,7 @@ test('settings:set gives the theme to Electron as it is kept, and the fallback w assert.equal(settings.values.preferences.theme, 'dark'); // A refusal leaves it. - assert.equal((await main.invoke('settings:set', 'theme', 'custom')).ok, false); + assert.equal((await main.invoke('settings:set', 'theme', 'blue')).ok, false); assert.equal(main.electron.nativeTheme.themeSource, 'dark'); assert.equal((await main.invoke('settings:set', 'theme', null)).settings.theme, 'system'); @@ -1931,6 +1931,39 @@ test('settings:set gives the theme to Electron as it is kept, and the fallback w assert.equal(main.electron.nativeTheme.themeSource, 'light'); }); +// A custom theme (#560) is given to Electron as the scheme its background +// comes to, so the native controls match the page, and the window is made +// and painted in that background, not the standard theme's. +test('settings:set gives Electron a custom theme\'s scheme, and paints the window its background (#560)', async () => { + const settings = fakeSettingsStore(); + const main = loadMain({ ready: true, stubs: { ...silentLogging(), ...settings.stubs, './i18n.cjs': { resolveCatalog: async () => null } } }); + await menuBuilt(main); + const [window] = main.windows; + + await main.invoke('settings:set', 'customBackground', '#102030'); + assert.deepEqual(window.backgrounds, [LIGHT_BACKGROUND], 'a colour of a theme that is not the one in use changes nothing'); + await main.invoke('settings:set', 'theme', 'custom'); + assert.equal(main.electron.nativeTheme.themeSource, 'dark'); + assert.deepEqual(window.backgrounds, [LIGHT_BACKGROUND, '#102030']); + + await main.invoke('settings:set', 'customBackground', '#FFF8E1'); + assert.equal(main.electron.nativeTheme.themeSource, 'light'); + assert.deepEqual(window.backgrounds, [LIGHT_BACKGROUND, '#102030', '#fff8e1']); + assert.equal(settings.values.preferences.customBackground, '#fff8e1', 'kept as the settings keep a colour'); + + // The primary colour is the page's; the window's colour is unchanged. + await main.invoke('settings:set', 'customPrimary', '#ff8800'); + assert.deepEqual(window.backgrounds, [LIGHT_BACKGROUND, '#102030', '#fff8e1', '#fff8e1']); + assert.equal((await main.invoke('settings:set', 'customPrimary', 'orange')).ok, false); + assert.equal(settings.values.preferences.customPrimary, '#ff8800'); + + // The Trac window is given the theme's colour too. + const openAndScrape = spy(async () => ({ status: 'ok', items: [], ticket: {} })); + const trac = loadMain({ ready: true, stubs: { ...silentLogging(), ...fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': {} }, preferences: { theme: 'custom', customBackground: '#102030' } }).stubs, './i18n.cjs': { resolveCatalog: async () => null }, './trac-view': { openAndScrape, fetchAttachment: async () => ({}) } } }); + await menuBuilt(trac); + assert.equal(trac.windows[0].options.backgroundColor, '#102030', 'the window is made in the custom background'); +}); + test('the ready path gives Electron the stored theme before the window is made, and makes the window in it (#560)', async () => { const main = loadMain({ ready: true, stubs: { ...silentLogging(), ...fakeSettingsStore({ preferences: { theme: 'dark' } }).stubs, './i18n.cjs': { resolveCatalog: async () => null } } }); await menuBuilt(main); @@ -3434,7 +3467,7 @@ test('trac:list-attachments opens the Trac window for a Trac site, and refuses a const coreMain = loadMain({ stubs: { ...silentLogging(), ...coreSettings.stubs, './trac-view': { openAndScrape, fetchAttachment: async () => ({}) } } }); const read = await coreMain.invoke('trac:list-attachments', core); assert.equal(read.status, 'ok'); - assert.deepEqual(openAndScrape.calls, [[49661]]); + assert.deepEqual(openAndScrape.calls, [[49661, { backgroundColor: LIGHT_BACKGROUND }]], 'opened in the colour of the app\'s theme'); }); // git:list-ticket-patches reads the stored ticket, then delegates to github-prs @@ -6755,10 +6788,10 @@ test('settings:set keeps a language the build has, and refuses one it has not (# stubs: { ...silentLogging(), ...settings.stubs, './i18n.cjs': { resolveCatalog: async () => null, languageChoices: () => [{ tag: 'de', label: 'Deutsch' }, { tag: 'en', label: 'English' }] } } }); - assert.deepEqual(await main.invoke('settings:set', 'locale', 'de'), { ok: true, settings: { locale: 'de', phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', newSiteLocation: null } }); + assert.deepEqual(await main.invoke('settings:set', 'locale', 'de'), { ok: true, settings: { locale: 'de', phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', customBackground: '#fcfcfc', customPrimary: '#3858e9', newSiteLocation: null } }); assert.equal((await main.invoke('settings:set', 'locale', 'fr')).ok, false); assert.equal(settings.values.preferences.locale, 'de'); - assert.deepEqual(await main.invoke('settings:set', 'locale', null), { ok: true, settings: { locale: null, phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', newSiteLocation: null } }); + assert.deepEqual(await main.invoke('settings:set', 'locale', null), { ok: true, settings: { locale: null, phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', customBackground: '#fcfcfc', customPrimary: '#3858e9', newSiteLocation: null } }); }); test('app:relaunch relaunches through a quit, so the child sweep runs, without the launch\'s link or --lang (#559)', async (t) => { diff --git a/tests/unit/settings-view.test.cjs b/tests/unit/settings-view.test.cjs index e2c649cf..24e171fd 100644 --- a/tests/unit/settings-view.test.cjs +++ b/tests/unit/settings-view.test.cjs @@ -74,11 +74,12 @@ test('the quit control offers stop and restart, and not the prototype\'s leaving assert.ok(quitItems().every((item) => item.label)); }); -test('the theme control offers light, dark and system, in that order, and not the prototype\'s custom colours (#560)', () => { +test('the theme control offers light, dark, system and custom, in that order (#560)', () => { assert.deepEqual(themeItems(), [ { value: 'light', label: 'Light' }, { value: 'dark', label: 'Dark' }, - { value: 'system', label: 'System' } + { value: 'system', label: 'System' }, + { value: 'custom', label: 'Custom' } ]); }); diff --git a/tests/unit/settings.test.cjs b/tests/unit/settings.test.cjs index 5c499e98..072c7866 100644 --- a/tests/unit/settings.test.cjs +++ b/tests/unit/settings.test.cjs @@ -12,19 +12,19 @@ const languages = (tags) => ({ isLanguage: (tag) => tags.includes(tag) }); const php = (versions) => ({ isPhpVersion: (version) => versions.includes(version) }); test('readSettings falls back for a store with nothing in it, and for values of the wrong kind', () => { - const fallbacks = { locale: null, phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', newSiteLocation: null }; + const fallbacks = { locale: null, phpVersion: '8.3', wpDebug: true, scriptDebug: true, autoStartServer: false, autoStartWatch: false, quitBehavior: 'stop', theme: 'system', customBackground: '#fcfcfc', customPrimary: '#3858e9', newSiteLocation: null }; assert.deepEqual(readSettings(), fallbacks); assert.deepEqual(readSettings(undefined), fallbacks); assert.deepEqual(readSettings({}), fallbacks); - assert.deepEqual(readSettings({ newSiteLocation: 42, locale: ['de'], phpVersion: 8.4, wpDebug: 'false', scriptDebug: 0, autoStartServer: 'yes', autoStartWatch: 1, quitBehavior: 'leave', theme: 'custom' }), fallbacks); + assert.deepEqual(readSettings({ newSiteLocation: 42, locale: ['de'], phpVersion: 8.4, wpDebug: 'false', scriptDebug: 0, autoStartServer: 'yes', autoStartWatch: 1, quitBehavior: 'leave', theme: 'blue', customBackground: '#ABC', customPrimary: 'red' }), fallbacks); assert.deepEqual(readSettings({ newSiteLocation: '', locale: '', phpVersion: '', wpDebug: null, scriptDebug: null }), fallbacks); assert.deepEqual(readSettings('not an object'), fallbacks); }); test('readSettings gives back a stored folder without asking the disk about it', () => { assert.deepEqual( - readSettings({ newSiteLocation: '/Users/jane/sites', locale: 'de', phpVersion: '8.4', wpDebug: false, scriptDebug: false, autoStartServer: true, autoStartWatch: true, quitBehavior: 'restart', theme: 'dark' }), - { locale: 'de', phpVersion: '8.4', wpDebug: false, scriptDebug: false, autoStartServer: true, autoStartWatch: true, quitBehavior: 'restart', theme: 'dark', newSiteLocation: '/Users/jane/sites' } + readSettings({ newSiteLocation: '/Users/jane/sites', locale: 'de', phpVersion: '8.4', wpDebug: false, scriptDebug: false, autoStartServer: true, autoStartWatch: true, quitBehavior: 'restart', theme: 'custom', customBackground: '#102030', customPrimary: '#ff8800' }), + { locale: 'de', phpVersion: '8.4', wpDebug: false, scriptDebug: false, autoStartServer: true, autoStartWatch: true, quitBehavior: 'restart', theme: 'custom', customBackground: '#102030', customPrimary: '#ff8800', newSiteLocation: '/Users/jane/sites' } ); }); @@ -91,15 +91,26 @@ test('a debug flag is on or off, nothing means the fallback, and a string is ref } }); -test('the theme is light, dark or system, nothing means the system\'s, and the prototype\'s custom is refused (#560)', () => { - for (const theme of ['light', 'dark', 'system']) assert.deepEqual(acceptSetting('theme', theme, {}), { ok: true, value: theme }); +test('the theme is light, dark, system or custom, nothing means the system\'s, and anything else is refused (#560)', () => { + for (const theme of ['light', 'dark', 'system', 'custom']) assert.deepEqual(acceptSetting('theme', theme, {}), { ok: true, value: theme }); assert.deepEqual(acceptSetting('theme', null, {}), { ok: true, value: null }); assert.deepEqual(acceptSetting('theme', '', {}), { ok: true, value: null }); - assert.deepEqual(acceptSetting('theme', 'custom', {}), { ok: false, error: 'Choose light, dark, or your system’s theme.' }); + assert.deepEqual(acceptSetting('theme', 'blue', {}), { ok: false, error: 'Choose light, dark, your system’s theme, or custom.' }); assert.equal(acceptSetting('theme', 'Dark', {}).ok, false); assert.equal(acceptSetting('theme', true, {}).ok, false); }); +test('a custom theme\'s colour is kept as six lowercase hex digits from what was typed, nothing means the fallback, and anything else is refused (#560)', () => { + for (const key of ['customBackground', 'customPrimary']) { + assert.deepEqual(acceptSetting(key, '#102030', {}), { ok: true, value: '#102030' }, key); + assert.deepEqual(acceptSetting(key, 'ABC', {}), { ok: true, value: '#aabbcc' }, key); + assert.deepEqual(acceptSetting(key, null, {}), { ok: true, value: null }, key); + assert.deepEqual(acceptSetting(key, '', {}), { ok: true, value: null }, key); + assert.deepEqual(acceptSetting(key, 'blue', {}), { ok: false, error: 'Choose a colour as six hex digits, like #3858e9.' }, key); + assert.equal(acceptSetting(key, 42, {}).ok, false, key); + } +}); + test('what happens on quit is stop or restart, nothing means the fallback, and anything else is refused', () => { assert.deepEqual(acceptSetting('quitBehavior', 'restart', {}), { ok: true, value: 'restart' }); assert.deepEqual(acceptSetting('quitBehavior', 'stop', {}), { ok: true, value: 'stop' }); diff --git a/tests/unit/theme.test.cjs b/tests/unit/theme.test.cjs index 89c22f42..597301ed 100644 --- a/tests/unit/theme.test.cjs +++ b/tests/unit/theme.test.cjs @@ -3,27 +3,65 @@ const test = require('node:test'); const assert = require('node:assert/strict'); -const { THEMES, themeColorSeeds, windowBackground, DARK_BACKGROUND } = require('../../src/theme.cjs'); +const { THEMES, THEME_KEYS, resolveTheme, nativeThemeSource, normalizeHexColor, isHexColor, isDarkColor, DARK_BACKGROUND, LIGHT_BACKGROUND, PRIMARY } = require('../../src/theme.cjs'); -test('the themes are light, dark and system: the prototype\'s custom colours are not one (#560)', () => { - assert.deepEqual(THEMES, ['light', 'dark', 'system']); +test('the themes are light, dark, system and custom, and the theme is made of three settings (#560)', () => { + assert.deepEqual(THEMES, ['light', 'dark', 'system', 'custom']); + assert.deepEqual(THEME_KEYS, ['theme', 'customBackground', 'customPrimary']); }); // In the light scheme the provider is given no colour, so the tokens // stylesheet's values stand as they ship: generating them again from a seed // would move every colour by a hair, and the light theme is what every // picture in the guide shows. In the dark scheme it is given the dark seed -// and builds every token from it. -test('the provider is seeded in the dark scheme only, and from the dark background', () => { - assert.deepEqual(themeColorSeeds(false), {}); - assert.deepEqual(themeColorSeeds(true), { background: DARK_BACKGROUND }); +// and builds every token from it. The window's colour and the provider's +// seed are the one colour, or the page changes colour as it mounts. +test('light, dark and system come to the two schemes, seeded in the dark one only', () => { + assert.deepEqual(resolveTheme({ theme: 'light' }), { scheme: 'light', seeds: {}, background: LIGHT_BACKGROUND, key: 'light' }); + assert.deepEqual(resolveTheme({ theme: 'dark' }), { scheme: 'dark', seeds: { background: DARK_BACKGROUND }, background: DARK_BACKGROUND, key: 'dark' }); + assert.deepEqual(resolveTheme({ theme: 'system', systemDark: false }), resolveTheme({ theme: 'light' })); + assert.deepEqual(resolveTheme({ theme: 'system', systemDark: true }), resolveTheme({ theme: 'dark' })); + assert.deepEqual(resolveTheme({ theme: 'system' }), resolveTheme({ theme: 'light' }), 'nothing known of the system is light'); }); -// The window's colour and the provider's seed have to be the one colour, or -// the page changes colour as it mounts; and both have to be what Electron -// takes for a window's colour, `#rrggbb`. -test('a window is made in the colour of its scheme, which in the dark scheme is the seed the theme is built from', () => { - assert.equal(windowBackground(true), themeColorSeeds(true).background); - assert.notEqual(windowBackground(false), windowBackground(true)); - for (const colour of [windowBackground(true), windowBackground(false)]) assert.match(colour, /^#[0-9a-f]{6}$/); +// A custom theme is its two colours, and a scheme by its background, so the +// native controls Chromium paints for the scheme match the page. +test('a custom theme is seeded from its two colours, is dark when its background is, and the window is made in its background', () => { + const dark = resolveTheme({ theme: 'custom', customBackground: '#102030', customPrimary: '#ff8800' }); + assert.deepEqual(dark, { scheme: 'dark', seeds: { background: '#102030', primary: '#ff8800' }, background: '#102030', key: 'custom:#102030:#ff8800' }); + const light = resolveTheme({ theme: 'custom', customBackground: '#fff8e1', customPrimary: '#3858e9', systemDark: true }); + assert.equal(light.scheme, 'light', 'the system\'s theme has no say'); + assert.equal(light.background, '#fff8e1'); + // A colour that is not one falls back to the light theme's, so the + // provider is never given something it cannot build from. + assert.deepEqual(resolveTheme({ theme: 'custom', customBackground: 'blue', customPrimary: undefined }).seeds, { background: LIGHT_BACKGROUND, primary: PRIMARY }); + // Two themes that paint the same have the same name, and two that do not + // do not. + assert.equal(dark.key, resolveTheme({ theme: 'custom', customBackground: '#102030', customPrimary: '#ff8800' }).key); + assert.notEqual(dark.key, resolveTheme({ theme: 'custom', customBackground: '#102030', customPrimary: '#ff8801' }).key); +}); + +test('a background is dark when white text reads better on it than black', () => { + assert.equal(isDarkColor('#000000'), true); + assert.equal(isDarkColor('#1e1e1e'), true); + assert.equal(isDarkColor('#3858e9'), true, 'the brand blue takes white text'); + assert.equal(isDarkColor('#ffffff'), false); + assert.equal(isDarkColor('#fcfcfc'), false); + assert.equal(isDarkColor('#808080'), false, 'a mid grey still takes black text'); +}); + +test('Electron is given the setting for the three it knows, and a custom theme\'s scheme', () => { + for (const theme of ['light', 'dark', 'system']) assert.equal(nativeThemeSource({ theme }), theme); + assert.equal(nativeThemeSource({ theme: 'custom', customBackground: '#102030', customPrimary: PRIMARY }), 'dark'); + assert.equal(nativeThemeSource({ theme: 'custom', customBackground: '#fcfcfc', customPrimary: PRIMARY }), 'light'); +}); + +test('a colour is kept as six lowercase hex digits, from three or six as typed, and anything else is not one', () => { + assert.equal(normalizeHexColor('#3858E9'), '#3858e9'); + assert.equal(normalizeHexColor(' 3858e9 '), '#3858e9'); + assert.equal(normalizeHexColor('#abc'), '#aabbcc'); + assert.equal(normalizeHexColor('ABC'), '#aabbcc'); + for (const bad of ['', '#', '#12345', '#1234567', 'blue', 'rgb(1,2,3)', '#ggg', 42, null, undefined]) assert.equal(normalizeHexColor(bad), null, String(bad)); + assert.equal(isHexColor('#3858e9'), true); + for (const bad of ['#3858E9', '#abc', '3858e9', '', null]) assert.equal(isHexColor(bad), false, String(bad)); }); diff --git a/tests/unit/trac-view.test.cjs b/tests/unit/trac-view.test.cjs index 0ee8733a..3b02be68 100644 --- a/tests/unit/trac-view.test.cjs +++ b/tests/unit/trac-view.test.cjs @@ -36,7 +36,6 @@ test('openAndScrape: a navigation that never finishes still reaches the ready ti const electron = { BrowserWindow: BrowserWindowStub, - nativeTheme: { shouldUseDarkColors: false }, session: { fromPartition: () => ({ setUserAgent() {} }) } }; const { openAndScrape } = loadTracView(electron); @@ -55,10 +54,11 @@ test('openAndScrape: a navigation that never finishes still reaches the ready ti // The window is shown when Trac's check needs a click, and where Trac's page // does not paint it is the colour it was made with: the app's theme (#560), -// not white on a dark desktop. A deadline of now skips the poll. +// which main holds and passes, not white on a dark desktop; the light theme's +// where main passes nothing. A deadline of now skips the poll. test('openAndScrape: the Trac window is made in the colour of the app\'s theme', async () => { const { DARK_BACKGROUND, LIGHT_BACKGROUND } = require('../../src/theme.cjs'); - for (const [dark, colour] of [[true, DARK_BACKGROUND], [false, LIGHT_BACKGROUND]]) { + for (const [given, colour] of [[DARK_BACKGROUND, DARK_BACKGROUND], ['#102030', '#102030'], [undefined, LIGHT_BACKGROUND]]) { const made = []; class BrowserWindowStub { constructor(options) { @@ -76,14 +76,13 @@ test('openAndScrape: the Trac window is made in the colour of the app\'s theme', } const electron = { BrowserWindow: BrowserWindowStub, - nativeTheme: { shouldUseDarkColors: dark }, session: { fromPartition: () => ({ setUserAgent() {} }) } }; const { openAndScrape } = loadTracView(electron); - await openAndScrape(56320, { readyTimeoutMs: 0 }); + await openAndScrape(56320, { readyTimeoutMs: 0, backgroundColor: given }); assert.equal(made.length, 1); - assert.equal(made[0].backgroundColor, colour, `dark: ${dark}`); + assert.equal(made[0].backgroundColor, colour, `given: ${given}`); } }); From 7fdf4e0768e7b179e04591e193c7fef771dc3f20 Mon Sep 17 00:00:00 2001 From: Francesco Bigiarini Date: Wed, 7 Oct 2026 11:09:25 +0200 Subject: [PATCH 2/5] Drive the colour picker from the draft, and name what the Trac assertion tests From the first review pass. A colour picked with the picker was never kept: the input was controlled by the colour kept, so it was put back to it after every step of a drag, and the picker's closing read the old colour. It is driven from the draft now, and the journey picks a colour. The fallback before the settings arrive is the module's 'system', not a branch in the component; the hex fields are in the terminal's type, which the rule for them reached only with a type on the input; the wiring test's Trac assertion opens a Trac window now, and the main window's colour is named as its own. The dark crossover is pinned, a colour spelled another way is shown as kept, and a prop nothing passed is gone. Co-Authored-By: Claude Fable 5.1 --- src/renderer/components/app-theme.jsx | 8 +++--- src/renderer/components/settings-dialog.jsx | 29 ++++++++++++--------- tests/e2e/journeys/settings.spec.js | 15 ++++++++++- tests/unit/ipc-wiring.test.cjs | 14 ++++++---- tests/unit/theme.test.cjs | 3 +++ 5 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/renderer/components/app-theme.jsx b/src/renderer/components/app-theme.jsx index 4471e1ad..1ce5c485 100644 --- a/src/renderer/components/app-theme.jsx +++ b/src/renderer/components/app-theme.jsx @@ -37,13 +37,13 @@ function usePrefersDark() { // they read, and for the app's own styles. // // `settings` is what main holds, or null until it has answered: until then -// the window is painted for the scheme alone, which for a custom theme is -// the standard theme of its scheme for the moment before the colours arrive. +// the window is painted as under 'system', for the scheme alone, which for a +// custom theme is the standard theme of its scheme for the moment before +// the colours arrive. export function AppTheme({ settings, children }) { const prefersDark = usePrefersDark(); const [warnings, setWarnings] = useState([]); - let theme = prefersDark ? 'dark' : 'light'; - if (settings) theme = settings.theme; + const theme = settings ? settings.theme : 'system'; const customBackground = settings ? settings.customBackground : undefined; const customPrimary = settings ? settings.customPrimary : undefined; const resolved = useMemo( diff --git a/src/renderer/components/settings-dialog.jsx b/src/renderer/components/settings-dialog.jsx index 357a1ca9..13681fc2 100644 --- a/src/renderer/components/settings-dialog.jsx +++ b/src/renderer/components/settings-dialog.jsx @@ -7,6 +7,7 @@ import { __experimentalToggleGroupControl as ToggleGroupControl, __experimentalT import { __, sprintf } from '@wordpress/i18n'; import { Button, Dialog, InputControl, Notice, SelectControl, Stack, SwitchControl, Tabs, Text } from '@wordpress/ui'; import { useThemeWarnings } from './app-theme.jsx'; +import { normalizeHexColor } from '../../theme.cjs'; import { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, SYSTEM_LANGUAGE } from '../settings-view.cjs'; import { FolderField } from './folder-field.jsx'; @@ -84,20 +85,23 @@ function LanguageControl({ settings, loaded, onChange }) { // One colour of the custom theme (#560): typed as hex, or picked with the // system's picker, which is the swatch before the field, showing the colour -// kept. What is typed is kept when the field is left or Enter is pressed, -// and main says what it accepts, so a colour that is not one is refused in -// main's words and the field goes back to what is kept. The picker's choice -// is kept when the picker is closed, not as it is dragged: each keep is a -// write to the store, and the field shows the colour under the pointer -// meanwhile. -function ColorField({ label, value, disabled, onKeep }) { +// being chosen. What is typed is kept when the field is left or Enter is +// pressed, and main says what it accepts, so a colour that is not one is +// refused in main's words and the field goes back to what is kept. The +// picker's choice is kept when the picker is closed, not as it is dragged: +// each keep is a write to the store, and the field shows the colour under +// the pointer meanwhile. The picker is driven from the draft and not from +// what is kept: a controlled input is put back to its prop after every +// step of a drag, and the picker's closing would then read the old colour. +function ColorField({ label, value, onKeep }) { const [draft, setDraft] = useState(value); useEffect(() => setDraft(value), [value]); - // A refusal leaves what is kept as it was, so nothing above changes the - // draft back: it is put back here. + // What is kept is put back in the field after a refusal, which leaves + // what is kept as it was, and after a keep of a colour spelled another + // way (`F80` for `#ff8800`), which leaves it as it was too. const keep = async (text) => { const result = await onKeep(text); - if (!result?.ok) setDraft(value); + setDraft(result?.ok ? (normalizeHexColor(text) ?? value) : value); }; const picker = useRef(null); useEffect(() => { @@ -111,9 +115,9 @@ function ColorField({ label, value, disabled, onKeep }) { return ( setDraft(event.currentTarget.value)} /> } diff --git a/tests/e2e/journeys/settings.spec.js b/tests/e2e/journeys/settings.spec.js index 3cefd02e..9b783170 100644 --- a/tests/e2e/journeys/settings.spec.js +++ b/tests/e2e/journeys/settings.spec.js @@ -371,6 +371,19 @@ test( 'a custom theme is built from the two colours chosen, is kept, and a colou await expect.poll( () => stored( 'customPrimary' ) ).toBe( '#ff8800' ); await expect( primary ).toHaveValue( '#ff8800' ); await expect.poll( () => tokenColour( 'var(--wpds-color-background-interactive-brand-strong)' ) ).toBe( 'rgb(255, 136, 0)' ); + // Spelled another way, the same colour is kept as it was, and the field + // shows it as kept. + await primary.fill( 'ff8800' ); + await primary.press( 'Tab' ); + await expect( primary ).toHaveValue( '#ff8800' ); + + // INVARIANT — a colour picked with the picker is kept when the picker is + // done, which is what a fill of a colour input is: the input, then the + // change. + await dialog.getByLabel( 'Primary colour picker', { exact: true } ).fill( '#204060' ); + await expect.poll( () => stored( 'customPrimary' ) ).toBe( '#204060' ); + await expect( primary ).toHaveValue( '#204060' ); + await expect.poll( () => tokenColour( 'var(--wpds-color-background-interactive-brand-strong)' ) ).toBe( 'rgb(32, 64, 96)' ); // INVARIANT — a colour that is not one is refused in main's words, and // what is kept stays, in the store and in the field. @@ -397,5 +410,5 @@ test( 'a custom theme is built from the two colours chosen, is kept, and a colou const kept = ui.settingsDialog( again.page ); await expect( kept.getByRole( 'radio', { name: 'Custom', exact: true } ) ).toBeChecked(); await expect( kept.getByLabel( 'Background', { exact: true } ) ).toHaveValue( '#fff8e1' ); - await expect( kept.getByLabel( 'Primary', { exact: true } ) ).toHaveValue( '#ff8800' ); + await expect( kept.getByLabel( 'Primary', { exact: true } ) ).toHaveValue( '#204060' ); } ); diff --git a/tests/unit/ipc-wiring.test.cjs b/tests/unit/ipc-wiring.test.cjs index 32559013..e91a025f 100644 --- a/tests/unit/ipc-wiring.test.cjs +++ b/tests/unit/ipc-wiring.test.cjs @@ -1934,7 +1934,7 @@ test('settings:set gives the theme to Electron as it is kept, and the fallback w // A custom theme (#560) is given to Electron as the scheme its background // comes to, so the native controls match the page, and the window is made // and painted in that background, not the standard theme's. -test('settings:set gives Electron a custom theme\'s scheme, and paints the window its background (#560)', async () => { +test('settings:set gives Electron a custom theme\'s scheme, and paints the window its background (#560)', async (t) => { const settings = fakeSettingsStore(); const main = loadMain({ ready: true, stubs: { ...silentLogging(), ...settings.stubs, './i18n.cjs': { resolveCatalog: async () => null } } }); await menuBuilt(main); @@ -1957,11 +1957,15 @@ test('settings:set gives Electron a custom theme\'s scheme, and paints the windo assert.equal((await main.invoke('settings:set', 'customPrimary', 'orange')).ok, false); assert.equal(settings.values.preferences.customPrimary, '#ff8800'); - // The Trac window is given the theme's colour too. + // Started with a custom theme kept, the main window is made in its + // background, and the Trac window is opened in it too. + const core = await fixtureRepo(t); const openAndScrape = spy(async () => ({ status: 'ok', items: [], ticket: {} })); - const trac = loadMain({ ready: true, stubs: { ...silentLogging(), ...fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': {} }, preferences: { theme: 'custom', customBackground: '#102030' } }).stubs, './i18n.cjs': { resolveCatalog: async () => null }, './trac-view': { openAndScrape, fetchAttachment: async () => ({}) } } }); - await menuBuilt(trac); - assert.equal(trac.windows[0].options.backgroundColor, '#102030', 'the window is made in the custom background'); + const custom = loadMain({ ready: true, stubs: { ...silentLogging(), ...fakeSettingsStore({ sites: [core], siteMeta: { [core]: { tracTicket: 49661 } }, preferences: { theme: 'custom', customBackground: '#102030' } }).stubs, './i18n.cjs': { resolveCatalog: async () => null }, './trac-view': { openAndScrape, fetchAttachment: async () => ({}) } } }); + await menuBuilt(custom); + assert.equal(custom.windows[0].options.backgroundColor, '#102030', 'the main window is made in the custom background'); + await custom.invoke('trac:list-attachments', core); + assert.deepEqual(openAndScrape.calls, [[49661, { backgroundColor: '#102030' }]], 'the Trac window is opened in it'); }); test('the ready path gives Electron the stored theme before the window is made, and makes the window in it (#560)', async () => { diff --git a/tests/unit/theme.test.cjs b/tests/unit/theme.test.cjs index 597301ed..189790ce 100644 --- a/tests/unit/theme.test.cjs +++ b/tests/unit/theme.test.cjs @@ -48,6 +48,9 @@ test('a background is dark when white text reads better on it than black', () => assert.equal(isDarkColor('#ffffff'), false); assert.equal(isDarkColor('#fcfcfc'), false); assert.equal(isDarkColor('#808080'), false, 'a mid grey still takes black text'); + // The crossover, where the design system's own ramp turns too. + assert.equal(isDarkColor('#757575'), true); + assert.equal(isDarkColor('#767676'), false); }); test('Electron is given the setting for the three it knows, and a custom theme\'s scheme', () => { From 0a9965d8c1a555b4b03e30752ec04ed6c61915af Mon Sep 17 00:00:00 2001 From: Francesco Bigiarini Date: Wed, 7 Oct 2026 11:19:54 +0200 Subject: [PATCH 3/5] Decide what a colour field shows in the view module, and keep newer typing From the second review pass. What the field shows once main has answered a keep, and what the picker holds while the field holds no colour, are settings-view's to say, with their cases in its suite; an answer to an older draft leaves what has been typed since. The wiring test's Trac assertion seeds a path rather than building a repository it never opens, and the journey says where its two exact colours come from. Co-Authored-By: Claude Fable 5.1 --- src/renderer/components/settings-dialog.jsx | 16 +++++----- src/renderer/settings-view.cjs | 35 ++++++++++++++++++++- tests/e2e/journeys/settings.spec.js | 6 ++++ tests/unit/ipc-wiring.test.cjs | 10 +++--- tests/unit/settings-view.test.cjs | 18 ++++++++++- 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/src/renderer/components/settings-dialog.jsx b/src/renderer/components/settings-dialog.jsx index 13681fc2..4f06ccba 100644 --- a/src/renderer/components/settings-dialog.jsx +++ b/src/renderer/components/settings-dialog.jsx @@ -7,8 +7,7 @@ import { __experimentalToggleGroupControl as ToggleGroupControl, __experimentalT import { __, sprintf } from '@wordpress/i18n'; import { Button, Dialog, InputControl, Notice, SelectControl, Stack, SwitchControl, Tabs, Text } from '@wordpress/ui'; import { useThemeWarnings } from './app-theme.jsx'; -import { normalizeHexColor } from '../../theme.cjs'; -import { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, SYSTEM_LANGUAGE } from '../settings-view.cjs'; +import { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, colorFieldDraft, pickerValue, SYSTEM_LANGUAGE } from '../settings-view.cjs'; import { FolderField } from './folder-field.jsx'; // A notice here is read by its role, and is not also spoken: the dialog it @@ -85,7 +84,8 @@ function LanguageControl({ settings, loaded, onChange }) { // One colour of the custom theme (#560): typed as hex, or picked with the // system's picker, which is the swatch before the field, showing the colour -// being chosen. What is typed is kept when the field is left or Enter is +// being chosen, or the colour kept while the field holds no colour. What is +// typed is kept when the field is left or Enter is // pressed, and main says what it accepts, so a colour that is not one is // refused in main's words and the field goes back to what is kept. The // picker's choice is kept when the picker is closed, not as it is dragged: @@ -96,12 +96,12 @@ function LanguageControl({ settings, loaded, onChange }) { function ColorField({ label, value, onKeep }) { const [draft, setDraft] = useState(value); useEffect(() => setDraft(value), [value]); - // What is kept is put back in the field after a refusal, which leaves - // what is kept as it was, and after a keep of a colour spelled another - // way (`F80` for `#ff8800`), which leaves it as it was too. + // What the field shows once main has answered is settings-view's to say: + // what is kept after a refusal, the colour as kept after a keep, and what + // has been typed since where the answer is to an older draft. const keep = async (text) => { const result = await onKeep(text); - setDraft(result?.ok ? (normalizeHexColor(text) ?? value) : value); + setDraft((current) => colorFieldDraft({ current, sent: text, ok: Boolean(result?.ok), kept: value })); }; const picker = useRef(null); useEffect(() => { @@ -127,7 +127,7 @@ function ColorField({ label, value, onKeep }) { className="color-swatch" // translators: %s: what the colour is for, "Background" or "Primary". aria-label={sprintf(__('%s colour picker'), label)} - value={normalizeHexColor(draft) ?? value} + value={pickerValue(draft, value)} onChange={(event) => setDraft(event.currentTarget.value)} /> } diff --git a/src/renderer/settings-view.cjs b/src/renderer/settings-view.cjs index 08ff9953..d46030e4 100644 --- a/src/renderer/settings-view.cjs +++ b/src/renderer/settings-view.cjs @@ -9,6 +9,7 @@ */ const { __, _x, sprintf } = require('@wordpress/i18n'); +const { normalizeHexColor } = require('../theme.cjs'); /** * The line under "GitHub": whose account the app holds, or why none. @@ -139,6 +140,38 @@ function themeItems() { ]; } +/** + * What a colour field shows after main has answered a keep (#560): the + * colour as kept where it was kept, so `F80` reads `#ff8800`; what was kept + * before where it was refused; and whatever is in the field now where that + * is no longer what was sent, since the contributor has typed on while main + * answered and the answer is to an older draft. + * + * @param {Object} root0 + * @param {string} root0.current What the field holds now. + * @param {string} root0.sent What was sent to be kept. + * @param {boolean} root0.ok Whether main kept it. + * @param {string} root0.kept The colour kept, as main holds it after the answer. + * @return {string} What the field shows. + */ +function colorFieldDraft({ current, sent, ok, kept }) { + if (current !== sent) return current; + return ok ? (normalizeHexColor(sent) ?? kept) : kept; +} + +/** + * What the colour picker beside a field holds: the colour being typed, or + * the colour kept while what is typed is no colour, since a picker cannot + * hold anything else. + * + * @param {string} draft What the field holds. + * @param {string} kept The colour kept. + * @return {string} `#rrggbb`. + */ +function pickerValue(draft, kept) { + return normalizeHexColor(draft) ?? kept; +} + /** * What the next launch starts for a site, from the list the last quit left: * its server, its watch, both, or nothing. @@ -154,4 +187,4 @@ function resumeFor(resume, sitePath) { return server || watch ? { server, watch } : null; } -module.exports = { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, resumeFor, SYSTEM_LANGUAGE }; +module.exports = { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, colorFieldDraft, pickerValue, resumeFor, SYSTEM_LANGUAGE }; diff --git a/tests/e2e/journeys/settings.spec.js b/tests/e2e/journeys/settings.spec.js index 9b783170..2215e412 100644 --- a/tests/e2e/journeys/settings.spec.js +++ b/tests/e2e/journeys/settings.spec.js @@ -356,6 +356,12 @@ test( 'a custom theme is built from the two colours chosen, is kept, and a colou // INVARIANT — a dark background typed and entered is kept as the settings // keep a colour, Electron is told the scheme it comes to, and the page // and the window are painted in it at once. + // CHARACTERISATION — the body is the seed itself, and below the brand + // surface is the primary seed itself: the design system's ramps pin + // their first step to the seed (`surface2`, `bgFill1`, contrast 1 + // against it) and move it only where the seed cannot meet a contrast + // target, which these do not. A design-system release that rescales + // differently moves these two numbers and nothing else here. await background.fill( '#102030' ); await background.press( 'Enter' ); await expect.poll( () => stored( 'customBackground' ) ).toBe( '#102030' ); diff --git a/tests/unit/ipc-wiring.test.cjs b/tests/unit/ipc-wiring.test.cjs index e91a025f..4adb85df 100644 --- a/tests/unit/ipc-wiring.test.cjs +++ b/tests/unit/ipc-wiring.test.cjs @@ -1934,7 +1934,7 @@ test('settings:set gives the theme to Electron as it is kept, and the fallback w // A custom theme (#560) is given to Electron as the scheme its background // comes to, so the native controls match the page, and the window is made // and painted in that background, not the standard theme's. -test('settings:set gives Electron a custom theme\'s scheme, and paints the window its background (#560)', async (t) => { +test('settings:set gives Electron a custom theme\'s scheme, and paints the window its background (#560)', async () => { const settings = fakeSettingsStore(); const main = loadMain({ ready: true, stubs: { ...silentLogging(), ...settings.stubs, './i18n.cjs': { resolveCatalog: async () => null } } }); await menuBuilt(main); @@ -1958,13 +1958,13 @@ test('settings:set gives Electron a custom theme\'s scheme, and paints the windo assert.equal(settings.values.preferences.customPrimary, '#ff8800'); // Started with a custom theme kept, the main window is made in its - // background, and the Trac window is opened in it too. - const core = await fixtureRepo(t); + // background, and the Trac window is opened in it too. The handler reads + // the site's record and never its folder, so a path is enough. const openAndScrape = spy(async () => ({ status: 'ok', items: [], ticket: {} })); - const custom = loadMain({ ready: true, stubs: { ...silentLogging(), ...fakeSettingsStore({ sites: [core], siteMeta: { [core]: { tracTicket: 49661 } }, preferences: { theme: 'custom', customBackground: '#102030' } }).stubs, './i18n.cjs': { resolveCatalog: async () => null }, './trac-view': { openAndScrape, fetchAttachment: async () => ({}) } } }); + const custom = loadMain({ ready: true, stubs: { ...silentLogging(), ...fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': { tracTicket: 49661 } }, preferences: { theme: 'custom', customBackground: '#102030' } }).stubs, './i18n.cjs': { resolveCatalog: async () => null }, './trac-view': { openAndScrape, fetchAttachment: async () => ({}) } } }); await menuBuilt(custom); assert.equal(custom.windows[0].options.backgroundColor, '#102030', 'the main window is made in the custom background'); - await custom.invoke('trac:list-attachments', core); + await custom.invoke('trac:list-attachments', '/sites/wp'); assert.deepEqual(openAndScrape.calls, [[49661, { backgroundColor: '#102030' }]], 'the Trac window is opened in it'); }); diff --git a/tests/unit/settings-view.test.cjs b/tests/unit/settings-view.test.cjs index 24e171fd..26f3b4c2 100644 --- a/tests/unit/settings-view.test.cjs +++ b/tests/unit/settings-view.test.cjs @@ -3,7 +3,7 @@ const test = require('node:test'); const assert = require('node:assert/strict'); -const { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, resumeFor, SYSTEM_LANGUAGE } = require('../../src/renderer/settings-view.cjs'); +const { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, colorFieldDraft, pickerValue, resumeFor, SYSTEM_LANGUAGE } = require('../../src/renderer/settings-view.cjs'); test('the GitHub line says the account is still being read, and offers no sign-out, until it is', () => { assert.deepEqual(githubAccountLine(null), { text: 'Reading…', canSignOut: false }); @@ -83,6 +83,22 @@ test('the theme control offers light, dark, system and custom, in that order (#5 ]); }); +test('a colour field shows the colour as kept after a keep, what was kept after a refusal, and newer typing over an older answer (#560)', () => { + assert.equal(colorFieldDraft({ current: 'F80', sent: 'F80', ok: true, kept: '#3858e9' }), '#ff8800'); + assert.equal(colorFieldDraft({ current: '#ff8800', sent: '#ff8800', ok: true, kept: '#ff8800' }), '#ff8800'); + assert.equal(colorFieldDraft({ current: 'navy', sent: 'navy', ok: false, kept: '#102030' }), '#102030'); + // Main answered a draft the field no longer holds. + assert.equal(colorFieldDraft({ current: '#1020', sent: 'navy', ok: false, kept: '#102030' }), '#1020'); + assert.equal(colorFieldDraft({ current: '#10203', sent: '#102030', ok: true, kept: '#102030' }), '#10203'); +}); + +test('the picker holds the colour being typed, and the colour kept while what is typed is no colour (#560)', () => { + assert.equal(pickerValue('#204060', '#102030'), '#204060'); + assert.equal(pickerValue('ABC', '#102030'), '#aabbcc'); + assert.equal(pickerValue('nav', '#102030'), '#102030'); + assert.equal(pickerValue('', '#102030'), '#102030'); +}); + test('what the next launch starts for a site comes from the list the quit left', () => { const resume = { servers: ['/a', '/both'], watches: ['/w', '/both'] }; assert.deepEqual(resumeFor(resume, '/a'), { server: true, watch: false }); From 31c125c7ac7d15c1611b2250ab1df8c67e10b3fe Mon Sep 17 00:00:00 2001 From: Francesco Bigiarini Date: Wed, 7 Oct 2026 11:41:56 +0200 Subject: [PATCH 4/5] Stand the colour swatch in the design system's prefix slot, as a square The swatch sat in the field's row without the slot the design system pads and centres a prefix with: it stuck to the top of the row, the row shrank it narrower than it is tall, and the gap before it was one of its own. It stands in the slot now, with the slot's minimal padding, centred, and a square of its own size the row cannot shrink. Co-Authored-By: Claude Fable 5.1 --- src/renderer/components/settings-dialog.jsx | 22 +++++++++++---------- src/renderer/shell.css | 8 ++++++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/renderer/components/settings-dialog.jsx b/src/renderer/components/settings-dialog.jsx index 4f06ccba..36f2e64c 100644 --- a/src/renderer/components/settings-dialog.jsx +++ b/src/renderer/components/settings-dialog.jsx @@ -5,7 +5,7 @@ import { useEffect, useId, useMemo, useRef, useState } from 'react'; // eslint-disable-next-line @wordpress/no-unsafe-wp-apis -- see above. import { __experimentalToggleGroupControl as ToggleGroupControl, __experimentalToggleGroupControlOption as ToggleGroupControlOption } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; -import { Button, Dialog, InputControl, Notice, SelectControl, Stack, SwitchControl, Tabs, Text } from '@wordpress/ui'; +import { Button, Dialog, InputControl, InputLayout, Notice, SelectControl, Stack, SwitchControl, Tabs, Text } from '@wordpress/ui'; import { useThemeWarnings } from './app-theme.jsx'; import { githubAccountLine, newSiteLocationNote, languageItems, languageValue, languageChanged, phpVersionChoice, quitItems, themeItems, colorFieldDraft, pickerValue, SYSTEM_LANGUAGE } from '../settings-view.cjs'; import { FolderField } from './folder-field.jsx'; @@ -121,15 +121,17 @@ function ColorField({ label, value, onKeep }) { spellCheck={false} autoComplete="off" prefix={ - setDraft(event.currentTarget.value)} - /> + + setDraft(event.currentTarget.value)} + /> + } onChange={(event) => setDraft(event.currentTarget.value)} onBlur={commit} diff --git a/src/renderer/shell.css b/src/renderer/shell.css index 154f7f41..d01015e0 100644 --- a/src/renderer/shell.css +++ b/src/renderer/shell.css @@ -1311,7 +1311,10 @@ body { the terminal's type after a swatch of the colour kept. The swatch is the system's colour input itself, which draws the colour it holds, with the browser's padding and border around that colour taken off so it fills - the swatch; pressing it opens the picker. */ + the swatch; pressing it opens the picker. It stands in the design + system's own slot for a prefix, which centres it and pads it as the + field's text is padded; a square of its own size, which the field's row + is not to shrink. */ .theme-colors { display: grid; grid-template-columns: 1fr 1fr; @@ -1328,10 +1331,11 @@ body { .color-swatch { display: block; + flex: none; box-sizing: border-box; width: var(--wpds-dimension-size-sm); height: var(--wpds-dimension-size-sm); - margin-inline-start: var(--wpds-dimension-gap-xs); + margin: 0; padding: 0; overflow: hidden; border: var(--wpds-border-width-xs) solid var(--wpds-color-stroke-surface-neutral); From 45ab50d48a487ce41a9eb8ca9166c51c7ca0945b Mon Sep 17 00:00:00 2001 From: Francesco Bigiarini Date: Wed, 7 Oct 2026 11:56:53 +0200 Subject: [PATCH 5/5] Let the log tail's test wait for its streams to close before the folder is removed The Windows unit job failed on this branch's head in a test this branch does not touch: the test's read helper settled on the stream's `end`, the stream closed its file a turn later, and the test's cleanup removed the folder in between, which Windows refuses while a file in it is open. The helper settles on `close`. Co-Authored-By: Claude Fable 5.1 --- tests/unit/log-tail.test.cjs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/log-tail.test.cjs b/tests/unit/log-tail.test.cjs index 0f1174c1..de6fe338 100644 --- a/tests/unit/log-tail.test.cjs +++ b/tests/unit/log-tail.test.cjs @@ -88,10 +88,14 @@ test('a range ends at the last byte of the size it was planned from', () => { assert.deepEqual(planTailRead(6, 1).read, { start: 0, end: 0 }); }); +// Settled on `close`, not on `end`: the stream closes its file a turn after +// the last byte, and the test's cleanup removes the folder as soon as the +// test settles. On Windows a file still open cannot be removed, and the +// folder is then "not empty" (ENOTEMPTY in `t.after`). const readAll = (stream) => new Promise((resolve, reject) => { let text = ''; stream.on('data', (chunk) => { text += chunk.toString(); }); - stream.on('end', () => resolve(text)); + stream.on('close', () => resolve(text)); stream.on('error', reject); });