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

Rogues styles messing with my navigation styles #63345

Closed
wolffe opened this issue Jul 10, 2024 · 33 comments · Fixed by #63403
Closed

Rogues styles messing with my navigation styles #63345

wolffe opened this issue Jul 10, 2024 · 33 comments · Fixed by #63403
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@wolffe
Copy link

wolffe commented Jul 10, 2024

Description

I have a rogue style overriding my navigation styles, namely underlines.

I do not have an underline on my navigation bar, but a very high-specificity style is being added (see attached screenshot):

:root :where(a:where(:not(.wp-element-button)))

My only solution is to add !important to my own styles. Who decided that all <a> elements that are not buttons should be underlined?!!!

Screenshot 2024-07-10 100749

Step-by-step reproduction instructions

  1. Install WordPress
  2. Install Gutenberg

Screenshots, screen recording, code snippet

No response

Environment info

Latest WordPress, latest Gutenberg plugin, PHP 8.3+, Apache and Nginx, Windows 10 and Windows 11.

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@wolffe wolffe added the [Type] Bug An existing feature does not function as intended label Jul 10, 2024
@youknowriad
Copy link
Contributor

@aaronrobertshaw @ellatrix I'm guessing this one is related to the specificity work?

@carolinan
Copy link
Contributor

carolinan commented Jul 10, 2024

By default, browsers underline links.
For the navigation block, WordPress adds CSS that removes the underline:
https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/navigation/style.scss#L79

The text decoration underline in the example above is added by a theme, plugin, or user.

To be able to help and troubleshoot this further, more information is needed. It is unclear what theme that is used for testing and what custom styles have been added.

@carolinan carolinan added the [Block] Navigation Affects the Navigation Block label Jul 10, 2024
@youknowriad
Copy link
Contributor

@carolinan I think this one doesn't relate to the navigation block, it comes from the default "core" provided theme.json (that all themes inherit)

@carolinan carolinan removed the [Block] Navigation Affects the Navigation Block label Jul 10, 2024
@wolffe
Copy link
Author

wolffe commented Jul 10, 2024

@carolinan There is no navigation block, it's just custom HTML + CSS. A list with links inside.

This code is added by Gutenberg:

:root :where(a:where(:not(.wp-element-button)))

Regardless of browsers adding underline by default, I have my own CSS rule to hide the underline, but the line above has higher priority. See my screenshot above. It's really simple.

@carolinan
Copy link
Contributor

Yes you are both right :)
I made an incorrect assumption that you were using a block theme, where these styles can be overridden multiple times depending on what is placed in theme.json.

@aaronrobertshaw
Copy link
Contributor

Thanks for reporting this issue @wolffe, it's greatly appreciated 👍

I'm guessing this one is related to the specificity work?

Yes, I believe the default element selectors for theme.json were levelled to 0-1-0 specificity in #61638. Some elements like captions were lowered in specificity and it appears the standard link bumped from 0-0-1 to 0-1-0.

A TL;DR on the reasoning for the change was to ensure base global styles were applied after general reset styles and still allow nesting of block style variations.


I do not have an underline on my navigation bar, but a very high-specificity style is being added (see attached screenshot):

@wolffe hopefully I can shed a little extra light on the :root :where(a:where(:not(.wp-element-button))) selector and why it was unfortunately overriding your nav styles.

From your screenshot (thank you 🙇), your nav link selector is made up purely of elements header nav ul li a which results in a specificity of 0-0-5.

The :root selector has the same specificity as a class name i.e. 0-1-0. The :where() selector makes anything inside them have zero specificity i.e. 0-0-0. While the whole selector looks complicated and appears to have a high specificity, it's only 0-1-0 and the same as say .wp-block-group.

As the :root :where() selector has a single classname level segment in its selector, that trumps all the element-only parts. For a better explanation of how the different columns in the CSS specificity are weighted, these docs do a better job than me 😅

Screenshot 2024-07-11 at 10 10 55 AM

My only solution is to add !important to my own styles.

There are some other options to !important that you could use until I can work on a fix. They would involve using a selector containing a classname from any container of the nav links e.g. .my-header a, .my-nav a etc.

Here's a codepen link that demonstrates those options which you can explore by uncommenting the styles one at a time from top to bottom and see them apply.


As for a solution, we might be able to avoid the :root :where() wrapper for top-level element styles that were previously element-only selectors i.e. a:where(:not(.wp-element-button)), h1 - h6, and cite.

I'll explore that today and update again when I know more.

@aaronrobertshaw
Copy link
Contributor

