-
Notifications
You must be signed in to change notification settings - Fork 597
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
Convey Spinner to assistive technologies #4140
Changes from 13 commits
fe7f92d
edada79
24a4eca
9a0308e
9c4359e
7448154
54660f9
12c5383
dd463c0
1ceff99
9de1e7d
69c955d
52a1657
b77f801
5d49c29
7c736b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'@primer/react': minor | ||
--- | ||
|
||
Adds a prop, `srText`, to the Spinner component to convey a loading message to assistive technologies such as screen readers. | ||
|
||
<!-- Changed components: Spinner --> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
import React from 'react' | ||
import type {ComponentMeta} from '@storybook/react' | ||
import Spinner from './Spinner' | ||
import {Box, Button} from '..' | ||
import {VisuallyHidden} from '../internal/components/VisuallyHidden' | ||
import {Status} from '../internal/components/Status' | ||
|
||
export default { | ||
title: 'Components/Spinner/Examples', | ||
component: Spinner, | ||
} as ComponentMeta<typeof Spinner> | ||
|
||
type LoadingState = 'initial' | 'loading' | 'done' | ||
|
||
async function wait(ms: number) { | ||
return new Promise(resolve => setTimeout(resolve, ms)) | ||
} | ||
|
||
// There should be an announcement when loading is completed or if there was an error loading | ||
export const FullLifecycle = () => { | ||
const [isLoading, setIsLoading] = React.useState(false) | ||
const [loadedContent, setLoadedContent] = React.useState('') | ||
let state: LoadingState = 'initial' | ||
|
||
if (isLoading) { | ||
state = 'loading' | ||
} else if (loadedContent) { | ||
state = 'done' | ||
} | ||
|
||
const initiateLoading = async () => { | ||
if (state === 'done') { | ||
return | ||
} | ||
|
||
setIsLoading(true) | ||
await wait(1000) | ||
setLoadedContent('Some content that had to be loaded.') | ||
setIsLoading(false) | ||
} | ||
|
||
return ( | ||
<> | ||
<Button onClick={initiateLoading} sx={{mb: '1em'}}> | ||
Load content | ||
</Button> | ||
{state === 'loading' && <Spinner />} | ||
<p>{loadedContent}</p> | ||
<VisuallyHidden> | ||
<Status>{state === 'done' && 'Content finished loading'}</Status> | ||
</VisuallyHidden> | ||
</> | ||
) | ||
} | ||
|
||
// We should avoid duplicate loading announcements | ||
export const FullLifecycleVisibleLoadingText = () => { | ||
const [isLoading, setIsLoading] = React.useState(false) | ||
const [loadedContent, setLoadedContent] = React.useState('') | ||
let state: LoadingState = 'initial' | ||
|
||
if (isLoading) { | ||
state = 'loading' | ||
} else if (loadedContent) { | ||
state = 'done' | ||
} | ||
|
||
const initiateLoading = async () => { | ||
if (state === 'done') { | ||
return | ||
} | ||
|
||
setIsLoading(true) | ||
await wait(1000) | ||
setLoadedContent('Some content that had to be loaded.') | ||
setIsLoading(false) | ||
} | ||
|
||
return ( | ||
<Box sx={{display: 'flex', alignItems: 'flex-start', flexDirection: 'column', gap: '0.5em'}}> | ||
<Button onClick={initiateLoading} sx={{mb: '1em'}}> | ||
Load content | ||
</Button> | ||
{state !== 'done' && ( | ||
<Box sx={{alignItems: 'center', display: 'flex', gap: '0.25rem'}}> | ||
{state === 'loading' && <Spinner size="small" srText={null} />} | ||
<Status>{state === 'loading' ? 'Content is loading...' : ''}</Status> | ||
</Box> | ||
)} | ||
<p>{loadedContent}</p> | ||
<VisuallyHidden> | ||
<Status>{state === 'done' && 'Content finished loading'}</Status> | ||
</VisuallyHidden> | ||
</Box> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,46 +1,62 @@ | ||
import React from 'react' | ||
import styled from 'styled-components' | ||
import type {SxProp} from '../sx' | ||
import sx from '../sx' | ||
import type {ComponentProps} from '../utils/types' | ||
import sx, {type SxProp} from '../sx' | ||
import {VisuallyHidden} from '../internal/components/VisuallyHidden' | ||
import type {HTMLDataAttributes} from '../internal/internal-types' | ||
import Box from '../Box' | ||
import {Status} from '../internal/components/Status' | ||
|
||
const sizeMap = { | ||
small: '16px', | ||
medium: '32px', | ||
large: '64px', | ||
} | ||
|
||
export interface SpinnerInternalProps { | ||
export type SpinnerProps = { | ||
/** Sets the width and height of the spinner. */ | ||
size?: keyof typeof sizeMap | ||
} | ||
/** Sets the text conveyed by assistive technologies such as screen readers. Set to `null` if the loading state is displayed in a text node somewhere else on the page. */ | ||
srText?: string | null | ||
/** @deprecated Use `srText` instead. */ | ||
'aria-label'?: string | null | ||
} & HTMLDataAttributes & | ||
SxProp | ||
|
||
function Spinner({size: sizeKey = 'medium', ...props}: SpinnerInternalProps) { | ||
function Spinner({size: sizeKey = 'medium', srText = 'Loading', 'aria-label': ariaLabel, ...props}: SpinnerProps) { | ||
const size = sizeMap[sizeKey] | ||
const hasSrAnnouncement = Boolean(srText || ariaLabel) | ||
|
||
return ( | ||
<svg height={size} width={size} viewBox="0 0 16 16" fill="none" {...props}> | ||
<circle | ||
cx="8" | ||
cy="8" | ||
r="7" | ||
stroke="currentColor" | ||
strokeOpacity="0.25" | ||
strokeWidth="2" | ||
vectorEffect="non-scaling-stroke" | ||
/> | ||
<path | ||
d="M15 8a7.002 7.002 0 00-7-7" | ||
stroke="currentColor" | ||
strokeWidth="2" | ||
strokeLinecap="round" | ||
vectorEffect="non-scaling-stroke" | ||
/> | ||
</svg> | ||
/* inline-flex removes the extra line height */ | ||
<Box sx={{display: 'inline-flex'}} role={hasSrAnnouncement ? 'status' : undefined}> | ||
<svg height={size} width={size} viewBox="0 0 16 16" fill="none" aria-hidden {...props}> | ||
<circle | ||
cx="8" | ||
cy="8" | ||
r="7" | ||
stroke="currentColor" | ||
strokeOpacity="0.25" | ||
strokeWidth="2" | ||
vectorEffect="non-scaling-stroke" | ||
/> | ||
<path | ||
d="M15 8a7.002 7.002 0 00-7-7" | ||
stroke="currentColor" | ||
strokeWidth="2" | ||
strokeLinecap="round" | ||
vectorEffect="non-scaling-stroke" | ||
/> | ||
</svg> | ||
{hasSrAnnouncement ? ( | ||
<VisuallyHidden> | ||
<Status>{srText || ariaLabel}</Status> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per #4140 (comment), we could make this opt-in and keep the live region, but I'll leave this up to you! It would probably involve an additional prop to toggle the live region being on/off with the default being off. We could also remove it and come back to it in the future, but there might be value in having it in now as opt-in. |
||
</VisuallyHidden> | ||
) : null} | ||
</Box> | ||
) | ||
} | ||
|
||
const StyledSpinner = styled(Spinner)<SxProp>` | ||
const StyledSpinner = styled(Spinner)` | ||
@keyframes rotate-keyframes { | ||
100% { | ||
transform: rotate(360deg); | ||
|
@@ -54,5 +70,4 @@ const StyledSpinner = styled(Spinner)<SxProp>` | |
|
||
StyledSpinner.displayName = 'Spinner' | ||
|
||
export type SpinnerProps = ComponentProps<typeof StyledSpinner> | ||
export default StyledSpinner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're using the
<Status>
component now, we can remove the role here.