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

Address additional ToggleSwitch a11y feedback #3902

Closed
wants to merge 12 commits into from
Closed
5 changes: 5 additions & 0 deletions .changeset/honest-gorillas-drive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Address additional ToggleSwitch a11y feedback
3 changes: 2 additions & 1 deletion script/test-e2e
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
#!/bin/bash

args="$@"
set -x

docker run --rm \
--network host \
-v $(pwd):/workspace \
-w /workspace \
-it mcr.microsoft.com/playwright:v1.37.0-jammy \
/bin/bash -c "npm install && STORYBOOK_URL=http://host.docker.internal:6006 npx playwright test $@"
/bin/bash -c "npm install && STORYBOOK_URL=http://host.docker.internal:6006 npx playwright test $args"
58 changes: 56 additions & 2 deletions src/ToggleSwitch/ToggleSwitch.features.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React from 'react'
import ToggleSwitch from './ToggleSwitch'
import React, {useState} from 'react'
import ToggleSwitch, {ToggleSwitchProps} from './ToggleSwitch'
import {Box, Text} from '..'
import {action} from '@storybook/addon-actions'
import ToggleSwitchStoryWrapper from './ToggleSwitchStoryWrapper'
import {StoryFn} from '@storybook/react'

export default {
title: 'Components/ToggleSwitch/Features',
Expand Down Expand Up @@ -67,6 +68,59 @@ export const Loading = () => (
</ToggleSwitchStoryWrapper>
)

type LoadingWithDelayProps = {
loadingDelay: number
}

export const LoadingWithDelay: StoryFn<ToggleSwitchProps & LoadingWithDelayProps> = args => {
const {loadingDelay, loadingLabelDelay} = args

const [isLoading, setIsLoading] = useState(false)
const [timeoutId, setTimeoutId] = useState<number | null>(null)

const handleToggleClick = () => {
setIsLoading(true)

if (timeoutId) {
clearTimeout(timeoutId)
setTimeoutId(null)
}

setTimeoutId(setTimeout(() => setIsLoading(false), loadingDelay) as unknown as number)
}

return (
<ToggleSwitchStoryWrapper>
<Text id="toggle" fontWeight={'bold'} fontSize={1}>
Toggle label
</Text>
<ToggleSwitch
loading={isLoading}
loadingLabelDelay={loadingLabelDelay}
aria-labelledby="toggle"
onClick={handleToggleClick}
/>
</ToggleSwitchStoryWrapper>
)
}

LoadingWithDelay.args = {
loadingDelay: 5000,
loadingLabelDelay: 2000,
}
LoadingWithDelay.argTypes = {
loadingDelay: {
control: {
type: 'number',
},
},
loadingLabelDelay: {
control: {
type: 'number',
},
},
}

export const LabelEnd = () => (
<ToggleSwitchStoryWrapper>
<Text id="toggle" fontWeight={'bold'} fontSize={1}>
Expand Down
34 changes: 32 additions & 2 deletions src/ToggleSwitch/ToggleSwitch.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react'
import {render} from '@testing-library/react'
import {render, waitFor} from '@testing-library/react'
import ToggleSwitch from './'
import {behavesAsComponent, checkExports, checkStoriesForAxeViolations} from '../utils/testing'
import userEvent from '@testing-library/user-event'
Expand Down Expand Up @@ -55,7 +55,7 @@ describe('ToggleSwitch', () => {
expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
})

it("renders a switch who's state is loading", async () => {
it('renders a switch whose state is loading', async () => {
const user = userEvent.setup()
const {getByLabelText, container} = render(
<>
Expand Down Expand Up @@ -103,6 +103,22 @@ describe('ToggleSwitch', () => {
expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
})

it('ensures the status label cannot toggle a disabled switch', async () => {
const user = userEvent.setup()
const {getByLabelText, getByText} = render(
<>
<div id="switchLabel">{SWITCH_LABEL_TEXT}</div>
<ToggleSwitch aria-labelledby="switchLabel" disabled />
</>,
)
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', 'false')
})

it('switches from off to on with a controlled prop', async () => {
const user = userEvent.setup()
const ControlledSwitchComponent = () => {
Expand Down Expand Up @@ -180,5 +196,19 @@ describe('ToggleSwitch', () => {
expect(ref).toHaveBeenCalledWith(expect.any(HTMLButtonElement))
})

it('displays a loading label', async () => {
const TEST_ID = 'a test id'

const {getByTestId} = render(
<>
<span id="label">label</span>
<ToggleSwitch data-testid={TEST_ID} aria-labelledby="label" loadingLabelDelay={0} loading />
</>,
)

const toggleSwitch = getByTestId(TEST_ID)
await waitFor(() => expect(toggleSwitch).toHaveTextContent('Loading'))
})

checkStoriesForAxeViolations('ToggleSwitch.features', '../ToggleSwitch/')
})
79 changes: 69 additions & 10 deletions src/ToggleSwitch/ToggleSwitch.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, {MouseEventHandler, useCallback, useEffect} from 'react'
import {useId} from '../hooks/useId'
import styled, {css} from 'styled-components'
import {variant} from 'styled-system'
import Box from '../Box'
Expand All @@ -8,11 +9,14 @@ import {get} from '../constants'
import {useProvidedStateOrCreate} from '../hooks'
import sx, {BetterSystemStyleObject, SxProp} from '../sx'
import {CellAlignment} from '../DataTable/column'
import VisuallyHidden from '../_VisuallyHidden'

const TRANSITION_DURATION = '80ms'
const EASE_OUT_QUAD_CURVE = 'cubic-bezier(0.5, 1, 0.89, 1)'

export interface ToggleSwitchProps extends Omit<React.HTMLAttributes<HTMLDivElement>, 'onChange'>, SxProp {
/** The id of the DOM node that labels the switch */
['aria-labelledby']: string
/** Uncontrolled - whether the switch is turned on */
defaultChecked?: boolean
/** Whether the switch is ready for user input */
Expand All @@ -31,6 +35,9 @@ export interface ToggleSwitchProps extends Omit<React.HTMLAttributes<HTMLDivElem
* **This should only be changed when the switch's alignment needs to be adjusted.** For example: It needs to be left-aligned because the label appears above it and the caption appears below it.
*/
statusLabelPosition?: CellAlignment
/** If the switch is in the loading state, this value controls the amount of delay in milliseconds before
* the word "Loading" is announced to screen readers. Default: 2000. */
loadingLabelDelay?: number
camertron marked this conversation as resolved.
Show resolved Hide resolved
}

const sizeVariants = variant({
Expand Down Expand Up @@ -127,8 +134,12 @@ const SwitchButton = styled.button<SwitchButtonProps>`
}

${props => {
if (props.disabled) {
if (props['aria-disabled']) {
camertron marked this conversation as resolved.
Show resolved Hide resolved
return css`
@media (forced-colors: active) {
border-color: GrayText;
}

background-color: ${get('colors.switchTrack.disabledBg')};
border-color: transparent;
cursor: not-allowed;
Expand Down Expand Up @@ -166,11 +177,12 @@ const SwitchButton = styled.button<SwitchButtonProps>`
${sx}
${sizeVariants}
`
const ToggleKnob = styled.div<{checked?: boolean; disabled?: boolean}>`
const ToggleKnob = styled.div<{checked?: boolean}>`
camertron marked this conversation as resolved.
Show resolved Hide resolved
background-color: ${get('colors.switchKnob.bg')};
border-width: 1px;
border-style: solid;
border-color: ${props => (props.disabled ? get('colors.switchTrack.disabledBg') : get('colors.switchKnob.border'))};
border-color: ${props =>
props['aria-disabled'] ? get('colors.switchTrack.disabledBg') : get('colors.switchKnob.border')};
border-radius: calc(${get('radii.2')} - 1px); /* -1px to account for 1px border around the control */
width: 50%;
position: absolute;
Expand All @@ -187,8 +199,12 @@ const ToggleKnob = styled.div<{checked?: boolean; disabled?: boolean}>`
}

${props => {
if (props.disabled) {
if (props['aria-disabled']) {
return css`
@media (forced-colors: active) {
color: GrayText;
}

border-color: ${get('colors.switchTrack.disabledBg')};
`
}
Expand Down Expand Up @@ -219,27 +235,63 @@ const ToggleSwitch = React.forwardRef<HTMLButtonElement, React.PropsWithChildren
onClick,
size = 'medium',
statusLabelPosition = 'start',
loadingLabelDelay = 2000,
sx: sxProp,
...rest
} = props
const isControlled = typeof checked !== 'undefined'
const [isOn, setIsOn] = useProvidedStateOrCreate<boolean>(checked, onChange, Boolean(defaultChecked))
const acceptsInteraction = !disabled && !loading

const [loadingLabelTimeoutId, setLoadingLabelTimeoutId] = React.useState<number | null>(null)
const [isLoadingLabelVisible, setIsLoadingLabelVisible] = React.useState(false)
const loadingLabelId = useId('loadingLabel')

const resetLoadingLabel = useCallback(() => {
if (loadingLabelTimeoutId) {
clearTimeout(loadingLabelTimeoutId)
setLoadingLabelTimeoutId(null)
}

if (isLoadingLabelVisible) {
setIsLoadingLabelVisible(false)
}
}, [loadingLabelTimeoutId, isLoadingLabelVisible])

const handleToggleClick: MouseEventHandler = useCallback(
e => {
if (disabled || loading) return

if (!isControlled) {
setIsOn(!isOn)
resetLoadingLabel()
}
onClick && onClick(e)
},
[onClick, isControlled, isOn, setIsOn],
[disabled, isControlled, loading, onClick, setIsOn, isOn, resetLoadingLabel],
)

useEffect(() => {
if (onChange && isControlled) {
if (onChange && isControlled && !disabled) {
onChange(Boolean(checked))
}
}, [onChange, checked, isControlled])
}, [onChange, checked, isControlled, disabled])

if (loading) {
if (!loadingLabelTimeoutId) {
setLoadingLabelTimeoutId(
setTimeout(() => {
setLoadingLabelTimeoutId(null)
setIsLoadingLabelVisible(true)
}, loadingLabelDelay) as unknown as number,
)
}
} else {
resetLoadingLabel()
}

let switchButtonDescribedBy = loadingLabelId
if (ariaDescribedby) switchButtonDescribedBy = `${switchButtonDescribedBy} ${ariaDescribedby}`

return (
<Box
Expand All @@ -249,6 +301,13 @@ const ToggleSwitch = React.forwardRef<HTMLButtonElement, React.PropsWithChildren
sx={sxProp}
{...rest}
>
{isLoadingLabelVisible ? (
<VisuallyHidden>
<span aria-live="polite" id={loadingLabelId}>
Loading
</span>
</VisuallyHidden>
) : null}
{loading ? <Spinner size="small" /> : null}
<Text
color={acceptsInteraction ? 'fg.default' : 'fg.muted'}
Expand All @@ -269,11 +328,11 @@ const ToggleSwitch = React.forwardRef<HTMLButtonElement, React.PropsWithChildren
ref={ref}
onClick={handleToggleClick}
aria-labelledby={ariaLabelledby}
aria-describedby={ariaDescribedby}
aria-describedby={switchButtonDescribedBy}
aria-pressed={isOn}
checked={isOn}
size={size}
disabled={!acceptsInteraction}
aria-disabled={!acceptsInteraction}
>
<Box aria-hidden="true" display="flex" alignItems="center" width="100%" height="100%" overflow="hidden">
<Box
Expand Down Expand Up @@ -305,7 +364,7 @@ const ToggleSwitch = React.forwardRef<HTMLButtonElement, React.PropsWithChildren
<CircleIcon size={size} />
</Box>
</Box>
<ToggleKnob aria-hidden="true" disabled={!acceptsInteraction} checked={isOn} />
<ToggleKnob aria-hidden="true" aria-disabled={!acceptsInteraction} checked={isOn} />
</SwitchButton>
</Box>
)
Expand Down
5 changes: 3 additions & 2 deletions src/ToggleSwitch/__snapshots__/ToggleSwitch.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,11 @@ exports[`ToggleSwitch renders consistently 1`] = `
</div>
</span>
<button
aria-describedby="loadingLabel"
aria-disabled={false}
aria-pressed={false}
checked={false}
className="c4"
disabled={false}
onClick={[Function]}
size="medium"
>
Expand Down Expand Up @@ -272,10 +273,10 @@ exports[`ToggleSwitch renders consistently 1`] = `
</div>
</div>
<div
aria-disabled={false}
aria-hidden="true"
checked={false}
className="c8"
disabled={false}
/>
</button>
</div>
Expand Down
Loading