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): improve stroke/fill icon swap for ToggleIconButton #893

Merged
merged 3 commits into from
Oct 7, 2020

Conversation

jzempel
Copy link
Member

@jzempel jzempel commented Oct 6, 2020

Description

  • We may need to ease the opacity transition between stroke/fill buttons on toggle.
  • Removes invalid theme attribute from the cloned icon.

Detail

@ginnywood please have a look at the "Toggle Button" example in the PR deployment. Note the affect of the various knobs (i.e. "Primary") on the transition animation.

Will impact or be impacted by #889.

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 🌐 Styleguidist 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 ginnywood October 6, 2020 21:30
@jzempel jzempel requested a review from a team as a code owner October 6, 2020 21:30
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 95.844% when pulling c69c739 on jzempel/toggle-icon into aefdef3 on main.

@zendesk-garden zendesk-garden temporarily deployed to staging October 6, 2020 22:09 Inactive
Copy link

@ginnywood ginnywood left a comment

Choose a reason for hiding this comment

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

Looks awesome.

@jzempel jzempel merged commit 4c2407e into main Oct 7, 2020
@jzempel jzempel deleted the jzempel/toggle-icon branch October 7, 2020 00:13
@austingreendev
Copy link
Contributor

@hzhu does this new example need to be included with the storybook PR for react-buttons?

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.

6 participants