-
Notifications
You must be signed in to change notification settings - Fork 534
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
Un-revert "Add loading
prop for Button
and IconButton
(#3582)"
#4485
Changes from all commits
60b3253
0c29ea7
27a7c58
14c0278
ac0801b
708f452
e765c05
22ea550
b881fd7
ccde61a
3399501
4fce38a
c454b3e
29e5955
2e1df44
86b14e7
852c9e2
f7616c9
f9cdfb5
1ee5854
2749e52
44845b3
b3f90ed
59d90c6
f61d1d6
95843eb
73cf068
3d2f917
78e99a3
9007105
e9e391a
d401782
8cb4830
50cfa75
be94467
76724c7
a7741ba
abbd4d8
f379211
c1b40a2
093a8bd
50eb950
a400ed9
abd6870
85d8358
691bb7a
85c68a6
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,5 @@ | ||
--- | ||
"@primer/react": minor | ||
--- | ||
|
||
Add `loading` state to `Button` and `IconButton` |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import React from 'react' | ||
import type {Meta} from '@storybook/react' | ||
import {Button} from '.' | ||
import {DownloadIcon} from '@primer/octicons-react' | ||
import {Banner} from '../drafts' | ||
import {AriaStatus} from '../live-region' | ||
|
||
const meta: Meta<typeof Button> = { | ||
title: 'Components/Button/Examples', | ||
} as Meta<typeof Button> | ||
|
||
export default meta | ||
|
||
export const LoadingStatusAnnouncementSuccessful = () => { | ||
const [loading, setLoading] = React.useState(false) | ||
const [success, setSuccess] = React.useState(false) | ||
|
||
const resolveAction = async () => { | ||
setLoading(true) | ||
await new Promise(resolve => setTimeout(resolve, 1500)) | ||
setLoading(false) | ||
|
||
return await true | ||
} | ||
|
||
const onClick = (resolveType: 'error' | 'success') => async () => { | ||
const actionResult = await resolveAction() | ||
|
||
if (resolveType === 'error') { | ||
setSuccess(!actionResult) | ||
return | ||
} | ||
|
||
setSuccess(actionResult) | ||
} | ||
|
||
return ( | ||
<> | ||
<AriaStatus>{!loading && success ? 'Export completed' : null}</AriaStatus> | ||
<Button loading={loading} leadingVisual={DownloadIcon} onClick={onClick('success')}> | ||
Export (success) | ||
</Button> | ||
</> | ||
) | ||
} | ||
|
||
export const LoadingStatusAnnouncementError = () => { | ||
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. In this example, the failure state is communicated to assistive technologies via announcement, but there's no visual indication that anything failed. Do we have guidance around communicating errors visually? If so, should this example include that? 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. Great point! I'll take a look through our docs and update this. |
||
const [loading, setLoading] = React.useState(false) | ||
const [error, setError] = React.useState(false) | ||
|
||
const resolveAction = async () => { | ||
setLoading(true) | ||
await new Promise(resolve => setTimeout(resolve, 1500)) | ||
setLoading(false) | ||
|
||
return await true | ||
} | ||
|
||
const onClick = (resolveType: 'error' | 'success') => async () => { | ||
const actionResult = await resolveAction() | ||
|
||
if (resolveType === 'error') { | ||
setError(actionResult) | ||
return | ||
} | ||
|
||
setError(!actionResult) | ||
} | ||
|
||
return ( | ||
<> | ||
{!loading && error ? <Banner title="Export failed" variant="critical" /> : null} | ||
|
||
<Button loading={loading} leadingVisual={DownloadIcon} onClick={onClick('error')}> | ||
Export (error) | ||
</Button> | ||
</> | ||
) | ||
} |
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.
In these examples, when the loading state is activated, the focus gets lost and removed from the button. I believe this is a 2.4.3 issue, and may result in some screen reader users ending up at the top of the page. Could we avoid focus loss?
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.
Good catch. I'll take a look.