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

Button improvements #23638

Merged
merged 3 commits into from Jul 11, 2018
Merged

Button improvements #23638

merged 3 commits into from Jul 11, 2018

Conversation

islemaster
Copy link
Contributor

While working on #23625 I noticed some things I don't like about our Button component, so I'm changing them. I've also updated the storybook entry for Buttons to be more browsable.

This will cause eyes changes.

Before
before
(Disabled)
before_disabled

  1. The button colors that use white lettering on bold colors can be difficult to read; the lettering is almost too fine.
  2. The "disabled" styles for some button colors are surprising, and for others (especially red) indicate no difference at all.
  3. A button with the disabled prop and an href prop is still a clickable link.

After
after
(Disabled)
after_disabled

  1. Buttons that use white lettering also use font-weight: bold for readability.
  2. All button colors have disabled styles.
  3. A button with the disabled prop and an href prop does nothing when clicked.

Copy link
Contributor

@Erin007 Erin007 left a comment

Choose a reason for hiding this comment

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

Yeah! These are so much more readable!

@islemaster islemaster requested review from balderdash and removed request for joshlory July 11, 2018 20:01
@islemaster
Copy link
Contributor Author

-Josh, +Ram (DOTD) since this will cause eyes changes on the next test run after merge.

@islemaster islemaster merged commit 97ab0d7 into staging Jul 11, 2018
@islemaster islemaster deleted the button-improvements branch July 11, 2018 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants