WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.36k stars 4.13k forks source link

Rogues styles messing with my navigation styles #63345

Closed wolffe closed 2 months ago

wolffe commented 2 months ago

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

youknowriad commented 2 months ago

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

carolinan commented 2 months ago

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.

youknowriad commented 2 months ago

@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)

wolffe commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 https://github.com/WordPress/gutenberg/pull/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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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: https://github.com/WordPress/gutenberg/issues/53468#issuecomment-2232907531

fgbarrios commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

1 is the global style, from WordPress.

2 is my fix.

image

aaronrobertshaw commented 2 months ago

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 https://github.com/WordPress/gutenberg/pull/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 commented 2 months ago

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 commented 2 months ago

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 commented 1 month ago

I agree with everything @wolffe has said.

eric-michel commented 3 weeks ago

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 commented 2 weeks ago

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.

https://github.com/WordPress/gutenberg/issues/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 commented 1 week ago

@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 https://github.com/WordPress/gutenberg/issues/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, https://github.com/WordPress/gutenberg/issues/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, https://github.com/WordPress/gutenberg/issues/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 commented 1 week ago

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, https://github.com/WordPress/gutenberg/issues/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 commented 1 week ago

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.