diff --git a/src/renderer/components/app-theme.jsx b/src/renderer/components/app-theme.jsx index 1ce5c485..c24a6632 100644 --- a/src/renderer/components/app-theme.jsx +++ b/src/renderer/components/app-theme.jsx @@ -36,10 +36,10 @@ function usePrefersDark() { // 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 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. +// `settings` is what main holds, read before the first render so that a +// custom theme is painted from the app's first render; null only where that +// read failed, and then the window is painted as under 'system', for the +// scheme alone, until main has answered again. export function AppTheme({ settings, children }) { const prefersDark = usePrefersDark(); const [warnings, setWarnings] = useState([]); diff --git a/src/renderer/hooks/use-settings.jsx b/src/renderer/hooks/use-settings.jsx index 6e2e534d..8477c4e6 100644 --- a/src/renderer/hooks/use-settings.jsx +++ b/src/renderer/hooks/use-settings.jsx @@ -2,7 +2,9 @@ import { useCallback, useEffect, useState } from 'react'; // The app's settings (#559), held once for the window for the reason the // contributor's details are: they are about the person and the machine, -// not about a checkout. `settings` is null until main has answered, so a +// not about a checkout. They are read before the first render, since the +// theme is one of them (#560), and handed in as `initial`; where that read +// failed they are null until main has answered a second ask here, so a // dialog opened before then can say it is still reading them. `loaded` is // what main answered first and does not change: a setting that takes a // relaunch, the language, is one whose value now differs from it. @@ -11,21 +13,23 @@ import { useCallback, useEffect, useState } from 'react'; // and the details say which one a server starts on, which is the fallback // where the one set is not among them. Null until read; `{ error }` when it // could not be, so the dialog can say so rather than offer nothing. -export function useSettings() { - const [settings, setSettings] = useState(null); - const [loaded, setLoaded] = useState(null); +export function useSettings(initial = null) { + const [settings, setSettings] = useState(initial); + const [loaded, setLoaded] = useState(initial); const [php, setPhp] = useState(null); useEffect(() => { let cancelled = false; - window.api.getSettings() - .then((res) => { - if (cancelled || !res?.ok) return; - setSettings(res.settings); - setLoaded(res.settings); - }) - // eslint-disable-next-line no-console -- reaches the log file, see the note in useDetectedEditors. - .catch((err) => console.error('Could not read the settings:', err)); + if (!initial) { + window.api.getSettings() + .then((res) => { + if (cancelled || !res?.ok) return; + setSettings(res.settings); + setLoaded(res.settings); + }) + // eslint-disable-next-line no-console -- reaches the log file, see the note in useDetectedEditors. + .catch((err) => console.error('Could not read the settings:', err)); + } window.api.listPhpVersions() .then((res) => { if (cancelled) return; @@ -37,7 +41,7 @@ export function useSettings() { if (!cancelled) setPhp({ error: true }); }); return () => { cancelled = true; }; - }, []); + }, [initial]); // One setting written, and the whole replaced by what main then holds. A // refusal comes back as one rather than being raised, with the words for diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index fcdcc8cd..6fb4341f 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -2406,15 +2406,31 @@ async function loadLocale() { document.title = __('WordPress Contributor Toolkit'); } +// The settings load before the first render too, beside the locale (#560): +// the theme is one of them, and a custom theme painted after a first render +// in the standard theme of its scheme was a flash of the wrong colours at +// every launch. A failed read leaves them to be read again once mounted, +// as they were. +async function loadSettings() { + try { + const reply = await window.api.getSettings(); + return reply?.ok ? reply.settings : null; + } catch (err) { + // eslint-disable-next-line no-console -- see the note in loadLocale. + console.error('Could not read the settings before the first render:', err); + return null; + } +} + // Under the design system's provider, in the theme the window is in (#560): -// see app-theme.jsx. The settings are read here, above the provider, since +// see app-theme.jsx. The settings are held here, above the provider, since // the theme is one of them; the app is handed what was read. -function Root() { - const settingsState = useSettings(); +function Root({ initialSettings }) { + const settingsState = useSettings(initialSettings); return ; } -loadLocale().then(() => { +Promise.all([loadLocale(), loadSettings()]).then(([, initialSettings]) => { const root = createRoot(document.getElementById('root')); - root.render(); + root.render(); }); diff --git a/tests/e2e/journeys/settings.spec.js b/tests/e2e/journeys/settings.spec.js index 2215e412..832c67a5 100644 --- a/tests/e2e/journeys/settings.spec.js +++ b/tests/e2e/journeys/settings.spec.js @@ -412,6 +412,25 @@ test( 'a custom theme is built from the two colours chosen, is kept, and a colou 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)' ); + + // INVARIANT — the page is in the custom colours from the app's first + // render, not after a first render in the standard theme: the design + // system's tokens on the document at the moment the app mounts, which is + // before the app's first frame is painted, are the custom ones, not the + // standard theme's and not none. Read on a reload, which loads the page + // as a launch does, by a script put in the page before any of its own, + // which notes the document's own style as the app mounts. + await again.page.addInitScript( () => { + window.__styleAtMount = null; + new window.MutationObserver( () => { + const root = document.getElementById( 'root' ); + if ( window.__styleAtMount === null && root && root.childElementCount > 0 ) { + window.__styleAtMount = document.documentElement.getAttribute( 'style' ) || ''; + } + } ).observe( document, { childList: true, subtree: true } ); + } ); + await again.page.reload(); + await expect.poll( () => again.page.evaluate( () => window.__styleAtMount ) ).toContain( '#fff8e1' ); await ui.settingsButton( again.page ).click(); const kept = ui.settingsDialog( again.page ); await expect( kept.getByRole( 'radio', { name: 'Custom', exact: true } ) ).toBeChecked();