diff --git a/src/color-schemes/README.md b/src/color-schemes/README.md index 54a41ac2a914..6a51c7c47d4d 100644 --- a/src/color-schemes/README.md +++ b/src/color-schemes/README.md @@ -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 ``. It runs before the first paint, reads the `color_mode` cookie, and sets `data-color-mode`, `data-light-theme`, and `data-dark-theme` on ``. + +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: @@ -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 `` `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. \ No newline at end of file diff --git a/src/color-schemes/components/useTheme.ts b/src/color-schemes/components/useTheme.ts index b5e9fea3aa3e..cb0ad4c841e8 100644 --- a/src/color-schemes/components/useTheme.ts +++ b/src/color-schemes/components/useTheme.ts @@ -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', @@ -14,7 +14,7 @@ enum ComponentColorMode { night = 'night', } -enum SupportedTheme { +export enum SupportedTheme { light = 'light', dark = 'dark', dark_dimmed = 'dark_dimmed', diff --git a/src/color-schemes/lib/color-mode-script.ts b/src/color-schemes/lib/color-mode-script.ts new file mode 100644 index 000000000000..c5272af904d0 --- /dev/null +++ b/src/color-schemes/lib/color-mode-script.ts @@ -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 , 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 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){} +})();` diff --git a/src/color-schemes/tests/color-mode-script.ts b/src/color-schemes/tests/color-mode-script.ts new file mode 100644 index 000000000000..cd83d53c030f --- /dev/null +++ b/src/color-schemes/tests/color-mode-script.ts @@ -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 = {} + 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)) + }) +}) diff --git a/src/content-render/unified/wrap-procedural-images.ts b/src/content-render/unified/wrap-procedural-images.ts index 94713d7ac061..4d39db65f34a 100644 --- a/src/content-render/unified/wrap-procedural-images.ts +++ b/src/content-render/unified/wrap-procedural-images.ts @@ -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

