Skip to content

Commit e36c57d

Browse files
danielguillandanielguillan
committed
Update SubdomainNavBar desktop pills
Copilot-Session: d277a6ee-e0c6-42d5-9ad8-2eaad758b176
1 parent e077038 commit e36c57d

24 files changed

Lines changed: 222 additions & 25 deletions

File tree

‎.changeset/quiet-pills-align.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react-brand': patch
3+
---
4+
5+
Updated `SubdomainNavBar` desktop navigation links with compact hover, current-page, focus, and expanded overflow-trigger pill states.

‎packages/e2e/scripts/playwright/playwright.generate-tests.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,19 @@ const beforeScreenshotLookup: Partial<Record<string, string>> = {
156156
const overflowMenu = page.locator(\`[id="\${await moreButton.getAttribute('aria-controls')}"]\`)
157157
await expect(moreButton).toHaveAttribute('aria-expanded', 'true')
158158
await expect(overflowMenu).toBeVisible()
159-
await expect(overflowMenu.getByRole('link', {name: 'Books'})).toBeVisible()
159+
await expect(overflowMenu.getByRole('link', {name: 'Resources'})).toBeVisible()
160160
`,
161161
}
162162

163163
const screenshotOptionsLookup: Partial<Record<string, string>> = {
164164
'components-subdomainnavbar--overflow-menu-open': `{animations: 'allow'}`,
165165
}
166166

167+
const viewportLookup: Partial<Record<string, {width: number; height: number}>> = {
168+
'components-subdomainnavbar--desktop-pill-states': {width: 1440, height: 900},
169+
'components-subdomainnavbar--overflow-menu-open': {width: 1440, height: 900},
170+
}
171+
167172
// const skipLocalizationsTestsFor = [
168173
// 'components-actionmenu-features--disabled-item', // for the menu to open
169174
// 'components-actionmenu-features--anchored-positioning', // for the menu to open
@@ -270,6 +275,7 @@ for (const key of Object.keys(categorisedStories)) {
270275
)
271276
272277
const requiresTabletViewport = storyName.toLowerCase().includes('tablet')
278+
const viewport = viewportLookup[id]
273279
if (skipTestLookup.includes(id)) {
274280
return acc
275281
}
@@ -293,6 +299,16 @@ for (const key of Object.keys(categorisedStories)) {
293299
const languagesToTest = ['en']
294300
const allLanguageTests = languagesToTest.map(language => generateTestForLanguage(language)).join('')
295301
302+
if (viewport) {
303+
return (acc += `
304+
// eslint-disable-next-line i18n-text/no-en
305+
test.describe('Custom viewport test for ${storyName}', () => {
306+
test.use({ viewport: { width: ${viewport.width}, height: ${viewport.height} } });
307+
${allLanguageTests}
308+
});
309+
`)
310+
}
311+
296312
if (requiresMobileViewport) {
297313
return (acc += `
298314
// eslint-disable-next-line i18n-text/no-en

‎packages/react/src/SubdomainNavBar/NavigationVisbilityObserver.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,10 @@ const OverflowButton = forwardRef<HTMLLIElement, OverflowButtonProps>(
188188
ref={buttonRef}
189189
tabIndex={visible ? undefined : -1}
190190
>
191-
More
192-
<ChevronDownIcon />
191+
<span className={styles['SubdomainNavBar-link-content']}>
192+
More
193+
<ChevronDownIcon />
194+
</span>
193195
</button>
194196
</li>
195197
)

‎packages/react/src/SubdomainNavBar/SubdomainNavBar.module.css‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,12 @@
308308
position: relative;
309309
}
310310

311+
.SubdomainNavBar-link-content {
312+
display: inline-flex;
313+
align-items: center;
314+
gap: var(--base-size-8);
315+
}
316+
311317
@media screen and (max-width: 767px) {
312318
.SubdomainNavBar-link:first-of-type {
313319
padding-top: var(--base-size-24);
@@ -885,6 +891,7 @@
885891
margin: 0;
886892
block-size: 100%;
887893
max-block-size: 100%;
894+
padding-inline-start: var(--base-size-16);
888895
}
889896

890897
.SubdomainNavBar .SubdomainNavBar-primary-nav-overflow {
@@ -902,10 +909,62 @@
902909
height: 100%;
903910
padding: 0 var(--base-size-16);
904911
color: var(--brand-color-text-muted);
905-
font-size: var(--base-size-14);
912+
font-size: var(--brand-text-size-100);
906913
line-height: var(--brand-text-lineHeight-200);
907914
}
908915

916+
.SubdomainNavBar
917+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
918+
> .SubdomainNavBar-link,
919+
.SubdomainNavBar .SubdomainNavBar-more-link {
920+
padding-inline: var(--base-size-4);
921+
}
922+
923+
.SubdomainNavBar
924+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
925+
> .SubdomainNavBar-link
926+
> .SubdomainNavBar-link-content,
927+
.SubdomainNavBar .SubdomainNavBar-more-link > .SubdomainNavBar-link-content {
928+
gap: var(--base-size-4);
929+
padding: var(--base-size-4) var(--base-size-12);
930+
background-color: transparent;
931+
border-radius: var(--brand-borderRadius-full);
932+
}
933+
934+
.SubdomainNavBar
935+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
936+
> .SubdomainNavBar-link:hover
937+
> .SubdomainNavBar-link-content,
938+
.SubdomainNavBar
939+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
940+
> .SubdomainNavBar-link:active
941+
> .SubdomainNavBar-link-content,
942+
.SubdomainNavBar
943+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
944+
> .SubdomainNavBar-link[aria-current]:not([aria-current='false'])
945+
> .SubdomainNavBar-link-content,
946+
.SubdomainNavBar .SubdomainNavBar-more-link:hover > .SubdomainNavBar-link-content,
947+
.SubdomainNavBar .SubdomainNavBar-more-link:active > .SubdomainNavBar-link-content,
948+
.SubdomainNavBar .SubdomainNavBar-more-link[aria-expanded='true'] > .SubdomainNavBar-link-content {
949+
background-color: var(--brand-SubNav-color-link-bgColor);
950+
}
951+
952+
.SubdomainNavBar
953+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
954+
> .SubdomainNavBar-link:focus-visible,
955+
.SubdomainNavBar .SubdomainNavBar-more-link:focus-visible {
956+
outline: none;
957+
}
958+
959+
.SubdomainNavBar
960+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
961+
> .SubdomainNavBar-link:focus-visible
962+
> .SubdomainNavBar-link-content,
963+
.SubdomainNavBar .SubdomainNavBar-more-link:focus-visible > .SubdomainNavBar-link-content {
964+
outline: var(--brand-borderWidth-thick) solid var(--brand-color-focus);
965+
outline-offset: var(--base-size-2);
966+
}
967+
909968
.SubdomainNavBar .SubdomainNavBar-link-text::after {
910969
display: none;
911970
}
@@ -947,6 +1006,16 @@
9471006
background-color: var(--brand-color-canvas-subtle);
9481007
}
9491008

1009+
@media (prefers-reduced-motion: no-preference) {
1010+
.SubdomainNavBar
1011+
.SubdomainNavBar-primary-nav-list-item:not(.SubdomainNavBar-overflow-menu-item)
1012+
> .SubdomainNavBar-link
1013+
> .SubdomainNavBar-link-content,
1014+
.SubdomainNavBar .SubdomainNavBar-more-link > .SubdomainNavBar-link-content {
1015+
transition: background-color var(--brand-animation-duration-fast) var(--brand-animation-easing-default);
1016+
}
1017+
}
1018+
9501019
.SubdomainNavBar .SubdomainNavBar-secondary-nav {
9511020
border-inline-start: var(--brand-borderWidth-thin) solid var(--brand-color-border-muted);
9521021
}

‎packages/react/src/SubdomainNavBar/SubdomainNavBar.module.css.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ declare const styles: {
1313
readonly "SubdomainNavBar-leading-component": string;
1414
readonly "SubdomainNavBar-link": string;
1515
readonly "SubdomainNavBar-link--title": string;
16+
readonly "SubdomainNavBar-link-content": string;
1617
readonly "SubdomainNavBar-link-text": string;
1718
readonly "SubdomainNavBar-logo-mark": string;
1819
readonly "SubdomainNavBar-menu-button": string;

‎packages/react/src/SubdomainNavBar/SubdomainNavBar.stories.tsx‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ type StoryArgs = {
2323
fullWidth: boolean
2424
} & SubdomainNavBarProps
2525

26+
const viewports = {
27+
...INITIAL_VIEWPORTS,
28+
desktop1440: {
29+
name: 'Desktop 1440',
30+
styles: {
31+
width: '1440px',
32+
height: '900px',
33+
},
34+
type: 'desktop',
35+
},
36+
}
37+
2638
const meta = {
2739
title: 'Components/SubdomainNavBar',
2840
component: SubdomainNavBar as Meta<StoryArgs>['component'],
@@ -48,7 +60,7 @@ const meta = {
4860
},
4961
parameters: {
5062
viewport: {
51-
viewports: INITIAL_VIEWPORTS,
63+
options: viewports,
5264
},
5365
},
5466
} satisfies Meta<StoryArgs>
@@ -883,6 +895,9 @@ export const OverflowMenuOpen: Story = {
883895
args: {
884896
numLinks: 13,
885897
},
898+
globals: {
899+
viewport: {value: 'desktop1440'},
900+
},
886901
play: async ({canvasElement}) => {
887902
const canvas = within(canvasElement)
888903
await document.fonts.ready
@@ -893,6 +908,29 @@ export const OverflowMenuOpen: Story = {
893908
name: 'Overflow Menu Open',
894909
}
895910

911+
export const DesktopPillStates: Story = {
912+
render: () => (
913+
<SubdomainNavBar title="Site title">
914+
<SubdomainNavBar.Link href="#default">Default</SubdomainNavBar.Link>
915+
<SubdomainNavBar.Link href="#hover">Hover</SubdomainNavBar.Link>
916+
<SubdomainNavBar.Link href="#focus">Focus</SubdomainNavBar.Link>
917+
<SubdomainNavBar.Link href="#current" aria-current="page">
918+
Current
919+
</SubdomainNavBar.Link>
920+
</SubdomainNavBar>
921+
),
922+
globals: {
923+
viewport: {value: 'desktop1440'},
924+
},
925+
parameters: {
926+
pseudo: {
927+
hover: ['a[href="#hover"]'],
928+
focusVisible: ['a[href="#focus"]'],
929+
},
930+
},
931+
name: 'Desktop Pill States',
932+
}
933+
896934
export const MobileView: Story = {
897935
render: (args: StoryArgs) => <SubdomainNavBarTemplate {...args} />,
898936
globals: {

‎packages/react/src/SubdomainNavBar/SubdomainNavBar.test.tsx‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,21 @@ describe('SubdomainNavBar', () => {
155155
expect(getByRole('banner')).toHaveStyle({opacity: '0.5'})
156156
})
157157

158+
it('forwards aria-current to the rendered link', () => {
159+
const {getByRole} = render(
160+
<SubdomainNavBar title="Subdomain">
161+
<SubdomainNavBar.Link href="#collections" aria-current="page">
162+
Collections
163+
</SubdomainNavBar.Link>
164+
</SubdomainNavBar>,
165+
)
166+
167+
const link = getByRole('link', {name: 'Collections'})
168+
169+
expect(link).toHaveAttribute('aria-current', 'page')
170+
expect(link.closest('li')).not.toHaveAttribute('aria-current')
171+
})
172+
158173
it('has no a11y violations by default', async () => {
159174
const {container} = render(<Component />)
160175

@@ -605,6 +620,30 @@ describe('SubdomainNavBar', () => {
605620
expect(within(overflowMenu as HTMLElement).queryByRole('link', {name: 'Topics'})).not.toBeInTheDocument()
606621
})
607622

623+
it('accounts for inline list padding when measuring desktop navigation overflow', async () => {
624+
mockUseWindowSize.mockImplementation(() => ({isSmall: true, isMedium: true}))
625+
626+
const {container, getByRole} = render(
627+
<SubdomainNavBar title="Subdomain">
628+
<SubdomainNavBar.Link href="#collections">Collections</SubdomainNavBar.Link>
629+
<SubdomainNavBar.Link href="#topics">Topics</SubdomainNavBar.Link>
630+
<SubdomainNavBar.Link href="#articles">Articles</SubdomainNavBar.Link>
631+
</SubdomainNavBar>,
632+
)
633+
634+
const navList = container.querySelector<HTMLElement>('.SubdomainNavBar-primary-nav-list')
635+
if (navList) {
636+
navList.style.paddingInlineStart = '16px'
637+
}
638+
639+
await updateNavigationLayout(container, {containerWidth: 150, itemWidth: 45, moreWidth: 30})
640+
641+
expect(getByRole('button', {name: 'More'})).toBeInTheDocument()
642+
expect(container.querySelector('[data-navitemid="0-Collections"]')).not.toHaveAttribute('aria-hidden')
643+
expect(container.querySelector('[data-navitemid="1-Topics"]')).not.toHaveAttribute('aria-hidden')
644+
expect(container.querySelector('[data-navitemid="2-Articles"]')).toHaveAttribute('aria-hidden', 'true')
645+
})
646+
608647
it('keeps overflowed desktop navigation links contiguous when later links are shorter', async () => {
609648
mockUseWindowSize.mockImplementation(() => ({isSmall: true, isMedium: true}))
610649

‎packages/react/src/SubdomainNavBar/SubdomainNavBar.tsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,7 @@ function Link({
597597
className,
598598
children,
599599
isExternal,
600+
'aria-current': ariaCurrent,
600601
'aria-hidden': ariaHidden,
601602
tabIndex,
602603
...rest
@@ -613,12 +614,15 @@ function Link({
613614
>
614615
<a
615616
href={href}
617+
aria-current={ariaCurrent}
616618
className={styles['SubdomainNavBar-link']}
617619
onClick={onLinkClick}
618620
tabIndex={isOverflowed ? -1 : undefined}
619621
>
620-
<span className={styles['SubdomainNavBar-link-text']}>{children}</span>
621-
{isExternal && <LinkExternalIcon size={16} aria-label="External link" />}
622+
<span className={styles['SubdomainNavBar-link-content']}>
623+
<span className={styles['SubdomainNavBar-link-text']}>{children}</span>
624+
{isExternal && <LinkExternalIcon size={16} aria-label="External link" />}
625+
</span>
622626
</a>
623627
</li>
624628
)

‎packages/react/src/SubdomainNavBar/SubdomainNavBar.visual.spec.ts‎

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -72,24 +72,43 @@ test.describe('Visual Comparison: SubdomainNavBar', () => {
7272
await expect(page).toHaveScreenshot({fullPage: true})
7373
})
7474

75-
test('SubdomainNavBar / Overflow Menu Open', async ({page}) => {
76-
await page.goto(
77-
'http://localhost:6006/iframe.html?args=&id=components-subdomainnavbar--overflow-menu-open&viewMode=story',
78-
{waitUntil: 'networkidle'},
79-
)
80-
await page.locator('body.sb-show-main').waitFor({state: 'visible'})
75+
// eslint-disable-next-line i18n-text/no-en
76+
test.describe('Custom viewport test for Overflow Menu Open', () => {
77+
test.use({viewport: {width: 1440, height: 900}})
78+
test('SubdomainNavBar / Overflow Menu Open', async ({page}) => {
79+
await page.goto(
80+
'http://localhost:6006/iframe.html?args=&id=components-subdomainnavbar--overflow-menu-open&viewMode=story',
81+
{waitUntil: 'networkidle'},
82+
)
83+
await page.locator('body.sb-show-main').waitFor({state: 'visible'})
8184

82-
await page.waitForTimeout(1500)
83-
const moreButton = page.getByRole('button', {name: 'More'})
84-
if ((await moreButton.getAttribute('aria-expanded')) !== 'true') {
85-
await moreButton.click()
86-
}
87-
const overflowMenu = page.locator(`[id="${await moreButton.getAttribute('aria-controls')}"]`)
88-
await expect(moreButton).toHaveAttribute('aria-expanded', 'true')
89-
await expect(overflowMenu).toBeVisible()
90-
await expect(overflowMenu.getByRole('link', {name: 'Books'})).toBeVisible()
91-
92-
await expect(page).toHaveScreenshot({animations: 'allow'})
85+
await page.waitForTimeout(1500)
86+
const moreButton = page.getByRole('button', {name: 'More'})
87+
if ((await moreButton.getAttribute('aria-expanded')) !== 'true') {
88+
await moreButton.click()
89+
}
90+
const overflowMenu = page.locator(`[id="${await moreButton.getAttribute('aria-controls')}"]`)
91+
await expect(moreButton).toHaveAttribute('aria-expanded', 'true')
92+
await expect(overflowMenu).toBeVisible()
93+
await expect(overflowMenu.getByRole('link', {name: 'Resources'})).toBeVisible()
94+
95+
await expect(page).toHaveScreenshot({animations: 'allow'})
96+
})
97+
})
98+
99+
// eslint-disable-next-line i18n-text/no-en
100+
test.describe('Custom viewport test for Desktop Pill States', () => {
101+
test.use({viewport: {width: 1440, height: 900}})
102+
test('SubdomainNavBar / Desktop Pill States', async ({page}) => {
103+
await page.goto(
104+
'http://localhost:6006/iframe.html?args=&id=components-subdomainnavbar--desktop-pill-states&viewMode=story',
105+
{waitUntil: 'networkidle'},
106+
)
107+
await page.locator('body.sb-show-main').waitFor({state: 'visible'})
108+
109+
await page.waitForTimeout(500)
110+
await expect(page).toHaveScreenshot({fullPage: true})
111+
})
93112
})
94113

95114
// eslint-disable-next-line i18n-text/no-en

‎packages/react/src/SubdomainNavBar/SubdomainNavBar.visual.spec.ts-snapshots/Visual-Comparison-SubdomainNavBar-Custom-view-20067-enu-Open-SubdomainNavBar-Overflow-Menu-Open-1-darwin.png‎

72.2 KB
Loading

0 commit comments

Comments
 (0)