Skip to content

Commit e9a7eaf

Browse files
EbonsignoriEbonsignori
authored
Migrate doc-tree sidebar to @primer/react-brand NavList (#62211)
1 parent a7c0da0 commit e9a7eaf

33 files changed

Lines changed: 1193 additions & 354 deletions

‎data/ui.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ header:
1919
menu: Menu
2020
open_menu_label: Open menu
2121
go_home: Home
22+
collapse_sidebar: Collapse sidebar
23+
expand_sidebar: Expand sidebar
24+
scroll_breadcrumbs_left: Scroll breadcrumbs left
25+
scroll_breadcrumbs_right: Scroll breadcrumbs right
2226

2327
picker:
2428
language_picker_label: Language

‎src/automated-pipelines/components/AutomatedPage.tsx‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { ArticleGridLayout } from '@/frame/components/article/ArticleGridLayout'
88
import { ArticleInlineLayout } from '@/frame/components/article/ArticleInlineLayout'
99
import { MiniTocs } from '@/frame/components/ui/MiniTocs'
1010
import { useAutomatedPageContext } from '@/automated-pipelines/components/AutomatedPageContext'
11-
import { Breadcrumbs } from '@/frame/components/page-header/Breadcrumbs'
1211
import { JourneyTrackCard, JourneyTrackNav } from '@/journeys/components'
1312

1413
type Props = {
@@ -70,7 +69,6 @@ export const AutomatedPage = ({ children, rawChildren, fullWidth }: Props) => {
7069
topper={<ArticleTitle>{title}</ArticleTitle>}
7170
intro={introProp}
7271
toc={toc}
73-
breadcrumbs={<Breadcrumbs />}
7472
>
7573
{articleContents}
7674
</ArticleInlineLayout>
@@ -84,14 +82,7 @@ export const AutomatedPage = ({ children, rawChildren, fullWidth }: Props) => {
8482
<div className="container-xl px-3 px-md-6 my-4">
8583
<ArticleGridLayout
8684
fullWidth={fullWidth}
87-
topper={
88-
<>
89-
<div className="d-none d-xl-block my-3 mr-auto width-full">
90-
<Breadcrumbs />
91-
</div>
92-
<ArticleTitle>{title}</ArticleTitle>
93-
</>
94-
}
85+
topper={<ArticleTitle>{title}</ArticleTitle>}
9586
intro={introProp}
9687
toc={toc}
9788
>

‎src/fixtures/fixtures/data/ui.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ header:
1919
menu: Menu
2020
open_menu_label: Open menu
2121
go_home: Home
22+
collapse_sidebar: Collapse sidebar
23+
expand_sidebar: Expand sidebar
24+
scroll_breadcrumbs_left: Scroll breadcrumbs left
25+
scroll_breadcrumbs_right: Scroll breadcrumbs right
2226

2327
picker:
2428
language_picker_label: Language

‎src/fixtures/tests/breadcrumbs.ts‎

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,70 +7,80 @@ import { getDOM } from '@/tests/helpers/e2etest'
77
describe('breadcrumbs', () => {
88
test('links always prefixed with language', async () => {
99
const $ = await getDOM('/get-started/start-your-journey/hello-world')
10-
const links = $('[data-testid=breadcrumbs-in-article] a')
10+
const links = $('[data-testid=breadcrumbs-bar] a')
1111
links.each((i, element) => {
12-
expect($(element).attr('href')!.startsWith('/en/')).toBe(true)
12+
const href = $(element).attr('href')!
13+
// The Home crumb points at the locale root (`/en` on the default version,
14+
// no trailing slash); every other crumb is under `/en/…`. Both are
15+
// language-prefixed, which is what this test guards.
16+
expect(href === '/en' || href.startsWith('/en/')).toBe(true)
1317
})
14-
expect.assertions(3)
18+
// Home crumb + the three trail crumbs for this path.
19+
expect.assertions(4)
1520
})
1621

1722
test('top-level hidden /search page has no breadcrumbs', async () => {
1823
const $ = await getDOM('/search')
19-
const links = $('[data-testid=breadcrumbs-in-article] a')
24+
const links = $('[data-testid=breadcrumbs-bar] a')
2025
expect(links.length).toBe(0)
2126
const headers = $('[data-testid=breadcrumbs-header]')
2227
expect(headers.length).toBe(0)
2328
})
2429

2530
test('short titles are preferred', async () => {
2631
const $ = await getDOM('/get-started/foo/bar')
27-
const links = $('[data-testid=breadcrumbs-in-article] li:last-child a')
32+
const links = $('[data-testid=breadcrumbs-bar] li:last-child a')
2833
expect(links.text()).toBe('Bar')
2934
})
3035

31-
test('article pages have breadcrumbs in article with product, category, subcategory, and article and last breadcrumb is not viewable', async () => {
36+
test('article pages have breadcrumbs in the secondary bar with home, product, category, subcategory, and article (all shown)', async () => {
3237
const $ = await getDOM('/get-started/start-your-journey/hello-world')
33-
const links = $('[data-testid=breadcrumbs-in-article] a')
34-
expect(links.length).toBe(3)
35-
expect($(links[0]).text()).toBe('Get started')
36-
expect($(links[0]).attr('class')!.includes('d-none')).toBe(false)
37-
expect($(links[1]).text()).toBe('Start your journey')
38+
const links = $('[data-testid=breadcrumbs-bar] a')
39+
// The secondary bar leads with a Home crumb, then the page trail.
40+
expect(links.length).toBe(4)
41+
expect($(links[0]).text()).toBe('Home')
42+
expect($(links[1]).text()).toBe('Get started')
3843
expect($(links[1]).attr('class')!.includes('d-none')).toBe(false)
39-
expect($(links[2]).text()).toBe('Hello World')
40-
expect($(links[2]).attr('class')!.includes('d-none')).toBe(true)
44+
expect($(links[2]).text()).toBe('Start your journey')
45+
expect($(links[2]).attr('class')!.includes('d-none')).toBe(false)
46+
expect($(links[3]).text()).toBe('Hello World')
47+
// The secondary-bar variant shows the full trail (no hidden last crumb).
48+
expect($(links[3]).attr('class')!.includes('d-none')).toBe(false)
4149
})
4250

4351
test('works for enterprise-server articles too', async () => {
4452
const $ = await getDOM('/enterprise-server@latest/get-started/start-your-journey/hello-world')
45-
const links = $('[data-testid=breadcrumbs-in-article] a')
46-
expect(links.length).toBe(3)
47-
expect($(links[0]).text()).toBe('Get started')
48-
expect($(links[1]).text()).toBe('Start your journey')
49-
expect($(links[2]).text()).toBe('Hello World')
53+
const links = $('[data-testid=breadcrumbs-bar] a')
54+
expect(links.length).toBe(4)
55+
expect($(links[0]).text()).toBe('Home')
56+
expect($(links[1]).text()).toBe('Get started')
57+
expect($(links[2]).text()).toBe('Start your journey')
58+
expect($(links[3]).text()).toBe('Hello World')
5059
})
5160

5261
test('works for titles that depend on Liquid', async () => {
5362
const $fpt = await getDOM('/get-started/start-your-journey/dynamic-title')
54-
const fptLinks = $fpt('[data-testid=breadcrumbs-in-article] a')
55-
expect($fpt(fptLinks[2]).text()).toBe('Hello HubGit')
63+
const fptLinks = $fpt('[data-testid=breadcrumbs-bar] a')
64+
// [0] is the Home crumb; the article is the last crumb.
65+
expect($fpt(fptLinks[3]).text()).toBe('Hello HubGit')
5666

5767
const $ghec = await getDOM(
5868
'/enterprise-cloud@latest/get-started/start-your-journey/dynamic-title',
5969
)
60-
const ghecLinks = $ghec('[data-testid=breadcrumbs-in-article] a')
61-
expect($ghec(ghecLinks[2]).text()).toBe('Greetings HubGit Enterprise Cloud')
70+
const ghecLinks = $ghec('[data-testid=breadcrumbs-bar] a')
71+
expect($ghec(ghecLinks[3]).text()).toBe('Greetings HubGit Enterprise Cloud')
6272
})
6373

64-
test('early access article pages have breadcrumbs with product, category, and article', async () => {
74+
test('early access article pages have breadcrumbs with home, product, category, and article', async () => {
6575
const $ = await getDOM('/early-access/secrets/deeper/mariana-trench')
66-
const $breadcrumbTitles = $(
67-
'[data-testid=breadcrumbs-in-article] [data-testid=breadcrumb-title]',
68-
)
69-
const $breadcrumbLinks = $('[data-testid=breadcrumbs-in-article] a')
76+
const $breadcrumbTitles = $('[data-testid=breadcrumbs-bar] [data-testid=breadcrumb-title]')
77+
const $breadcrumbLinks = $('[data-testid=breadcrumbs-bar] a')
7078

7179
expect($breadcrumbTitles.length).toBe(0)
72-
expect($breadcrumbLinks.length).toBe(2)
73-
expect(($breadcrumbLinks[0] as Element).attribs.title).toBe('Deeper secrets')
74-
expect(($breadcrumbLinks[1] as Element).attribs.title).toBe('Mariana Trench')
80+
// Home crumb + the two early-access crumbs.
81+
expect($breadcrumbLinks.length).toBe(3)
82+
expect(($breadcrumbLinks[0] as Element).attribs.title).toBe('Home')
83+
expect(($breadcrumbLinks[1] as Element).attribs.title).toBe('Deeper secrets')
84+
expect(($breadcrumbLinks[2] as Element).attribs.title).toBe('Mariana Trench')
7585
})
7686
})

‎src/fixtures/tests/playwright-rendering.spec.ts‎

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -540,21 +540,21 @@ test.describe('test nav at different viewports', () => {
540540
})
541541
await page.goto('/get-started/foo/bar')
542542

543-
// in article breadcrumbs at our custom xl viewport should remove last
544-
// breadcrumb so for this page we should only have 'Get Started / Foo'
545-
expect(await page.getByTestId('breadcrumbs-in-article').getByRole('link').all()).toHaveLength(2)
546-
await expect(page.getByTestId('breadcrumbs-in-article').getByText('Foo')).toBeVisible()
547-
await expect(page.getByTestId('breadcrumbs-in-article').getByText('Bar')).not.toBeVisible()
543+
// The Docs 2026 secondary bar leads with a Home crumb, then the full trail
544+
// 'Get started / Foo / Bar' (no hidden last crumb).
545+
expect(await page.getByTestId('breadcrumbs-bar').getByRole('link').all()).toHaveLength(4)
546+
await expect(page.getByTestId('breadcrumbs-bar').getByText('Foo')).toBeVisible()
547+
await expect(page.getByTestId('breadcrumbs-bar').getByText('Bar')).toBeVisible()
548548

549549
// breadcrumbs show up in rest reference pages
550550
await page.goto('/rest/actions/artifacts')
551-
await expect(page.getByTestId('breadcrumbs-in-article')).toBeVisible()
551+
await expect(page.getByTestId('breadcrumbs-bar')).toBeVisible()
552552

553553
// breadcrumbs show up in one of the pages that use the AutomatedPage
554554
// component (e.g. graphql, audit log, etc.) -- we test the webhooks
555555
// reference page here
556556
await page.goto('/webhooks/webhook-events-and-payloads')
557-
await expect(page.getByTestId('breadcrumbs-in-article')).toBeVisible()
557+
await expect(page.getByTestId('breadcrumbs-bar')).toBeVisible()
558558
})
559559

560560
test('large -> x-large viewports - 1012+', async ({ page }) => {
@@ -584,17 +584,15 @@ test.describe('test nav at different viewports', () => {
584584
})
585585
await page.goto('/get-started/foo/bar')
586586

587-
// breadcrumbs show up in the header, for this page we should have
588-
// 3 items 'Get Started / Foo / Bar'
589-
// in-article breadcrumbs don't show up
590-
await expect(page.getByTestId('breadcrumbs-header')).toBeVisible()
591-
expect(await page.getByTestId('breadcrumbs-header').getByRole('link').all()).toHaveLength(3)
592-
await expect(page.getByTestId('breadcrumbs-in-article')).not.toBeVisible()
587+
// breadcrumbs show up in the secondary bar; for this page we should have
588+
// a Home crumb plus 'Get started / Foo / Bar'
589+
await expect(page.getByTestId('breadcrumbs-bar')).toBeVisible()
590+
expect(await page.getByTestId('breadcrumbs-bar').getByRole('link').all()).toHaveLength(4)
593591

594-
// hamburger button for sidebar overlay is visible
595-
await expect(page.getByTestId('sidebar-hamburger')).toBeVisible()
596-
await page.getByTestId('sidebar-hamburger').click()
597-
await expect(page.locator('[role="dialog"][class*="Header_dialog"]')).toBeVisible()
592+
// the mobile nav toggle is visible and expands the doc-tree nav inline
593+
await expect(page.getByTestId('sidebar-mobile-toggle')).toBeVisible()
594+
await page.getByTestId('sidebar-mobile-toggle').click()
595+
await expect(page.getByTestId('sidebar')).toBeVisible()
598596
})
599597

600598
test('medium viewports - 768-1011', async ({ page }) => {
@@ -617,9 +615,9 @@ test.describe('test nav at different viewports', () => {
617615
await expect(page.getByTestId('mobile-signup')).toBeVisible()
618616

619617
// hamburger button for sidebar overlay is visible
620-
await expect(page.getByTestId('sidebar-hamburger')).toBeVisible()
621-
await page.getByTestId('sidebar-hamburger').click()
622-
await expect(page.locator('[role="dialog"][class*="Header_dialog"]')).toBeVisible()
618+
await expect(page.getByTestId('sidebar-mobile-toggle')).toBeVisible()
619+
await page.getByTestId('sidebar-mobile-toggle').click()
620+
await expect(page.getByTestId('sidebar')).toBeVisible()
623621
})
624622

625623
test('small viewports - 544-767', async ({ page }) => {
@@ -646,9 +644,9 @@ test.describe('test nav at different viewports', () => {
646644
await expect(page.getByTestId('mobile-signup')).toBeVisible()
647645

648646
// hamburger button for sidebar overlay is visible
649-
await expect(page.getByTestId('sidebar-hamburger')).toBeVisible()
650-
await page.getByTestId('sidebar-hamburger').click()
651-
await expect(page.locator('[role="dialog"][class*="Header_dialog"]')).toBeVisible()
647+
await expect(page.getByTestId('sidebar-mobile-toggle')).toBeVisible()
648+
await page.getByTestId('sidebar-mobile-toggle').click()
649+
await expect(page.getByTestId('sidebar')).toBeVisible()
652650
})
653651

654652
test('x-small viewports - 0-544', async ({ page }) => {
@@ -680,9 +678,9 @@ test.describe('test nav at different viewports', () => {
680678
await expect(page.getByTestId('mobile-signup')).toBeVisible()
681679

682680
// hamburger button for sidebar overlay is visible
683-
await expect(page.getByTestId('sidebar-hamburger')).toBeVisible()
684-
await page.getByTestId('sidebar-hamburger').click()
685-
await expect(page.locator('[role="dialog"][class*="Header_dialog"]')).toBeVisible()
681+
await expect(page.getByTestId('sidebar-mobile-toggle')).toBeVisible()
682+
await page.getByTestId('sidebar-mobile-toggle').click()
683+
await expect(page.getByTestId('sidebar')).toBeVisible()
686684
})
687685

688686
test('do a search when the viewport is x-small', async ({ page }) => {
@@ -881,7 +879,10 @@ test.describe('survey', () => {
881879
await expect(page.getByRole('button', { name: 'Send' })).toBeVisible()
882880
await expect(page.locator('[for=survey-comment]')).toBeVisible()
883881

884-
await page.getByTestId('product-sidebar').getByLabel('Bar', { exact: true }).click()
882+
await page
883+
.getByTestId('product-sidebar')
884+
.getByRole('link', { name: 'Bar', exact: true })
885+
.click()
885886
await expect(page.getByRole('button', { name: 'Send' })).not.toBeVisible()
886887
await expect(page.locator('[for=survey-comment]')).not.toBeVisible()
887888
})
@@ -894,8 +895,13 @@ test.describe('rest API reference pages', () => {
894895
// URL that has that `?apiVersion=` query parameter.
895896
await expect(page).toHaveURL(/\/en\/rest\?apiVersion=/)
896897
await page.getByTestId('sidebar').getByText('Actions').click()
897-
await page.getByTestId('sidebar').getByLabel('Artifacts').click()
898-
await page.getByLabel('About artifacts in HubGit Actions').click()
898+
// Brand NavList renders leaf articles as <a> links (not the label-associated
899+
// controls Primer used), so locate them by link role rather than getByLabel.
900+
await page.getByTestId('sidebar').getByRole('link', { name: 'Artifacts' }).click()
901+
await page
902+
.getByTestId('sidebar')
903+
.getByRole('link', { name: 'About artifacts in HubGit Actions' })
904+
.click()
899905
await expect(page).toHaveURL(/\/en\/rest\/actions\/artifacts\?apiVersion=/)
900906
await expect(page).toHaveTitle(/GitHub Actions Artifacts - GitHub Docs/)
901907
})

‎src/fixtures/tests/sidebar.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ describe('sidebar', () => {
1010
const sidebarProduct = $('[data-testid="sidebar-product-xl"]')
1111
expect(sidebarProduct.text()).toBe('Get started')
1212
expect(sidebarProduct.attr('href')).toBe('/en/get-started')
13-
// Mobile
14-
expect($('[data-testid="header-subnav"]').length).toBe(1)
15-
expect($('[data-testid="header-subnav-hamburger"]').length).toBe(1)
13+
// Docs 2026 secondary bar (breadcrumbs + nav toggle) replaces the old subnav
14+
expect($('[data-testid="docs-secondary-bar"]').length).toBe(1)
15+
expect($('[data-testid="sidebar-mobile-toggle"]').length).toBe(1)
1616
})
1717

1818
test('REST pages get the REST sidebar', async () => {
@@ -73,9 +73,9 @@ describe('sidebar', () => {
7373
const $: CheerioAPI = await getDOM('/early-access/secrets/deeper/mariana-trench')
7474
// Deskop
7575
expect($('[data-testid="sidebar-product-xl"]').length).toBe(0)
76-
// Mobile
77-
expect($('[data-testid="header-subnav"]').length).toBe(1)
78-
expect($('[data-testid="header-subnav-hamburger"]').length).toBe(0)
76+
// The secondary bar renders, but early-access has no nav toggle
77+
expect($('[data-testid="docs-secondary-bar"]').length).toBe(1)
78+
expect($('[data-testid="sidebar-mobile-toggle"]').length).toBe(0)
7979
})
8080

8181
test('category-landing pages show title entry in sidebar', async () => {

‎src/frame/components/DefaultLayout.module.scss‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
@import "src/frame/stylesheets/breakpoint-xxl.scss";
2+
13
.mainContent {
24
scroll-margin-top: 5rem;
35
}
6+
7+
// When the inline mobile/tablet nav is open, it takes over the viewport — hide
8+
// the content column so the full-width rail isn't squeezed beside it. Above the
9+
// xxl breakpoint the rail is a fixed-width sibling, so content always shows.
10+
.contentHiddenForNav {
11+
display: none;
12+
13+
@include breakpoint-xxl {
14+
display: flex;
15+
}
16+
}

0 commit comments

Comments
 (0)