Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
84 commits
Select commit Hold shift + click to select a range
a1c03e2
draft loading state
langermank Aug 1, 2023
cfd0944
cleanup
langermank Aug 1, 2023
9408220
add story to trigger loading
langermank Aug 1, 2023
18d7954
Merge branch 'main' into button-loading-state
langermank Sep 21, 2023
1cc0465
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Sep 22, 2023
a9a7771
merge
langermank Sep 28, 2023
5c901d4
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Oct 4, 2023
fae28dd
Merge branch 'main' into button-loading-state
langermank Oct 11, 2023
72a0247
icon button
langermank Oct 11, 2023
71b747d
Create lazy-jobs-pump.md
langermank Oct 11, 2023
556aff9
add prop for loading message
langermank Oct 11, 2023
03ce69f
Merge branch 'button-loading-state' of https://github.com/primer/reac…
langermank Oct 11, 2023
f7b316d
Merge branch 'button-loading-state' of github.com:primer/react into b…
mperrotti Oct 12, 2023
6f98511
handle no visuals loading state
langermank Oct 17, 2023
5080989
Merge branch 'button-loading-state' of github.com:primer/react into b…
mperrotti Nov 28, 2023
f3e6d07
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Nov 28, 2023
ee21a63
updates snapshots after merging from main
mperrotti Nov 28, 2023
5956393
Merge branch 'main' into button-loading-state
mperrotti Nov 28, 2023
9687486
uses unique ID for loading messages, preserves aria-describedby passe…
mperrotti Nov 28, 2023
2fc7c4d
adds Storybook examples for btn loading error message
mperrotti Nov 28, 2023
7f981b0
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Nov 28, 2023
49d8853
reverts unintentional default link underlining
mperrotti Nov 28, 2023
3aa7225
changes loadingMessage to loadingAnnouncement
mperrotti Nov 28, 2023
7f7f326
updates draft Button component with loading prop
mperrotti Nov 28, 2023
73b59c7
updates legacy button counter behavior when loading
mperrotti Nov 29, 2023
fc15b72
Merge branch 'main' into button-loading-state
mperrotti Nov 29, 2023
c277957
Revert "updates draft Button component with loading prop"
mperrotti Nov 29, 2023
d556bc9
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Dec 1, 2023
ccaa4ae
moves error behavior stories to 'Examples' section
mperrotti Dec 1, 2023
30fb01d
screenreader fixes
mperrotti Dec 1, 2023
22c7c65
adds and updates unit tests
mperrotti Dec 1, 2023
787bfaa
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Dec 1, 2023
6dcf5b3
re-updates snapshots after using correct VisuallyHidden
mperrotti Dec 1, 2023
09a3199
Merge branch 'main' into button-loading-state
mperrotti Dec 5, 2023
0e276de
documents loading props
mperrotti Dec 5, 2023
e8fcd21
adds VRTs, updates loading feature stories
mperrotti Dec 5, 2023
3eee19a
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Dec 5, 2023
e63e61c
simplifies inner visual/spinner rendering logic
mperrotti Dec 5, 2023
8a901da
removes example stories (we can put them back when Flash supports foc…
mperrotti Dec 5, 2023
39f2d4e
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Dec 5, 2023
24abbcd
Merge branch 'main' into button-loading-state
mperrotti Dec 7, 2023
ffdff03
excludes loading buttons from axe contrast check
mperrotti Dec 7, 2023
9d133a0
fixes visual regression: button counter vertical alignment
mperrotti Dec 7, 2023
e069e04
Merge branch 'main' into button-loading-state
mperrotti Dec 11, 2023
611eea0
prevents double spinners when leading and trailing visuals are passed
mperrotti Dec 11, 2023
7cf14ca
test(e2e): update story ids
joshblack Dec 13, 2023
1a4a08d
test(vrt): update snapshots
joshblack Dec 13, 2023
a091125
test(e2e): disable animations in screenshots
joshblack Dec 13, 2023
e4a6f41
Merge branch 'button-loading-state' of github.com:primer/react into b…
joshblack Dec 13, 2023
c9a0743
test(vrt): update snapshots
joshblack Dec 13, 2023
4d9128e
preserves rest state styles when button is loading
mperrotti Dec 14, 2023
3c38084
adds story for success and error announcement
mperrotti Dec 14, 2023
9408882
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Dec 14, 2023
ceb5ba2
test(vrt): update snapshots
langermank Dec 14, 2023
c7501e2
Merge branch 'main' into button-loading-state
mperrotti Dec 18, 2023
ad02496
Merge branch 'main' into button-loading-state
mperrotti Dec 18, 2023
db094c5
fixes ButtonGroup regression
mperrotti Dec 18, 2023
fe2dcee
Merge branch 'main' into button-loading-state
mperrotti Dec 19, 2023
e300738
Merge branch 'main' into button-loading-state
mperrotti Dec 19, 2023
fb6d639
Merge branch 'main' into button-loading-state
mperrotti Dec 19, 2023
0911586
delete broken snapshots
langermank Dec 19, 2023
1416fcb
also targets anchor tags in ButtonGroup styles
mperrotti Dec 19, 2023
e245fe9
Merge branch 'main' into button-loading-state
mperrotti Dec 19, 2023
5e8877e
add conditional wrapper
langermank Dec 19, 2023
f2ccc8e
Merge branch 'button-loading-state' of https://github.com/primer/reac…
langermank Dec 19, 2023
f301ffe
fix group
langermank Dec 19, 2023
e7a25ef
test(vrt): update snapshots
langermank Dec 19, 2023
71de776
Merge branch 'main' into button-loading-state
langermank Dec 19, 2023
2e5ed47
lint
mperrotti Dec 20, 2023
6c425f9
Merge branch 'button-loading-state' of github.com:primer/react into b…
mperrotti Dec 20, 2023
01e07a1
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Dec 20, 2023
20d81f4
fixes unit tests, updates snapshots
mperrotti Dec 20, 2023
b3cdce6
Update src/Button/Button.docs.json
mperrotti Dec 21, 2023
c38523c
Merge branch 'main' into button-loading-state
mperrotti Dec 21, 2023
9ad96f2
fixes 'block' layout for loading buttons
mperrotti Dec 21, 2023
7e5bc66
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Mar 12, 2024
f781434
uses new internal Status component for loading announcement
mperrotti Mar 12, 2024
6f9f6ed
updates Tooltip V2 tests to account for loading messageID in button's…
mperrotti Mar 12, 2024
8809ba5
fixes BoxProps type import to new preferred syntax
mperrotti Mar 12, 2024
9649e4e
appease the linter
mperrotti Mar 12, 2024
87fa84b
rms ConditionalWrapper
mperrotti Mar 12, 2024
5acdcb9
revert back to using ConditionalWrapper with aria-describedby
mperrotti Mar 13, 2024
c0e2a3d
Merge branch 'main' of github.com:primer/react into button-loading-state
mperrotti Mar 14, 2024
a7b114b
test(vrt): update snapshots
mperrotti Mar 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
icon button
  • Loading branch information
langermank committed Oct 11, 2023
commit 72a02473572c3e4933c43fc9b23150d89bd92947
3 changes: 3 additions & 0 deletions src/Button/Button.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ export const LoadingTrigger = () => {

const handleClick = () => {
setIsLoading(true)
setTimeout(() => {
setIsLoading(false)
}, 3000)
}

return (
Expand Down
11 changes: 9 additions & 2 deletions src/Button/ButtonBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,14 @@ const ButtonBase = forwardRef(
data-size={size === 'small' || size === 'large' ? size : undefined}
data-no-visuals={!LeadingVisual && !TrailingVisual && !TrailingAction ? true : undefined}
aria-disabled={loading ? true : undefined}
aria-describedby={loading ? 'loading-message' : undefined}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be?

Suggested change
aria-describedby={loading ? 'loading-message' : undefined}
aria-describedby={loading ? loadingMessage : undefined}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure aria-describedby is meant to reference the id of the message/string, not actually contain the string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh sorry yes 100% 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a unique loading message ID for each button. We could have a page with multiple buttons in a loading state, and we can't have duplicate IDs.

I've picked up this PR, so I'll take care of this

>
{Icon ? (
<Icon />
loading ? (
<Spinner size="small" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the spinner size always small or should it depend on the size of the button? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it looks good in the large button as well. I had the same thought 😆 I'll check again

Copy link
Member

@siddharthkp siddharthkp Oct 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bonus of not changing the size: it always matches the size of leadingVisual for this satisfying interaction:

CleanShot.2023-10-11.at.14.55.13.mp4

) : (
<Icon />
)
) : (
<>
<Box as="span" data-component="buttonContent" sx={getAlignContentSize(alignContent)}>
Expand Down Expand Up @@ -108,7 +113,9 @@ const ButtonBase = forwardRef(
</StyledButton>
{loading && (
<VisuallyHidden>
<span aria-live="polite">Loading</span>
<span aria-live="polite" aria-busy="true" id="loading-message">
Loading
</span>
</VisuallyHidden>
)}
</>
Expand Down
19 changes: 17 additions & 2 deletions src/Button/IconButton.features.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {HeartIcon} from '@primer/octicons-react'
import React from 'react'
import {HeartIcon, DownloadIcon} from '@primer/octicons-react'
import React, {useState} from 'react'
import {IconButton} from '.'

export default {
Expand All @@ -19,3 +19,18 @@ export const Small = () => <IconButton size="small" icon={HeartIcon} aria-label=
export const Medium = () => <IconButton size="medium" icon={HeartIcon} aria-label="Default" />

export const Large = () => <IconButton size="large" icon={HeartIcon} aria-label="Default" />

export const Loading = () => <IconButton loading icon={HeartIcon} variant="primary" aria-label="Primary" />

export const LoadingTrigger = () => {
const [isLoading, setIsLoading] = useState(false)

const handleClick = () => {
setIsLoading(true)
setTimeout(() => {
setIsLoading(false)
}, 3000)
}

return <IconButton loading={isLoading} onClick={handleClick} icon={DownloadIcon} aria-label="Download" />
}