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(button): use currentColor as icon fill for all states #5270

Merged
merged 4 commits into from
Feb 6, 2020

Conversation

janhassel
Copy link
Member

I've noticed that in the button styles the icon path fill is initially set to currentColor but overwritten in most states (disabled, hover...) event hough the color value changes the same.

If this is intentional, I'd be interested in the reasoning behind it. Otherwise, this PR applies the same currentColor logic to the states.

Changelog

Changed

  • removed explicit icon fill color in button states which were identical to color value

Testing / Reviewing

  1. Run react storybook
  2. Apply different button states

@janhassel janhassel requested a review from a team as a code owner February 5, 2020 09:01
@ghost ghost requested review from asudoh and emyarod February 5, 2020 09:01
@netlify
Copy link

netlify bot commented Feb 5, 2020

Deploy preview for carbon-elements ready!

Built with commit 96beda4

https://deploy-preview-5270--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Feb 5, 2020

Deploy preview for carbon-components-react failed.

Built with commit 96beda4

https://app.netlify.com/sites/carbon-components-react/deploys/5e3c97092e9d4000096da03c

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @janhassel!

@asudoh asudoh requested review from a team and designertyler and removed request for a team February 6, 2020 00:00
Copy link
Member

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ✅

@asudoh asudoh merged commit 0f9aef2 into carbon-design-system:master Feb 6, 2020
@janhassel janhassel deleted the button-icon-color branch April 2, 2020 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants