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: Update hover styles to account for pressed state for tertiary button #68542

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

yogeshbhutkar
Copy link
Contributor

@yogeshbhutkar yogeshbhutkar commented Jan 8, 2025

What, Why and How?

As noted in the comment, the is-pressed CSS rule was overridden on hover, resulting in a dark blue accent that reduced readability.

color: $components-color-accent-darker-20;

This PR introduces an additional :not condition to ensure the color property is applied only when the is-pressed state is not active.

Testing Instructions

  1. Start storybook using npm run storybook:dev command.
  2. Navigate to Button component.
  3. Make sure the variant is tertiary.
  4. Notice, the text color stays white on hover.

Screencasts

Before After
before after

The settings button hover effect

MOV to GIF output
It was found to work as expected.

Closes: #68535

@yogeshbhutkar yogeshbhutkar marked this pull request as ready for review January 8, 2025 04:55
Copy link

github-actions bot commented Jan 8, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: yogeshbhutkar <[email protected]>
Co-authored-by: afercia <[email protected]>
Co-authored-by: patil-vipul <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Components /packages/components labels Jan 8, 2025
@afercia
Copy link
Contributor

afercia commented Jan 8, 2025

I'd suggest to wait for some feedback from @WordPress/gutenberg-design and see what is the desired styling.
Edge case: I think the :active non-hovered state makes the background color turn to a light gray? To test:

Screenshot:

Screenshot 2025-01-08 at 12 20 24

@yogeshbhutkar
Copy link
Contributor Author

yogeshbhutkar commented Jan 8, 2025

Thanks for the review, @afercia. I think I've considered the :active state already and have fixed the bug by explicitly setting the color when the button is active. However, I've noticed a similar issue with the secondary variant of the button.

I'll await design feedback on this one, post which, I should have a clear path forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tertiary button is-pressed style insufficient contrast ratio on hover
2 participants