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

fix(buttons): disabled IconButton background color #1009

Merged
merged 3 commits into from
Feb 3, 2021

Conversation

jzempel
Copy link
Member

@jzempel jzempel commented Feb 3, 2021

Description

This PR is comprised of a bit of design rationale combined with dev rationale.

Design rationale

  • Disabled icon buttons should reflect enabled icon buttons.
  • Icon buttons don’t have a background when enabled, and they should not suddenly get a background when disabled. It would throw off our language around icon buttons if they suddenly got a heavy background.
  • Garden icon buttons in split buttons do get a background, so they should still have a background when disabled.

Dev rationale

Since added state combinations are available on the code side, the design rationale is enhanced with:

  • if the button isPrimary, we’ll retain the grey-200 disabled background
  • if the button participates within any grouping (GroupButton or SplitButton), we’ll retain the grey-200 disabled background

Detail

Supports styling seen with zendeskgarden/ckeditor#5.

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 🌐 demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • analyzed via axe and evaluated using VoiceOver
  • 💂‍♂️ includes new unit tests
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

@jzempel jzempel requested a review from a team as a code owner February 3, 2021 01:17
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 95.759% when pulling 5ff8238 on jzempel/disabled-icon-buttons into e5cd252 on main.

@zendesk-garden zendesk-garden temporarily deployed to staging February 3, 2021 01:49 Inactive
@jzempel jzempel requested a review from nyckie February 3, 2021 21:37
@jzempel jzempel merged commit d103cb1 into main Feb 3, 2021
@jzempel jzempel deleted the jzempel/disabled-icon-buttons branch February 3, 2021 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

6 participants