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.

Sign up for GitHub

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

Button: Update disabled state to be without background. #50496

Merged
merged 4 commits into from
May 15, 2023
Merged

Button: Update disabled state to be without background. #50496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
92e8b2b
Button: Update disabled state to be without background.
jasmussen May 10, 2023
7861590
Update changelog.
jasmussen May 10, 2023
c171377
Merge changelog section
mirka May 12, 2023
63d9394
Remove outdated TODO comment
mirka May 12, 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
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Bug Fix

- `NavigableContainer`: do not trap focus in `TabbableContainer` ([#49846](https://github.com/WordPress/gutenberg/pull/49846)).
- Update `<Button>` component to have a transparent background for its tertiary disabled state, to match its enabled state. ([#50496](https://github.com/WordPress/gutenberg/pull/50496)).

### Internal

Expand Down
17 changes: 5 additions & 12 deletions packages/components/src/button/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,14 @@
outline: 1px solid transparent;

&:active:not(:disabled) {
background: $components-color-gray-300;
color: $components-color-accent-darker-10;
Copy link
Member

Choose a reason for hiding this comment

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

Not that I object but just checking since it wasn't mentioned in the PR description — is it intentional for the text to turn black when clicked?

CleanShot.2023-05-13.at.00.20.06.mp4

box-shadow: none;
}

&:hover:not(:disabled) {
color: $components-color-accent-darker-10;
}

&:disabled,
&[aria-disabled="true"],
&[aria-disabled="true"]:hover {
// TODO: Prepare for theming (https://github.com/WordPress/gutenberg/pull/45466/files#r1030872724)
Copy link
Contributor

@ciampo ciampo May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think that this comment still applies, right? We should still think about adding variables at the theme level for disabled UI

Copy link
Member

Choose a reason for hiding this comment

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

The TODO comments were specifically for the issue of darken/lighten Sass functions being used on theme variables.

As for the theme-ready gray variables, we simply haven't put them to use anywhere yet. So I was kind of thinking of that as a big separate task rather than a contextual TODO item 😄

color: lighten($gray-700, 5%);
background: lighten($gray-300, 5%);
color: $gray-600;
background: transparent;
transform: none;
opacity: 1;
box-shadow: none;
Expand Down Expand Up @@ -165,12 +158,12 @@
color: $components-color-accent;
background: transparent;

&:hover:not(:disabled) {
&:hover:not(:disabled, [aria-disabled="true"]) {
// TODO: Prepare for theming (https://github.com/WordPress/gutenberg/pull/45466/files#r1030872724)
background: rgba(var(--wp-admin-theme-color--rgb), 0.04);
}

&:active:not(:disabled) {
&:active:not(:disabled, [aria-disabled="true"]) {
// TODO: Prepare for theming (https://github.com/WordPress/gutenberg/pull/45466/files#r1030872724)
background: rgba(var(--wp-admin-theme-color--rgb), 0.08);
}
Expand Down Expand Up @@ -239,7 +232,7 @@
}
}

&:not([aria-disabled="true"]):active {
&:not(:disabled, [aria-disabled="true"]):active {
color: $components-color-foreground;
}

Expand Down