Skip to content

Fix: Text colour on focused primary buttons#6714

Closed
nfmohit wants to merge 1 commit intoWordPress:masterfrom
nfmohit:update/component-button/6668
Closed

Fix: Text colour on focused primary buttons#6714
nfmohit wants to merge 1 commit intoWordPress:masterfrom
nfmohit:update/component-button/6668

Conversation

@nfmohit
Copy link
Copy Markdown
Member

@nfmohit nfmohit commented May 11, 2018 •
edited
Loading

Description

This PR addresses #6668 and fixes the wrong colour on not just the focused "Publish" button but also all other buttons with the .button-primary class on focus.

How has this been tested?

This PR has been tested by making sure the text colour of the "Publish" button is white on focus. This was tested in WP 4.9.5, Gutenberg 2.8.0, Apache server with PHP 7.2.0 and MySQL 5.6.34. According to initial tests, the code doesn’t seem to affect any other areas.

Screenshots

gutenberg-6668

Types of changes

This PR adds a new style which enforces the buttons having the .button-primary class to have the white colour. I'm open to recommendations if a new mixin should be introduced for these buttons or if this can be done in a more optimal way.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@nfmohit nfmohit mentioned this pull request May 14, 2018
Closed
@youknowriad youknowriad added Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended labels May 16, 2018
@youknowriad youknowriad requested a review from a team May 16, 2018 09:08
@jasmussen
Copy link
Copy Markdown
Contributor

jasmussen commented May 16, 2018

Thanks for looking at this.

The rules are becoming a bit complex, I'm going to try and take a stab at this as well, see if there's a nicer way to do it. It feels like there's an opportunity to leverage #6562 to do this.

jasmussen pushed a commit that referenced this pull request May 16, 2018
Fix issues with hover, active, focus states.
e587426 
This fixes #6668, inspired by @nfmohit work in #6714 (props).
@jasmussen
Copy link
Copy Markdown
Contributor

jasmussen commented May 16, 2018

I took inspiration from this PR to push a fix to #6562. Let me know what you think, and we'll give you props for that PR. Thanks again for your contributions!

@nfmohit
Copy link
Copy Markdown
Member Author

nfmohit commented May 16, 2018

Thank you so much for checking this out @jasmussen!

I just checked out and tested #6562 and it seems to be a much better and consistent solution than mine. I believe that is the most important part of open source contributions where beginners like me get to watch and learn from inspiring professionals like you 😊

Thank you very much!

@jasmussen
Copy link
Copy Markdown
Contributor

jasmussen commented May 16, 2018

Awesome! Thanks again for your contribution. Let's get that other branch in, then!

@karmatosed
Copy link
Copy Markdown
Member

karmatosed commented May 19, 2018

Looking at this I think I'm right in closing this as the other branch is the focus. Thanks again @nfmohit-wpmudev for your contribution here.

@karmatosed karmatosed closed this May 19, 2018
youknowriad pushed a commit that referenced this pull request May 21, 2018
@youknowriad
Fix issues with hover, active, focus states.
2465dc4 
This fixes #6668, inspired by @nfmohit work in #6714 (props).
youknowriad added a commit that referenced this pull request May 21, 2018
@youknowriad
Button Component: Update styling to avoid relying on Core Styles maki…
ce61b59 
…ng it reusable (#6562)

* Button Component: Update styling to avoid relying on Core Styles making it reusable

* Fix admin schemes by using theme colors

* Fix issues with hover, active, focus states.

This fixes #6668, inspired by @nfmohit work in #6714 (props).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nfmohit @jasmussen @karmatosed @youknowriad