Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves keyboard navigation for the SegmentedControl #2145

Merged
merged 8 commits into from
Jul 21, 2022
5 changes: 5 additions & 0 deletions .changeset/tiny-radios-wait.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Updates SegmentedControl component's keyboard navigation to align with the recommendations of GitHub's accessibility team.
84 changes: 77 additions & 7 deletions src/SegmentedControl/SegmentedControl.test.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,34 @@
import React from 'react'
import '@testing-library/jest-dom/extend-expect'
import {render} from '@testing-library/react'
import {fireEvent, render} from '@testing-library/react'
import {EyeIcon, FileCodeIcon, PeopleIcon} from '@primer/octicons-react'
import userEvent from '@testing-library/user-event'
import {behavesAsComponent, checkExports, checkStoriesForAxeViolations} from '../utils/testing'
import {SegmentedControl} from '.' // TODO: update import when we move this to the global index

const segmentData = [
{label: 'Preview', iconLabel: 'EyeIcon', icon: () => <EyeIcon aria-label="EyeIcon" />},
{label: 'Raw', iconLabel: 'FileCodeIcon', icon: () => <FileCodeIcon aria-label="FileCodeIcon" />},
{label: 'Blame', iconLabel: 'PeopleIcon', icon: () => <PeopleIcon aria-label="PeopleIcon" />}
{label: 'Preview', id: 'preview', iconLabel: 'EyeIcon', icon: () => <EyeIcon aria-label="EyeIcon" />},
{label: 'Raw', id: 'raw', iconLabel: 'FileCodeIcon', icon: () => <FileCodeIcon aria-label="FileCodeIcon" />},
{label: 'Blame', id: 'blame', iconLabel: 'PeopleIcon', icon: () => <PeopleIcon aria-label="PeopleIcon" />}
]

// TODO: improve test coverage
describe('SegmentedControl', () => {
const mockWarningFn = jest.fn()

beforeAll(() => {
jest.spyOn(global.console, 'warn').mockImplementation(mockWarningFn)
})

behavesAsComponent({
Component: SegmentedControl,
toRender: () => (
<SegmentedControl aria-label="File view">
<SegmentedControl.Button selected>Preview</SegmentedControl.Button>
<SegmentedControl.Button>Raw</SegmentedControl.Button>
<SegmentedControl.Button>Blame</SegmentedControl.Button>
{segmentData.map(({label}, index) => (
<SegmentedControl.Button selected={index === 0} key={label}>
{label}
</SegmentedControl.Button>
))}
</SegmentedControl>
)
})
Expand Down Expand Up @@ -133,6 +141,68 @@ describe('SegmentedControl', () => {
}
expect(handleClick).toHaveBeenCalled()
})

it('focuses the selected button first', () => {
const {getByRole} = render(
<>
<button>Before</button>
<SegmentedControl aria-label="File view">
{segmentData.map(({label, id}, index) => (
<SegmentedControl.Button selected={index === 1} key={label} id={id}>
{label}
</SegmentedControl.Button>
))}
</SegmentedControl>
</>
)
const initialFocusButtonNode = getByRole('button', {name: segmentData[1].label})

expect(document.activeElement?.id).not.toEqual(initialFocusButtonNode.id)

userEvent.tab() // focus the button before the segmented control
userEvent.tab() // move focus into the segmented control

expect(document.activeElement?.id).toEqual(initialFocusButtonNode.id)
})

it('focuses the previous button when keying ArrowLeft, and the next button when keying ArrowRight', () => {
const {getByRole} = render(
<SegmentedControl aria-label="File view">
{segmentData.map(({label, id}, index) => (
<SegmentedControl.Button selected={index === 1} key={label} id={id}>
{label}
</SegmentedControl.Button>
))}
</SegmentedControl>
)
const initialFocusButtonNode = getByRole('button', {name: segmentData[1].label})
const nextFocusButtonNode = getByRole('button', {name: segmentData[0].label})

expect(document.activeElement?.id).not.toEqual(nextFocusButtonNode.id)

fireEvent.focus(initialFocusButtonNode)
fireEvent.keyDown(initialFocusButtonNode, {key: 'ArrowLeft'})

expect(document.activeElement?.id).toEqual(nextFocusButtonNode.id)

fireEvent.keyDown(initialFocusButtonNode, {key: 'ArrowRight'})

expect(document.activeElement?.id).toEqual(initialFocusButtonNode.id)
})

it('should warn the user if they neglect to specify a label for the segmented control', () => {
render(
<SegmentedControl>
{segmentData.map(({label, id}) => (
<SegmentedControl.Button id={id} key={label}>
{label}
</SegmentedControl.Button>
))}
</SegmentedControl>
)

expect(mockWarningFn).toHaveBeenCalled()
})
})

