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

Adds 'inactive' state to buttons #3812

Merged
merged 46 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
a45f5d5
adds inactive state to button components
mperrotti Oct 12, 2023
557b012
update snapshots
mperrotti Oct 12, 2023
f3f0f40
adds changeset
mperrotti Oct 12, 2023
749e57a
Merge branch 'main' into mp/inactive-button
mperrotti Oct 12, 2023
16f34a0
test(vrt): update snapshots
mperrotti Oct 12, 2023
032dc7b
Merge branch 'main' into mp/inactive-button
mperrotti Oct 12, 2023
78d638e
Merge branch 'main' into mp/inactive-button
mperrotti Oct 13, 2023
4f64d94
Merge branch 'main' into mp/inactive-button
mperrotti Oct 13, 2023
38e9cae
Update src/Button/ButtonBase.tsx
mperrotti Oct 16, 2023
a6395a9
Merge branch 'main' of github.com:primer/react into mp/inactive-button
mperrotti Nov 9, 2023
e9f368d
follow-up on resolving merge conflicts
mperrotti Nov 9, 2023
7ee2525
adds new button style for inactive buttons
mperrotti Nov 9, 2023
fd935c0
refactors inactive styles, lets disabled style override inactive style
mperrotti Nov 9, 2023
5814f73
adds docs for 'inactive' prop
mperrotti Nov 9, 2023
d17ff65
Merge branch 'main' into mp/inactive-button
mperrotti Nov 9, 2023
e76533d
Merge branch 'main' into mp/inactive-button
mperrotti Nov 13, 2023
2d40c6a
Merge branch 'main' into mp/inactive-button
mperrotti Nov 14, 2023
466b029
upgrades styled components, fixes hover and active style bugs on inac…
mperrotti Nov 14, 2023
e51226a
Merge branch 'mp/inactive-button' of github.com:primer/react into mp/…
mperrotti Nov 14, 2023
83c424b
Merge branch 'main' of github.com:primer/react into mp/inactive-button
mperrotti Nov 14, 2023
2f515f9
fix drafts/Button2 inactive implementation
mperrotti Nov 14, 2023
48d08d7
cursor style tweak, update snapshots, update package-lock.json
mperrotti Nov 14, 2023
36b2c53
test(vrt): update snapshots
mperrotti Nov 14, 2023
7820947
Merge branch 'main' of github.com:primer/react into mp/inactive-button
mperrotti Nov 16, 2023
1f04313
upgrades @primer/primitives
mperrotti Nov 16, 2023
ab5cc5d
Merge branch 'mp/inactive-button' of github.com:primer/react into mp/…
mperrotti Nov 16, 2023
63928f5
Merge branch 'main' into mp/inactive-button
mperrotti Nov 20, 2023
772d196
Merge branch 'main' into mp/inactive-button
mperrotti Nov 21, 2023
c019d0e
test next primitives release
mperrotti Nov 21, 2023
d48a3e2
upgrades @primer/primitives to 7.15.3
mperrotti Nov 21, 2023
bfa9133
updates jest snaps
mperrotti Nov 21, 2023
7c0acc2
test(vrt): update snapshots
mperrotti Nov 21, 2023
80a85da
Merge branch 'main' into mp/inactive-button
mperrotti Nov 22, 2023
5c9ab5a
Merge branch 'main' into mp/inactive-button
ericwbailey Nov 27, 2023
2758520
Merge branch 'main' of github.com:primer/react into mp/inactive-button
mperrotti Nov 27, 2023
5e4cba9
Merge branch 'mp/inactive-button' of github.com:primer/react into mp/…
mperrotti Nov 27, 2023
5650687
Merge branch 'main' into mp/inactive-button
mperrotti Nov 28, 2023
d4117fa
Merge branch 'main' into mp/inactive-button
mperrotti Nov 29, 2023
4865b69
rm disabled styles
mperrotti Nov 30, 2023
d2aeaee
Merge branch 'mp/inactive-button' of github.com:primer/react into mp/…
mperrotti Nov 30, 2023
8d3cd3a
Merge branch 'main' of github.com:primer/react into mp/inactive-button
mperrotti Nov 30, 2023
fc9d713
preserves border on inactive buttons
mperrotti Nov 30, 2023
737b6ca
test(vrt): update snapshots
mperrotti Dec 1, 2023
05e8b53
Merge branch 'main' of github.com:primer/react into mp/inactive-button
mperrotti Dec 5, 2023
2196c4a
Merge branch 'mp/inactive-button' of github.com:primer/react into mp/…
mperrotti Dec 5, 2023
cdfaebe
Merge branch 'main' into mp/inactive-button
mperrotti Dec 5, 2023
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
7 changes: 7 additions & 0 deletions .changeset/soft-pans-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': minor
---

