Navigation block: Allow themes to override block library text-decoration rule #63406
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
talldan
added
[Type] Bug
|
Size Change: +1.6 kB (+0.09%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
talldan
changed the title
ellatrix
reviewed
Jul 11, 2024
| &:where(:not([class*="has-text-decoration"])) { | ||
| a { | ||
| text-decoration: none; | ||
| &:where(:not([class*="has-text-decoration"])) :where(a) { |
There was a problem hiding this comment.
Could these be combined into a single where?
There was a problem hiding this comment.
Another question: do we still need the :not([class*="has-text-decoration"]) if the specificity is lowered? Ideally if it has that class, it should just override this rule and the exception should no longer be needed.
There was a problem hiding this comment.
Could these be combined into a single where?
I had already tried this:
&:where(:not([class*="has-text-decoration"]) a)But that selector doesn't work. Is there another way?
There was a problem hiding this comment.
Another question: do we still need the :not([class*="has-text-decoration"]) if the specificity is lowered? Ideally if it has that class, it should just override this rule and the exception should no longer be needed.
It does seem needed, it overrides a general rule for links that are not buttons which gives them underlines:
That should be a bit more obvious when testing now that I've rebased to include the fix from #63403.
There was a problem hiding this comment.
I've applied Aaron's suggestion now so it looks a little different - #63406 (comment)
There was a problem hiding this comment.
It does seem needed, it overrides a general rule for links that are not buttons which gives them underlines:
Removing the :where(:not([class*=has-text-decoration])) won't change the specificity. So the selector would remain 0-1-0 specificity and continue to override a:where(:not(.wp-element-button)) which is only 0-0-1.
There was a problem hiding this comment.
Oh, I misunderstood what was meant in the original comment. I've gone ahead and removed that. 👍
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Thanks for putting this together @talldan 👍
This tests as advertised for me regarding the :hover styles set on link elements in the nav block like TT4 does.
The :focus and :active pseudo classes still bump the specificity and override anything set for those link pseudo selectors in theme.json.
For the most part it appears that setting the text decoration at the block level (not elements) still works due to the way browsers render text decoration differently.
I primarily tested using both emptytheme and TT4. To replicate the issue with :focus and :active:
- Activate emptytheme
- Add active/focus styles to link elements in the nav block within your theme.json using the snippet below
- Add nav block to a page without customizing its styles
- Load the page on the frontend and confirm the
text-decoration: nonerule still overrides the global styles.
{
"$schema": "../../schemas/json/theme.json",
"version": 3,
"settings": {
"appearanceTools": true,
"layout": {
"contentSize": "840px",
"wideSize": "1100px"
}
},
"styles": {
"blocks": {
"core/navigation": {
"elements": {
"link": {
":focus": {
"typography": {
"textDecoration": "line-through"
}
},
":active": {
"typography": {
"textDecoration": "line-through"
}
}
}
}
}
}
}
}| This PR | With Suggested Tweak |
|---|---|
Screen.Recording.2024-07-12.at.5.18.20.PM.mp4 |
Screen.Recording.2024-07-12.at.5.17.41.PM.mp4 |
Comment on lines
82
to
83
| &:focus, | ||
| &:active { |
There was a problem hiding this comment.
These pseudo styles get appended so the resulting specificity is 0-2-0 which is still higher than global styles.
To my knowledge there isn't an easy way in SCSS to insert the inner :focus/:active into a CSS function in the parent selector i.e. :where(). So I think they need to be written out in full.
&:where(a),
&:where(a:focus),
&:where(a:active) {
text-decoration: none;
}There was a problem hiding this comment.
Good point, updated in 0f9669a
talldan
force-pushed
the
fix/navigation-link-hover-style
branch
from
1586d54
to
be5ae55
Compare
There was a problem hiding this comment.
I've retested this and can confirm the :focus and :active styles have been fixed via 0f9669a.
Also, I believe the wrapping &:where(:not([class*="has-text-decoration"])) is not required. I should have been clearer in my review and provided a full diff.
Updated suggestion after `0f9669a`
diff --git a/packages/block-library/src/navigation/style.scss b/packages/block-library/src/navigation/style.scss
index 734e3bd04c..3d7a8810ae 100644
--- a/packages/block-library/src/navigation/style.scss
+++ b/packages/block-library/src/navigation/style.scss
@@ -76,12 +76,10 @@ $navigation-icon-size: 24px;
}
}
- &:where(:not([class*="has-text-decoration"])) {
- & :where(a),
- & :where(a:focus),
- & :where(a:active) {
- text-decoration: none;
- }
+ & :where(a),
+ & :where(a:focus),
+ & :where(a:active) {
+ text-decoration: none;
}
// Submenu indicator.
Using that suggested change, here's what I get using emptytheme and no theme.json block type element styles
Note: Core element styles are correctly overridden by the block library's link styles.
Screen.Recording.2024-07-14.at.4.27.47.PM.mp4
When defining link element styles for the navigation block type in theme.json, this is what I get.
Note: Global Styles for navigation block type link element are applied, overriding the block library styles.
Screen.Recording.2024-07-14.at.4.29.25.PM.mp4
Example theme.json styles
{
"$schema": "../../schemas/json/theme.json",
"version": 3,
"settings": {
"appearanceTools": true,
"layout": {
"contentSize": "840px",
"wideSize": "1100px"
}
},
"styles": {
"blocks": {
"core/navigation": {
"elements": {
"link": {
"typography": {
"textDecoration": "none"
},
":hover": {
"typography": {
"textDecoration": "underline"
}
},
":focus": {
"typography": {
"textDecoration": "line-through"
}
},
":active": {
"typography": {
"textDecoration": "none"
}
}
}
}
}
}
}
}Applying block instance styles for text decoration continue to take precedence over the block library and global styles due to their compound class selector e.g. .wp-block-navigation.has-text-decoration-line-through
aaronrobertshaw
approved these changes
Jul 17, 2024
There was a problem hiding this comment.
Thanks for the continued iteration here 👍
This is testing well for me now.
The default theme.json element styles are overridden by block library styles, which are in turn correctly overridden by block type element global styles. Applying text decoration on the block instance then takes precedence over all the others.
Given the chance this might need to be included in a WP 6.6.1 point release, it would be good to update the PR test instructions with more detail for others coming to this in the near future.
andrewserong
added
the
Backport to Gutenberg RC
|
I've added the Is it worth adding the |
andrewserong
pushed a commit
that referenced
this pull request
Jul 17, 2024
…ion rule (#63406) * Reduce specificity of navigation text decoration rule * Apply Aaron suggestion to also lower specificity for focus/active * Remove `:not([class*="has-text-decoration"])` from selector ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
|
I just cherry-picked this PR to the release/18.8 branch to get it included in the next release: 976df1e |
andrewserong
removed
the
Backport to Gutenberg RC
I think usually it'd be cherry-picked to that branch as part of the I saw there's a slack thread discussing this issue - https://wordpress.slack.com/archives/C02QB2JS7/p1721166077452989, though I'm not sure you need to actually merge into the 6.6 branch to test, you can possibly make a PR that merges into the branch that includes the fix, and a PR usually has a job that builds the plugin zip. |
andrewserong
pushed a commit
that referenced
this pull request
Jul 17, 2024
…ion rule (#63406) * Reduce specificity of navigation text decoration rule * Apply Aaron suggestion to also lower specificity for focus/active * Remove `:not([class*="has-text-decoration"])` from selector ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
|
A separate PR to be on the safe side sounds good, that way folks can grab a zip file from that build if they need to. I've opened up a PR targeting the I figure that at least gives folks working on a point release enough options to help ensure this fix gets included 🙂 |
ellatrix
added
Backport to WP 6.7 Beta/RC
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Jul 18, 2024
…ion rule (#63406) * Reduce specificity of navigation text decoration rule * Apply Aaron suggestion to also lower specificity for focus/active * Remove `:not([class*="has-text-decoration"])` from selector ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.6 branch to get it included in the next release: bca5381 |
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Jul 18, 2024
Bugfixes included: * [WordPress/gutenberg#63637 Elements: Avoid specificity bump for top-level element-only selectors]. * [WordPress/gutenberg#63406 Navigation block: Allow themes to override block library text-decoration rule]. * [WordPress/gutenberg#63436 Fix invalid css for nested fullwidth layouts with zero padding applied]. * [WordPress/gutenberg#63397 Prevent empty void at the bottom of editor when block directory results are present]. * [WordPress/gutenberg#63291 Pattern overrides: Ensure "Reset" button always shows as last item and with border]. * [WordPress/gutenberg#63562 Global Styles: Disable "Reset styles" button when there are no changes]. * [WordPress/gutenberg#63093 Fix: Removed shuffle button when only 1 pattern is present]. * [WordPress/gutenberg#62675 fix: wp icon focus issue]. * [WordPress/gutenberg#63565 useBlockElement: return null until ref callback has time to clean up the old element]. Props ellatrix. Fixes #61692. See #61660, #61630, #61656. git-svn-id: https://develop.svn.wordpress.org/trunk@58757 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Jul 18, 2024
Bugfixes included: * [WordPress/gutenberg#63637 Elements: Avoid specificity bump for top-level element-only selectors]. * [WordPress/gutenberg#63406 Navigation block: Allow themes to override block library text-decoration rule]. * [WordPress/gutenberg#63436 Fix invalid css for nested fullwidth layouts with zero padding applied]. * [WordPress/gutenberg#63397 Prevent empty void at the bottom of editor when block directory results are present]. * [WordPress/gutenberg#63291 Pattern overrides: Ensure "Reset" button always shows as last item and with border]. * [WordPress/gutenberg#63562 Global Styles: Disable "Reset styles" button when there are no changes]. * [WordPress/gutenberg#63093 Fix: Removed shuffle button when only 1 pattern is present]. * [WordPress/gutenberg#62675 fix: wp icon focus issue]. * [WordPress/gutenberg#63565 useBlockElement: return null until ref callback has time to clean up the old element]. Props ellatrix. Fixes #61692. See #61660, #61630, #61656. Built from https://develop.svn.wordpress.org/trunk@58757 git-svn-id: http://core.svn.wordpress.org/trunk@58159 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot
pushed a commit
to gilzow/wordpress-performance
that referenced
this pull request
Jul 18, 2024
Bugfixes included: * [WordPress/gutenberg#63637 Elements: Avoid specificity bump for top-level element-only selectors]. * [WordPress/gutenberg#63406 Navigation block: Allow themes to override block library text-decoration rule]. * [WordPress/gutenberg#63436 Fix invalid css for nested fullwidth layouts with zero padding applied]. * [WordPress/gutenberg#63397 Prevent empty void at the bottom of editor when block directory results are present]. * [WordPress/gutenberg#63291 Pattern overrides: Ensure "Reset" button always shows as last item and with border]. * [WordPress/gutenberg#63562 Global Styles: Disable "Reset styles" button when there are no changes]. * [WordPress/gutenberg#63093 Fix: Removed shuffle button when only 1 pattern is present]. * [WordPress/gutenberg#62675 fix: wp icon focus issue]. * [WordPress/gutenberg#63565 useBlockElement: return null until ref callback has time to clean up the old element]. Props ellatrix. Fixes #61692. See #61660, #61630, #61656. Built from https://develop.svn.wordpress.org/trunk@58757 git-svn-id: https://core.svn.wordpress.org/trunk@58159 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Jul 18, 2024
Bugfixes included: * [WordPress/gutenberg#63637 Elements: Avoid specificity bump for top-level element-only selectors]. * [WordPress/gutenberg#63406 Navigation block: Allow themes to override block library text-decoration rule]. * [WordPress/gutenberg#63436 Fix invalid css for nested fullwidth layouts with zero padding applied]. * [WordPress/gutenberg#63397 Prevent empty void at the bottom of editor when block directory results are present]. * [WordPress/gutenberg#63291 Pattern overrides: Ensure "Reset" button always shows as last item and with border]. * [WordPress/gutenberg#63562 Global Styles: Disable "Reset styles" button when there are no changes]. * [WordPress/gutenberg#63093 Fix: Removed shuffle button when only 1 pattern is present]. * [WordPress/gutenberg#62675 fix: wp icon focus issue]. * [WordPress/gutenberg#63565 useBlockElement: return null until ref callback has time to clean up the old element]. Reviewed by spacedmonkey. Merges [58757] to the 6.6 branch. Props ellatrix. Fixes #61692. See #61660, #61630, #61656. git-svn-id: https://develop.svn.wordpress.org/branches/6.6@58760 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Jul 18, 2024
Bugfixes included: * [WordPress/gutenberg#63637 Elements: Avoid specificity bump for top-level element-only selectors]. * [WordPress/gutenberg#63406 Navigation block: Allow themes to override block library text-decoration rule]. * [WordPress/gutenberg#63436 Fix invalid css for nested fullwidth layouts with zero padding applied]. * [WordPress/gutenberg#63397 Prevent empty void at the bottom of editor when block directory results are present]. * [WordPress/gutenberg#63291 Pattern overrides: Ensure "Reset" button always shows as last item and with border]. * [WordPress/gutenberg#63562 Global Styles: Disable "Reset styles" button when there are no changes]. * [WordPress/gutenberg#63093 Fix: Removed shuffle button when only 1 pattern is present]. * [WordPress/gutenberg#62675 fix: wp icon focus issue]. * [WordPress/gutenberg#63565 useBlockElement: return null until ref callback has time to clean up the old element]. Reviewed by spacedmonkey. Merges [58757] to the 6.6 branch. Props ellatrix. Fixes #61692. See #61660, #61630, #61656. Built from https://develop.svn.wordpress.org/branches/6.6@58760 git-svn-id: http://core.svn.wordpress.org/branches/6.6@58162 1a063a9b-81f0-0310-95a4-ce76da25c4cd
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
…ion rule (WordPress#63406) * Reduce specificity of navigation text decoration rule * Apply Aaron suggestion to also lower specificity for focus/active * Remove `:not([class*="has-text-decoration"])` from selector ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
In a default install of WordPress 6.5, when hovering over a navigation link an underline is shown.
In 6.6 this doesn't happen. Theme developers are unable to override block library styles for text decoration.
Why?
In 6.5, there's a CSS rule that sets the underline hover with the specificity of 0,2,1 (it comes from the TT4's theme.json):
It overrides some of the block library css (specificity 0,1,1):
In 6.6, the first rule's specificity was reduced to 0,1,0, and now the block library css has a higher specificity it results in that taking precedence and no hover underline styles.
How?
This reduces the specificity of the block library css to 0,1,0, following the general rule that theme.json/global styles should be able to override block css.
Testing Instructions
Simple test of hover styles
Expected: the link should be underlined
Active and Focus styles
Expected: Navigation block links have line-through styles when focused or active
{ "$schema": "../../schemas/json/theme.json", "version": 3, "settings": { "appearanceTools": true, "layout": { "contentSize": "840px", "wideSize": "1100px" } }, "styles": { "blocks": { "core/navigation": { "elements": { "link": { ":focus": { "typography": { "textDecoration": "line-through" } }, ":active": { "typography": { "textDecoration": "line-through" } } } } } } } }Screenshots or screencast
Before
After