Quick update:

It does appear to be possible to isolate the "top-level" element styles and prevent them being wrapped in :root :where() when the element's selector previously had "element-only" specificity e.g. 0-0-*.

The approach still needs to be replicated on the JS side for the editors and tested against global block type element styles, block style variations, etc. I hope to have a PR published later this afternoon or tomorrow morning.

@aaronrobertshaw
Copy link
Contributor

I have a PR up that addresses this issue. I'm not sure whether this will make it into WP 6.6.0 but at the very least it should be in the first point release.

@wolffe
Copy link
Author

wolffe commented Jul 11, 2024

Thanks, @aaronrobertshaw.

I don't want to modify my layout as it's pretty specific - header nav ul li a. I only have one <header> element on the page.

I wish Gutenberg would not try to impose opinionated styles on us. Things used to work perfectly fine before, but I have 400 websites that are in danger of being broken when WordPress 6.6 comes out.

Is there a way to opt out of these styles? Some filter or action or theme support I can disable?

@aaronrobertshaw
Copy link
Contributor

I appreciate your patience on this one @wolffe, thank you 👍

The fix has landed and will be available in Gutenberg 18.9. It will also be included in the first WordPress point release, 6.6.1.

I don't want to modify my layout as it's pretty specific - header nav ul li a. I only have one <header> element on the page.

FWIW the temporary workaround I was suggesting was to leverage an existing class name you might have already within your layout.

Another option could be to tweak your selector to header nav:not(.wp-block-navigation) ul li a. The :not() clause is a class-level selector and would override the core link element styles until the fix lands. Hopefully, this would be easier than removing any filter, action, or theme support 🤞

@wolffe
Copy link
Author

wolffe commented Jul 12, 2024

Okay, the header nav:not(.wp-block-navigation) ul li a but why do I need to patch my theme for every WordPress release?

I will never have a .wp-block-navigation element on my page, so why do I need to exclude it?

Nevertheless, thanks for the fix! I appreciate it.

@aaronrobertshaw
Copy link
Contributor

I will never have a .wp-block-navigation element on my page, so why do I need to exclude it?

The :not(.wp-block-navigation) segment in the selector was only to bump the overall selector's class-level specificity to 1 which allows it to take precedence over an element only selector. The :not() clause could have been anything like :not(.does-not-exist) etc.

@wolffe
Copy link
Author

wolffe commented Jul 17, 2024

I know what it does, but it shouldn't be there. It assumes ALL links that are :not a specific class.

This assumption should not be made by the core. In fact, no classes should be added by the core, except very specific ones, not umbrella ones.

@aaronrobertshaw
Copy link
Contributor

Thanks for the candid feedback @wolffe, I appreciate it 👍

The fix has landed and will be available in Gutenberg 18.9. It will also be included in the first WordPress point release, 6.6.1.

The fix for this issue managed to make the Gutenberg 18.8 release which happened a few hours ago. If you're using Gutenberg, updating to 18.8 should allow you to avoid the use of :not() in your selector now.

@Uncreative-Dev
Copy link

Hi, we are currently facing the same problems as @wolffe we are mostly using the Divi Theme. As i already read that the issue is fixed in release 6.6.1. I just want to ask when this release is expected to be available. In the meantime we are just using a fix.

@ghost
Copy link

ghost commented Jul 17, 2024

Hello, similar to both @wolffe and @Uncreative-Dev, I am encountering the exact same problem of underlining on all my clients' websites created with Divi. No issues with Elementor based websites! When will the 6.6.1 version be available as many of my clients have contacted me regarding this problem. Thank you for the update. Sorry for my english, but I am Franch ;-)

@cbirdsong
Copy link

Is there a way to opt out of these styles? Some filter or action or theme support I can disable?

This reply in another thread goes into some ways, though it could use some more comments: #53468 (comment)

@fgbarrios
Copy link

fgbarrios commented Jul 17, 2024

Is there a way to opt out of these styles? Some filter or action or theme support I can disable?

This worked for us (using DIVI theme):


//REMOVE GUTENBERG BLOCK LIBRARY CSS FROM LOADING ON FRONTEND
function remove_wp_block_library_css(){
    wp_dequeue_style( 'wp-block-library' );
    wp_dequeue_style( 'wp-block-library-theme' );
    wp_dequeue_style( 'wc-block-style' ); // REMOVE WOOCOMMERCE BLOCK CSS
    wp_dequeue_style( 'global-styles' ); // REMOVE THEME.JSON
    }
add_action( 'wp_enqueue_scripts', 'remove_wp_block_library_css', 100 );