checkStoriesForAxeViolations('examples', '../SegmentedControl/')
Expand Down
52 changes: 46 additions & 6 deletions src/SegmentedControl/SegmentedControl.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React from 'react'
import React, {RefObject, useRef} from 'react'
import Button, {SegmentedControlButtonProps} from './SegmentedControlButton'
import SegmentedControlIconButton, {SegmentedControlIconButtonProps} from './SegmentedControlIconButton'
import {Box, useTheme} from '..'
import {merge, SxProp} from '../sx'
import {FocusKeys, FocusZoneHookSettings, useFocusZone} from '../hooks/useFocusZone'

type SegmentedControlProps = {
'aria-label'?: string
Expand All @@ -28,10 +29,16 @@ const getSegmentedControlStyles = (props?: SegmentedControlProps) => ({
})

// TODO: implement `variant` prop for responsive behavior
// TODO: implement `loading` prop
siddharthkp marked this conversation as resolved.
Show resolved Hide resolved
// TODO: log a warning if no `ariaLabel` or `ariaLabelledBy` prop is passed
// TODO: implement keyboard behavior to move focus using the arrow keys
const Root: React.FC<SegmentedControlProps> = ({children, fullWidth, onChange, sx: sxProp = {}, ...rest}) => {
const Root: React.FC<SegmentedControlProps> = ({
'aria-label': ariaLabel,
'aria-labelledby': ariaLabelledby,
children,
fullWidth,
onChange,
sx: sxProp = {},
...rest
}) => {
const segmentedControlContainerRef = useRef<HTMLSpanElement>(null)
const {theme} = useTheme()
const selectedChildren = React.Children.toArray(children).map(
child =>
Expand All @@ -46,8 +53,41 @@ const Root: React.FC<SegmentedControlProps> = ({children, fullWidth, onChange, s
sxProp as SxProp
)

const focusInStrategy: FocusZoneHookSettings['focusInStrategy'] = () => {
if (segmentedControlContainerRef.current) {
// we need to use type assertion because querySelector returns "Element", not "HTMLElement"
type SelectedButton = HTMLButtonElement | undefined

const selectedButton = segmentedControlContainerRef.current.querySelector(
'button[aria-current="true"]'
siddharthkp marked this conversation as resolved.
Show resolved Hide resolved
) as SelectedButton

return selectedButton
}
}

useFocusZone({
containerRef: segmentedControlContainerRef,
bindKeys: FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd,
focusInStrategy
})

if (!ariaLabel && !ariaLabelledby) {
// eslint-disable-next-line no-console
console.warn(
'Use the `aria-label` or `aria-labelledby` prop to provide an accessible label for assistive technology'
)
}

return (
<Box role="toolbar" sx={sx} {...rest}>
<Box
role="toolbar"
sx={sx}
aria-label={ariaLabel}
aria-labelledby={ariaLabelledby}
ref={segmentedControlContainerRef as RefObject<HTMLDivElement>}
{...rest}
>
{React.Children.map(children, (child, i) => {
if (React.isValidElement<SegmentedControlButtonProps | SegmentedControlIconButtonProps>(child)) {
return React.cloneElement(child, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ exports[`SegmentedControl renders consistently 1`] = `
width: 1px;
}

.c1:focus:focus-visible:not(:last-child):after {
width: 0;
}

.c1 .segmentedControl-text:after {
content: "Preview";
display: block;
Expand Down Expand Up @@ -180,6 +184,10 @@ exports[`SegmentedControl renders consistently 1`] = `
width: 1px;
}

.c2:focus:focus-visible:not(:last-child):after {
width: 0;
}

.c2 .segmentedControl-text:after {
content: "Raw";
display: block;
Expand Down Expand Up @@ -274,6 +282,10 @@ exports[`SegmentedControl renders consistently 1`] = `
width: 1px;
}

.c3:focus:focus-visible:not(:last-child):after {
width: 0;
}

.c3 .segmentedControl-text:after {
content: "Blame";
display: block;
Expand Down
2 changes: 1 addition & 1 deletion src/SegmentedControl/examples.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const Default = (args: Args) => (
)

export const Controlled = (args: Args) => {
const [selectedIndex, setSelectedIndex] = useState(1)
const [selectedIndex, setSelectedIndex] = useState(0)
const handleChange = (i: number) => {
setSelectedIndex(i)
}
Expand Down
5 changes: 5 additions & 0 deletions src/SegmentedControl/getSegmentedControlStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ const getSegmentedControlButtonStyles = (props?: SegmentedControlButtonProps & {
}
},

// fixes an issue where the focus outline shows over the pseudo-element
':focus:focus-visible:not(:last-child):after': {
width: 0
},

'.segmentedControl-text': {
':after': {
content: `"${props?.children}"`,
Expand Down