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

Change the order of the pseudo-states in the pseudo selectors array #45559

Merged
merged 2 commits into from
Nov 7, 2022

Conversation

mikachan
Copy link
Member

@mikachan mikachan commented Nov 4, 2022

What?

When the :visited state is applied in theme.json, it should have the least specificity of all the pseudo-states to allow the other interaction-based states to take precedence when appropriate (on hover, on focus, etc.)

Why?

There was a bug noticed in Twenty Twenty-Three when both the visited and hover states are active on a button, the visited text color overrides the hover text color, which means the two colors are not contrasting in the default style variation.

image

How?

This PR changes the order of the pseudo-states in the VALID_ELEMENT_PSEUDO_SELECTORS array, so that :visited is at the beginning. This means it receives the lowest specificity in the generated CSS.

Props to @sabernhardt for originally proposing this fix!

Partially addresses #34448 (in order for this PR to fix this issue, you need to specify a :visited state in your theme.json)

Testing Instructions

  1. Activate Twenty Twenty-Three
  2. Insert a button with a link to a site you have already visited
  3. Check that the button receives the correct text color specified in theme.json for each pseudo-state

Screenshots or screencast

Hovering over a button with a visited link:

Before After
image image

Co-Authored-By: Stephen A. Bernhardt <17100257+sabernhardt@users.noreply.github.com>
@mikachan mikachan added [Type] Bug An existing feature does not function as intended [Block] Buttons Affects the Buttons Block labels Nov 4, 2022
@mikachan mikachan self-assigned this Nov 4, 2022
@codesandbox
Copy link

codesandbox bot commented Nov 4, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

Copy link
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

Good spot.

What do you think about adding a comment here to explain why this order needs to be maintained?

@mikachan
Copy link
Member Author

mikachan commented Nov 6, 2022

Thanks, @getdave! I've added a comment.

I also did some quick reading around this, to confirm this is a good order for these selectors.

@Mamaduka Mamaduka added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Nov 7, 2022
@Mamaduka Mamaduka merged commit 41d5f4c into trunk Nov 7, 2022
@Mamaduka Mamaduka deleted the update/pseudo-state-selector-order branch November 7, 2022 17:19
@github-actions github-actions bot added this to the Gutenberg 14.6 milestone Nov 7, 2022
@Mamaduka
Copy link
Member

Mamaduka commented Nov 8, 2022

The core backport PR is ready: WordPress/wordpress-develop#3582

Mamaduka pushed a commit that referenced this pull request Nov 10, 2022
…45559)

* Move visited to start of array

Co-Authored-By: Stephen A. Bernhardt <17100257+sabernhardt@users.noreply.github.com>

* Add comment

Co-authored-by: Stephen A. Bernhardt <17100257+sabernhardt@users.noreply.github.com>
Mamaduka pushed a commit that referenced this pull request Nov 10, 2022
…45559)

* Move visited to start of array

Co-Authored-By: Stephen A. Bernhardt <17100257+sabernhardt@users.noreply.github.com>

* Add comment

Co-authored-by: Stephen A. Bernhardt <17100257+sabernhardt@users.noreply.github.com>
@anastas10s-afk
Copy link

Affecting WooCommerce stores as well.

Screen recordings with TT3 version 1.0 and WooCommerce version 7.1.0 below:

Screen.Recording.2022-12-05.at.20.25.23.mov
Screen.Recording.2022-12-05.at.20.27.52.mov

I hope that helps!

@mikachan
Copy link
Member Author

mikachan commented Dec 6, 2022

Hey @anastas10s-afk 👋 This PR has been merged and should now be fixed in WordPress 6.1.1. Are you still seeing this issue?

@anastas10s-afk
Copy link

anastas10s-afk commented Dec 11, 2022

Hey @mikachan 👋 Yes, I'm afraid so. Testing with WordPress 6.1.1 and TT3 1.0, I get the same behavior as showcased in my previous message here.

As this one is merged already, should I open a new issue about it still happening when WooCommerce is active?

@sabernhardt
Copy link
Contributor

The "View Cart" (or "Continue Shopping") link text color is a separate bug in WooCommerce:
woocommerce/woocommerce#35418

@Mamaduka Mamaduka removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

5 participants