Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/color-schemes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ Primer React uses slightly different terminology than the underlying CSS or the
- CSS `light` -> Component `day`
- CSS `dark` -> Component `night`

### Pre-paint Inline Script

`useTheme` only runs after the React bundle hydrates, so the page would first paint with the SSR default theme and then switch, causing a visible flash. To avoid that, `src/color-schemes/lib/color-mode-script.ts` exports `colorModeScript`: a small synchronous script that `_document.tsx` inlines in the `<head>`. It runs before the first paint, reads the `color_mode` cookie, and sets `data-color-mode`, `data-light-theme`, and `data-dark-theme` on `<html>`.

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).
- **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.

## Setup & Usage

To access the current theme in a component:
Expand Down Expand Up @@ -66,7 +75,8 @@ This hook is primarily used at the root of the application (e.g., in `src/frame/

## Current State & Known Issues

- **Hydration Mismatch / Flash of Unstyled Content**: Since the theme is read from a cookie on the client side (in `useEffect`), there can be a brief moment where the default theme is applied before the user's preference loads.
- **Page background flash (fixed)**: The page-level theme (the `<html>` `data-*` attributes that drive the background color) is now set before first paint by the inline `colorModeScript`, so there is no longer a light-to-dark flash of the page background on load.
- **Primer component theming**: Primer React components still resolve their theme from the post-hydration `useTheme` state, so component-level theming applies slightly after the page background. The `setTimeout` workaround below is still required for that path.
- **Race Condition Workaround**: There is a `setTimeout` hack in `useTheme.ts` to delay the theme application. This is necessary to prevent Primer React's internal logic from overriding the user's preference with `auto` on initial load.
- *Reference*: [Primer React Issue #2229](https://github.com/primer/react/issues/2229)
- **Future**: The long-term goal is to rely entirely on CSS variables, removing the need for complex JavaScript state management for theming.
4 changes: 2 additions & 2 deletions src/color-schemes/components/useTheme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useState, useEffect } from 'react'
import Cookies from '../../frame/components/lib/cookies'
import { COLOR_MODE_COOKIE_NAME } from '@/frame/lib/constants'

enum CssColorMode {
export enum CssColorMode {
auto = 'auto',
light = 'light',
dark = 'dark',
Expand All @@ -14,7 +14,7 @@ enum ComponentColorMode {
night = 'night',
}

enum SupportedTheme {
export enum SupportedTheme {
light = 'light',
dark = 'dark',
dark_dimmed = 'dark_dimmed',
Expand Down
38 changes: 38 additions & 0 deletions src/color-schemes/lib/color-mode-script.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { COLOR_MODE_COOKIE_NAME } from '@/frame/lib/constants'
import { CssColorMode, SupportedTheme, defaultCSSTheme } from '@/color-schemes/components/useTheme'

// A tiny script that runs synchronously in the document <head>, before the
// browser's first paint. It reads the `color_mode` cookie (set by github.com,
// not HttpOnly) and writes the matching `data-color-mode`, `data-light-theme`,
// and `data-dark-theme` attributes onto the <html> element. Without this, the
// 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.
//
// 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
// exact string for the CSP `script-src` allowance (no nonce, no unsafe-inline).
const modes = JSON.stringify(Object.values(CssColorMode))
const themes = JSON.stringify(Object.values(SupportedTheme))
const defaults = JSON.stringify(defaultCSSTheme)
const cookieName = JSON.stringify(COLOR_MODE_COOKIE_NAME)

export const colorModeScript = `(function(){
var MODES=${modes},THEMES=${themes},D=${defaults};
var css=D;
try{
var m=document.cookie.match(new RegExp('(?:^|; )'+${cookieName}+'=([^;]*)'));
if(m){
var p=JSON.parse(decodeURIComponent(m[1]));
var fMode=function(x){return MODES.indexOf(x)>-1?x:null;};
var fTheme=function(t){if(!t)return null;if(THEMES.indexOf(t.name)>-1)return t.name;if(THEMES.indexOf(t.color_mode)>-1)return t.color_mode;return null;};
css={colorMode:fMode(p.color_mode)||D.colorMode,lightTheme:fTheme(p.light_theme)||D.lightTheme,darkTheme:fTheme(p.dark_theme)||D.darkTheme};
}
}catch(e){}
try{
var h=document.documentElement;
h.setAttribute('data-color-mode',css.colorMode);
h.setAttribute('data-light-theme',css.lightTheme);
h.setAttribute('data-dark-theme',css.darkTheme);
}catch(e){}
})();`
81 changes: 81 additions & 0 deletions src/color-schemes/tests/color-mode-script.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { describe, expect, test } from 'vitest'

import { colorModeScript } from '../lib/color-mode-script'
import { getCssTheme } 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) {
const attrs: Record<string, string> = {}
const fakeDocument = {
cookie: rawCookie,
documentElement: {
setAttribute(name: string, value: string) {
attrs[name] = value
},
},
}
new Function('document', colorModeScript)(fakeDocument)
return attrs
}

function cookieFor(value: object) {
// The real cookie value is URL-encoded JSON, like the browser stores it.
return `color_mode=${encodeURIComponent(JSON.stringify(value))}`
}

function expectMatchesGetCssTheme(rawCookie: string, cookieValue: string) {
const css = getCssTheme(cookieValue)
expect(runScript(rawCookie)).toEqual({
'data-color-mode': css.colorMode,
'data-light-theme': css.lightTheme,
'data-dark-theme': css.darkTheme,
})
}

describe('colorModeScript', () => {
test('falls back to defaults when no cookie is set', () => {
expectMatchesGetCssTheme('', '')
})

test('falls back to defaults on junk cookie values', () => {
expectMatchesGetCssTheme('color_mode=not-valid-json', '')
})

test('respects a valid color_mode cookie', () => {
const value = {
color_mode: 'dark',
light_theme: { name: 'light_colorblind', color_mode: 'light' },
dark_theme: { name: 'dark_tritanopia', color_mode: 'dark' },
}
expectMatchesGetCssTheme(cookieFor(value), JSON.stringify(value))
})

test('honors supported named themes', () => {
const value = {
color_mode: 'auto',
light_theme: { name: 'light', color_mode: 'light' },
dark_theme: { name: 'dark_dimmed', color_mode: 'dark' },
}
expectMatchesGetCssTheme(cookieFor(value), JSON.stringify(value))
})

test('ignores unknown modes and themes', () => {
const value = {
color_mode: 'sepia',
light_theme: { name: 'rainbow', color_mode: 'rainbow' },
dark_theme: { name: 'midnight', color_mode: 'midnight' },
}
expectMatchesGetCssTheme(cookieFor(value), JSON.stringify(value))
})

test('reads the cookie even when other cookies are present', () => {
const value = { color_mode: 'light' }
const rawCookie = `_octo=GH1.1; color_mode=${encodeURIComponent(
JSON.stringify(value),
)}; logged_in=no`
expectMatchesGetCssTheme(rawCookie, JSON.stringify(value))
})
})
35 changes: 21 additions & 14 deletions src/content-render/unified/wrap-procedural-images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,28 @@ function insideOlLi(ancestors: Parent[]): boolean {
}

function visitor(node: Element, ancestors: Parent[]): void {
if (insideOlLi(ancestors)) {
const shallowClone: Element = Object.assign({}, node)
shallowClone.tagName = 'div'
shallowClone.properties = { class: 'procedural-image-wrapper' }
shallowClone.children = [node]
const parent = ancestors.at(-1)
if (parent && parent.children) {
parent.children = parent.children.map((child) => {
if (child.type === 'element' && (child as Element).tagName === 'img') {
return shallowClone
}
return child
})
if (!insideOlLi(ancestors)) return
const parent = ancestors.at(-1)
if (!parent || !parent.children) return

// When the image is already inside a <p> (the writer left a blank line before
// it), the paragraph already provides spacing. Wrapping a <div> inside that
// <p> produces invalid HTML (`<div>` cannot be a descendant of `<p>`), which
// the browser silently repairs for dangerouslySetInnerHTML but causes a React
// hydration mismatch when the content is rendered as real elements. So only
// add the wrapper for the no-<p> (tight list) case it was designed for.
if ((parent as Element).tagName === 'p') return

const shallowClone: Element = Object.assign({}, node)
shallowClone.tagName = 'div'
shallowClone.properties = { class: 'procedural-image-wrapper' }
shallowClone.children = [node]
parent.children = parent.children.map((child) => {
if (child.type === 'element' && (child as Element).tagName === 'img') {
return shallowClone
}
}
return child
})
}

export default function wrapProceduralImages() {
Expand Down
19 changes: 14 additions & 5 deletions src/fixtures/tests/images.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
import { describe, expect, test } from 'vitest'
import sharp from 'sharp'
import type { CheerioAPI } from 'cheerio'
import type { Cheerio, CheerioAPI } from 'cheerio'
import type { Element } from 'domhandler'

import { get, head, getDOM } from '@/tests/helpers/e2etest'
import { MAX_WIDTH } from '@/content-render/unified/rewrite-asset-img-tags'

// `getDOM` parses with `xmlMode: true`, which is case-sensitive on attribute
// names. The legacy string render path emits a lowercase `srcset`, but the
// React render path (hast -> JSX) emits React 19's camelCase `srcSet`. Both are
// valid HTML (attribute names are case-insensitive in browsers), so read either.
function srcsetOf(el: Cheerio<Element>): string | undefined {
return el.attr('srcset') ?? el.attr('srcSet')
}

describe('render Markdown image tags', () => {
test('page with a single image', async () => {
const $: CheerioAPI = await getDOM('/get-started/images/single-image')
Expand All @@ -14,7 +23,7 @@ describe('render Markdown image tags', () => {

const sources = $('source', pictures)
expect(sources.length).toBe(1)
const srcset = sources.attr('srcset')
const srcset = srcsetOf(sources)
expect(srcset).toMatch(
new RegExp(`^/assets/cb-\\w+/mw-${MAX_WIDTH}/images/_fixtures/screenshot\\.webp 2x$`),
)
Expand Down Expand Up @@ -54,9 +63,9 @@ describe('render Markdown image tags', () => {
const sources = $('source', pictures)
expect(sources.length).toBe(3)

expect(sources.eq(0).attr('srcset')).toContain('1x') // 0
expect(sources.eq(1).attr('srcset')).toContain('2x') // 1
expect(sources.eq(2).attr('srcset')).toContain('2x') // 2
expect(srcsetOf(sources.eq(0))).toContain('1x') // 0
expect(srcsetOf(sources.eq(1))).toContain('2x') // 1
expect(srcsetOf(sources.eq(2))).toContain('2x') // 2
})

test('image inside a list keeps its span', async () => {
Expand Down
Loading
Loading