From 9d1bb93cfcb9ae86141c847ca830a313512f5e43 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Tue, 8 Nov 2022 13:20:40 +1000 Subject: [PATCH 1/6] aria-hidden for vosible counters and sr-only for screen readers --- src/UnderlineNav2/UnderlineNavItem.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index be89953c72a..d7b74d9cc20 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -7,6 +7,7 @@ import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' import {getLinkStyles, wrapperStyles, iconWrapStyles, counterStyles} from './styles' import {LoadingCounter} from './LoadingCounter' +import VisuallyHidden from '../_VisuallyHidden' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -175,7 +176,8 @@ export const UnderlineNavItem = forwardRef( ) : ( counter !== undefined && ( - {counter} + +  {counter} ) )} From e80f5af540b399e582e0ed3739cc89837a1633f7 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Tue, 8 Nov 2022 13:36:08 +1000 Subject: [PATCH 2/6] add changeset --- .changeset/soft-sheep-glow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/soft-sheep-glow.md diff --git a/.changeset/soft-sheep-glow.md b/.changeset/soft-sheep-glow.md new file mode 100644 index 00000000000..fff5c2469b4 --- /dev/null +++ b/.changeset/soft-sheep-glow.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +UnderlineNav2: Add aria-hidden and sr-only class support for descriptive counters From 87ce3cae9e1c2d4a6eceed05afaa798d93abef39 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Tue, 8 Nov 2022 14:37:08 +1000 Subject: [PATCH 3/6] menu item counter a11y support --- src/UnderlineNav2/UnderlineNav.tsx | 9 +++++++-- src/UnderlineNav2/UnderlineNavItem.tsx | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index b58470ecdc3..815a46bd5bc 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -353,10 +353,15 @@ export const UnderlineNav = forwardRef( {actionElementChildren} {loadingCounters ? ( - + + + ) : ( actionElementProps.counter !== undefined && ( - {actionElementProps.counter} + + +  ({actionElementProps.counter}) + ) )} diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index d7b74d9cc20..926a63cc6a7 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -177,7 +177,7 @@ export const UnderlineNavItem = forwardRef( counter !== undefined && ( -  {counter} +  ({counter}) ) )} From 916958f62a0c0ff52424886673a282edf4d11adf Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 9 Nov 2022 10:53:35 +1000 Subject: [PATCH 4/6] remove unnecessary wrapper around loading counters --- src/UnderlineNav2/UnderlineNav.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index b1660fad137..5bb020f20c4 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -353,9 +353,7 @@ export const UnderlineNav = forwardRef( {actionElementChildren} {loadingCounters ? ( - - - + ) : ( actionElementProps.counter !== undefined && ( From 83c5ab5f106f948f1a5827c52b62f9bdc8183c36 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 10 Nov 2022 12:34:36 +1000 Subject: [PATCH 5/6] update tests for counters to have accessible names --- src/UnderlineNav2/UnderlineNav.tsx | 2 +- src/UnderlineNav2/UnderlineNavItem.tsx | 2 +- .../__snapshots__/UnderlineNav.test.tsx.snap | 36 +++++++++++++++++++ src/UnderlineNav2/interactions.stories.tsx | 10 +++--- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 6d3c2ebfafe..e58e44cb684 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -353,7 +353,7 @@ export const UnderlineNav = forwardRef( actionElementProps.counter !== undefined && ( -  ({actionElementProps.counter}) + {` (${actionElementProps.counter})`} ) )} diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 6d301268dad..3dda77bbacf 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -179,7 +179,7 @@ export const UnderlineNavItem = forwardRef( counter !== undefined && ( -  ({counter}) + {` (${counter})`} ) )} diff --git a/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap b/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap index 593e604d97e..4e15b2815e6 100644 --- a/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap +++ b/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap @@ -309,10 +309,16 @@ exports[`UnderlineNav renders consistently 1`] = ` data-component="counter" > + +  (120) + @@ -370,10 +376,16 @@ exports[`UnderlineNav renders consistently 1`] = ` data-component="counter" > + +  (13) + @@ -431,10 +443,16 @@ exports[`UnderlineNav renders consistently 1`] = ` data-component="counter" > + +  (5) + @@ -464,10 +482,16 @@ exports[`UnderlineNav renders consistently 1`] = ` data-component="counter" > + +  (4) + @@ -525,10 +549,16 @@ exports[`UnderlineNav renders consistently 1`] = ` data-component="counter" > + +  (9) + @@ -609,10 +639,16 @@ exports[`UnderlineNav renders consistently 1`] = ` data-component="counter" > + +  (10) + diff --git a/src/UnderlineNav2/interactions.stories.tsx b/src/UnderlineNav2/interactions.stories.tsx index c38e1aa0e82..e87542871d6 100644 --- a/src/UnderlineNav2/interactions.stories.tsx +++ b/src/UnderlineNav2/interactions.stories.tsx @@ -47,11 +47,11 @@ KeyboardNavigation.play = async ({canvasElement}: {canvasElement: HTMLElement}) await delay(500) await userEvent.tab() await delay(500) - let menuItem = canvas.getByRole('link', {name: 'Settings 10'}) + let menuItem = canvas.getByRole('link', {name: 'Settings  (10)'}) userEvent.click(menuItem) expect(activeElement).toHaveFocus() - menuItem = canvas.getByRole('link', {name: 'Settings 10'}) + menuItem = canvas.getByRole('link', {name: 'Settings  (10)'}) expect(menuItem).toHaveAttribute('aria-current', 'page') const lastListItem = canvas.getByRole('list').children[5] @@ -85,11 +85,11 @@ SelectAMenuItem.play = async ({canvasElement}: {canvasElement: HTMLElement}) => userEvent.click(moreBtn) await delay(1000) - let menuItem = canvas.getByRole('link', {name: 'Settings 10'}) + let menuItem = canvas.getByRole('link', {name: 'Settings  (10)'}) userEvent.click(menuItem) expect(moreBtn).toHaveFocus() - menuItem = canvas.getByRole('link', {name: 'Settings 10'}) + menuItem = canvas.getByRole('link', {name: 'Settings  (10)'}) expect(menuItem).toHaveAttribute('aria-current', 'page') const lastListItem = canvas.getByRole('list').children[5] @@ -107,7 +107,7 @@ KeepSelectedItemVisible.play = async ({canvasElement}: {canvasElement: HTMLEleme const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) const canvas = within(canvasElement) // await delay(2000) - const selectedItem = canvas.getByRole('link', {name: 'Settings 10'}) + const selectedItem = canvas.getByRole('link', {name: 'Settings  (10)'}) expect(selectedItem).toHaveAttribute('aria-current', 'page') // change viewport canvasElement.style.width = '900px' From 1c87674d2a385170542077f7d54860347d26a2a6 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 10 Nov 2022 13:08:43 +1000 Subject: [PATCH 6/6] update tests --- src/UnderlineNav2/UnderlineNav.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index a54c747dc22..cc0b169bbab 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -136,7 +136,7 @@ describe('UnderlineNav', () => { }) it('respects counter prop', () => { const {getByRole} = render() - const item = getByRole('link', {name: 'Issues 120'}) + const item = getByRole('link', {name: 'Issues  (120)'}) const counter = item.getElementsByTagName('span')[3] expect(counter.className).toContain('CounterLabel') expect(counter.textContent).toBe('120') @@ -162,7 +162,7 @@ describe('Keyboard Navigation', () => { it('should move focus to the next/previous item on the list with the tab key', async () => { const {getByRole} = render() const item = getByRole('link', {name: 'Code'}) - const nextItem = getByRole('link', {name: 'Issues 120'}) + const nextItem = getByRole('link', {name: 'Issues  (120)'}) const user = userEvent.setup() await user.tab() // tab into the story, this should focus on the first link expect(item).toEqual(document.activeElement) // check if the first item is focused