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

Color palette: make the Clear link consistent with buttons in other settings #8875

Merged
merged 2 commits into from Aug 23, 2018

Conversation

Projects
None yet
5 participants
@eliorivero
Contributor

eliorivero commented Aug 11, 2018

This PR updates the Clear link in the color palette to make it consistent with similar actionable elements found in other panels like the Reset buttons in Text Settings:

captura de pantalla 2018-08-11 a la s 13 38 17

or the Image Settings panel:

captura de pantalla 2018-08-11 a la s 13 38 10

Description

This PR converts the link into a small default button.

How has this been tested?

Tested manually

Screenshots

Before

captura de pantalla 2018-08-11 a la s 13 40 20

After

captura de pantalla 2018-08-11 a la s 13 40 04

Types of changes

Several attributes are set for the Button component so it's rendered as a small button.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
Color palette: solve inconsistency between the Clear link and buttons…
… in other panels like the Reset buttons in Text or Image settings

@eliorivero eliorivero changed the title from Color palette: solve inconsistency between the Clear link and buttons in other settings to Color palette: mae the Clear link and buttons in other settings Aug 11, 2018

@eliorivero eliorivero changed the title from Color palette: mae the Clear link and buttons in other settings to Color palette: make the Clear link consistent with buttons in other settings Aug 11, 2018

@karmatosed karmatosed self-requested a review Aug 13, 2018

@karmatosed

I don't mind this although I do still feel it could be a text link. With not minding it in mind let's add and see if people benefit from it.

@eliorivero

This comment has been minimized.

Show comment
Hide comment
@eliorivero

eliorivero Aug 17, 2018

Contributor

Thanks for the approval Tammie. I've updated the snapshot to reflect the changes introduced and now tests pass.

Contributor

eliorivero commented Aug 17, 2018

Thanks for the approval Tammie. I've updated the snapshot to reflect the changes introduced and now tests pass.

@tofumatt

Yaaaay this has always bothered me so much, thanks for making it a button ❤️

@tofumatt tofumatt merged commit 24b935f into WordPress:master Aug 23, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tofumatt tofumatt added this to the 3.7 milestone Aug 23, 2018

@eliorivero

This comment has been minimized.

Show comment
Hide comment
@eliorivero

eliorivero Aug 24, 2018

Contributor

Thanks to you @tofumatt for reviewing it!

Contributor

eliorivero commented Aug 24, 2018

Thanks to you @tofumatt for reviewing it!

@eliorivero eliorivero deleted the eliorivero:update/color-palette-buttons branch Aug 24, 2018

@mtias

This comment has been minimized.

Show comment
Hide comment
@mtias

mtias Aug 30, 2018

Contributor

Nice, thanks for the update!

Contributor

mtias commented Aug 30, 2018

Nice, thanks for the update!

@Soean Soean referenced this pull request Sep 11, 2018

Merged

Issue/9785 - Remove isButton prop #9786

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment