From 82471b2fc97b9ad8ae901f8c09357f7638599117 Mon Sep 17 00:00:00 2001
From: Cameron Dutro <camertron@gmail.com>
Date: Fri, 12 May 2023 21:33:47 -0700
Subject: [PATCH] Address ToggleSwitch a11y feedback (#3251)

* Address ToggleSwitch a11y feedback

* Remove unused import

* Add changeset

* Fix tests
---
 .changeset/two-cycles-provide.md              |  5 +++
 e2e/components/ToggleSwitch.test.ts           |  5 +++
 src/ToggleSwitch/ToggleSwitch.tsx             |  8 ++--
 src/__tests__/ToggleSwitch.test.tsx           | 35 +++++++++++-----
 .../__snapshots__/ToggleSwitch.test.tsx.snap  | 41 ++++++-------------
 5 files changed, 50 insertions(+), 44 deletions(-)
 create mode 100644 .changeset/two-cycles-provide.md

diff --git a/.changeset/two-cycles-provide.md b/.changeset/two-cycles-provide.md
new file mode 100644
index 00000000000..803bbd43fcb
--- /dev/null
+++ b/.changeset/two-cycles-provide.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Address ToggleSwitch accessibility feedback
diff --git a/e2e/components/ToggleSwitch.test.ts b/e2e/components/ToggleSwitch.test.ts
index 33ed880d336..bde4521a25c 100644
--- a/e2e/components/ToggleSwitch.test.ts
+++ b/e2e/components/ToggleSwitch.test.ts
@@ -30,6 +30,11 @@ test.describe('ToggleSwitch', () => {
               'color-contrast': {
                 enabled: theme !== 'dark_dimmed',
               },
+
+              // the 'default' preview does not associate a label with the button
+              'button-name': {
+                enabled: false,
+              },
             },
           })
         })
diff --git a/src/ToggleSwitch/ToggleSwitch.tsx b/src/ToggleSwitch/ToggleSwitch.tsx
index 851757d9bab..6606f5edc1c 100644
--- a/src/ToggleSwitch/ToggleSwitch.tsx
+++ b/src/ToggleSwitch/ToggleSwitch.tsx
@@ -7,7 +7,6 @@ import Text from '../Text'
 import {get} from '../constants'
 import {useProvidedStateOrCreate} from '../hooks'
 import sx, {BetterSystemStyleObject, SxProp} from '../sx'
-import VisuallyHidden from '../_VisuallyHidden'
 import {CellAlignment} from '../DataTable/column'
 
 const TRANSITION_DURATION = '80ms'
@@ -254,7 +253,8 @@ const ToggleSwitch: React.FC<React.PropsWithChildren<ToggleSwitchProps>> = ({
         fontSize={size === 'small' ? 0 : 1}
         mx={2}
         aria-hidden="true"
-        sx={{position: 'relative'}}
+        sx={{position: 'relative', cursor: 'pointer'}}
+        onClick={handleToggleClick}
       >
         <Box textAlign="right" sx={isOn ? null : hiddenTextStyles}>
           On
@@ -267,13 +267,11 @@ const ToggleSwitch: React.FC<React.PropsWithChildren<ToggleSwitchProps>> = ({
         onClick={handleToggleClick}
         aria-labelledby={ariaLabelledby}
         aria-describedby={ariaDescribedby}
-        aria-checked={isOn}
-        role="switch"
+        aria-pressed={isOn}
         checked={isOn}
         size={size}
         disabled={!acceptsInteraction}
       >
-        <VisuallyHidden>{isOn ? 'On' : 'Off'}</VisuallyHidden>
         <Box aria-hidden="true" display="flex" alignItems="center" width="100%" height="100%" overflow="hidden">
           <Box
             flexGrow={1}
diff --git a/src/__tests__/ToggleSwitch.test.tsx b/src/__tests__/ToggleSwitch.test.tsx
index 6a632d1b6c4..f610866e251 100644
--- a/src/__tests__/ToggleSwitch.test.tsx
+++ b/src/__tests__/ToggleSwitch.test.tsx
@@ -23,7 +23,7 @@ it('renders a switch that is turned off', () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
 })
 it('renders a switch that is turned on', () => {
   const {getByLabelText} = render(
@@ -34,7 +34,7 @@ it('renders a switch that is turned on', () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'true')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
 })
 it('renders a switch that is disabled', async () => {
   const user = userEvent.setup()
@@ -46,9 +46,9 @@ it('renders a switch that is disabled', async () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
 })
 it("renders a switch who's state is loading", async () => {
   const user = userEvent.setup()
@@ -62,9 +62,9 @@ it("renders a switch who's state is loading", async () => {
   const loadingSpinner = container.querySelector('svg')
 
   expect(loadingSpinner).toBeDefined()
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
 })
 it('switches from off to on uncontrolled', async () => {
   const user = userEvent.setup()
@@ -76,9 +76,24 @@ it('switches from off to on uncontrolled', async () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'true')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
+})
+it('switches from off to on uncontrolled when clicking on status label', async () => {
+  const user = userEvent.setup()
+  const {getByLabelText, getByText} = render(
+    <>
+      <div id="switchLabel">{SWITCH_LABEL_TEXT}</div>
+      <ToggleSwitch aria-labelledby="switchLabel" />
+    </>,
+  )
+  const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
+  const toggleSwitchStatusLabel = getByText('Off')
+
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  await user.click(toggleSwitchStatusLabel)
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
 })
 it('switches from off to on with a controlled prop', async () => {
   const user = userEvent.setup()
@@ -99,9 +114,9 @@ it('switches from off to on with a controlled prop', async () => {
   const {getByLabelText} = render(<ControlledSwitchComponent />)
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-checked', 'true')
+  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
 })
 it('calls onChange when the switch is toggled', async () => {
   const user = userEvent.setup()
diff --git a/src/__tests__/__snapshots__/ToggleSwitch.test.tsx.snap b/src/__tests__/__snapshots__/ToggleSwitch.test.tsx.snap
index be527cef621..764e81dd26d 100644
--- a/src/__tests__/__snapshots__/ToggleSwitch.test.tsx.snap
+++ b/src/__tests__/__snapshots__/ToggleSwitch.test.tsx.snap
@@ -11,7 +11,7 @@ exports[`renders consistently 1`] = `
   text-align: right;
 }
 
-.c6 {
+.c5 {
   display: -webkit-box;
   display: -webkit-flex;
   display: -ms-flexbox;
@@ -25,7 +25,7 @@ exports[`renders consistently 1`] = `
   align-items: center;
 }
 
-.c7 {
+.c6 {
   color: #ffffff;
   line-height: 0;
   -webkit-box-flex: 1;
@@ -48,7 +48,7 @@ exports[`renders consistently 1`] = `
   transition-duration: 80ms;
 }
 
-.c8 {
+.c7 {
   color: #656d76;
   line-height: 0;
   -webkit-box-flex: 1;
@@ -85,25 +85,13 @@ exports[`renders consistently 1`] = `
   flex-direction: row;
 }
 
-.c5 {
-  position: absolute;
-  width: 1px;
-  height: 1px;
-  padding: 0;
-  margin: -1px;
-  overflow: hidden;
-  -webkit-clip: rect(0,0,0,0);
-  clip: rect(0,0,0,0);
-  white-space: nowrap;
-  border-width: 0;
-}
-
 .c1 {
   font-size: 14px;
   color: #1F2328;
   margin-left: 8px;
   margin-right: 8px;
   position: relative;
+  cursor: pointer;
 }
 
 .c4 {
@@ -152,7 +140,7 @@ exports[`renders consistently 1`] = `
   background-color: hsla(210,24%,88%,1);
 }
 
-.c9 {
+.c8 {
   background-color: #ffffff;
   border-width: 1px;
   border-style: solid;
@@ -202,7 +190,7 @@ exports[`renders consistently 1`] = `
 }
 
 @media (prefers-reduced-motion) {
-  .c9 {
+  .c8 {
     -webkit-transition: none;
     transition: none;
   }
@@ -217,6 +205,7 @@ exports[`renders consistently 1`] = `
     className="c1"
     color="fg.default"
     fontSize={1}
+    onClick={[Function]}
   >
     <div
       className="c2"
@@ -230,29 +219,23 @@ exports[`renders consistently 1`] = `
     </div>
   </span>
   <button
-    aria-checked={false}
+    aria-pressed={false}
     checked={false}
     className="c4"
     disabled={false}
     onClick={[Function]}
-    role="switch"
     size="medium"
   >
-    <span
-      className="c5"
-    >
-      Off
-    </span>
     <div
       aria-hidden="true"
-      className="c6"
+      className="c5"
       display="flex"
       height="100%"
       overflow="hidden"
       width="100%"
     >
       <div
-        className="c7"
+        className="c6"
         color="switchTrack.checked.fg"
       >
         <svg
@@ -269,7 +252,7 @@ exports[`renders consistently 1`] = `
         </svg>
       </div>
       <div
-        className="c8"
+        className="c7"
         color="switchTrack.fg"
       >
         <svg
@@ -289,7 +272,7 @@ exports[`renders consistently 1`] = `
     <div
       aria-hidden="true"
       checked={false}
-      className="c9"
+      className="c8"
       disabled={false}
     />
   </button>