From 9df66568d1dc2e62752b42c7d30de298582adb34 Mon Sep 17 00:00:00 2001 From: Evan Bonsignori Date: Wed, 9 Sep 2026 23:51:40 +0000 Subject: [PATCH 1/8] Migrate the search results page to Primer Brand (#62942) --- data/ui.yml | 5 +- src/fixtures/fixtures/data/ui.yml | 5 +- src/fixtures/tests/playwright-a11y.spec.ts | 34 +++++ .../components/DefaultLayout.module.scss | 10 ++ src/frame/components/DefaultLayout.tsx | 21 +++- .../page-header/DocsSecondaryBar.tsx | 6 +- .../components/sidebar/SidebarNav.module.scss | 62 ++++++++++ src/frame/components/sidebar/SidebarNav.tsx | 49 ++++++-- .../results/Aggregations.module.scss | 117 +++++++++++++++++- .../components/results/Aggregations.tsx | 115 ++++++++++++----- .../components/results/NoQuery.module.scss | 7 ++ src/search/components/results/NoQuery.tsx | 10 +- .../components/results/SearchPage.module.scss | 32 +++++ .../results/SearchResults.module.scss | 111 ++++++++++++++++- .../components/results/SearchResults.tsx | 62 ++++++---- .../SidebarSearchAggregates.module.scss | 88 +++++++++++++ .../results/SidebarSearchAggregates.tsx | 78 +++++++++++- src/search/components/results/index.tsx | 14 ++- .../middleware/general-search-middleware.ts | 19 ++- src/search/tests/rendering.ts | 13 +- 20 files changed, 765 insertions(+), 93 deletions(-) create mode 100644 src/search/components/results/SearchPage.module.scss create mode 100644 src/search/components/results/SidebarSearchAggregates.module.scss diff --git a/data/ui.yml b/data/ui.yml index 7b4565bf4036..7708b929f7a9 100644 --- a/data/ui.yml +++ b/data/ui.yml @@ -93,7 +93,10 @@ search_results: n_results: '{n} results' search_validation_error: Validation error with search query filter: Filter - clear_filter: Clear + clear_all_filters: Clear all + show_filters: Show filters + hide_filters: Hide filters + filter_search_results: Filter search results homepage: explore_by_product: Explore by product diff --git a/src/fixtures/fixtures/data/ui.yml b/src/fixtures/fixtures/data/ui.yml index 7b4565bf4036..7708b929f7a9 100644 --- a/src/fixtures/fixtures/data/ui.yml +++ b/src/fixtures/fixtures/data/ui.yml @@ -93,7 +93,10 @@ search_results: n_results: '{n} results' search_validation_error: Validation error with search query filter: Filter - clear_filter: Clear + clear_all_filters: Clear all + show_filters: Show filters + hide_filters: Hide filters + filter_search_results: Filter search results homepage: explore_by_product: Explore by product diff --git a/src/fixtures/tests/playwright-a11y.spec.ts b/src/fixtures/tests/playwright-a11y.spec.ts index 5aff5bc5bec9..0eacad14ed50 100644 --- a/src/fixtures/tests/playwright-a11y.spec.ts +++ b/src/fixtures/tests/playwright-a11y.spec.ts @@ -2,6 +2,8 @@ import { test, expect } from '@playwright/test' import AxeBuilder from '@axe-core/playwright' import { turnOffExperimentsInPage, turnOnExperimentsInPage } from '../helpers/turn-off-experiments' +const SEARCH_TESTS = !!process.env.ELASTICSEARCH_URL + const pages: { [key: string]: string } = { category: '/actions/category', codeAnnotations: '/get-started/markdown/code-annotations', @@ -45,3 +47,35 @@ for (const pageName of Object.keys(pages)) { }) }) } + +// The search facet filters collapse behind a "Show filters" disclosure below +// Primer Brand's `medium` breakpoint. The scans above run at the default desktop +// viewport, where that disclosure is display:none, so the expanded panel would +// otherwise never be scanned. +test.describe('search filters (narrow viewport)', () => { + // Without a local Elasticsearch the middleware proxies to production, so there are no + // aggregations, the disclosure never renders, and this would time out rather than skip + // — matching the guard every search test in playwright-rendering.spec.ts uses. + test.skip(!SEARCH_TESTS, 'No local Elasticsearch, no tests involving search') + + test('expanded filter disclosure passes axe', async ({ page }) => { + await page.setViewportSize({ width: 390, height: 820 }) + await page.goto('/search?query=foo') + await turnOffExperimentsInPage(page) + + const toggle = page.getByTestId('search-filter-toggle') + await expect(toggle).toBeVisible() + await expect(toggle).toHaveAttribute('aria-expanded', 'false') + + await toggle.click() + await expect(toggle).toHaveAttribute('aria-expanded', 'true') + // Scoped to the disclosure's own panel: a bare `fieldset` locator would hit strict + // mode the moment anything else on the page renders one. + const panelId = await toggle.getAttribute('aria-controls') + await expect(page.locator(`#${panelId} fieldset`)).toBeVisible() + + const accessibilityScanResults = await new AxeBuilder({ page }).analyze() + + expect(accessibilityScanResults.violations).toEqual([]) + }) +}) diff --git a/src/frame/components/DefaultLayout.module.scss b/src/frame/components/DefaultLayout.module.scss index 9d113e41e172..f6dea1f56509 100644 --- a/src/frame/components/DefaultLayout.module.scss +++ b/src/frame/components/DefaultLayout.module.scss @@ -1,9 +1,19 @@ @import "src/frame/stylesheets/breakpoint-xxl.scss"; +@import "@primer/react-brand/lib/design-tokens/scss/tokens/functional/size/breakpoints.scss"; .mainContent { scroll-margin-top: 5rem; } +// The search results page splits into rail + results at brand's `medium` +// breakpoint rather than waiting for `d-lg-flex` at 1012px, so the facet rail is +// available on tablets. Applied only on /search. +.searchColumns { + @media (min-width: $brand-breakpoint-medium) { + display: flex; + } +} + // When the inline mobile/tablet nav is open, it takes over the viewport — hide // the content column so the full-width rail isn't squeezed beside it. Above the // xxl breakpoint the rail is a fixed-width sibling, so content always shows. diff --git a/src/frame/components/DefaultLayout.tsx b/src/frame/components/DefaultLayout.tsx index 68e014aa4261..dbbd9ddf6b35 100644 --- a/src/frame/components/DefaultLayout.tsx +++ b/src/frame/components/DefaultLayout.tsx @@ -263,14 +263,29 @@ export const DefaultLayout = (props: Props) => { type LayoutBodyProps = { children?: React.ReactNode; scrollToTopLabel: string } const LayoutBody = ({ children, scrollToTopLabel }: LayoutBodyProps) => { const { collapsed, mobileNavOpen } = useSidebarCollapsed() + const { currentProduct } = useMainContext() + // Matches SidebarNav's own gate rather than testing router.route. There are two search + // pages — src/pages/search.tsx and src/pages/[versionId]/search.tsx — so a route test + // for '/search' misses every versioned search URL, and this check would then disagree + // with SidebarNav about whether the rail is a facet rail. + const isSearchResultsPage = currentProduct?.id === 'search' return ( -
+ // `d-lg-flex` only goes side-by-side at 1012px. The search page's facet rail + // is meant to sit beside the results from brand's `medium` breakpoint, so it + // gets an earlier split of its own. Route-gated, so no other page moves. +
{/* `collapsed` is the desktop rail-collapse state (persisted). The inline mobile nav is independent, so still render the sidebar when it's open — otherwise opening the mobile nav while the desktop rail is collapsed hides the content column (contentHiddenForNav) with no drawer to show, - so the open nav displays a blank area instead of the doc tree. */} - {collapsed && !mobileNavOpen ? null : } + so the open nav displays a blank area instead of the doc tree. + + Search is exempt: the cookie is shared with the doc-tree rail, but the + search page has no toggle to undo it (DocsSecondaryBar returns null + there), so honouring it would strand the filters with no way back. */} + {collapsed && !mobileNavOpen && !isSearchResultsPage ? null : ( + + )} {/* Need to set an explicit height for sticky elements since we also set overflow to auto */}
{ const { t } = useTranslation('header') const { collapsed, toggleCollapsed, mobileNavOpen, toggleMobileNav } = useSidebarCollapsed() - const isSearchResultsPage = router.route === '/search' + // Product id rather than router.route: there are two search pages + // (src/pages/search.tsx and src/pages/[versionId]/search.tsx), so a route test for + // '/search' left this bar rendering on versioned search URLs only. The search rail's + // sticky offset assumes this bar is absent, so the two have to agree. + const isSearchResultsPage = currentProduct?.id === 'search' const isEarlyAccessPage = currentProduct && currentProduct.id === 'early-access' // Mirror the visibility rule of the header subnav this replaces. diff --git a/src/frame/components/sidebar/SidebarNav.module.scss b/src/frame/components/sidebar/SidebarNav.module.scss index 4d5d74c55339..e5abc5d21bdd 100644 --- a/src/frame/components/sidebar/SidebarNav.module.scss +++ b/src/frame/components/sidebar/SidebarNav.module.scss @@ -1,4 +1,5 @@ @import "src/frame/stylesheets/breakpoint-xxl.scss"; +@import "@primer/react-brand/lib/design-tokens/scss/tokens/functional/size/breakpoints.scss"; .sidebarFull { @include breakpoint-xxl { @@ -50,3 +51,64 @@ // Extra padding for REST pages (includes API version picker) padding-bottom: 250px; } + +// Docs 2026 search facet rail. Unlike the doc-tree rail this renders at every +// width: below brand's `medium` breakpoint it is a full-width block holding the +// "Show filters" disclosure, and from `medium` up it becomes the sidebar column. +// +// From `medium` up its right divider is the SINGLE border around the filters — +// the panel inside drops its own, so they don't read as a box within a box. +// +// It also deliberately does NOT reuse .sidebarFull's sticky offsets. Those assume +// a 109px header stack (65px header + 44px secondary bar), but DocsSecondaryBar +// returns null on /search, so the rail would otherwise sit 44px too low. +.searchRail { + // Display is set here rather than with a `d-block` utility: those carry + // `!important`, which would beat the `display: flex` below and leave the rail + // a block box, so the filter panel could never bound itself to the viewport. + display: block; + width: 100%; + + // A search with no hits has no facets to offer, so SidebarSearchAggregates renders + // nothing — but the column would still reserve its width and draw its divider, leaving + // an empty 300px gutter beside the "0 results" message. Collapse it when there is no + // filter group inside. Browsers without :has() simply keep the old empty column. + &:not(:has(fieldset)) { + display: none; + } + + @media (min-width: $brand-breakpoint-medium) { + position: sticky; + top: 65px; + display: flex; + flex-direction: column; + width: 300px; + height: calc(100vh - 65px); + flex-shrink: 0; + overflow: hidden; + border-right: var(--brand-borderWidth-thin) solid + var(--brand-color-border-subtle); + + // The nav has to join the same flex column, otherwise it sizes to its content + // and the filter card has no bounded height to scroll within. + > nav { + display: flex; + flex: 1; + flex-direction: column; + min-height: 0; + } + } + + @include breakpoint-xxl { + width: 326px; + } +} + +.searchRailContent { + @media (min-width: $brand-breakpoint-medium) { + display: flex; + flex: 1; + flex-direction: column; + min-height: 0; + } +} diff --git a/src/frame/components/sidebar/SidebarNav.tsx b/src/frame/components/sidebar/SidebarNav.tsx index 210a435ff86a..e53a1819a7ff 100644 --- a/src/frame/components/sidebar/SidebarNav.tsx +++ b/src/frame/components/sidebar/SidebarNav.tsx @@ -2,6 +2,7 @@ import cx from 'classnames' import { useRouter } from 'next/router' import { useMainContext } from '@/frame/components/context/MainContext' +import { useTranslation } from '@/languages/components/useTranslation' import { SidebarProduct } from '@/landings/components/SidebarProduct' import { SidebarSearchAggregates } from '@/search/components/results/SidebarSearchAggregates' import { ApiVersionPicker } from '@/rest/components/ApiVersionPicker' @@ -28,6 +29,10 @@ export const SidebarNav = ({ variant = 'full', mobileOpen = false }: Props) => { (process.env.NODE_ENV === 'development' || currentProduct.id !== 'early-access') const isSearch = currentProduct?.id === 'search' + // `search_results` only ships in the page props on /search, and + // createTranslationFunctions warns about a missing namespace at construction — not at + // t() call — so asking for it unconditionally would log on every render of every page. + const { t } = useTranslation(isSearch ? 'search_results' : 'header') return (
{ className={cx( // Desktop rail: sticky, hidden below xxl. When mobileOpen, it also // renders on mobile (block at all widths), full-width in the page flow. + // + // Search is the exception. Its rail holds the facet filters rather than + // a doc tree, and filters have to stay reachable on narrow viewports, so + // it renders at every width: a rail from brand's `medium` breakpoint up, + // and below that a "Show filters" disclosure (see SidebarSearchAggregates). variant === 'full' && - (mobileOpen - ? cx( - 'd-block d-xxl-block border-right', - styles.sidebarFull, - styles.sidebarFullMobileOpen, - ) - : cx('position-sticky d-none border-right d-xxl-block', styles.sidebarFull)), + (isSearch + ? styles.searchRail + : mobileOpen + ? cx( + 'd-block d-xxl-block border-right', + styles.sidebarFull, + styles.sidebarFullMobileOpen, + ) + : cx('position-sticky d-none border-right d-xxl-block', styles.sidebarFull)), )} >
) : ( <> - {props.children} + {props.children} )} @@ -260,8 +251,8 @@ export const DefaultLayout = (props: Props) => { // collapsed; on mobile it shows inline (in the page flow, like desktop) only // when the nav is opened from the secondary bar. The content column (flex-1) // fills the row when the rail is absent. -type LayoutBodyProps = { children?: React.ReactNode; scrollToTopLabel: string } -const LayoutBody = ({ children, scrollToTopLabel }: LayoutBodyProps) => { +type LayoutBodyProps = { children?: React.ReactNode } +const LayoutBody = ({ children }: LayoutBodyProps) => { const { collapsed, mobileNavOpen } = useSidebarCollapsed() const { currentProduct } = useMainContext() // Matches SidebarNav's own gate rather than testing router.route. There are two search @@ -300,14 +291,7 @@ const LayoutBody = ({ children, scrollToTopLabel }: LayoutBodyProps) => { {children} -
- - - -
+
) diff --git a/src/frame/components/page-footer/Contribution.tsx b/src/frame/components/page-footer/Contribution.tsx index 17875ec0e82a..5fdb93ab3243 100644 --- a/src/frame/components/page-footer/Contribution.tsx +++ b/src/frame/components/page-footer/Contribution.tsx @@ -1,5 +1,3 @@ -import { GitPullRequestIcon } from '@primer/octicons-react' - import { useMainContext } from '@/frame/components/context/MainContext' import { useTranslation } from '@/languages/components/useTranslation' @@ -11,19 +9,16 @@ export const Contribution = () => { ? `https://github.com/github/docs/blob/main/content/${relativePath}` : 'https://github.com/github/docs' + // Heading and body styling comes from the footer column rules in + // SupportSection.module.scss — the Docs 2026 design renders these as plain body + // text rather than a bold heading plus muted copy. return ( ) } diff --git a/src/frame/components/page-footer/DocsFooter.module.scss b/src/frame/components/page-footer/DocsFooter.module.scss new file mode 100644 index 000000000000..740cd196179c --- /dev/null +++ b/src/frame/components/page-footer/DocsFooter.module.scss @@ -0,0 +1,180 @@ +// In dark mode the footer sits on the repo's canvas (#0d1117) while brand's +// canvas-subtle comes from brand's own palette (#0f1511). Those are near-identical, +// so the controls would lose their fill and read as bare outlines. Use Primer's +// surfaces there instead — they're built to contrast with this background. Selector +// pattern follows octicon-table-optimization.scss. +@mixin footer-control-surfaces-dark { + --color-btn-bg: var(--color-canvas-subtle); + --color-btn-border: var(--color-border-default); + --color-btn-text: var(--color-fg-default); + --brand-footer-backToTop-bgColor-rest: var(--color-canvas-subtle); + // Brand's own dark border token is a different grey from Primer's, which would + // leave Back to top outlined differently from the three buttons beside it. + --brand-footer-backToTop-borderColor-rest: var(--color-border-default); +} + +.docsFooter { + // Brand defaults the footer to canvas-subtle (a grey panel); the design puts it on + // the page canvas so it reads as continuous with the article and the band above. + // Deliberately the *repo's* canvas, not brand's: brand's canvas-default is pure + // black in dark mode while these pages sit on #0d1117, which would make the footer + // a visibly darker slab. + --brand-footer-bg-color: var(--bgColor-default, var(--color-canvas-default)); + + // The design draws all four footer controls — Yes, No, Make a contribution and + // Back to top — on brand's subtle canvas with a subtle border (#f2f5f3 on #d2d9d4, + // which is exactly what these brand tokens resolve to). + // + // Yes/No and the contribution CTA are Primer `.btn`s, so set the variables they + // read rather than overriding the rules: the survey's selected states apply + // `color-bg-success-emphasis` / `color-bg-danger-emphasis`, which set + // background-color directly and must keep winning over the rest state. + --color-btn-bg: var(--brand-color-canvas-subtle); + --color-btn-border: var(--brand-color-border-subtle); + --color-btn-text: var(--brand-color-text-default); + + // Brand gives Back to top a near-transparent fill by default; the design gives it + // the same treatment as the other three. Its border already matches. + --brand-footer-backToTop-bgColor-rest: var(--brand-color-canvas-subtle); + + :global([data-color-mode="dark"]) & { + @include footer-control-surfaces-dark; + } + + @media (prefers-color-scheme: dark) { + :global([data-color-mode="auto"][data-dark-theme*="dark"]) & { + @include footer-control-surfaces-dark; + } + } + + // The band above supplies the visual break, and the design puts a 2px emphasized + // rule directly beneath it — on the footer's own top edge. Brand's default here is + // a much brighter green (scale-green-3), so this stays an explicit override. + border-top: 2px solid var(--brand-color-text-emphasized); + + // Brand's section padding is roughly double the design's. Targeted through markers + // we own rather than brand's hashed module classes, which change between releases. + // + // Top bar: 76px in the design, sized by the 44px back-to-top control plus 16px. + > div:has(.backToTop) > section { + padding-top: var(--base-size-16); + padding-bottom: var(--base-size-16); + } + + // Help region: the columns own their padding, so the section adds none. Without + // this the columns sit 32px below the rule that introduces them, and — because the + // rules between rows are column borders — those rules would stop short of the + // footer edges instead of spanning it as the design shows. The inline inset moves + // onto the columns in SupportSection.module.scss so content stays aligned with the + // top and bottom bars. + > div:has(.centerSlot) > section { + padding: 0; + } + + // Bottom bar: 76px in the design around a single 18px line. + > div:has(.bottomRow) > section { + padding-top: var(--base-size-28); + padding-bottom: var(--base-size-28); + } + + // `copyrightStatement` renders inside a brand that is a flex item of + // the bottom row. Let it fill the row so the copyright and links can sit at + // opposite ends, and drop the uppercasing the component applies to the whole

+ // (the design uppercases only the copyright, not the links). + p:has(> .bottomRow) { + flex: 1; + // Inert while the bottom row is horizontal (flex-basis wins), but needed at + // narrow, where brand's bottom row turns into a column with align-items: + // flex-start and would otherwise shrink this to its content width. + width: 100%; + min-width: 0; + text-transform: none; + } +} + +.bottomRow { + display: flex; + flex-wrap: wrap; + align-items: center; + justify-content: space-between; + gap: var(--base-size-16); + width: 100%; +} + +.copyrightText { + text-transform: uppercase; +} + +// Narrow: the design stacks the legal links above the copyright, and gives each its +// own band separated by a rule that spans the footer. So the bottom section hands its +// padding to the two rows — the same move the help region makes — which lets the rule +// between them reach the edges while both rows stay inset and aligned with each other. +@media (max-width: 767px) { + .docsFooter > div:has(.bottomRow) > section { + padding-block: 0; + } + + .docsFooter > div:has(.bottomRow) > section > div { + padding-inline: 0; + } + + .bottomRow { + flex-direction: column; + align-items: stretch; + gap: 0; + } + + .legalLinks { + order: 1; + padding: var(--base-size-20); + } + + .copyrightText { + order: 2; + padding: var(--base-size-20); + border-top: 1px solid + var(--borderColor-muted, var(--color-border-muted, #d2d9d4)); + } +} + +.legalLinks { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: var(--base-size-24); +} + +.legalLink { + // The design sets these apart from the muted monospace copyright: link blue, and + // the body font at the same size as the other footer link columns. + font-family: var(--brand-body-fontFamily, inherit); + font-size: 14px; + font-weight: 500; + text-transform: none; + // Brand's link token, not Primer's accent blue. There is no + // `--brand-color-text-link`; the real token is `-rest`, and naming it wrong meant + // this quietly fell back to Primer (#0969da) instead of brand (#0055d5). + color: var(--brand-color-text-link-rest); + + &:hover { + text-decoration: underline; + } + + // Pipe separators between links, per the design. Decorative, so a pseudo-element + // keeps them out of the accessibility tree and out of the copied text. + &:not(:first-child)::before { + content: ""; + display: inline-block; + width: 1px; + height: 10px; + margin-right: var(--base-size-24); + vertical-align: -1px; + background-color: var(--brand-color-border-subtle); + } + + // KO law requires the privacy statement link to be conspicuous. + &[data-conspicuous="true"] { + color: var(--fgColor-attention, var(--color-fg-attention, #9a6700)); + font-weight: 600; + } +} diff --git a/src/frame/components/page-footer/DocsFooter.tsx b/src/frame/components/page-footer/DocsFooter.tsx new file mode 100644 index 000000000000..995040c7429a --- /dev/null +++ b/src/frame/components/page-footer/DocsFooter.tsx @@ -0,0 +1,137 @@ +import { type MouseEvent } from 'react' +import { useRouter } from 'next/router' +import { MinimalFooter, Text } from '@primer/react-brand' +import cx from 'classnames' + +import { FooterDivider } from '@/frame/components/page-footer/FooterDivider' +import { SupportSection } from '@/frame/components/page-footer/SupportSection' +import { useTranslation } from '@/languages/components/useTranslation' + +import styles from './DocsFooter.module.scss' + +// The Docs 2026 site footer (Figma node 123-6013): decorative band, then brand +// MinimalFooter supplying the logomark + back-to-top row, the help region, and the +// legal/copyright strip. +// +// The design puts the legal links in the *bottom* row beside the copyright. +// MinimalFooter.Link children render in the top row instead, and the two rows live +// in separate DOM subtrees so no amount of CSS moves one into the other. Passing the +// links through `copyrightStatement` — which accepts a ReactElement and renders in +// the bottom row — gets the designed layout without overriding brand internals. +// It also sidesteps the component's hard cap of five links. +// +// Note `copyrightStatement` is rendered inside a , so everything here +// must be phrasing content: spans and anchors only, no lists or