From 05b3d69af62d43e0b5d4dbb0b6eccada63264667 Mon Sep 17 00:00:00 2001 From: Evan Bonsignori Date: Wed, 16 Sep 2026 18:51:00 +0000 Subject: [PATCH 1/2] Use Primer Brand color tokens on Brand-migrated surfaces (Docs 2026) (#63177) Co-authored-by: Claude --- .../components/BrandThemeProvider.tsx | 28 ++++- .../stylesheets/accessibility.scss | 8 +- .../stylesheets/syntax-highlighting.scss | 6 +- .../tests/playwright-rendering.spec.ts | 52 +++++++- .../components/DefaultLayout.module.scss | 10 ++ src/frame/components/DefaultLayout.tsx | 2 +- src/frame/components/GenericError.module.scss | 17 +++ src/frame/components/GenericError.tsx | 8 +- .../article/ArticleGridLayout.module.scss | 12 +- .../page-footer/DocsFooter.module.scss | 53 ++++----- .../page-footer/SupportSection.module.scss | 3 +- .../BreadcrumbsScroller.module.scss | 9 +- .../page-header/DocsSecondaryBar.module.scss | 27 +++-- .../page-header/DocsSecondaryBar.tsx | 4 +- .../sidebar/AllProductsLink.module.scss | 7 ++ .../components/sidebar/AllProductsLink.tsx | 5 +- .../components/sidebar/SidebarNav.module.scss | 14 +++ src/frame/components/sidebar/SidebarNav.tsx | 16 ++- src/frame/components/ui/Lead/Lead.module.scss | 14 ++- src/frame/components/ui/Lead/Lead.tsx | 6 +- .../ui/MarkdownContent/stylesheets/code.scss | 44 ++++++- .../ui/MarkdownContent/stylesheets/table.scss | 11 ++ .../ui/MiniTocs/OverviewMenu.module.scss | 6 +- src/frame/stylesheets/index.scss | 111 ++++++++++++++++++ .../components/JourneyTrackNav.module.scss | 10 +- .../components/ArticleList.module.scss | 7 +- src/landings/components/ArticleList.tsx | 13 +- .../CookBookArticleCard.module.scss | 24 ++++ .../components/CookBookArticleCard.tsx | 18 ++- .../components/CookBookFilter.module.scss | 6 +- .../components/HomePageHero.module.scss | 10 +- .../ProductSelectionCard.module.scss | 2 +- .../components/TableOfContents.module.scss | 10 ++ src/landings/components/TableOfContents.tsx | 9 +- .../journey/JourneyLearningTracks.module.scss | 39 ++---- .../LandingArticleGridWithFilter.module.scss | 6 +- .../shared/LandingCarousel.module.scss | 4 +- .../components/shared/LandingHero.module.scss | 6 +- .../results/SearchResults.module.scss | 2 +- 39 files changed, 501 insertions(+), 138 deletions(-) create mode 100644 src/frame/components/sidebar/AllProductsLink.module.scss create mode 100644 src/landings/components/TableOfContents.module.scss diff --git a/src/color-schemes/components/BrandThemeProvider.tsx b/src/color-schemes/components/BrandThemeProvider.tsx index 67f63c9107bd..b59c4add706e 100644 --- a/src/color-schemes/components/BrandThemeProvider.tsx +++ b/src/color-schemes/components/BrandThemeProvider.tsx @@ -1,4 +1,4 @@ -import type { PropsWithChildren } from 'react' +import { useEffect, useState, type PropsWithChildren } from 'react' import { useTheme as usePrimerTheme } from '@primer/react' import { ThemeProvider } from '@primer/react-brand' @@ -9,7 +9,31 @@ export const BrandThemeProvider = ({ children }: PropsWithChildren) => { // otherwise many unhandled edge cases. // E.g. auto mode + dark mode + light scheme. const { resolvedColorScheme } = usePrimerTheme() - const colorMode = getBrandColorMode(resolvedColorScheme) + + // 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), []) + + const colorMode = hydrated ? getBrandColorMode(resolvedColorScheme) : 'auto' return {children} } diff --git a/src/content-render/stylesheets/accessibility.scss b/src/content-render/stylesheets/accessibility.scss index a3f11ccfaa30..4f859c02cf71 100644 --- a/src/content-render/stylesheets/accessibility.scss +++ b/src/content-render/stylesheets/accessibility.scss @@ -48,7 +48,13 @@ .tooltipped { &:focus-visible::before, &:focus-visible::after { - outline: 2px solid var(--color-focus-outset); + // --color-focus-outset is defined nowhere in this app, which made the whole + // `outline` shorthand invalid at computed-value time — so the longhands reset + // and `outline-style: none` won, leaving high-contrast users with NO focus + // ring at all, in either colour mode. Brand's focus token, matching the same + // repair already made in annotate.scss. + outline: var(--brand-borderWidth-thick, 2px) solid + var(--brand-color-focus, #0377ff); outline-offset: 2px; } } diff --git a/src/content-render/stylesheets/syntax-highlighting.scss b/src/content-render/stylesheets/syntax-highlighting.scss index 791d64163f02..30b9b9efa5ae 100644 --- a/src/content-render/stylesheets/syntax-highlighting.scss +++ b/src/content-render/stylesheets/syntax-highlighting.scss @@ -8,7 +8,11 @@ from https://unpkg.com/highlight.js@9.15.8/styles/github.css .hljs { display: block; padding: 0.5em; - color: var(--color-fg-default); + // The block's BASE text colour — the tokens below are prettylights, which has + // no Brand equivalent and stays on Primer deliberately, but this one is just + // "default text" and was painting Primer's #e6edf3 inside a Brand-framed code + // block. The `background` is inert here (markdown-overrides paints the `pre`). + color: var(--brand-color-text-default); background: var(--color-canvas-subtle); } diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index c238b036fa50..4dc9e1909542 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -1,7 +1,11 @@ import dotenv from 'dotenv' import { test, expect } from '@playwright/test' import { turnOffExperimentsInPage } from '../helpers/turn-off-experiments' -import { HOVERCARDS_ENABLED, ANALYTICS_ENABLED } from '../../frame/lib/constants' +import { + HOVERCARDS_ENABLED, + ANALYTICS_ENABLED, + COLOR_MODE_COOKIE_NAME, +} from '../../frame/lib/constants' // This exists for the benefit of local testing. // In GitHub Actions, we rely on setting the environment variable directly @@ -19,6 +23,52 @@ test('view home page', async ({ page }) => { await expect(page).toHaveTitle(/GitHub Docs/) }) +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') + + // Check both the initial dark paint and live preference changes without reloading. + for (const colorScheme of ['dark', 'light', 'dark'] as const) { + await page.emulateMedia({ colorScheme }) + const backgroundColor = colorScheme === 'dark' ? 'rgb(0, 0, 0)' : 'rgb(255, 255, 255)' + const textColor = colorScheme === 'dark' ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)' + + for (const selector of ['html', 'body']) { + await expect(page.locator(selector)).toHaveCSS('background-color', backgroundColor) + await expect(page.locator(selector)).toHaveCSS('color', textColor) + } + } + }) + + for (const colorMode of ['light', 'dark'] as const) { + test(`preserves explicit ${colorMode} mode against the opposite system preference`, async ({ + page, + context, + baseURL, + }) => { + await page.emulateMedia({ colorScheme: colorMode === 'light' ? 'dark' : 'light' }) + await context.addCookies([ + { + name: COLOR_MODE_COOKIE_NAME, + value: encodeURIComponent(JSON.stringify({ color_mode: colorMode })), + url: new URL('/', baseURL).href, + }, + ]) + await page.goto('/get-started/foo/bar') + await expect(page.locator('html')).toHaveAttribute('data-color-mode', colorMode) + + const backgroundColor = colorMode === 'dark' ? 'rgb(0, 0, 0)' : 'rgb(255, 255, 255)' + const textColor = colorMode === 'dark' ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)' + for (const selector of ['html', 'body']) { + await expect(page.locator(selector)).toHaveCSS('background-color', backgroundColor) + await expect(page.locator(selector)).toHaveCSS('color', textColor) + } + }) + } +}) + test('logo link keeps current version', async ({ page }) => { await page.goto('/enterprise-cloud@latest') await turnOffExperimentsInPage(page) diff --git a/src/frame/components/DefaultLayout.module.scss b/src/frame/components/DefaultLayout.module.scss index 93c832ddee7f..695b62ceb52b 100644 --- a/src/frame/components/DefaultLayout.module.scss +++ b/src/frame/components/DefaultLayout.module.scss @@ -61,3 +61,13 @@ display: flex; } } + +// The a11y skip link is a filled accent chip. Was primer/css +// `color-bg-accent-emphasis color-fg-on-emphasis`, which paints the chip from +// the PRC palette on a Brand surface. The Brand accent fill inverts between +// modes (dark green in light, light green in dark), so onEmphasis text is +// correct here. +.skipButton { + background-color: var(--brand-color-accent-primary); + color: var(--brand-color-text-onEmphasis); +} diff --git a/src/frame/components/DefaultLayout.tsx b/src/frame/components/DefaultLayout.tsx index b7d3bf153f6e..f61788f770e1 100644 --- a/src/frame/components/DefaultLayout.tsx +++ b/src/frame/components/DefaultLayout.tsx @@ -223,7 +223,7 @@ export const DefaultLayout = (props: Props) => { {/* a11y */} diff --git a/src/frame/components/GenericError.module.scss b/src/frame/components/GenericError.module.scss index 247697d1cbf3..3b498e3ec00a 100644 --- a/src/frame/components/GenericError.module.scss +++ b/src/frame/components/GenericError.module.scss @@ -1,3 +1,20 @@ .logoContainer { z-index: 3; } + +// Header rule under the simple error-page banner. Was primer/css +// `border-bottom color-border-muted`; the utility emits the whole shorthand +// with !important, so the replacement has to own the shorthand too. +.headerBorder { + border-bottom: 1px solid var(--brand-color-border-muted); +} + +// Wordmark logo + "GitHub Docs" link. Was primer/css `color-fg-default`. +.fgDefault { + color: var(--brand-color-text-default); +} + +// Footer copyright line. Was primer/css `color-fg-muted`. +.fgMuted { + color: var(--brand-color-text-muted); +} diff --git a/src/frame/components/GenericError.tsx b/src/frame/components/GenericError.tsx index 15e3dd3a56c4..9febf868dc35 100644 --- a/src/frame/components/GenericError.tsx +++ b/src/frame/components/GenericError.tsx @@ -40,7 +40,7 @@ export function GenericError() { export const SimpleHeader = () => { const router = useRouter() return ( -
+
{ >
- + GitHub Docs @@ -68,7 +68,7 @@ export const SimpleFooter = () => {