@wolffe
Copy link
Author

wolffe commented Jul 17, 2024

@fgbarrios Thanks! I will try it and see what works and what doesn't.

I still want to keep the core ones, columns, buttons, covers, groups and so on.

@wolffe
Copy link
Author

wolffe commented Jul 18, 2024

The filter below fixed the entire issue for me:

add_action(
    'wp_head',
    function () {
    	global $wp_styles;

        if ( ! empty( $wp_styles->registered['global-styles']->extra['after'][0] ) ){
    		$wp_styles->registered['global-styles']->extra['after'][0] = str_replace(
                [
                    ':root :where',
                    ':root',
                    ':where',
                ],
                '',
                $wp_styles->registered['global-styles']->extra['after'][0]
            );
        }
    },
    1
);

@nickfmc
Copy link

nickfmc commented Jul 18, 2024

This also effects the body padding, say you have a fixed header and applied a padding-top:150px to simply body {} then the new root style specifics will overide the padding to, easy fix to update in your theme but not good for existing sites

@wolffe
Copy link
Author

wolffe commented Jul 18, 2024

I also have lots of other issues, but I don't think it's worth opening new tickets.

For example, All centered images got a display: table, which is pretty ancient. As a result, all centered images across all my sites became left aligned.

I had to force display: block on them to fix them.

@richtabor
Copy link
Member

For example, All centered images got a display: table, which is pretty ancient. As a result, all centered images across all my sites became left aligned.

Curious, I'm not experiencing that. Could you share more?

CleanShot 2024-07-18 at 19 20 42

@wolffe
Copy link
Author

wolffe commented Jul 19, 2024

1 is the global style, from WordPress.

2 is my fix.

image

@aaronrobertshaw
Copy link
Contributor

Thanks for the screenshot @wolffe, it definitely helps 👍

The style with display: table for centered images appears to be a block library style rather than from the theme.json/global styles feature.

Looking through the history of the Image block's styles, the display: table rule was introduced back in 2018 via #7721. So that rule should have been in effect for a long time.

The other styles in the screenshot appear to make sense but unfortunately it doesn't show every matched style so it's hard to tell if a different style setting display had its selector lowered to where this old rule now took effect.

If this is still an issue, perhaps you could filter the styles in dev tools by display and share another screenshot of all matching styles? Alternatively, if you would like to share a link to your site with me, you're welcome to reach out on WordPress Slack and I can help from there.

@wolffe
Copy link
Author

wolffe commented Jul 21, 2024

Something has changed lately, and it changed alignment for all my images.

I patch all these issues by adding counter-measures - CSS rules that target those elements and are being loaded after WordPress' inline styles.

I don't think it's worth reporting them, as they keep getting added. Is there a way to stop this "specificity" project from going ahead? This is just a buzzword that's been floating around recently, and it's not beneficial to WordPress. You can see how many sites are being broken with every WordPress update.

It's like forcefully trying to squeeze a square in a circle shape. This is WordPress these days. These years...

image

@aaronrobertshaw
Copy link
Contributor

Something has changed lately, and it changed alignment for all my images.

I'm more than happy to help identify what may have changed but as I noted in my last comment I'd need to see all the styles matching the image block rather than a subset in the screenshot provided.

I don't think it's worth reporting them, as they keep getting added

That is ultimately your choice however I still recommend reporting any issues. Even if you've already addressed them, they might still help others and inform how WordPress can be improved for everyone.

In that spirit, perhaps we can refocus the discussion here on any specific styles and selectors that are causing a problem 🙏

@erikgripestam
Copy link

I agree with everything @wolffe has said.

@eric-michel
Copy link

I don't think it's worth reporting them, as they keep getting added

That is ultimately your choice however I still recommend reporting any issues. Even if you've already addressed them, they might still help others and inform how WordPress can be improved for everyone.

In that spirit, perhaps we can refocus the discussion here on any specific styles and selectors that are causing a problem 🙏

@aaronrobertshaw Respectfully, sometimes it does feel like reporting pretty egregious CSS changes is like yelling into the void. Here are my three biggest ones:

All three of these issues broke sites we maintain, and did so 100% unnecessarily. Only the worst one has been fixed, and there is no plan to actually deploy that fix.

There have been a lot of conversations about how disruptive every major WP release is these days for theme and site developers. These issues have only been made worse over time as more styling options have been added to the editor. I am starting to get heartburn every quarter knowing that month's maintenance cycle is inevitably going to involve patching every site we manage to accommodate some kind of breaking change - often times one that is, frankly, poorly conceived and unnecessarily aggressive.

