diff --git a/src/color-schemes/README.md b/src/color-schemes/README.md index 0e514d7dd9a5..a59079125500 100644 --- a/src/color-schemes/README.md +++ b/src/color-schemes/README.md @@ -41,9 +41,18 @@ Primer React uses slightly different terminology than the underlying CSS or the Key properties: - **Cache-safe**: The script is identical for every request, so the HTML stays shared-cacheable in the CDN. The theme is never server-rendered from the cookie (that would vary per user and poison the cache). +- **`auto` is resolved**: `data-color-mode` is always a concrete `light` or `dark`, kept in step by a `matchMedia` listener; the raw preference stays on `data-color-mode-preference` for analytics. - **No drift**: Its validation allowlists and defaults are derived from the same `CssColorMode`, `SupportedTheme`, and `defaultCSSTheme` exports used by `useTheme`. A test in `tests/color-mode-script.ts` runs the script against a fake `document` and asserts parity with `getCssTheme`. - **CSP**: Because the script is inline, `src/frame/middleware/helmet.ts` adds its `sha256` hash to the `script-src` directive. The hash is computed from the exact script string at startup, so it never needs manual maintenance, and a hash (not a nonce) keeps the response cacheable. +### Why `auto` is resolved before paint + +`@primer/react-brand` has no `auto` color mode: it declares its palette on `:root, [data-color-mode="light"]` and `[data-color-mode="dark"]`, with no `prefers-color-scheme` at-rules (`@primer/primitives` has them, so it is unaffected). That makes `` a _light_ root, and a nested wrapper in a different mode re-declares the whole palette for its subtree. Resolving once, at the root, avoids both: `BrandThemeProvider` mirrors `` and declares no mode of its own. + +The resolved mode comes from the **effective theme**, not the raw `color_mode`, because github.com's day and night themes are chosen independently — `light` mode can itself be a dark theme. + +This is a workaround for a gap in Brand and belongs upstream; until it lands, every consumer has to hand Brand a concrete mode. + ## Setup & Usage To access the current theme in a component: diff --git a/src/color-schemes/components/BrandThemeProvider.tsx b/src/color-schemes/components/BrandThemeProvider.tsx index b59c4add706e..4c1387cfdb35 100644 --- a/src/color-schemes/components/BrandThemeProvider.tsx +++ b/src/color-schemes/components/BrandThemeProvider.tsx @@ -1,39 +1,30 @@ import { useEffect, useState, type PropsWithChildren } from 'react' -import { useTheme as usePrimerTheme } from '@primer/react' import { ThemeProvider } from '@primer/react-brand' -import { getBrandColorMode } from '@/color-schemes/lib/get-brand-color-mode' +import { getBrandColorMode, type BrandColorMode } from '@/color-schemes/lib/get-brand-color-mode' +// Brand reads `colorMode="auto"` as "snapshot the OS on mount" rather than +// "inherit", so this only ever passes a concrete mode. export const BrandThemeProvider = ({ children }: PropsWithChildren) => { - // We need to resolve the color scheme through PRC first, because there are - // otherwise many unhandled edge cases. - // E.g. auto mode + dark mode + light scheme. - const { resolvedColorScheme } = usePrimerTheme() + // Seeded to match SSR; reading the DOM here would break hydration. + const [colorMode, setColorMode] = useState('light') - // Brand's ThemeProvider renders a real `
`, and brand - // declares its ENTIRE palette on the bare `[data-color-mode="light"]` / - // `[data-color-mode="dark"]` attribute. So a nested wrapper re-declares every - // brand token for its own subtree — canvas, text, borders, links, the lot. - // - // `resolvedColorScheme` is only correct after PRC's cookie-reading effect has - // run. On the server it resolves to light, and the first client render has to - // match the server markup, so it is light there too. Emitting `light` would - // override the correct mode colorModeScript has already stamped on `` - // before first paint, and every brand token on the page would resolve to its - // light value until React hydrates — a white flash on every dark-mode load. - // - // We cannot server-render the real mode: that HTML is shared-cacheable in the - // CDN and must be identical for every request, which is why the pre-paint - // script exists at all. - // - // `auto` matches none of brand's blocks, so the wrapper declares nothing and - // brand's tokens inherit from ``. Verified in-browser: with `` at - // dark, a wrapper at `auto` resolves byte-identical token values to the root, - // whereas a wrapper at `light` flips all 16 tokens this app uses. - const [hydrated, setHydrated] = useState(false) - useEffect(() => setHydrated(true), []) + useEffect(() => { + setColorMode(getBrandColorMode()) + // colorModeScript re-stamps when the OS flips under `auto`. + const observer = new MutationObserver(() => setColorMode(getBrandColorMode())) + observer.observe(document.documentElement, { + attributes: true, + attributeFilter: ['data-color-mode'], + }) + return () => observer.disconnect() + }, []) - const colorMode = hydrated ? getBrandColorMode(resolvedColorScheme) : 'auto' - - return {children} + // Brand spreads rest props after its own attribute, so `data-color-mode={undefined}` + // drops it from the wrapper div; the prop still feeds brand's context. + return ( + + {children} + + ) } diff --git a/src/color-schemes/components/useTheme.ts b/src/color-schemes/components/useTheme.ts index cb0ad4c841e8..6d34612987f4 100644 --- a/src/color-schemes/components/useTheme.ts +++ b/src/color-schemes/components/useTheme.ts @@ -62,7 +62,12 @@ function filterMode(mode = ''): CssColorMode | undefined { } } -function filterTheme({ name = '', color_mode = '' } = {}): SupportedTheme | undefined { +// `?? {}` rather than a default parameter: a default only covers `undefined`, and +// the cookie can carry an explicit `null` (`{"light_theme":null}`). +function filterTheme( + theme?: { name?: string; color_mode?: string } | null, +): SupportedTheme | undefined { + const { name = '', color_mode = '' } = theme ?? {} if (Object.values(SupportedTheme).includes(name)) { return name as SupportedTheme } diff --git a/src/color-schemes/lib/color-mode-script.ts b/src/color-schemes/lib/color-mode-script.ts index c5272af904d0..bf5833843821 100644 --- a/src/color-schemes/lib/color-mode-script.ts +++ b/src/color-schemes/lib/color-mode-script.ts @@ -8,6 +8,10 @@ import { CssColorMode, SupportedTheme, defaultCSSTheme } from '@/color-schemes/c // page first paints with the SSR default theme and only switches to the user's // real theme after the React bundle hydrates, causing a visible flash. // +// `data-color-mode` is always concrete, never `auto` — @primer/react-brand has +// no `auto` palette — and follows the effective theme, because a `light` mode +// can carry a dark day theme. See src/color-schemes/README.md. +// // The output is identical for every request, so the HTML stays shared-cacheable // in our CDN. The validation allowlists and defaults are derived from the same // enums used by `useTheme`, so they can't drift, and `helmet.ts` hashes this @@ -31,8 +35,21 @@ css={colorMode:fMode(p.color_mode)||D.colorMode,lightTheme:fTheme(p.light_theme) }catch(e){} try{ var h=document.documentElement; -h.setAttribute('data-color-mode',css.colorMode); +var q=window.matchMedia?window.matchMedia('(prefers-color-scheme: dark)'):null; +var apply=function(){ +var night=css.colorMode==='auto'?!!(q&&q.matches):css.colorMode==='dark'; +var theme=night?css.darkTheme:css.lightTheme; +var mode=theme.indexOf('dark')===0?'dark':'light'; +h.setAttribute('data-color-mode',mode); +h.setAttribute('data-'+mode+'-theme',theme); +}; +h.setAttribute('data-color-mode-preference',css.colorMode); h.setAttribute('data-light-theme',css.lightTheme); h.setAttribute('data-dark-theme',css.darkTheme); +apply(); +if(css.colorMode==='auto'&&q){ +if(q.addEventListener)q.addEventListener('change',apply); +else if(q.addListener)q.addListener(apply); +} }catch(e){} })();` diff --git a/src/color-schemes/lib/get-brand-color-mode.ts b/src/color-schemes/lib/get-brand-color-mode.ts index 0c26f26628fb..cd317a6e9766 100644 --- a/src/color-schemes/lib/get-brand-color-mode.ts +++ b/src/color-schemes/lib/get-brand-color-mode.ts @@ -1,3 +1,8 @@ -export function getBrandColorMode(resolvedColorScheme?: string) { - return resolvedColorScheme?.startsWith('dark') ? 'dark' : 'light' +export type BrandColorMode = 'light' | 'dark' + +// Brand's palette follows 's `data-color-mode`, resolved to a concrete mode +// before first paint — not PRC's `resolvedColorScheme`, which is the THEME. +export function getBrandColorMode(): BrandColorMode { + if (typeof document === 'undefined') return 'light' // SSR fallback + return document.documentElement.getAttribute('data-color-mode') === 'dark' ? 'dark' : 'light' } diff --git a/src/color-schemes/tests/color-mode-script.ts b/src/color-schemes/tests/color-mode-script.ts index cd83d53c030f..a97b1773e24a 100644 --- a/src/color-schemes/tests/color-mode-script.ts +++ b/src/color-schemes/tests/color-mode-script.ts @@ -1,14 +1,23 @@ import { describe, expect, test } from 'vitest' import { colorModeScript } from '../lib/color-mode-script' -import { getCssTheme } from '../components/useTheme' +import { getCssTheme, SupportedTheme } from '../components/useTheme' -// The inline script can't import the React `useTheme` module at runtime (it -// runs before any bundle loads), so it reimplements the same validation. These -// tests run the script against a fake `document` and assert it produces the -// exact same result as `getCssTheme`, which keeps the two in sync. -function runScript(rawCookie: string) { +// The inline script runs before any bundle loads, so it reimplements +// `useTheme`'s validation instead of importing it. These tests assert the two +// stay in sync. +function runScript( + rawCookie: string, + { prefersDark = false, matchMedia = true, legacyListener = false } = {}, +) { const attrs: Record = {} + const listeners: Array<(event: { matches: boolean }) => void> = [] + // `matches` reads this through a getter, so `flipSystemPreference` changes + // what an already-registered handler sees. + const os = { prefersDark } + const subscribe = (handler: (event: { matches: boolean }) => void) => { + listeners.push(handler) + } const fakeDocument = { cookie: rawCookie, documentElement: { @@ -17,8 +26,33 @@ function runScript(rawCookie: string) { }, }, } - new Function('document', colorModeScript)(fakeDocument) - return attrs + const fakeWindow = matchMedia + ? { + matchMedia(query: string) { + return { + get matches() { + return query.includes('dark') ? os.prefersDark : !os.prefersDark + }, + ...(legacyListener + ? { addListener: subscribe } + : { + addEventListener: (_: string, handler: (event: { matches: boolean }) => void) => + subscribe(handler), + }), + } + }, + } + : {} + + new Function('document', 'window', colorModeScript)(fakeDocument, fakeWindow) + return { + attrs, + listeners, + flipSystemPreference() { + os.prefersDark = !os.prefersDark + for (const handler of listeners) handler({ matches: os.prefersDark }) + }, + } } function cookieFor(value: object) { @@ -26,12 +60,18 @@ function cookieFor(value: object) { return `color_mode=${encodeURIComponent(JSON.stringify(value))}` } -function expectMatchesGetCssTheme(rawCookie: string, cookieValue: string) { +function expectMatchesGetCssTheme(rawCookie: string, cookieValue: string, prefersDark = false) { const css = getCssTheme(cookieValue) - expect(runScript(rawCookie)).toEqual({ - 'data-color-mode': css.colorMode, - 'data-light-theme': css.lightTheme, - 'data-dark-theme': css.darkTheme, + // Primitives select on the (mode, theme) pair, so the effective theme has to + // land on the attribute for the resolved mode. + const mode = css.colorMode === 'auto' ? (prefersDark ? 'dark' : 'light') : css.colorMode + const theme = mode === 'dark' ? css.darkTheme : css.lightTheme + const resolved = theme.startsWith('dark') ? 'dark' : 'light' + expect(runScript(rawCookie, { prefersDark }).attrs).toEqual({ + 'data-color-mode': resolved, + 'data-color-mode-preference': css.colorMode, + 'data-light-theme': resolved === 'light' ? theme : css.lightTheme, + 'data-dark-theme': resolved === 'dark' ? theme : css.darkTheme, }) } @@ -71,6 +111,13 @@ describe('colorModeScript', () => { expectMatchesGetCssTheme(cookieFor(value), JSON.stringify(value)) }) + test('survives an explicitly null theme without discarding the mode', () => { + // A default parameter covers `undefined`, not `null`. + const value = { color_mode: 'dark', light_theme: null } + expectMatchesGetCssTheme(cookieFor(value), JSON.stringify(value)) + expect(runScript(cookieFor(value)).attrs['data-color-mode']).toBe('dark') + }) + test('reads the cookie even when other cookies are present', () => { const value = { color_mode: 'light' } const rawCookie = `_octo=GH1.1; color_mode=${encodeURIComponent( @@ -78,4 +125,128 @@ describe('colorModeScript', () => { )}; logged_in=no` expectMatchesGetCssTheme(rawCookie, JSON.stringify(value)) }) + + describe('resolves `auto` against the system preference', () => { + for (const [prefersDark, expected] of [ + [false, 'light'], + [true, 'dark'], + ] as const) { + test(`prefers-color-scheme: ${expected}`, () => { + const { attrs } = runScript(cookieFor({ color_mode: 'auto' }), { prefersDark }) + expect(attrs['data-color-mode']).toBe(expected) + expect(attrs['data-color-mode-preference']).toBe('auto') + }) + } + + test('no cookie at all still resolves, because the default mode is auto', () => { + expect(runScript('', { prefersDark: true }).attrs['data-color-mode']).toBe('dark') + expect(runScript('').attrs['data-color-mode']).toBe('light') + }) + + test('keeps following the OS after load, but only for `auto`', () => { + expect(runScript(cookieFor({ color_mode: 'auto' })).listeners).toHaveLength(1) + expect(runScript(cookieFor({ color_mode: 'light' })).listeners).toHaveLength(0) + expect(runScript(cookieFor({ color_mode: 'dark' })).listeners).toHaveLength(0) + }) + + test('the registered listener actually re-resolves the attributes', () => { + const run = runScript(cookieFor({ color_mode: 'auto' })) + expect(run.attrs['data-color-mode']).toBe('light') + run.flipSystemPreference() + expect(run.attrs['data-color-mode']).toBe('dark') + run.flipSystemPreference() + expect(run.attrs['data-color-mode']).toBe('light') + expect(run.attrs['data-color-mode-preference']).toBe('auto') + }) + + test('falls back to the deprecated addListener when addEventListener is absent', () => { + // Pre-14 Safari exposes only `addListener`, so this branch is live. + const run = runScript(cookieFor({ color_mode: 'auto' }), { legacyListener: true }) + expect(run.attrs['data-color-mode']).toBe('light') + expect(run.listeners).toHaveLength(1) + run.flipSystemPreference() + expect(run.attrs['data-color-mode']).toBe('dark') + + expect( + runScript(cookieFor({ color_mode: 'dark' }), { legacyListener: true }).listeners, + ).toHaveLength(0) + }) + + test('still writes the attributes when matchMedia is unavailable', () => { + // The script's DOM block sits in a try/catch, so an unguarded matchMedia + // call would leave with no attributes at all. + const { attrs } = runScript(cookieFor({ color_mode: 'auto' }), { matchMedia: false }) + expect(attrs['data-color-mode']).toBe('light') + expect(attrs['data-color-mode-preference']).toBe('auto') + expect( + runScript(cookieFor({ color_mode: 'dark' }), { matchMedia: false }).attrs[ + 'data-color-mode' + ], + ).toBe('dark') + }) + + test('an explicit mode wins over the opposite system preference', () => { + expect( + runScript(cookieFor({ color_mode: 'light' }), { prefersDark: true }).attrs[ + 'data-color-mode' + ], + ).toBe('light') + expect(runScript(cookieFor({ color_mode: 'dark' })).attrs['data-color-mode']).toBe('dark') + }) + }) + + describe('follows the effective theme, not the raw mode', () => { + test('a dark day theme makes the document dark', () => { + const { attrs } = runScript( + cookieFor({ color_mode: 'light', light_theme: { name: 'dark', color_mode: 'dark' } }), + ) + expect(attrs['data-color-mode']).toBe('dark') + expect(attrs['data-dark-theme']).toBe('dark') + }) + + test('a dark day theme variant reaches primitives intact', () => { + const { attrs } = runScript( + cookieFor({ + color_mode: 'light', + light_theme: { name: 'dark_dimmed', color_mode: 'dark' }, + dark_theme: { name: 'dark_high_contrast', color_mode: 'dark' }, + }), + ) + // Resolved dark by the DAY theme, so data-dark-theme carries that, not the + // separately configured night theme. + expect(attrs['data-color-mode']).toBe('dark') + expect(attrs['data-dark-theme']).toBe('dark_dimmed') + }) + + test('dark theme variants still count as dark', () => { + for (const name of ['dark_dimmed', 'dark_high_contrast']) { + const { attrs } = runScript( + cookieFor({ color_mode: 'dark', dark_theme: { name, color_mode: 'dark' } }), + ) + expect(attrs['data-color-mode']).toBe('dark') + expect(attrs['data-dark-theme']).toBe(name) + } + }) + + test('auto picks the theme belonging to the side the OS is on', () => { + const value = cookieFor({ + color_mode: 'auto', + light_theme: { name: 'dark', color_mode: 'dark' }, + dark_theme: { name: 'dark_dimmed', color_mode: 'dark' }, + }) + expect(runScript(value).attrs['data-color-mode']).toBe('dark') + expect(runScript(value, { prefersDark: true }).attrs['data-color-mode']).toBe('dark') + }) + + test('every supported theme classifies the same under both operators', () => { + // must classify a theme's lightness the same way @primer/react does + // for its own wrapper: `startsWith('dark')` here, `includes('dark')` there. + // A name like `high_contrast_dark` would split them. + for (const name of Object.values(SupportedTheme)) { + expect(`${name} startsWith:${name.startsWith('dark')}`).toBe( + `${name} startsWith:${name.includes('dark')}`, + ) + } + }) + }) }) diff --git a/src/color-schemes/tests/get-brand-color-mode.ts b/src/color-schemes/tests/get-brand-color-mode.ts index 7a141c305238..1653ffcacc9c 100644 --- a/src/color-schemes/tests/get-brand-color-mode.ts +++ b/src/color-schemes/tests/get-brand-color-mode.ts @@ -1,14 +1,37 @@ -import { describe, expect, test } from 'vitest' +import { afterEach, describe, expect, test, vi } from 'vitest' import { getBrandColorMode } from '@/color-schemes/lib/get-brand-color-mode' +function withColorMode(value: string | null) { + const element = { getAttribute: (name: string) => (name === 'data-color-mode' ? value : null) } + vi.stubGlobal('document', { documentElement: element }) +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + describe('getBrandColorMode', () => { test.each([ - ['light', 'light'], ['dark', 'dark'], - ['dark_dimmed', 'dark'], - ['dark_high_contrast', 'dark'], - ])('maps the %s scheme to %s mode', (scheme, mode) => { - expect(getBrandColorMode(scheme)).toBe(mode) + ['light', 'light'], + ])('follows data-color-mode=%s', (attribute, expected) => { + withColorMode(attribute) + expect(getBrandColorMode()).toBe(expected) + }) + + test.each([ + // Brand has no `auto` mode, so anything not `dark` has to render light. + ['auto', 'light'], + ['nonsense', 'light'], + [null, 'light'], + ])('falls back to light for data-color-mode=%s', (attribute, expected) => { + withColorMode(attribute) + expect(getBrandColorMode()).toBe(expected) + }) + + test('returns light when there is no document', () => { + vi.stubGlobal('document', undefined) + expect(getBrandColorMode()).toBe('light') }) }) diff --git a/src/content-render/stylesheets/octicon-table-optimization.scss b/src/content-render/stylesheets/octicon-table-optimization.scss index 1ba34658a4ce..23d1f9af5728 100644 --- a/src/content-render/stylesheets/octicon-table-optimization.scss +++ b/src/content-render/stylesheets/octicon-table-optimization.scss @@ -26,6 +26,7 @@ $dark-color: "%23ffffff"; background-image: octicon-svg($path, $dark-color); } + // No-JS fallback; see the note in src/frame/stylesheets/index.scss. @media (prefers-color-scheme: dark) { [data-color-mode="auto"][data-dark-theme*="dark"] & { background-image: octicon-svg($path, $dark-color); diff --git a/src/events/components/events.ts b/src/events/components/events.ts index 31548c3bf87e..127de20ee0d1 100644 --- a/src/events/components/events.ts +++ b/src/events/components/events.ts @@ -210,7 +210,11 @@ function getColorModePreference() { // color mode is set as attributes on , we'll use that information // along with media query checking rather than parsing the cookie value // set by github.com - let color_mode_preference = document.querySelector('html')?.dataset.colorMode + // + // `data-color-mode` is the resolved mode; the preference attribute is what + // keeps `auto` reportable. + const html = document.querySelector('html') + let color_mode_preference = html?.dataset.colorModePreference || html?.dataset.colorMode if (color_mode_preference === 'auto') { if (window.matchMedia('(prefers-color-scheme: light)').matches) { diff --git a/src/fixtures/helpers/color-contrast.ts b/src/fixtures/helpers/color-contrast.ts new file mode 100644 index 000000000000..4d2e6fc8fe77 --- /dev/null +++ b/src/fixtures/helpers/color-contrast.ts @@ -0,0 +1,37 @@ +// WCAG contrast for computed `rgb()`/`rgba()` colours. Keywords, hex and +// translucent values throw rather than being coerced — `rgba(0, 0, 0, 0)` would +// otherwise read as opaque black and yield a confident, wrong ratio. + +function parseComputedColor(color: string) { + const parts = color.match(/\d+(?:\.\d+)?/g)?.map(Number) + if (!parts || parts.length < 3) { + throw new Error( + `Expected a computed rgb()/rgba() colour, got ${JSON.stringify(color)}. ` + + 'Pass getComputedStyle(...).color or .backgroundColor, not a keyword or hex string.', + ) + } + const [r, g, b, alpha = 1] = parts + if (alpha !== 1) { + throw new Error( + `Cannot measure contrast against the translucent colour ${JSON.stringify(color)}: ` + + 'the result depends on what is painted behind it. Measure an opaque pairing instead.', + ) + } + return [r, g, b] +} + +export function relativeLuminance(color: string) { + const [r, g, b] = parseComputedColor(color) + const channel = (value: number) => { + const ratio = value / 255 + return ratio <= 0.03928 ? ratio / 12.92 : ((ratio + 0.055) / 1.055) ** 2.4 + } + return 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b) +} + +export function contrastRatio(foreground: string, background: string) { + const [lighter, darker] = [relativeLuminance(foreground), relativeLuminance(background)].sort( + (a, b) => b - a, + ) + return (lighter + 0.05) / (darker + 0.05) +} diff --git a/src/fixtures/tests/playwright-header.spec.ts b/src/fixtures/tests/playwright-header.spec.ts index 15c39b8f9470..36261f1d7ffc 100644 --- a/src/fixtures/tests/playwright-header.spec.ts +++ b/src/fixtures/tests/playwright-header.spec.ts @@ -1,6 +1,7 @@ import { test, expect, type Locator, type Page } from '@playwright/test' import { turnOffExperimentsInPage } from '../helpers/turn-off-experiments' +import { relativeLuminance } from '@/fixtures/helpers/color-contrast' import { COLOR_MODE_COOKIE_NAME, USER_LANGUAGE_COOKIE_NAME, @@ -104,21 +105,6 @@ async function resolveTokenValues(locator: Locator, tokens: string[]) { }, tokens) } -// Used only to prove the emulated color scheme actually reached Brand's tokens. -// Without it, a dark-mode run that silently stayed light would satisfy every -// "resolved token" assertion below and the dark coverage would be vacuous. -function relativeLuminance(color: string) { - const [r, g, b] = color - .match(/\d+(?:\.\d+)?/g)! - .slice(0, 3) - .map(Number) - const channel = (value: number) => { - const ratio = value / 255 - return ratio <= 0.03928 ? ratio / 12.92 : ((ratio + 0.055) / 1.055) ** 2.4 - } - return 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b) -} - async function expectHeaderPlanPicker(page: Page) { const picker = page.getByTestId('desktop-header').getByTestId('version-picker') const label = picker.getByText(PLAN_LABEL, { exact: true }) @@ -399,6 +385,8 @@ async function expectHeaderDropdownDesign( '--brand-color-text-default', '--brand-color-success-fg', ]) + // Proves the emulated scheme reached Brand's tokens: a dark run that silently + // stayed light would satisfy every assertion above on its own. const luminance = relativeLuminance(tokens['--brand-color-canvas-default']) if (colorScheme === 'dark') { expect(luminance).toBeLessThan(0.2) @@ -909,9 +897,8 @@ test.describe('Brand header', () => { page, }) => { await page.setViewportSize({ width: 1440, height: 800 }) - // No color_mode cookie, so the page stays in `auto` and resolves the - // scheme from this media emulation. Set before navigating so the first - // paint already uses it. + // No color_mode cookie, so colorModeScript resolves `auto` from this + // emulation. Set before navigating so the first paint already uses it. await page.emulateMedia({ colorScheme }) await page.goto(ARTICLE) await turnOffExperimentsInPage(page) diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index 4dc9e1909542..766f68394506 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -1,6 +1,7 @@ import dotenv from 'dotenv' import { test, expect } from '@playwright/test' import { turnOffExperimentsInPage } from '../helpers/turn-off-experiments' +import { contrastRatio } from '@/fixtures/helpers/color-contrast' import { HOVERCARDS_ENABLED, ANALYTICS_ENABLED, @@ -27,7 +28,8 @@ test.describe('Brand document canvas', () => { test('follows system color scheme changes in auto mode without a cookie', async ({ page }) => { await page.emulateMedia({ colorScheme: 'dark' }) await page.goto('/get-started/foo/bar') - await expect(page.locator('html')).toHaveAttribute('data-color-mode', 'auto') + // `auto` is resolved before first paint, so the raw preference gets its own attribute. + await expect(page.locator('html')).toHaveAttribute('data-color-mode-preference', 'auto') // Check both the initial dark paint and live preference changes without reloading. for (const colorScheme of ['dark', 'light', 'dark'] as const) { @@ -35,6 +37,7 @@ test.describe('Brand document canvas', () => { const backgroundColor = colorScheme === 'dark' ? 'rgb(0, 0, 0)' : 'rgb(255, 255, 255)' const textColor = colorScheme === 'dark' ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)' + await expect(page.locator('html')).toHaveAttribute('data-color-mode', colorScheme) for (const selector of ['html', 'body']) { await expect(page.locator(selector)).toHaveCSS('background-color', backgroundColor) await expect(page.locator(selector)).toHaveCSS('color', textColor) @@ -67,6 +70,123 @@ test.describe('Brand document canvas', () => { } }) } + + // A concrete [data-color-mode] below re-declares brand's whole palette + // for that subtree. + const MISMATCHES = [ + { name: 'OS dark, explicit light mode', colorScheme: 'dark', cookie: { color_mode: 'light' } }, + { name: 'OS light, explicit dark mode', colorScheme: 'light', cookie: { color_mode: 'dark' } }, + { + // Day and night themes are picked independently on github.com, so `light` + // mode can itself resolve to a dark theme. + name: 'light mode whose day theme is itself dark', + colorScheme: 'light', + cookie: { + color_mode: 'light', + light_theme: { name: 'dark_dimmed', color_mode: 'dark' }, + dark_theme: { name: 'dark', color_mode: 'dark' }, + }, + }, + ] as const + + for (const scenario of MISMATCHES) { + test(`declares brand's palette only on (${scenario.name})`, async ({ + page, + context, + baseURL, + }) => { + // A settled assertion cannot catch a wrapper that self-corrects within a + // macrotask, so record every data-color-mode below from first paint on. + await page.addInitScript(() => { + const seen: string[] = [] + ;(window as unknown as { __modes: string[] }).__modes = seen + const note = (node: Node) => { + if (!(node instanceof Element) || node === document.documentElement) return + const value = node.getAttribute('data-color-mode') + if (value) seen.push(value) + } + new MutationObserver((records) => { + for (const record of records) { + if (record.type === 'attributes') note(record.target) + for (const node of record.addedNodes) { + note(node) + if (node instanceof Element) { + for (const nested of node.querySelectorAll('[data-color-mode]')) note(nested) + } + } + } + }).observe(document, { + subtree: true, + childList: true, + attributes: true, + attributeFilter: ['data-color-mode'], + }) + }) + await page.emulateMedia({ colorScheme: scenario.colorScheme }) + await context.addCookies([ + { + name: COLOR_MODE_COOKIE_NAME, + value: encodeURIComponent(JSON.stringify(scenario.cookie)), + url: new URL('/', baseURL).href, + }, + ]) + await page.goto('/get-started/foo/for-playwright') + + const rootMode = await page.locator('html').getAttribute('data-color-mode') + expect(rootMode).toMatch(/^(light|dark)$/) + + // Brand's ActionMenu.Overlay wraps an open menu in its own ThemeProvider, + // which emits a data-color-mode from brand's context, and only while open. + await page.getByTestId('version-picker-button').first().click() + await expect(page.getByRole('menu').first()).toBeVisible() + + // `auto` is exempt: brand has no `auto` block, so such a wrapper declares + // nothing and inherits. + await expect(async () => { + const offenders = await page + .locator('body [data-color-mode]') + .evaluateAll( + (nodes, mode) => + nodes + .map((node) => node.getAttribute('data-color-mode')!) + .filter((value) => value !== 'auto' && value !== mode), + rootMode, + ) + expect(offenders).toEqual([]) + }).toPass() + + const everSeen = await page.evaluate( + () => (window as unknown as { __modes: string[] }).__modes, + ) + expect(everSeen.filter((value) => value !== 'auto' && value !== rootMode)).toEqual([]) + + // heading-links.ts wraps every heading's text in an `` + // held at heading color, so a bare `a[href]` here picks a heading. The + // exclusions mirror article-link-overrides.scss. + const link = page + .locator('#article-contents .markdown-body a[href]:not(.heading-link):not(.btn)') + .first() + const linkColor = await link.evaluate((element) => getComputedStyle(element).color) + const canvas = await page + .locator('body') + .evaluate((element) => getComputedStyle(element).backgroundColor) + + const expectedLinkColor = await page.locator('html').evaluate((element) => { + const probe = document.createElement('span') + probe.style.color = 'var(--brand-color-text-link-rest)' + element.append(probe) + try { + return getComputedStyle(probe).color + } finally { + probe.remove() + } + }) + // Equality alone passes if is wrong; contrast alone passes if the + // selector drifts off brand links. + expect(linkColor).toBe(expectedLinkColor) + expect(contrastRatio(linkColor, canvas)).toBeGreaterThanOrEqual(4.5) + }) + } }) test('logo link keeps current version', async ({ page }) => { diff --git a/src/frame/components/ui/MiniTocs/OverviewMenu.module.scss b/src/frame/components/ui/MiniTocs/OverviewMenu.module.scss index 3bd37de74dd1..d5eb4f72729a 100644 --- a/src/frame/components/ui/MiniTocs/OverviewMenu.module.scss +++ b/src/frame/components/ui/MiniTocs/OverviewMenu.module.scss @@ -74,6 +74,7 @@ box-shadow: 0 3px 6px rgba(0, 0, 0, 0.6); } + // No-JS fallback; see the note in src/frame/stylesheets/index.scss. @media (prefers-color-scheme: dark) { :global([data-color-mode="auto"][data-dark-theme*="dark"]) & { box-shadow: 0 3px 6px rgba(0, 0, 0, 0.6); diff --git a/src/frame/pages/app.tsx b/src/frame/pages/app.tsx index ff875377863d..6c0edb05f810 100644 --- a/src/frame/pages/app.tsx +++ b/src/frame/pages/app.tsx @@ -115,8 +115,6 @@ const MyApp = ({ Component, pageProps, languagesContext, stagingName }: MyAppPro components receive brand theme context during the Docs 2026 migration (github/docs-engineering#5879). Runs alongside the @primer/react ThemeProvider above while the component-by-component swap is in progress. - Resolve Brand's color mode from Primer React's active color scheme so - opposite-mode day/night schemes stay in sync. */} diff --git a/src/frame/stylesheets/index.scss b/src/frame/stylesheets/index.scss index b9e4543216a1..948ab8603731 100644 --- a/src/frame/stylesheets/index.scss +++ b/src/frame/stylesheets/index.scss @@ -58,9 +58,10 @@ // background-color: var(--bgColor-default, var(--color-canvas-default)); // } // -// The Brand `ThemeProvider` renders a real wrapping `
` -// (see src/color-schemes/components/BrandThemeProvider.tsx), so it matched that -// selector and painted itself Primer's canvas. In dark mode that is #0d1117 — a +// @primer/react's `ThemeProvider` renders a real wrapping `
` +// (brand's no longer does; see src/color-schemes/components/BrandThemeProvider.tsx), +// so it matched that selector and painted itself Primer's canvas. In dark mode +// that is #0d1117 — a // blue-tinted charcoal — while every surface migrated to Brand tokens sits on // Brand's true black (#000000) or its green-tinted near-black (#0f1511). The // result was up to three different "blacks" stacked on one page, and fills @@ -85,31 +86,17 @@ html[data-color-mode] body { } // ...and every NESTED [data-color-mode] wrapper stays transparent so the root -// canvas shows through. There are two of them: @primer/react's ThemeProvider and -// brand's, which the app nests inside it (src/frame/pages/app.tsx). -// -// This rule must NOT resolve a `--brand-*` token, and that is the whole point of -// it. Brand declares its palette on the bare `[data-color-mode="light"]` / -// `[data-color-mode="dark"]` attribute, so a nested wrapper RE-DECLARES the -// entire brand palette for its own subtree. BrandThemeProvider resolves its mode -// through PRC's `resolvedColorScheme`, which is only correct after the -// cookie-reading effect runs — so it server-renders `data-color-mode="light"`. -// Painting that wrapper with `var(--brand-color-canvas-default)` therefore -// resolves brand's LIGHT canvas (#ffffff) on a dark load, and the whole page -// flashes white until React hydrates. Verified: with `` dark and the -// wrapper at `light`, the wrapper computes rgb(255,255,255) on black text. -// -// `:not(html)` is also what wins the cascade here: @primer/css uses -// `[data-color-mode]` (0,1,0), and `:not(html)` adds `html`'s (0,0,1) to make -// this (0,1,1). +// canvas shows through — @primer/react's ThemeProvider still renders one. Never +// a `--brand-*` token here: a nested wrapper re-declares brand's palette and +// would paint its own mode, not the page's. `:not(html)` adds html's (0,0,1) to +// beat @primer/css's `[data-color-mode]` (0,1,0). [data-color-mode]:not(html) { background-color: transparent; color: inherit; } -// Brand only defines explicit light/dark modes, but the document defaults to -// auto. Match Brand's dark canvas/text tokens when the OS prefers dark; nested -// ThemeProviders still resolve their own explicit modes. +// No-JS fallback, not dead code: without the pre-paint script keeps the +// SSR `auto` default, which @primer/primitives honours and brand does not. @media (prefers-color-scheme: dark) { html[data-color-mode="auto"][data-dark-theme*="dark"] { --brand-color-canvas-default: var(--base-color-scale-black-0);