Skip to content

Commit bf785d6

Browse files
authored
Updated the accessible label of the SubNav's open/close toggle button to include the current page (#838)
* remove test-ids from SubNav tests * remove aria-label from SubNav open/close toggle * add changeset
1 parent e3d0a94 commit bf785d6

File tree

3 files changed

+40
-36
lines changed

3 files changed

+40
-36
lines changed

.changeset/shiny-mice-promise.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 the accessible label of the SubNav's open/close toggle button to include the name of the current page.

packages/react/src/SubNav/SubNav.test.tsx

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {HTMLAttributes} from 'react'
2-
import React, {render, cleanup, fireEvent, within} from '@testing-library/react'
2+
import React, {render, cleanup, within} from '@testing-library/react'
33
import '@testing-library/jest-dom'
44
import {axe, toHaveNoViolations} from 'jest-axe'
55

@@ -57,64 +57,65 @@ describe('SubNav', () => {
5757
afterEach(cleanup)
5858

5959
it('renders the root element correctly into the document', () => {
60-
const {getByTestId} = render(<MockSubNavFixture />)
60+
const {getByRole} = render(<MockSubNavFixture />)
6161

62-
expect(getByTestId(SubNav.testIds.root)).toBeInTheDocument() // expect the root element to be in the document
63-
expect(getByTestId(SubNav.testIds.root).tagName).toBe('nav'.toUpperCase()) // expect root to be a <nav> element
62+
expect(getByRole('navigation')).toBeInTheDocument()
6463
})
6564
it('renders a title as a link', () => {
66-
const {getByTestId} = render(<MockSubNavFixture />)
65+
const {getByRole} = render(<MockSubNavFixture />)
6766

68-
expect(getByTestId(SubNav.testIds.heading)).toBeInTheDocument()
69-
expect(getByTestId(SubNav.testIds.heading).tagName).toBe('a'.toUpperCase())
70-
expect(getByTestId(SubNav.testIds.heading)).toHaveAttribute('href', headingLink)
67+
expect(getByRole('link', {name: heading})).toHaveAttribute('href', headingLink)
7168
})
7269

7370
it('renders the correct number of links into the document', () => {
74-
const {getByTestId} = render(<MockSubNavFixture />)
75-
expect(getByTestId(SubNav.testIds.overlay).querySelectorAll('a').length).toBe(mockLinkData.length)
71+
const {getByRole} = render(<MockSubNavFixture />)
72+
73+
const list = getByRole('list')
74+
const links = within(list).getAllByRole('link')
75+
76+
expect(links).toHaveLength(mockLinkData.length)
7677
})
7778

7879
it('has a button that opens the menu when clicked', async () => {
79-
const {getByTestId} = render(<MockSubNavFixture />)
80+
const {getByRole} = render(<MockSubNavFixture />)
8081

81-
const buttonEl = getByTestId('SubNav-root-button')
82-
const overlayEl = getByTestId(SubNav.testIds.overlay)
83-
84-
expect(buttonEl).toBeInTheDocument()
82+
const buttonEl = getByRole('button', {name: 'Navigation menu. Current page: page three'})
83+
const overlayEl = getByRole('list')
8584

86-
// check aria roles are correct by default
85+
expect(overlayEl).not.toHaveClass('SubNav__links-overlay--open')
8786
expect(buttonEl).toHaveAttribute('aria-expanded', 'false')
8887

8988
userEvent.click(buttonEl)
9089

9190
expect(overlayEl).toHaveClass('SubNav__links-overlay--open')
92-
// check aria roles have updated
9391
expect(buttonEl).toHaveAttribute('aria-expanded', 'true')
9492
})
9593

9694
it('closes the overlay when button is pressed again', () => {
97-
const {getByTestId} = render(<MockSubNavFixture />)
95+
const {getByRole} = render(<MockSubNavFixture />)
9896

99-
const buttonEl = getByTestId(SubNav.testIds.button)
100-
const overlayEl = getByTestId(SubNav.testIds.overlay)
97+
const buttonEl = getByRole('button', {name: 'Navigation menu. Current page: page three'})
98+
const overlayEl = getByRole('list')
10199

102-
fireEvent.click(buttonEl)
100+
userEvent.click(buttonEl)
103101
expect(overlayEl).toHaveClass('SubNav__links-overlay--open')
104102

105-
fireEvent.click(buttonEl)
103+
userEvent.click(buttonEl)
106104
expect(overlayEl).not.toHaveClass('SubNav__links-overlay--open')
107105
})
108106

109-
it('shows the aria-current text next to the button by default', () => {
110-
const {getByTestId} = render(<MockSubNavFixture />)
107+
it('includes the aria-current text in the aria-label of the button', () => {
108+
const {getByRole} = render(<MockSubNavFixture />)
109+
110+
const buttonEl = getByRole('button', {name: 'Navigation menu. Current page: page three'})
111+
expect(buttonEl).toBeInTheDocument()
112+
})
111113

112-
const buttonEl = getByTestId(SubNav.testIds.button)
113-
const activeLink = mockLinkData.find(link => link['aria-current']) as {title: string} | undefined
114+
it('shows the aria-current text next to the button by default', () => {
115+
const {getByRole} = render(<MockSubNavFixture />)
114116

115-
if (activeLink) {
116-
expect(buttonEl).toHaveTextContent(activeLink.title)
117-
}
117+
const buttonEl = getByRole('button', {name: 'Navigation menu. Current page: page three'})
118+
expect(buttonEl).toHaveTextContent('page three')
118119
})
119120

120121
it('has no a11y violations on initial render', async () => {
@@ -127,7 +128,7 @@ describe('SubNav', () => {
127128
it('shows subitems when the submenu toggle is activated at large viewports', async () => {
128129
mockUseWindowSize.mockImplementation(() => ({isLarge: true}))
129130

130-
const {getByRole, getAllByTestId} = render(
131+
const {getByRole} = render(
131132
<SubNav fullWidth>
132133
<SubNav.Link href="#" aria-current="page">
133134
Copilot
@@ -157,16 +158,14 @@ describe('SubNav', () => {
157158
expect(toggleSubmenuButton).toHaveFocus()
158159
expect(toggleSubmenuButton).toHaveAttribute('aria-expanded', 'true')
159160

160-
const expanded = getAllByTestId(SubNav.testIds.subMenu)[0]
161-
162161
userEvent.tab()
163-
expect(within(expanded).getByRole('link', {name: 'Copilot feature page one'})).toHaveFocus()
162+
expect(getByRole('link', {name: 'Copilot feature page one'})).toHaveFocus()
164163

165164
userEvent.tab()
166-
expect(within(expanded).getByRole('link', {name: 'Copilot feature page two'})).toHaveFocus()
165+
expect(getByRole('link', {name: 'Copilot feature page two'})).toHaveFocus()
167166

168167
userEvent.tab()
169-
expect(within(expanded).getByRole('link', {name: 'Copilot feature page three'})).toHaveFocus()
168+
expect(getByRole('link', {name: 'Copilot feature page three'})).toHaveFocus()
170169

171170
userEvent.tab()
172171
expect(getByRole('link', {name: 'Code review'})).toHaveFocus()

packages/react/src/SubNav/SubNav.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,8 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
270270
onClick={isOpenAtNarrow ? closeMenuCallback : handleMenuToggle}
271271
aria-expanded={isOpenAtNarrow ? 'true' : 'false'}
272272
aria-controls={idForLinkContainer}
273-
aria-label={`${isOpenAtNarrow ? 'close' : 'open'} navigation menu`}
274273
>
274+
<span className="visually-hidden">Navigation menu. Current page: </span>
275275
<span className={styles['SubNav__overlay-toggle-content']}>
276276
<Text as="span" size="200">
277277
{activeLinklabel}

0 commit comments

Comments
 (0)