Skip to content

Commit

Permalink
Un-revert "Add loading prop for Button and IconButton (#3582)" (#…
Browse files Browse the repository at this point in the history
…4485)

* Revert "Revert "Add `loading` prop for `Button` and `IconButton` (#3582)" (#4…"

This reverts commit c01901f.

* only overrides btn label when loading

* update tooltip tests to accomodate loading message aria-describedby

* import missing modules in IconButton stories

* makes aria-labelledby prop remain set when when button is in a loading state

* Update packages/react/src/Button/Button.examples.stories.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* Update packages/react/src/Button/Button.examples.stories.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* updates textinput snapshots

* addresses remaining PR feedback

* appeases the linter, updates snaps

* leaves loading prop undefined so we do not always render the wrapper

* test(vrt): update snapshots

* trying again without changing colors on faux-disabled buttons

* replaces Status with AriaStatus

* replaces one more Status with AriaStatus

* rms added aria-disabled styles

* test(vrt): update snapshots

* Update icon button tests to make it work with the loading state

* add story with leading visual and count

* misc bugfixes:
- ensures counter stays rendered even when no children are passed
- preserves space between elements that are children of span[data-component=text]
- adds story and VRT for buttons with a trailing action but no leading/trailing visuals

* test(vrt): update snapshots

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>
Co-authored-by: mperrotti <mperrotti@users.noreply.github.com>
Co-authored-by: Armagan Ersoz <broccolinisoup@github.com>
  • Loading branch information
5 people authored Jul 25, 2024
1 parent 8574027 commit 991839c
Show file tree
Hide file tree
Showing 91 changed files with 1,063 additions and 304 deletions.
5 changes: 5 additions & 0 deletions .changeset/lazy-jobs-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": minor
---

Add `loading` state to `Button` and `IconButton`
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
178 changes: 178 additions & 0 deletions e2e/components/Button.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,184 @@ test.describe('Button', () => {
}
})

test.describe('Loading', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(`Button.Loading.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})

test.describe('Loading Custom Announcement', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-custom-announcement',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
`Button.Loading Custom Announcement.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-custom-announcement',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})

test.describe('Loading With Leading Visual', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-with-leading-visual',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
`Button.Loading With Leading Visual.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-with-leading-visual',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})

test.describe('Loading With Trailing Visual', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-with-trailing-visual',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
`Button.Loading With Trailing Visual.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-with-trailing-visual',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})

test.describe('Loading With Trailing Action', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-with-trailing-action',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
`Button.Loading With Trailing Action.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-button-features--loading-with-trailing-action',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})

test.describe('Dev: Invisible Variants', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions packages/react/src/Button/Button.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@
"type": "number | string",
"description": "For counter buttons, the number to display."
},
{
"name": "disabled",
"type": "boolean",
"description": "Items that are disabled can not be clicked, selected, or navigated through."
},
{
"name": "inactive",
"type": "boolean",
Expand All @@ -53,6 +48,17 @@
"type": "React.ElementType",
"description": "A visual to display before the button text."
},
{
"name": "loading",
"type": "boolean",
"description": "When true, the button is in a loading state."
},
{
"name": "loadingAnnouncement",
"type": "string",
"description": "The content to announce to screen readers when loading. This requires `loading` prop to be true"
},

{
"name": "ref",
"type": "React.RefObject<HTMLButtonElement>"
Expand Down
79 changes: 79 additions & 0 deletions packages/react/src/Button/Button.examples.stories.tsx
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 = () => {
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>
</>
)
}
42 changes: 41 additions & 1 deletion packages/react/src/Button/Button.features.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {EyeIcon, TriangleDownIcon, HeartIcon, CommentIcon} from '@primer/octicons-react'
import {EyeIcon, TriangleDownIcon, HeartIcon, DownloadIcon, CommentIcon} from '@primer/octicons-react'
import React, {useState} from 'react'
import {Button} from '.'
import {Stack} from '../Stack/Stack'
Expand Down Expand Up @@ -141,6 +141,46 @@ export const Medium = () => <Button size="medium">Default</Button>

export const Large = () => <Button size="large">Default</Button>

export const Loading = () => <Button loading>Default</Button>

export const LoadingCustomAnnouncement = () => (
<Button loading loadingAnnouncement="This is a custom loading announcement">
Default
</Button>
)

export const LoadingWithLeadingVisual = () => (
<Button loading leadingVisual={DownloadIcon}>
Export
</Button>
)

export const LoadingWithTrailingVisual = () => (
<Button loading trailingVisual={DownloadIcon}>
Export
</Button>
)

export const LoadingWithTrailingAction = () => (
<Button loading trailingAction={TriangleDownIcon}>
Export dropdown
</Button>
)

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

const handleClick = () => {
setIsLoading(true)
}

return (
<Button loading={isLoading} onClick={handleClick} leadingVisual={DownloadIcon}>
Export
</Button>
)
}

export const LabelWrap = () => {
return (
<Stack style={{width: '200px'}}>
Expand Down
11 changes: 11 additions & 0 deletions packages/react/src/Button/Button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,21 @@ Playground.argTypes = {
type: 'boolean',
},
},
loading: {
control: {
type: 'boolean',
},
},
labelWrap: {
control: {
type: 'boolean',
},
},
count: {
control: {
type: 'number',
},
},
leadingVisual: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
trailingVisual: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
trailingAction: OcticonArgType([TriangleDownIcon]),
Expand All @@ -64,6 +74,7 @@ Playground.args = {
inactive: false,
variant: 'default',
alignContent: 'center',
loading: false,
trailingVisual: null,
leadingVisual: null,
trailingAction: null,
Expand Down
Loading

0 comments on commit 991839c

Please sign in to comment.