Adds an 'inactive' state to buttons. An inactive button looks disabled and has aria-disabled, but it can still be clicked and focused. This was added to support buttons that are broken due to availability issues, but can't be removed from the page.

<!-- Changed components: Button, Button2, 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.
34 changes: 34 additions & 0 deletions e2e/components/Button.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,40 @@ test.describe('Button', () => {
}
})

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

// Default state
expect(await page.screenshot()).toMatchSnapshot(`Button.Inactive.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-button-features--inactive',
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
5 changes: 5 additions & 0 deletions src/Button/Button.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@
"type": "React.ElementType",
"description": "A visual to display after the button text."
},
{
"name": "inactive",
"type": "boolean",
"description": "Whether the button looks visually disabled, but can still accept all the same interactions as an enabled button."
},
{
"name": "as",
"type": "React.ElementType",
Expand Down
6 changes: 6 additions & 0 deletions src/Button/Button.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ export const Block = () => <Button block>Default</Button>

export const Disabled = () => <Button disabled>Default</Button>

export const Inactive = () => (
<Button variant="primary" inactive>
Default
</Button>
)

export const Small = () => <Button size="small">Default</Button>

export const Medium = () => <Button size="medium">Default</Button>
Expand Down
6 changes: 6 additions & 0 deletions src/Button/Button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ export default {
type: 'boolean',
},
},
inactive: {
control: {
type: 'boolean',
},
},
variant: {
control: {
type: 'radio',
Expand Down Expand Up @@ -48,6 +53,7 @@ export default {
block: false,
size: 'medium',
disabled: false,
inactive: false,
variant: 'default',
alignContent: 'center',
trailingVisual: null,
Expand Down
2 changes: 2 additions & 0 deletions src/Button/ButtonBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const ButtonBase = forwardRef(
size = 'medium',
alignContent = 'center',
block = false,
inactive,
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, what if we just continue to use disabled as a prop but change it to use aria-disabled instead of disabled across the board?

Is this primarily used for the loading state? So it looked disabled but it can be focused.. can it also still be clicked and follow through with the on click action?

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 think there are still rare cases where we want a real disabled button.

The loading state is one way we can use this pattern, but it's mostly to support "degraded" buttons: https://primer.style/design/ui-patterns/degraded-experiences#indicating-a-button-is-non-functional-without-disabling-it

Copy link
Member

Choose a reason for hiding this comment

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

Curious, what if we just continue to use disabled as a prop but change it to use aria-disabled instead of disabled across the board?

I like the general idea of that, but it should come with handling click events with a useful message, which I'm not confident would happen :(

I think there are still rare cases where we want a real disabled button.

Also fair!

...rest
} = props
const LeadingVisual = leadingVisual ?? leadingIcon
Expand Down Expand Up @@ -70,6 +71,7 @@ const ButtonBase = forwardRef(
data-block={block ? 'block' : null}
data-size={size === 'small' || size === 'large' ? size : undefined}
data-no-visuals={!LeadingVisual && !TrailingVisual && !TrailingAction ? true : undefined}
aria-disabled={inactive}
mperrotti marked this conversation as resolved.
Show resolved Hide resolved
>
{Icon ? (
<Icon />
Expand Down
5 changes: 5 additions & 0 deletions src/Button/IconButton.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
"defaultValue": "",
"description": "Changes the size of the icon button component"
},
{
"name": "inactive",
"type": "boolean",
"description": "Whether the button looks visually disabled, but can still accept all the same interactions as an enabled button."
},
{
"name": "icon",
"type": "Component",
Expand Down
5 changes: 5 additions & 0 deletions src/Button/IconButton.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ const meta: Meta<ComponentProps<typeof IconButton>> = {
type: 'boolean',
},
},
inactive: {
control: {
type: 'boolean',
},
},
variant: {
control: {
type: 'radio',
Expand Down
Loading
Loading