@aaronrobertshaw
Copy link
Contributor

Sorry for the slow reply @eric-michel I've been offline for a couple of weeks 🙏

Respectfully, sometimes it does feel like reporting pretty egregious CSS changes is like yelling into the void

I can understand the sentiment, especially in a large community-driven project. Hopefully, my lack of availability recently didn't compound that experience for you 😅

Here are my three biggest ones:

Unfortunately, I can't speak too much on the first two here.

I do recall seeing work around supporting negative margins on blocks which I think required some changes for position styles on Group blocks. Finding a balance between supporting negative margins and the way things were might be a factor in this one needing some time to chart a path forward. I'll save any further discussion about this for the relevant issue.

#63912 (fixed and core patch made, but no plans to actually release 6.6.2 to get the fix deployed)

This fix was always slated for 6.6.2 as you noted, however, there are a few factors that go into scheduling point releases and that is hard to communicate across fractured GitHub issues, PRs etc. Most of the discussion around this sort of thing occurs in the WordPress Slack channels and is a great source of information if you are looking for up-to-the-minute details on a pending point release.

There have been a lot of conversations about how disruptive every major WP release is these days for theme and site developers.

100% understandable. It is an important area to improve.

To this end, I do have some hope that in the not-too-distant future CSS Layers will drastically reduce a lot of this friction by isolating the different levels of styling across WordPress.

@eric-michel
Copy link

@aaronrobertshaw no apologies necessary at all! I recognize this is largely a volunteer-led operation and never expect people to drop what they're doing to answer these threads right away. My main concern is for long-term sustainable growth of Gutenberg as a software.

I was happy to see that 6.6.2 did finally release, so #63912 is now resolved.

I am also excited for the eventual adoption of CSS layers, and think that will do a lot to iron out some of the minor conflicts that we run into sometimes.

Having said that, the two other issues I linked will not be solved with better CSS specificity. In both cases, they are CSS rules that are unnecessarily being applied in cases where they are not needed.

The first, #64276, applies position: relative to (effectively) all Group blocks, whether they need it for negative margin or not. To prevent this from breaking our sites, we would have needed to have the foresight to set all of our Group blocks to position: static in our theme CSS. No developer is going to do that, because the default value of static is assumed. I'm not going to redefine all defaults for all CSS properties for all blocks on my site on the off chance the next WP version breaks those defaults.

The second, #60922, is a problem for similar issues, but is made even worse by the fact that it's an inline style, which means it will not be helped by CSS layers even when they are adopted.

There is an extensive review process for pull requests (as there should be!). My only request is that this review process includes a question akin to: "Are the CSS changes in this PR as limited in scope as they reasonably can be?" I think any reviewer asking that question of the two issues I mentioned above would answer with a resounding "no."

@aaronrobertshaw
Copy link
Contributor

My only request is that this review process includes a question akin to: "Are the CSS changes in this PR as limited in scope as they reasonably can be?" I think any reviewer asking that question of the two issues I mentioned above would answer with a resounding "no."

For the position: relative change, there was considerable discussion on the relevant PR so if it interests you there might be some good context to gain there.

In particular, there were some considerations made towards trying to minimise the potential impact of the change, e.g. lowering specificity of the selector with :where(), limiting it to the Group block etc. I fully appreciate this doesn't alleviate the issue faced.

I'd also encourage anyone with a stake in WordPress theme development to join the #outreach channel in WordPress Slack. It's a great place to not only provide feedback but also participate in discussions around some features/changes early on. It may also be possible to join the WordPress Outreach team as they are often pinged to seek feedback across diverse use cases and perspectives. They were also involved on the linked PR.

The second, #60922, is a problem for similar issues, but is made even worse by the fact that it's an inline style, which means it will not be helped by CSS layers even when they are adopted.

Leaving aside the aspect ratio issue for a moment, regarding inline styles, I'd see part of the effort of implementing CSS layers in WordPress as working out if inline styles should/could also be migrated into a separate stylesheet and CSS layer. It's unclear at the moment how much of a difference that might make as these are styles specific to a block instance and "should" override all other theme and global styles as they are applied by the end user.

I want to thank you again for the feedback. At this stage, I'd like to move any further discussion to the relevant issue so it can help inform solutions and hopefully build some momentum rather than be lost on an old closed issue.

@eric-michel
Copy link

Thanks @aaronrobertshaw! I'll take a look at the PR and see what the discussion looked like and will respond in the other issues as you suggested. I do really appreciate your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.