From cf9b2d839af374b708c9e7d5e273e7ee9405663a Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Thu, 23 Jun 2022 12:39:42 +0000 Subject: [PATCH 1/9] Add focusZone to TabNav --- src/TabNav.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/TabNav.tsx b/src/TabNav.tsx index fc6cd810a83..2294ca7c7f5 100644 --- a/src/TabNav.tsx +++ b/src/TabNav.tsx @@ -3,6 +3,7 @@ import {To} from 'history' import React from 'react' import styled from 'styled-components' import {get} from './constants' +import {FocusKeys, useFocusZone} from './hooks/useFocusZone' import sx, {SxProp} from './sx' import {ComponentProps} from './utils/types' import getGlobalFocusStyles from './_getGlobalFocusStyles' @@ -28,8 +29,11 @@ const TabNavNav = styled.nav` export type TabNavProps = ComponentProps function TabNav({children, 'aria-label': ariaLabel, ...rest}: TabNavProps) { + const {containerRef: navRef} = useFocusZone({ + bindKeys: FocusKeys.ArrowHorizontal + }) return ( - + }> {children} From 89af3ffe2869804fb9565b3c9f6f2b7191b7e99c Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Thu, 23 Jun 2022 14:46:53 +0000 Subject: [PATCH 2/9] Add aria-selected to tabs --- src/TabNav.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/TabNav.tsx b/src/TabNav.tsx index 2294ca7c7f5..cabccb2e09d 100644 --- a/src/TabNav.tsx +++ b/src/TabNav.tsx @@ -49,7 +49,8 @@ type StyledTabNavLinkProps = { const TabNavLink = styled.a.attrs(props => ({ activeClassName: typeof props.to === 'string' ? 'selected' : '', className: classnames(ITEM_CLASS, props.selected && SELECTED_CLASS, props.className), - role: 'tab' + role: 'tab', + 'aria-selected': props.selected ? true : false }))` padding: 8px 12px; font-size: ${get('fontSizes.1')}; From 5cf1695b410e2c02a6225d5284658ac593f475ae Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Fri, 24 Jun 2022 09:18:07 +0000 Subject: [PATCH 3/9] Custom strategy to ensure selected tab is focused on re-entry --- src/TabNav.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/TabNav.tsx b/src/TabNav.tsx index cabccb2e09d..f203843741e 100644 --- a/src/TabNav.tsx +++ b/src/TabNav.tsx @@ -1,6 +1,6 @@ import classnames from 'classnames' import {To} from 'history' -import React from 'react' +import React, {useRef} from 'react' import styled from 'styled-components' import {get} from './constants' import {FocusKeys, useFocusZone} from './hooks/useFocusZone' @@ -29,8 +29,18 @@ const TabNavNav = styled.nav` export type TabNavProps = ComponentProps function TabNav({children, 'aria-label': ariaLabel, ...rest}: TabNavProps) { + const customContainerRef = useRef(null) + const customStrategy = React.useCallback(() => { + if (customContainerRef.current) { + const tab = Array.from(customContainerRef.current.querySelectorAll('a.selected')) + return tab[0] + } + }, [customContainerRef]) const {containerRef: navRef} = useFocusZone({ - bindKeys: FocusKeys.ArrowHorizontal + containerRef: customContainerRef, + bindKeys: FocusKeys.ArrowHorizontal, + focusOutBehavior: 'wrap', + focusInStrategy: customStrategy }) return ( }> From 67519e515bbc672f6ebb59b5b776a0625f7ff708 Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Fri, 24 Jun 2022 09:36:31 +0000 Subject: [PATCH 4/9] Fix tab consistency test --- src/__tests__/__snapshots__/TabNav.test.tsx.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/src/__tests__/__snapshots__/TabNav.test.tsx.snap b/src/__tests__/__snapshots__/TabNav.test.tsx.snap index edcc428b949..52d5e00853b 100644 --- a/src/__tests__/__snapshots__/TabNav.test.tsx.snap +++ b/src/__tests__/__snapshots__/TabNav.test.tsx.snap @@ -45,6 +45,7 @@ exports[`TabNav TabNav.Link renders consistently 1`] = ` } From cc0a10d1c7751dc390b5ec089a77fb6b8e743081 Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Mon, 27 Jun 2022 18:04:12 +0000 Subject: [PATCH 5/9] Add tests for new TabNav focus management --- src/__tests__/TabNav.test.tsx | 99 ++++++++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 2 deletions(-) diff --git a/src/__tests__/TabNav.test.tsx b/src/__tests__/TabNav.test.tsx index 3362e2d917e..8cd1f5157a7 100644 --- a/src/__tests__/TabNav.test.tsx +++ b/src/__tests__/TabNav.test.tsx @@ -1,12 +1,32 @@ import React from 'react' import {TabNav} from '..' -import {behavesAsComponent, checkExports} from '../utils/testing' -import {render as HTMLRender, cleanup} from '@testing-library/react' +import {mount, behavesAsComponent, checkExports} from '../utils/testing' +import {fireEvent, render as HTMLRender, cleanup} from '@testing-library/react' +import userEvent from '@testing-library/user-event' import {axe, toHaveNoViolations} from 'jest-axe' import 'babel-polyfill' +import {Button} from '../Button' +import Box from '../Box' expect.extend(toHaveNoViolations) describe('TabNav', () => { + const tabNavMarkup = ( + + + + First + + + Middle + + + Last + + + + + ) + behavesAsComponent({Component: TabNav}) checkExports('TabNav', { @@ -29,4 +49,79 @@ describe('TabNav', () => { expect(getByLabelText('stuff')).toBeTruthy() expect(getByLabelText('stuff').tagName).toEqual('NAV') }) + + it('selects a tab when tab is loaded', () => { + const component = mount(tabNavMarkup) + const tab = component.find('#middle').first() + expect(tab.getDOMNode().classList).toContain('selected') + }) + + it('selects next tab when pressing right arrow', () => { + const {getByText} = HTMLRender(tabNavMarkup) + const middleTab = getByText('Middle') + const lastTab = getByText('Last') + + fireEvent.focus(middleTab) + fireEvent.keyDown(middleTab, {key: 'ArrowRight'}) + + expect(lastTab).toHaveFocus() + }) + + it('selects previous tab when pressing left arrow', () => { + const {getByText} = HTMLRender(tabNavMarkup) + const middleTab = getByText('Middle') + const firstTab = getByText('First') + + fireEvent.focus(middleTab) + fireEvent.keyDown(middleTab, {key: 'ArrowLeft'}) + + expect(firstTab).toHaveFocus() + }) + + it('selects last tab when pressing left arrow on first tab', () => { + const {getByText} = HTMLRender(tabNavMarkup) + const firstTab = getByText('First') + const lastTab = getByText('Last') + + fireEvent.focus(firstTab) + fireEvent.keyDown(firstTab, {key: 'ArrowLeft'}) + + expect(lastTab).toHaveFocus() + }) + + it('selects first tab when pressing right arrow on last tab', () => { + const {getByText} = HTMLRender(tabNavMarkup) + const lastTab = getByText('Last') + const firstTab = getByText('First') + + fireEvent.focus(lastTab) + fireEvent.keyDown(lastTab, {key: 'ArrowRight'}) + + expect(firstTab).toHaveFocus() + }) + + it('moves focus away from TabNav when pressing tab', () => { + const {getByText, getByRole} = HTMLRender(tabNavMarkup) + const middleTab = getByText('Middle') + const button = getByRole('button') + + userEvent.click(middleTab) + expect(middleTab).toHaveFocus() + userEvent.tab() + + expect(button).toHaveFocus() + }) + + it('moves focus to selected tab when TabNav regains focus', () => { + const {getByText, getByRole} = HTMLRender(tabNavMarkup) + const middleTab = getByText('Middle') + // const lastTab = getByText('Last') + const button = getByRole('button') + + userEvent.click(button) + expect(button).toHaveFocus() + userEvent.tab({shift: true}) + + expect(middleTab).toHaveFocus() + }) }) From 777d2c7dfd2fa7203ee88c4915644fe8d2e4dfda Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Mon, 27 Jun 2022 18:07:55 +0000 Subject: [PATCH 6/9] Add changeset --- .changeset/orange-spiders-study.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/orange-spiders-study.md diff --git a/.changeset/orange-spiders-study.md b/.changeset/orange-spiders-study.md new file mode 100644 index 00000000000..8b39b560793 --- /dev/null +++ b/.changeset/orange-spiders-study.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Changes focus rules of TabNav to match WAI-ARIA rules for tablist From 344a8a238ca27d92d3b6561d2dc57076ba4b6fae Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 28 Jun 2022 09:25:44 +0100 Subject: [PATCH 7/9] Apply suggestions from code review Co-authored-by: Katie Foster Co-authored-by: Kendall Gassner --- src/TabNav.tsx | 4 ++-- src/__tests__/TabNav.test.tsx | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/TabNav.tsx b/src/TabNav.tsx index f203843741e..317bc2479d2 100644 --- a/src/TabNav.tsx +++ b/src/TabNav.tsx @@ -32,7 +32,7 @@ function TabNav({children, 'aria-label': ariaLabel, ...rest}: TabNavProps) { const customContainerRef = useRef(null) const customStrategy = React.useCallback(() => { if (customContainerRef.current) { - const tab = Array.from(customContainerRef.current.querySelectorAll('a.selected')) + const tabs = Array.from(customContainerRef.current.querySelectorAll('a[aria-selected]')) return tab[0] } }, [customContainerRef]) @@ -60,7 +60,7 @@ const TabNavLink = styled.a.attrs(props => ({ activeClassName: typeof props.to === 'string' ? 'selected' : '', className: classnames(ITEM_CLASS, props.selected && SELECTED_CLASS, props.className), role: 'tab', - 'aria-selected': props.selected ? true : false + 'aria-selected': props.selected }))` padding: 8px 12px; font-size: ${get('fontSizes.1')}; diff --git a/src/__tests__/TabNav.test.tsx b/src/__tests__/TabNav.test.tsx index 8cd1f5157a7..09cc2ea06d0 100644 --- a/src/__tests__/TabNav.test.tsx +++ b/src/__tests__/TabNav.test.tsx @@ -115,7 +115,6 @@ describe('TabNav', () => { it('moves focus to selected tab when TabNav regains focus', () => { const {getByText, getByRole} = HTMLRender(tabNavMarkup) const middleTab = getByText('Middle') - // const lastTab = getByText('Last') const button = getByRole('button') userEvent.click(button) From 5577f2e64d6dcc39808f6aa2427379399aba8ebb Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 28 Jun 2022 09:25:10 +0000 Subject: [PATCH 8/9] Fix tests after changes --- src/TabNav.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/TabNav.tsx b/src/TabNav.tsx index 317bc2479d2..fa318fdbf76 100644 --- a/src/TabNav.tsx +++ b/src/TabNav.tsx @@ -32,8 +32,8 @@ function TabNav({children, 'aria-label': ariaLabel, ...rest}: TabNavProps) { const customContainerRef = useRef(null) const customStrategy = React.useCallback(() => { if (customContainerRef.current) { - const tabs = Array.from(customContainerRef.current.querySelectorAll('a[aria-selected]')) - return tab[0] + const tabs = Array.from(customContainerRef.current.querySelectorAll('a[aria-selected=true]')) + return tabs[0] } }, [customContainerRef]) const {containerRef: navRef} = useFocusZone({ @@ -60,7 +60,7 @@ const TabNavLink = styled.a.attrs(props => ({ activeClassName: typeof props.to === 'string' ? 'selected' : '', className: classnames(ITEM_CLASS, props.selected && SELECTED_CLASS, props.className), role: 'tab', - 'aria-selected': props.selected + 'aria-selected': !!props.selected }))` padding: 8px 12px; font-size: ${get('fontSizes.1')}; From c22437af229f8d8834e9ec39180811a531575f75 Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 28 Jun 2022 21:10:31 +0100 Subject: [PATCH 9/9] Update src/TabNav.tsx Co-authored-by: Siddharth Kshetrapal --- src/TabNav.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/TabNav.tsx b/src/TabNav.tsx index fa318fdbf76..6e1ec7f964e 100644 --- a/src/TabNav.tsx +++ b/src/TabNav.tsx @@ -38,7 +38,7 @@ function TabNav({children, 'aria-label': ariaLabel, ...rest}: TabNavProps) { }, [customContainerRef]) const {containerRef: navRef} = useFocusZone({ containerRef: customContainerRef, - bindKeys: FocusKeys.ArrowHorizontal, + bindKeys: FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd, focusOutBehavior: 'wrap', focusInStrategy: customStrategy })