Improves keyboard navigation for the SegmentedControl (#2145)

* implements keyboard navigation strategy for the SegmentedControl suggested by the a11y team, and warns component users if the control does not have a label

* adds changeset

* crappy fix for bug where useFocusZone breaks after selecting a button

* corrects typo in test name

* consolidates button focus test, and adds comment to focusInStrategy test

* remove getNextFocusable by tweaking focusInStrategy

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

.changeset/tiny-radios-wait.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Updates SegmentedControl component's keyboard navigation to align with the recommendations of GitHub's accessibility team.

src/SegmentedControl/SegmentedControl.test.tsx

@@ -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>
     )
   })
@@ -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/')

src/SegmentedControl/SegmentedControl.tsx

@@ -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
@@ -28,10 +29,16 @@ const getSegmentedControlStyles = (props?: SegmentedControlProps) => ({
 })
 
 // TODO: implement `variant` prop for responsive behavior
-// TODO: implement `loading` prop
-// 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 =>
@@ -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"]'
+      ) 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, {

src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap

@@ -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;
@@ -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;
@@ -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;

src/SegmentedControl/examples.stories.tsx

@@ -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)
   }

src/SegmentedControl/getSegmentedControlStyles.ts

@@ -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}"`,