(the writer left a blank line before + // it), the paragraph already provides spacing. Wrapping a

inside that + //

produces invalid HTML (`

` cannot be a descendant of `

`), 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-

(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() { diff --git a/src/fixtures/tests/images.ts b/src/fixtures/tests/images.ts index 229d9c188d24..a1eaaccfec5d 100644 --- a/src/fixtures/tests/images.ts +++ b/src/fixtures/tests/images.ts @@ -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): 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') @@ -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$`), ) @@ -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 () => { diff --git a/src/frame/components/CodeTabsGroup.tsx b/src/frame/components/CodeTabsGroup.tsx new file mode 100644 index 000000000000..2d326d26eb03 --- /dev/null +++ b/src/frame/components/CodeTabsGroup.tsx @@ -0,0 +1,164 @@ +import { + createContext, + useCallback, + useContext, + useEffect, + useId, + useMemo, + useState, + isValidElement, + Children, + cloneElement, + type ReactElement, + type ReactNode, + type MouseEvent as ReactMouseEvent, + type KeyboardEvent as ReactKeyboardEvent, +} from 'react' +import { useRouter } from 'next/router' +import { UnderlineNav } from '@primer/react' +import cx from 'classnames' + +import Cookies from '@/frame/components/lib/cookies' +import { CODE_SAMPLE_LANGUAGE_COOKIE_NAME } from '@/frame/lib/constants' +import { sendEvent } from '@/events/components/events' +import { EventType } from '@/events/types' +import { useTranslation } from '@/languages/components/useTranslation' + +// React-native replacement for the imperative CodeTabs enhancer (#6619). The old +// component scanned `#article-contents` for `.ghd-codetabs`, inserted a foreign +// `.ghd-codetabs-nav` mountPoint as the container's first child, portaled a nav +// into it, and toggled panel attributes — destructive surgery on React-owned +// nodes that breaks on client-side navigation teardown. Instead, the article body +// hast maps each `.ghd-codetabs` container to , which reads its +// `.ghd-codetab` panel children straight from props and renders the nav + panels +// itself. No DOM scanning, no portal, no foreign nodes. +// +// The selected language lives in CodeLanguageContext so multiple code-tab groups +// on one page stay in sync and share the language cookie, matching the previous +// single-component behavior. + +type CodeLanguageContextT = { + language: string + setLanguage: (value: string) => void +} + +const CodeLanguageContext = createContext({ + language: '', + setLanguage: () => {}, +}) + +export function CodeTabsProvider({ children }: { children: ReactNode }) { + // Start empty so server + first client render select each group's first tab + // (deterministic, hydration-safe). The cookie preference is applied after + // hydration, the same moment the old imperative enhancer used to run. + const [language, setLanguageState] = useState('') + + useEffect(() => { + const cookieValue = Cookies.get(CODE_SAMPLE_LANGUAGE_COOKIE_NAME) + if (cookieValue) setLanguageState(cookieValue) + }, []) + + const setLanguage = useCallback((value: string) => { + setLanguageState(value) + Cookies.set(CODE_SAMPLE_LANGUAGE_COOKIE_NAME, value) + sendEvent({ + type: EventType.preference, + preference_name: 'code_language', + preference_value: value, + }) + }, []) + + const value = useMemo( + () => ({ language, setLanguage }), + [language, setLanguage], + ) + + return {children} +} + +type PanelTab = { + key: string + label: string + panel: ReactElement<{ className?: string }> +} + +function hasClass(className: unknown, target: string): boolean { + return String(className || '') + .split(/\s+/) + .includes(target) +} + +function getActiveKey(tabs: PanelTab[], selectedLanguage: string): string { + return tabs.some((tab) => tab.key === selectedLanguage) ? selectedLanguage : (tabs[0]?.key ?? '') +} + +type CodeTabsGroupProps = { + className?: string + children?: ReactNode + [key: string]: unknown +} + +export function CodeTabsGroup({ className, children, ...rest }: CodeTabsGroupProps) { + const router = useRouter() + const { t } = useTranslation('code_tabs') + const { language, setLanguage } = useContext(CodeLanguageContext) + const baseId = useId() + + // Pull the `.ghd-codetab` panel children straight from the converted hast. Fail + // open (render the original markup) if the expected metadata isn't present. + const tabs: PanelTab[] = Children.toArray(children) + .filter((child): child is ReactElement<{ className?: string }> => isValidElement(child)) + .filter((child) => hasClass(child.props.className, 'ghd-codetab')) + .map((panel) => { + const props = panel.props as { 'data-lang'?: string; 'data-label'?: string } + const key = props['data-lang'] + const label = props['data-label'] + if (!key || !label) return null + return { key, label, panel } + }) + .filter((tab): tab is PanelTab => tab !== null) + + if (!tabs.length) { + return ( +

+ {children} +
+ ) + } + + const activeKey = getActiveKey(tabs, language) + + return ( +
+
+ {/* key on asPath works around a Primer UnderlineNav re-render bug. */} + + {tabs.map((tab) => ( + { + event.preventDefault() + setLanguage(tab.key) + }} + > + {tab.label} + + ))} + +
+ {tabs.map((tab, index) => { + const isActive = tab.key === activeKey + return cloneElement(tab.panel, { + key: tab.key, + id: `${baseId}-panel-${index}`, + role: 'tabpanel', + tabIndex: 0, + hidden: !isActive, + className: cx(tab.panel.props.className, { 'ghd-codetab-hidden': !isActive }), + } as Record) + })} +
+ ) +} diff --git a/src/frame/components/article/ArticlePage.tsx b/src/frame/components/article/ArticlePage.tsx index 212909ccd7da..68a6a826597e 100644 --- a/src/frame/components/article/ArticlePage.tsx +++ b/src/frame/components/article/ArticlePage.tsx @@ -21,6 +21,8 @@ import { CodeTabs } from '@/frame/components/CodeTabs' import { JourneyTrackCard, JourneyTrackNav } from '@/journeys/components' import { CopyMarkdownMenu } from './ViewMarkdownButton' import { ExperimentContentSwap } from '@/events/components/experiments/ExperimentContentSwap' +import { SelectionProvider } from '@/tools/components/SelectionContext' +import { CodeTabsProvider } from '@/frame/components/CodeTabsGroup' const ClientSideRefresh = dynamic(() => import('@/frame/components/ClientSideRefresh'), { ssr: false, @@ -34,6 +36,7 @@ export const ArticlePage = () => { intro, effectiveDate, renderedPage, + renderedPageHast, permissions, includesPlatformSpecificContent, includesToolSpecificContent, @@ -77,7 +80,11 @@ export const ArticlePage = () => { const articleContents = (
- {renderedPage} + {renderedPageHast ? ( + + ) : ( + {renderedPage} + )} {effectiveDate && (
@@ -92,56 +99,62 @@ export const ArticlePage = () => { return ( - - - - {isDev && } - {router.pathname.includes('/rest/') && } - {currentLayout === 'inline' ? ( - <> - {title}} - intro={introProp} - introCallOuts={introCalloutsProp} - toc={toc} - breadcrumbs={} - > - {articleContents} - - {isJourneyTrack ? ( -
- -
- ) : null} - - ) : ( -
-
- -
+ + + + + {/* The imperative CodeTabs enhancer only runs for the string fallback + path; on the hast path, CodeTabsGroup renders tabs React-natively. */} + {!renderedPageHast && } + {isDev && } + {router.pathname.includes('/rest/') && } + {currentLayout === 'inline' ? ( + <> + {title}} + intro={introProp} + introCallOuts={introCalloutsProp} + toc={toc} + breadcrumbs={} + > + {articleContents} + + {isJourneyTrack ? ( +
+ +
+ ) : null} + + ) : ( +
+
+ +
- {title}} - intro={ - <> - {introProp} - {introCalloutsProp} - - } - toc={toc} - > - {articleContents} - + {title}} + intro={ + <> + {introProp} + {introCalloutsProp} + + } + toc={toc} + > + {articleContents} + - {isJourneyTrack ? ( -
- + {isJourneyTrack ? ( +
+ +
+ ) : null}
- ) : null} -
- )} + )} + + ) } diff --git a/src/frame/components/context/ArticleContext.tsx b/src/frame/components/context/ArticleContext.tsx index c91ab9253234..43dc59e2d3f2 100644 --- a/src/frame/components/context/ArticleContext.tsx +++ b/src/frame/components/context/ArticleContext.tsx @@ -17,6 +17,7 @@ export type ArticleContextT = { intro: string effectiveDate: string renderedPage: string | JSX.Element[] + renderedPageHast?: import('hast').Root miniTocItems: Array permissions?: string includesPlatformSpecificContent: boolean @@ -60,6 +61,7 @@ interface ContextRequest { context: { page: Record & { fullPath: string; title: string; intro: string } renderedPage?: string + renderedPageHast?: import('hast').Root miniTocItems?: MiniTocItem[] currentJourneyTrack?: JourneyContext currentLayoutName?: string @@ -97,6 +99,7 @@ export const getArticleContextFromRequest = (req: ContextRequest): ArticleContex intro: page.intro, effectiveDate, renderedPage: (req.context.renderedPage as string) || '', + renderedPageHast: req.context.renderedPageHast, miniTocItems: req.context.miniTocItems || [], permissions: (page.permissions as string) || '', includesPlatformSpecificContent: (page.includesPlatformSpecificContent as boolean) || false, diff --git a/src/frame/components/context/MainContext.tsx b/src/frame/components/context/MainContext.tsx index 9a48ac250781..31e2afc7e877 100644 --- a/src/frame/components/context/MainContext.tsx +++ b/src/frame/components/context/MainContext.tsx @@ -191,6 +191,11 @@ export const getMainContext = async ( if (context.currentJourneyTrack?.trackId) { addUINamespaces(req, ui, ['journey_track_nav']) } + // CodeTabs (rendered React-natively from the article body hast) needs its i18n + // strings shipped to the page; only articles can contain code tabs. + if (documentType === 'article') { + addUINamespaces(req, ui, ['code_tabs']) + } // Product index pages (depth-2 index.md, e.g. actions/index.md) need the // full product tree for landing rendering. diff --git a/src/frame/components/ui/MarkdownContent/MarkdownContent.tsx b/src/frame/components/ui/MarkdownContent/MarkdownContent.tsx index 123058a88eff..ef2102c7aaef 100644 --- a/src/frame/components/ui/MarkdownContent/MarkdownContent.tsx +++ b/src/frame/components/ui/MarkdownContent/MarkdownContent.tsx @@ -6,6 +6,7 @@ import type { Root as HastRoot } from 'hast' import cx from 'classnames' import { CopyButton } from '@/frame/components/CopyButton' +import { CodeTabsGroup } from '@/frame/components/CodeTabsGroup' import { ToggleableContent } from '@/tools/components/ToggleableContent' import { isToggleClass } from '@/tools/components/SelectionContext' import styles from './MarkdownContent.module.scss' @@ -35,6 +36,10 @@ const markdownComponents = { // and runs first, so only the handful of toggleable elements become context // consumers; every other div/span renders as a plain element with no hook. div(props: ComponentProps<'div'>) { + const classes = String(props.className || '').split(/\s+/) + if (classes.includes('ghd-codetabs')) { + return + } if (isToggleClass(props.className)) { return } diff --git a/src/frame/components/ui/MiniTocs/MiniTocs.tsx b/src/frame/components/ui/MiniTocs/MiniTocs.tsx index 46ee2f5e5a81..99920b1ae9e1 100644 --- a/src/frame/components/ui/MiniTocs/MiniTocs.tsx +++ b/src/frame/components/ui/MiniTocs/MiniTocs.tsx @@ -4,6 +4,11 @@ import cx from 'classnames' import type { MiniTocItem } from '@/frame/components/context/ArticleContext' import { useTranslation } from '@/languages/components/useTranslation' +import { + classifyToggleClass, + isContentVisible, + useSelection, +} from '@/tools/components/SelectionContext' import styles from './Minitocs.module.scss' @@ -13,6 +18,7 @@ export type MiniTocsPropsT = { function RenderTocItem(item: MiniTocItem) { const [currentAnchor, setCurrentAnchor] = useState('') + const { platform, tool } = useSelection() useEffect(() => { const onHashChanged = () => { @@ -26,6 +32,14 @@ function RenderTocItem(item: MiniTocItem) { } }, []) + // `item.platform` holds the class string of the heading's `.ghd-tool` ancestor + // (platform OR tool value). Hide the TOC entry when its platform/tool isn't the + // selected one, replacing the old imperative parent-
  • `style.display` hack. + const classification = classifyToggleClass(item.platform) + if (classification && !isContentVisible(classification, { platform, tool })) { + return null + } + return ( <> { return (await pageRenderTimed(context)) as string } +/** + * Spike for #6619 (remove dangerouslySetInnerHTML): produce the article body as + * a serializable hast (HTML AST) tree alongside the legacy HTML string. + * + * Must run AFTER buildRenderedPage, which calls page.render and populates the + * context fields the pipeline reads (englishHeadings, alertTitles). We render + * the same raw `page.markdown`, but with a context clone that omits + * `collectMiniToc` so the mini-TOC isn't collected a second time. + * + * Wrapped so a hast failure can never break the page: the React layer falls + * back to the string path when this is undefined. NOTE: this currently renders + * the body pipeline twice; the production design (see #6619 plan) should produce + * hast once and derive the string from it. + */ +async function buildRenderedPageHast(req: ExtendedRequest) { + const { context } = req + if (!context) throw new Error('request not contextualized') + const { page } = context + if (!page || !page.markdown) return undefined + + try { + const hastContext = { ...context, collectMiniToc: undefined } + const { hast } = await renderContentToHast(page.markdown, hastContext) + return hast || undefined + } catch (error) { + logger.error( + 'buildRenderedPageHast failed; falling back to string path', + error instanceof Error ? error : new Error(String(error)), + { path: req.pagePath || req.path }, + ) + return undefined + } +} + function buildMiniTocItems(req: ExtendedRequest) { const { context } = req if (!context) throw new Error('request not contextualized') @@ -119,6 +154,7 @@ export default async function renderPage(req: ExtendedRequest, res: Response) { ) } else { req.context.renderedPage = await buildRenderedPage(req) + req.context.renderedPageHast = await buildRenderedPageHast(req) req.context.miniTocItems = buildMiniTocItems(req) } diff --git a/src/frame/pages/app.tsx b/src/frame/pages/app.tsx index 357532d2cec4..979be3598c20 100644 --- a/src/frame/pages/app.tsx +++ b/src/frame/pages/app.tsx @@ -86,37 +86,6 @@ const MyApp = ({ Component, pageProps, languagesContext, stagingName }: MyAppPro } }, [router, router.query, pageProps.mainContext]) - useEffect(() => { - // The CSS from primer looks something like this: - // - // @media (prefers-color-scheme: dark) [data-color-mode=auto][data-dark-theme=dark] { - // --color-canvas-default: black; - // } - // html { - // background-color: var(--color-canvas-default); - // } - // - // So if that `[data-color-mode][data-dark-theme=dark]` isn't present - // on the html, but on a top-level wrapping `
    ` then the `` - // doesn't get the right CSS. - // Normally, with Primer you make sure you set these things in the - // `` tag and you can use `_document.tsx` for that but that's - // only something you can do in server-side rendering. So, - // we use a hook to assure that the `` tag has the correct - // dataset attribute values. - const html = document.querySelector('html') - if (html) { - // Note, this is the same as setting `` - // But you can't do `html.dataset['color-mode']` so you use the - // camelCase variant and you get the same effect. - // Appears Next.js can't modify after server rendering: - // https://stackoverflow.com/a/54774431 - html.dataset.colorMode = theme.css.colorMode - html.dataset.darkTheme = theme.css.darkTheme - html.dataset.lightTheme = theme.css.lightTheme - } - }, [theme]) - return ( <> diff --git a/src/ghes-releases/lib/release-issues.ts b/src/ghes-releases/lib/release-issues.ts new file mode 100644 index 000000000000..dfcb373786b6 --- /dev/null +++ b/src/ghes-releases/lib/release-issues.ts @@ -0,0 +1,52 @@ +export type IssueState = 'open' | 'closed' | 'all' + +const VALID_ISSUE_STATES: IssueState[] = ['open', 'closed', 'all'] +const EXCLUDED_RELEASE_LABELS = new Set(['public roadmap', 'not planned']) + +interface IssueLike { + labels: { name: string }[] +} + +/** + * Parse and validate the issue state filter. Defaults to "all". + */ +export function parseIssueState(value?: string): IssueState { + if (!value) return 'all' + + const normalized = value.toLowerCase() + if (VALID_ISSUE_STATES.includes(normalized as IssueState)) { + return normalized as IssueState + } + + throw new Error( + `Invalid issue state "${value}". Expected one of: ${VALID_ISSUE_STATES.join(', ')}`, + ) +} + +/** + * Build gh CLI args for listing release issues. + */ +export function buildReleaseIssueListArgs(version: string, issueState: IssueState): string[] { + const label = `GHES ${version}` + return [ + 'issue', + 'list', + '--repo', + 'github/releases', + '--label', + label, + '--state', + issueState, + '--limit', + '200', + '--json', + 'number,title,url,body,labels', + ] +} + +/** + * Excludes release issues that should not produce GHES release notes. + */ +export function isExcludedReleaseIssue(issue: IssueLike): boolean { + return issue.labels.some((l) => EXCLUDED_RELEASE_LABELS.has(l.name.toLowerCase())) +} diff --git a/src/ghes-releases/scripts/generate-release-notes.ts b/src/ghes-releases/scripts/generate-release-notes.ts index ca0fb075554e..aae793be2c2e 100644 --- a/src/ghes-releases/scripts/generate-release-notes.ts +++ b/src/ghes-releases/scripts/generate-release-notes.ts @@ -3,7 +3,7 @@ * @description Generate GHES release notes from github/releases issues using Copilot CLI * * Generate GHES release notes by: - * 1. Querying github/releases for open issues labeled "GHES " + * 1. Querying github/releases issues labeled "GHES " (all states by default) * 2. Finding corresponding changelog PRs in github/blog * 3. Running each through the ghes-release-notes agent via Copilot CLI * 4. Stitching the YAML outputs into a release notes file @@ -25,6 +25,12 @@ import { buildReleaseNotesYaml, appendNoteLines, } from '@/ghes-releases/lib/parse-release-notes' +import { + type IssueState, + buildReleaseIssueListArgs, + isExcludedReleaseIssue, + parseIssueState, +} from '@/ghes-releases/lib/release-issues' // ─── Ctrl+C handling ───────────────────────────────────────────────────────── // Copilot CLI puts the terminal in raw mode, so we catch Ctrl+C (0x03) manually. @@ -94,25 +100,12 @@ function gh(args: string[]): string { } /** - * Fetch open release issues labeled "GHES " + * Fetch release issues labeled "GHES " using the selected issue state. */ -function fetchReleaseIssues(version: string): ReleaseIssue[] { - const label = `GHES ${version}` - const output = gh([ - 'issue', - 'list', - '--repo', - 'github/releases', - '--label', - label, - '--state', - 'open', - '--limit', - '200', - '--json', - 'number,title,url,body,labels', - ]) - return JSON.parse(output) as ReleaseIssue[] +function fetchReleaseIssues(version: string, issueState: IssueState): ReleaseIssue[] { + const output = gh(buildReleaseIssueListArgs(version, issueState)) + const issues = JSON.parse(output) as ReleaseIssue[] + return issues.filter((issue) => !isExcludedReleaseIssue(issue)) } interface ChangelogInfo { @@ -484,6 +477,11 @@ program return true }) .option('--stdout', 'Print output to console instead of writing to file') + .option( + '--issue-state ', + 'Issue state filter for github/releases issues (open, closed, all). Defaults to all.', + 'all', + ) .option( '-i, --issue ', 'Process a single issue by number or URL (replaces its entry if it already exists)', @@ -507,12 +505,20 @@ program release: string rc: boolean stdout?: boolean + issueState?: string issue?: number force?: boolean }) => { const { release, stdout, issue: singleIssue, force } = options const rc = options.rc ?? false const spinner = ora() + let issueState: IssueState + try { + issueState = parseIssueState(options.issueState) + } catch (error) { + console.error(`Error: ${(error as Error).message}`) + process.exit(1) + } // ── Prerequisite checks ── try { @@ -560,6 +566,12 @@ program 'number,title,url,body,labels', ]) const issue = JSON.parse(output) as ReleaseIssue + if (isExcludedReleaseIssue(issue)) { + spinner.fail( + `Issue #${singleIssue} is excluded by label (public roadmap or not planned).`, + ) + process.exit(0) + } issues = [issue] spinner.succeed(`Fetched issue #${singleIssue}: ${issue.title}`) } catch (error) { @@ -567,10 +579,12 @@ program process.exit(1) } } else { - spinner.start(`Fetching open issues labeled "GHES ${release}"...`) + spinner.start(`Fetching issues labeled "GHES ${release}" (state: ${issueState})...`) try { - issues = fetchReleaseIssues(release) - spinner.succeed(`Found ${issues.length} open issues labeled "GHES ${release}"`) + issues = fetchReleaseIssues(release, issueState) + spinner.succeed( + `Found ${issues.length} issues labeled "GHES ${release}" (state: ${issueState})`, + ) } catch (error) { spinner.fail(`Failed to fetch issues: ${(error as Error).message}`) process.exit(1) diff --git a/src/ghes-releases/tests/release-issues.ts b/src/ghes-releases/tests/release-issues.ts new file mode 100644 index 000000000000..10355bf04a34 --- /dev/null +++ b/src/ghes-releases/tests/release-issues.ts @@ -0,0 +1,59 @@ +import { describe, expect, test } from 'vitest' + +import { + buildReleaseIssueListArgs, + isExcludedReleaseIssue, + parseIssueState, + type IssueState, +} from '@/ghes-releases/lib/release-issues' + +describe('parseIssueState', () => { + test('defaults to all when not provided', () => { + expect(parseIssueState()).toBe('all') + }) + + test('accepts valid values case-insensitively', () => { + expect(parseIssueState('OPEN')).toBe('open') + expect(parseIssueState('closed')).toBe('closed') + expect(parseIssueState('All')).toBe('all') + }) + + test('throws on invalid values', () => { + expect(() => parseIssueState('anything-else')).toThrow('Invalid issue state') + }) +}) + +describe('buildReleaseIssueListArgs', () => { + test('builds gh issue list args with release label and state', () => { + const args = buildReleaseIssueListArgs('3.20', 'closed' satisfies IssueState) + + expect(args).toEqual([ + 'issue', + 'list', + '--repo', + 'github/releases', + '--label', + 'GHES 3.20', + '--state', + 'closed', + '--limit', + '200', + '--json', + 'number,title,url,body,labels', + ]) + }) +}) + +describe('isExcludedReleaseIssue', () => { + test('returns true for public roadmap label', () => { + expect(isExcludedReleaseIssue({ labels: [{ name: 'public roadmap' }] })).toBe(true) + }) + + test('returns true for not planned label (case-insensitive)', () => { + expect(isExcludedReleaseIssue({ labels: [{ name: 'Not Planned' }] })).toBe(true) + }) + + test('returns false when no excluded labels are present', () => { + expect(isExcludedReleaseIssue({ labels: [{ name: 'GHES 3.20' }, { name: 'bug' }] })).toBe(false) + }) +}) diff --git a/src/pages/_document.tsx b/src/pages/_document.tsx index 168d735433dd..b1fbb30b3088 100644 --- a/src/pages/_document.tsx +++ b/src/pages/_document.tsx @@ -1,19 +1,24 @@ import Document, { Html, Head, Main, NextScript } from 'next/document' import { defaultCSSTheme } from '@/color-schemes/components/useTheme' +import { colorModeScript } from '@/color-schemes/lib/color-mode-script' export default class MyDocument extends Document { render() { return ( - + +