WordPress inline CSS doesn't respect editor-font-sizes

billerickson commented 2 years ago

Description

In WordPress 5.9, inline styles are added with !important, generated by theme.json (for a discussion disabling !important, see #37590). If a theme does not contain a theme.json file, it uses the theme.json file in WordPress core.

This issue occurs with the Gutenberg plugin active or inactive.

Previous versions of WordPress allowed themes to define the editor font sizes using add_theme_support( 'editor-font-sizes' ). These sizes are not being respected in WordPress 5.9.

In the backend, the typography selector shows the sizes specified by add_theme_support(), but the actual rendered font size uses the WP core defaults. In the screenshot below, the theme set "Large" to 21px, but WP core is overriding that with 36px.

If a theme does not use theme.json, the add_theme_support( 'editor-font-sizes' ) should be used when determining the values for those CSS variables.

Step-by-step reproduction instructions

Install a theme that does not use theme.json
In functions.php, define a "large" font size using add_theme_support: https://gist.github.com/billerickson/1111ac70585cf9ca7cf6c5bd306a2cd3
In your theme's stylesheet, style the font size appropriately. Ex: .has-large-font-size { font-size: 21px; }
When WP 5.9 is active, the large font style will appear as 36px

Screenshots, screen recording, code snippet

screenshot

Environment info

No response

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

webmandesign commented 2 years ago

Hi,

I can confirm this. My theme is "classic" one (non-FSE) and I've noticed the new WP 5.9 is adding global-styles-inline-css styles into HTML head such as: .has-large-font-size { font-size: var(--wp--preset--font-size--large) !important; }

This effectively overrides my theme styles as I also use "large" as one of custom block editor font sizes:

wp5 9-global-styles-overrides

Why this is even being outputted with classic theme?

annezazu commented 2 years ago

@oandregal tagging you in here. This seems very similar to this previous issue that was resolved around font sizes https://github.com/WordPress/gutenberg/issues/37617 Would love if you can dig in here to see what might be going on/what needs to be cleared up.

cbirdsong commented 2 years ago

My current workaround for this on legacy themes is to just get rid of the entire theme.json variable block via wp_dequeue_style( 'global-styles' );. However, that doesn't remove all the .wp-container- classes generated in the footer (#36135).

luminuu commented 2 years ago

I can confirm this as well on a staging site.

webmandesign commented 2 years ago

My current workaround for this on legacy themes is to just get rid of the entire theme.json variable block via wp_dequeue_style( 'global-styles' );.

Great workaround. As we are removing core functionality this way I'm just wondering whether it passes theme checks in Theme Check and Theme Sniffer plugins and WordPress theme review.

webmandesign commented 2 years ago

The question for me stays: why is this even produced? Isn't this a theme territory?

oandregal commented 2 years ago

:wave: We've made some changes to the default presets coming from WordPress, see the section "Changes to the global stylesheet > Default font sizes, colors, and gradients" in the devnote.

The TLDR is that themes with theme.json can modify the default values directly. Themes that don't have a theme.json need to update their CSS to use the CSS Custom Properties to set the values, as in:

:root {
 --wp--preset--font-size--large: <NEW_VALUE>;
}

webmandesign commented 2 years ago

@oandregal Thanks for explanation and for the link.

I already use CSS custom properties in my theme but they are custom ones, not WordPress ones. It seems contraproductive to basically repeat the code for the new WP CSS custom properties - it adds redundant code to my stylesheets. Could you please explain why this is produced by WordPress even for classic themes that does not use theme.json file?

Alternatively, can I provide empty theme.json file in my theme - would that get rid of that WordPress CSS code? But then, wouldn't it enable FSE features as theme.json file is present in my classic theme (which is what I don't want)?

What happens if I dequeue the styles with above workaround? Would it break something in WordPress?

webmandesign commented 2 years ago

Another thing is, WordPress is using body{ --wp--preset--...: ... } and this is being enqueued after my theme's stylesheet. This effectively overrides my theme's :root { --theme-css-variable: ...; --wp--preset--...: ... } declarations. I now have to use more specific CSS selector or/and !important.

Also, the font size classes are now produced twice: by theme in theme stylesheet and by WordPress in inline global styles. Why this happens when providing custom colors and font sizes was presented as a theme territory? WP5.9 doesn't seem to honor these theme declarations.

For years I produced CSS code with as minimal specificity as possible and with no !important. I never presumed I'd have to use opposite in 2021 onwards.

Is there a better place I can rase my concerns regarding global styles?

PS: I've tested creating theme.json file with basically no declarations. Doesn't help and it even messes up the editor layout that is working fine now in the theme.

oandregal commented 2 years ago

Hey, I'm happy to expand on this. This is the issue we face: block & theme selectors can have higher specificity than user styles attached to blocks, for example:

/* block classes that themes use to set styles */
.wp-block-post-title h1 { /* this has higher specificity than the preset class */
  color: green;
}

/*
 * Preset class that is only attached to blocks by the user,
 * otherwise they're not present. If they're attached,
 * they should override any other rule the theme has set.
 */
.has-red-color {
  color: red;
}

User styles should prevail at all times over block & theme styles. We considered alternatives but none were satisfactory. It was also discussed in this thread at length. We're all for iterating if there're alternatives to the issue we face.

Note that default presets have been enqueued by WordPress since the block editor was launched. In 5.9, we moved them from an embedded stylesheet (wp-block-library-css) to the global styles one (global-styles-inline-css). See https://github.com/WordPress/gutenberg/pull/34510 and https://github.com/WordPress/gutenberg/pull/35182 In terms of default presets, WordPress 5.9 ships the same CSS as before, just in a different place. The reason default classes have been enqueued since the beginning is that they can be used by users: in existing content, used in patterns by themes, or now directly through the UI color controls that show theme & default colors.

cbirdsong commented 2 years ago

In terms of default presets, WordPress 5.9 ships the same CSS as before, just in a different place.

Use of !important means this just isn't the case. Previously, I could override .has-font-size-large with responsive sizes and now Wordpress overrides it. (#34575)

johnstonphilip commented 2 years ago

It would seem to me that if a theme doesn't have a theme.json file, than no theme.json things should be rendered on the front-end at all. Does anyone disagree with that? If so, why?

mwhiteley16 commented 2 years ago

I would agree.

To go further on this specific issue (as noted by Bill in the initial issue), if there is no theme.json AND add_theme_support( 'editor-font-sizes' ) is being used, that should be respected, not overridden.

Seems pretty logical and straight forward.

billerickson commented 2 years ago

@oandregal For themes that don't have a theme.json file, why don't the font sizes defined in add_theme_support( 'editor-font-sizes' ); take precedence over the default font sizes in WP core?

webmandesign commented 2 years ago

@oandregal But isn't this a theme territory anyway? If the theme overrides WordPress styles, maybe it is doing on purpose. WordPress should not force its own styles in my opinion.

In my opinion, the example you've provided seems to be an insufficient design on the theme's side. I personally deal with such things this way in my themes:

/* block classes that themes use to set styles */
/* this way even user modified color via editor should work without any !important or higher specificity classes */
.wp-block-post-title h1:not(.has-text-color) {
  color: green;
}

/*
 * Preset class that is only attached to blocks by the user,
 * otherwise they're not present. If they're attached,
 * they should override any other rule the theme has set.
 */
.has-red-color {
  color: red;
}

User styles should prevail at all times over block & theme styles.

I 100% agree. But it should not be forced by WordPress by overriding theme declared styles and compatibility with block editor provided using recommended ways.

Note that default presets have been enqueued by WordPress since the block editor was launched.

That is perfectly fine. But I've never had issues with WordPress overriding my add_theme_support() theme styles previously and I build Gutenberg compatible themes for 2+ years.

It's funny that this seems to affect only themes that are actually compatible with Gutenberg editor (but are not block themes/FSE themes). Themes that are not compatible with Gutenberg or are block themes, they seem to work perfectly fine also with this WordPress change. I'm just sad that the themes that advocate for using Gutenberg editor are affected the most. (Which only still convinces users to use alternative editors, unfortunately...)

The reason default classes have been enqueued since the beginning is that they can be used by users: in existing content, used in patterns by themes, or now directly through the UI color controls that show theme & default colors.

I understand that. But such case can also occur when user switches between 2 themes that are compatible with Gutenberg. And in that case if a content was built with color and font presets of the first theme it may no longer work when the second theme is activated. There is no naming convention in place for such custom theme presets, which causes issues. I don't think there is a solution for this now. But I understand that WordPress keeps default values for these in the code.

It's just that I don't think WordPress should force its own default styles on themes. Especially those ones that uses the same custom font size labels but different actual font size values (for example). No default WordPress style should be outputted with such high specificity and !important declaration.

Tip: This is not related, but I've dealt with colors in block patterns too. If I may advice someone who could benefit, rather use custom color values and not preset colors in block patterns. That way the patterns have higher chance to look the same even if the color patterns are changed some "wildish" way.

My suggestions:

I understand that WordPress has to keep the styles for backwards compatibility. But could this be done with non-obtrusive way like it was until now? It would be beneficial to lower priority and specificity of the global styles, such as:

enqueue the global styles as early as possible, maybe even with negative priority: add_action( 'wp_enqueue_scripts', 'wp_enqueue_global_styles', -100 ); That way any theme and plugin styles hooked with default priority of 10 may override the global CSS with the same CSS selectors at least.
lower CSS selector specificity from body { ... } to :root { ... } in global styles code.
do not use !important in the global styles declarations. That way if there is a style declaration for existing WordPress class in the theme, the theme styles get precedence.
and finally, if none of the above is possible (or even if it is), provide hook to filter the output of wp_get_global_stylesheet() function. That way themes can control output of these styles themselves too.

mkkeck commented 2 years ago

Dear Wordpress and Gutenberg developers,

does anybody of you know, that code duplication specially in CSS styles could lead in bad page speed and bad web vitals?

Styles, and of course other things like SVG, which are loaded or inlined but not used, increase the time and filesize to load, parse and display the above fold content. Running tests with Lighthouse and Webhints shows me longer tasks for recalculating and rendering the layout. Of course, this may be not essential, but such every little thing added, decrease pagespeed perfomance.

PLEASE: if a user, I'm not talking about non professionells - which are installing WordPress in 5 minutes and than only use it for simple blogging, not need your "global / core" stuff, give them an opt-out (I prefer opt-ins). If anybody uses a custom made theme, I personally think, he/she/it knows what he/she/it does. So why do you force and dictate them how and why using your "global / core" stuff and that they have to accept, many junk is added to rendered pages?

The other point is: Are you really testing before release? Perhaps that could be done better. Thats why there are many junk inline styles and SVG added before closing the body and some essentials things like responsive font sizes not working without terrible hacks.

PLEASE: Add all this in your own Twenty-Themes, but not global to all other themes out there, without an explizit opt-in.

I'm really frustrated, that since WordPress version 5 nearly every minor and major update breaks my (front) pages and/or leads in bad pagespeed performance and web vitals.

tommusrhodus commented 2 years ago

The use of !important is definitely a huge issue here, another is that without declaring add_theme_support( 'experimental-link-color' ), font sizes and colour styles created through add_theme_support do not override the WP Presets if they share the same name, e.g large or black

I have detailed this issue in https://core.trac.wordpress.org/ticket/54954 and have a PR to remove the dependancy of experimental-link-color at WordPress/wordpress-develop/pull/2236

Whilst that PR does not resolve issues caused by the use of !important, it does at least correctly merge your defined style choices with the WP defaults before they get set as important.

webmandesign commented 2 years ago

Could you provide hook to filter global styles please?

The more I think about this, the more I incline to my last suggestion about filtering wp_get_global_stylesheet(). That way WP5.9 code could stay the same while theme authors could manipulate global styles code to their needs in a specific theme that requires such manipulation.

(Though still, no !important should be used :) )

paulgibbs commented 2 years ago

In terms of default presets, WordPress 5.9 ships the same CSS as before, just in a different place.

Use of !important means this just isn't the case. Previously, I could override .has-font-size-large with responsive sizes and now Wordpress overrides it. (#34575)

As @cbirdsong wrote, the change with !important makes the original statement here inaccurate, and we've noticed it on the first few of our clients' sites that we've updated. I think we're going to rollback to WP 5.8.x as we have hundreds of existing client sites which will be affected. This ought to be considered a breaking change / concern because it impacts our sites that do not have a theme.json and are classic theme.

I don't follow Gutenberg development closely enough to know whether the best fix is to merge the styles correctly, or drop the !important as a quick fix, but something needs to happen for WP 5.9.1 please.

webmandesign commented 2 years ago

Hey guys,

I've got a temporary fix for now (would be great if we can filter the global styles output though).

Here is PHP7+ code that should go into your (child) theme's functions.php file:

/**
 * Lowering specificity of WordPress global styles.
 */
add_action( 'init', function() {

    // WP5.9+ only.
    if ( ! function_exists( 'wp_get_global_stylesheet' ) ) {
        return;
    }

    // Dequeue original WP global styles.
    remove_action( 'wp_enqueue_scripts', 'wp_enqueue_global_styles' );

    // Enqueue WP global styles early.
    add_action( 'wp_enqueue_scripts', function() {

        // Lower CSS code specificity.
        $stylesheet = str_replace( [ 'body', '!important' ], [ ':root', '' ], wp_get_global_stylesheet() );

        if ( empty( $stylesheet ) ) {
            return;
        }

        wp_register_style( 'wp-global-styles', false );
        wp_add_inline_style( 'wp-global-styles', $stylesheet );
        wp_enqueue_style( 'wp-global-styles' );
    }, 0 );

    // Treat also editor styles.
    add_filter( 'block_editor_settings_all', function( $editor_settings ) {

        // Lower CSS code specificity.
        $editor_settings['styles'] = array_map( function( $style ) {
            if ( ! empty( $style['css'] ) ) {
                $style['css'] = str_replace( [ 'body', '!important' ], [ ':root', '' ], $style['css'] );
            }
            return $style;
        }, $editor_settings['styles'] );

        return $editor_settings;
    } );
} );

The code notes describe what's going on in there.

👉 This code was ported into a plugin Global Styles Mods.

mkkeck commented 2 years ago

Hey guys,

I've got a temporary fix for now (would be great if we can filter the global styles output though).

Here is PHP7+ code that should go into your (child) theme's functions.php file:

Nice :)

I've also a fix for the functions.php to remove the wp-container- styles produced by blockGap and the duotone-styles:

remove_filter('render_block', 'wp_render_duotone_support');
remove_filter('render_block', 'wp_restore_group_inner_container');
remove_filter('render_block', 'wp_render_layout_support_flag');

But the junk SVG can not be removed at the moment, missing an action/ filter to remove them. The problem is:

// file: wp-includes//block-supports/duotone.php; 
// line: #459 ... 467
//       produces the junk SVG, even if duot0ne is not used
add_action(
  // Safari doesn't render SVG filters defined in data URIs,
  // and SVG filters won't render in the head of a document,
  // so the next best place to put the SVG is in the footer.
  is_admin() ? 'admin_footer' : 'wp_footer',
    function () use ( $svg ) {
      echo $svg;
    }
);

pattyok commented 2 years ago

Hey guys,

I've got a temporary fix for now (would be great if we can filter the global styles output though).

Here is PHP7+ code that should go into your (child) theme's functions.php file:

Thanks for this!

This fixes the output on the front end. But still having issues trying to fix the view in the editor. The WordPress variable are set at .editor-styles-wrapper level...

.editor-styles-wrapper {
     ...
     --wp--preset--font-size--small: 13px;
    --wp--preset--font-size--medium: 20px;
    --wp--preset--font-size--large: 36px;
    --wp--preset--font-size--x-large: 42px;
}

Making it another challenge to override.

webmandesign commented 2 years ago

Hi @pattyok, I've updated the code in my reply above so it includes fix for editor too.

badlydrawnben commented 2 years ago

It's a shame that we have to resort to these temporary fixes or overwriting the :root CSS variables in existing sites to 'unbreak' them for 5.9.

It's another example of feeling like theme developers are punished for trying to customise Gutenberg by following the WP guidelines at the time - i.e. using add_theme_support( 'editor-font-sizes' ).

Is this planned to be fixed in an upcoming minor release? Surely WP core shouldn't be overriding the CSS set by a Theme, especially when the core CSS variables are using !important ?

webmandesign commented 2 years ago

It's another example of feeling like theme developers are punished for trying to customise Gutenberg

I agree with you. It also feels like Gutenberg sets a particular styling standard (meaning applying max-width on all blocks within a full width container) and everything is subordinated to this new approach - like themes should have no other option to follow. And when themes have a different approach, they experience problems with almost every recent WordPress major release.

For example, I don't really understand why widgets were made block based, Gutenberg ready. It is certain that sidebars and widgets have no future in WordPress any more so why the previous widgets interface was even changed? It caused more issues for theme authors than benefits as we still have to support both approaches → more styles are being loaded on front-end making the themes constantly less optimized.

As a side note I have to say that add_editor_style() resolves many issues I've experienced with WP5.9 (not this particular one, but #38240). But it also means there is no way to distinguish between classic editor (that is still being used by plugins such as WooCommerce and also by many real world customers/users) and Gutenberg stylesheets - you simply can not provide a specific stylesheet for specific editor. As an example of usecase: while classic editor still requires a reset styles, Gutenberg doesn't.

The biggest issue here is not informing about the changes in WP 5.9 sufficiently enough so theme authors can prepare. Or have I missed something? It seems very strange that Gutenberg ready themes were affected the most... And yes, we could test beta and RC versions, but sometimes it's not possible when you are one-man company and may even have bad experience doing so in the past...

Is this planned to be fixed in an upcoming minor release? Surely WP core shouldn't be overriding the CSS set by a Theme, especially when the core CSS variables are using !important ?

This is really just my presumption, and rather pessimistic one, but I personally don'f feel like the fix I've provided is "temporary" any more...

annezazu commented 2 years ago

The biggest issue here is not informing about the changes in WP 5.9 sufficiently enough so theme authors can prepare. Or have I missed something?

I wanted to chime in on this piece of things both in order to learn where you all would expect to see this communicated and to help share where it was for transparency. First and foremost, it was shared in this Dev Note as mentioned above. This was noted in the Field Guide for 5.9 and sent out to plugin and theme authors ahead of the release as well (source from the 5.9 release leads channel). It's also spread across various PRs and GitHub issues (just one example), shared in meetings like core editor & block themes, etc. I would definitely not expect folks to pay attention to that degree!

Obviously, this didn't reach folks in the way it needed to though and that means there's a great chance that together we can perhaps help prevent this problem in the future, especially since one of the 2022 goals is "Create a developer-focused communications site". What would have helped from your perspective? This could be everything from saying this would have worked well as a separate, more detailed dev note to something else entirely. If you would rather be more anonymous and not comment publicly, I welcome DMs on Make Slack (I'm also @annezazu there).

badlydrawnben commented 2 years ago

Hi @annezazu

I can obviously only speak from my own perspective - I'm more of a site builder than a theme developer - so I customize WP sites for clients and provide support for them. I don't use Page builders and I have been customizing Gutenberg for the last couple of years and creating custom blocks where necessary. I'm not a good enough developer to contribute to WP core or Gutenberg so I don't really tend to read Dev notes or Github threads unless something breaks and I come on here to find out how to fix it.

For me it's not so much of an issue about where the changes were publicised, but why they were made in the first place. The documentation about adding custom font sizes is still in the WP Docs here https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-support/#block-font-sizes but there's no indication in that this method is redundant because the core CSS in WP5.9 will supercede it?

So in my case, I look after 30 or so sites and quite a few of them will look broken when they upgrade to 5.9 because they make use of the has-large-font-size class and the WP font size is quite a lot bigger than the size set in my Theme. So that's a lot of sites to go through and fix. (And adding in a theme.json file to legacy sites isn't a quick fix because the Group block's inner-container has disappeared and I had targeted that in my Theme CSS).

Anyway, that's my $0.02 worth.

Also, for what it's worth - although it sounds like I'm complaining, I do have an enormous admiration and respect for everyone who works on the Gutenberg project and constantly makes it better. I'm sure it feels like a thankless task sometimes and I am very grateful for all your work.

annezazu commented 2 years ago

This is extremely helpful @badlydrawnben! It sounds like a few things would help:

Ensuring these changes are properly and promptly updated appropriately across documentation (this is officially on my list to follow up on to see what that needs to reflect).
Perhaps share in the dev notes where concerns can be raised to save folks time in bringing up problems (so you aren't dancing across GitHub issues and Dev Notes putting pieces together).

In the long run, if a developer focused communication site can happen too, it sounds like posts sharing why things are changing and more information about possible impact would really help. I'm a big believer in the power of knowing why something is done in order to then adopt whatever it is. Without the why, it's often just frustrating so I hear you there.

For what it's worth too -- I think folks "complain because they care". I do exactly the same and I deeply appreciate when others choose to engage rather than disengage, especially in a world where there are so many ways we could be spending our time. Thanks for spending your time sharing your thoughts here.

mkkeck commented 2 years ago

My opinion is, putting features, mainly used and created for full site edit should be only opt-in and active for themes, like the TwentyTwenty themes, which have the corresponding theme features activated. It is frustrating, that on nearly every WordPress update there is big chance to break existing themes and sites, which are not using FSE. Theres no hint on the dashboard, before updating the WordPress core, what and which new feature, setting or global style may perhaps break existing sites.

Never should the core add global styles, which forcing us to write hacks, so these added global styles won't overwriting theme styles. Not everybody is using TwentyTwenty themes, and not everybody want using FSE. FSE is not stable enough to forcing everybody to use it (thats my personal opinion).

Releasing new features, which may break existing sites and themes, without opt-in should be avoided.

Not every one is reading dev-notes or equivalent docs before simply pressing the update button. This is mainly readed after than if the site is broken (after core update).

I think thats the point, why here are many issues with sometimes hard and critical statements, of course my own too. A piece of software, like a CMS, should never break the work of users and developers using it. Not every user, using WordPress, Gutenberg and a third party theme is a developer or deep involved in WordPress and Gutenberg to fix issues happening after simply pressing the WordPress core update button.

I like the Gutenberg editor and it is really a new experience for authors and content creators. It has a big potential to make third party visual composers obselete. But there should be more control for users and specially developers, to enable and disable features. New features should be disabled by default, to minify chances breaking existing sites. This concept of opting in is used by near all other software projects, but not here in Gutenberg. Often experimental features and/or knitted with a hot needle are merged into WordPress core, by default activated but with no opt-out or any filter to remove. Not every feature is required by users or developers, but may bloat the rendered html, break sites, overwrites styles or results in unespected behavior.

oandregal commented 2 years ago

Hey, we're preparing a PR that sets the value of the presets automatically for themes that use the add_theme_support mechanism. This means that classic themes that override the default presets won't need to add any CSS to set it by themselves.

My hope is that it can go in a potential 5.9.1 release, so I'm adding the minor label to this one.

oandregal commented 2 years ago

@badlydrawnben the documentation issue you bring up is a good point. That change should have been documented there as well before the WordPress 5.9 release. Thanks for sharing.

While the PR above will provide a bridge for existing themes so they don't have to be updated, it's probably worth it to reflect the current state in the docs anyway - at least until a potential 5.9.1 that's what is required. Here's a PR https://github.com/WordPress/gutenberg/pull/38514

Graphnic commented 2 years ago

Firstly, massive and genuine respect to everyone involved in the WP release cycles. Thank you for your efforts.

Secondly, can I encourage all of those talented folks to look at how the process can be developed to catch more of these breaking changes before they hit 40% of the web.

I've just finished a weekly call with a handful of other small WP agency owners where this issue was a topic of discussion. All of us expressed our ongoing fatigue of not only keeping up with the changes, but moreover, handling these kind of issues across our client base.

I know I'm not the only one evaluating whether WordPress is going to be a viable tool for client website builds in the future. I hope it is, but my eyes are wandering. The future depends not only on our ability to build tools that help clients maintain a design system, but also on whether we can keep the support overhead under control.

Please, please, look at ways of catching these breaking changes to core functionality so they don't cumulatively ruin the reputation of the platform. Once clients start experiencing multiple issues, it's only natural they start to look to other solutions. Now, more than ever, there is great choice out there. Let's keep WP at the top of the list.

wmalexander commented 2 years ago

From my perspective discussing documentation related to this issue is confusing. This surely isn’t the intended behavior here, it’s a bug that needs to be fixed. There is value in communicating to people “heads up there’s a breaking issue in the new release that will be fixed in the next update” but it seems to me that if this problem had been discovered well enough in advance to be documented, that it would have been fixed, rather than documented. Quoting an earlier comment:

“if there is no theme.json AND add_theme_support( 'editor-font-sizes' ) is being used, that should be respected, not overridden.

Seems pretty logical and straight forward.”

That must be the intended behavior, which means this contrary behavior is a bug to fix, not a feature to document.

cbirdsong commented 2 years ago

More robust documentation on the editor would absolutely be welcome, but that assumes every Wordpress site has a developer actively maintaining it.

we're preparing a PR that sets the value of the presets automatically for themes that use the add_theme_support mechanism.

This fix is still going to break any responsive typography sizes on every site I've implemented that was built on the block editor. That problem was raised in an issue back in September (#34575), and I can only assume the team at best missed it or at worst ignored it?

On a larger level, it feels like the team doesn't adequately consider the way their implementation choices will interact with the outside world. A project that powers 40% of the web should not suddenly ship a bunch of !important styles to the front end (#38252, #36135), or insert a bunch of SVG markup at the bottom of every page (#38299), or add a bunch of redundant and repetitive <style> tags (#36053, #38127).

This is a long list, and it's only taking into account new things that are actively causing problems for theme authors in this release. Many of the HTML/CSS-related implementation choices over the years have made things more difficult than they needed to be. #35840 is a recent example, and it doesn't feel like things are getting any better on that front.

mkkeck commented 2 years ago

User styles should prevail at all times over block & theme styles.

No! This should be configurable. And presets and block styles should never overwrite theme styles.

Note that default presets have been enqueued by WordPress since the block editor was launched.

Yes that's right, but it was easier to hook into actions and filters to adjust what should enqueued and what not. But today, with every minor or major versions it becomes more difficult and/or is not planned to give us rights to adjust, configure or disable. That's bad. And it's a shame, that existing pages, themes or plugins out there may be suddenly broken with nearly every minor core update.

In 5.9, we moved them from an embedded stylesheet (wp-block-library-css) to the global styles one (global-styles-inline-css).

That's the point, why here are many issues. Global styles should never overwrite theme styles.

In terms of default presets, WordPress 5.9 ships the same CSS as before, just in a different place. The reason default classes have been enqueued since the beginning is that they can be used by users: in existing content, used in patterns by themes, or now directly through the UI color controls that show theme & default colors.

I agree, but it is only necessary for TwentyTwenty themes. For third party or own themes, this should be deactivated by default. If a user need this let him opt-in with an action, filter or theme setting in theme.json.

annezazu commented 2 years ago

Once more, thank you all for this feedback and continued discussion. I want you all to know that I've flagged both this issue and the SVG markup one https://github.com/WordPress/gutenberg/issues/38299 for the 5.9 release leads (you can see this here) so this can help inform the 5.9.x release(s). If you don't read the rest of this comment, just know I deeply encourage you to continue to engage in giving feedback, testing early, etc. Things might not always be fixed in the way that matches your specific situation but, if that's the case, it does allow those of us communicating these changes to be more proactive in informing folks like yourself so you can be better prepared.

Secondly, can I encourage all of those talented folks to look at how the process can be developed to catch more of these breaking changes before they hit 40% of the web.

This is always the aim. Breaking changes are painful for everyone all around - those experiencing them and those trying to resolve them after the fact. I'm very biased here as the co-test lead for 5.9 but, if I can make any ask in turn, it would be to help test the release during the beta/RC phases to help those of us working on the project catch these problems too. It all helps immensely. On the flip side, I know lots of work is being done to stabilize e2e tests, improve communication through ideas like the developer communication site, build more robust testing/auditing into earlier stages of development (previous example: https://github.com/WordPress/gutenberg/issues/31126), etc. There's lots of room for improvement as this issue highlights!

I've just finished a weekly call with a handful of other small WP agency owners where this issue was a topic of discussion. All of us expressed our ongoing fatigue of not only keeping up with the changes, but moreover, handling these kind of issues across our client base.

Thank you for passing on this perspective. How WordPress feels to folks matters immensely (does it feel stable? easy to keep up with? fun to work with? or the opposite?). Adapting to the latest updates is always going to be something to prepare for but that doesn't mean we can't reduce unnecessary stress for situations like this :) Please know I'm soaking this all up and will bring back to community spaces as there's the potential for a developer focused communication site that I think might help address some of these concerns. For example, imagine not needing to keep up with Make Core but instead getting curated posts less frequently with the key information/changes you need to know and specific call outs to audiences that might be impacted ("theme authors be aware that...").

That problem was raised in an issue back in September (https://github.com/WordPress/gutenberg/issues/34575), and I can only assume the team at best missed it or at worst ignored it?

I triaged that issue personally (you can see I added some labels and just added an additional one). I also did share it with folks working on this project. Please keep opening issues, sharing impact, etc as it allows for those working on implementing these projects to get real world information around the impact changes have, where better documentation is needed, and more. There's obviously a lot to be learned here and you are right to point this out. I had the same thing happen with this issue too https://github.com/WordPress/gutenberg/issues/37634 until a duplicate of the same problem received greater attention. It's also on my list to learn from after the dust settles more on this release.

FSE is not stable enough to forcing everybody to use it (thats my personal opinion).

That's completely correct and not at all the intention of the collection of features of full site editing. There's a reason a key milestone of this work is gradual adoption and ensuring there are pathways for folks to adopt as they want/are ready/makes sense for them, whether that's opting into the template editor or using theme.json on a classic theme. I run the FSE Outreach Program and can say with certainty that that is the intent of this project since this is still very much a v1.

From my perspective discussing documentation related to this issue is confusing. This surely isn’t the intended behavior here, it’s a bug that needs to be fixed.

To quickly address this comment, I think it's important for those who are using 5.9 right now and may not end up here in this thread that documentation properly reflects the current state of things to get items to work. After 5.9.1, I think the docs will need to be updated once more. Perhaps though the docs can say something more to the effect of "here’s what you can do in the meantime before 5.9.1" to reflect this as something that is going to be fixed. Thankfully, these are easy updates to make.

webmandesign commented 2 years ago

I wanted to chime in on this piece of things both in order to learn where you all would expect to see this communicated and to help share where it was for transparency. First and foremost...

Hi @annezazu,

Thank you for your feedback. My apologies as I presumed the changes mentioned in the article are related to themes that contain theme.json file, which none of my themes contains. So I really must have read that article too fast and must have missed the !important in the global stylesheet code example or I've ignored it as it was not mentioned in the actual text of the article (it is only mentioned that !important is being removed from custom link color styles).

After WP 5.9 release I was also put on the wrong track with comment about enqueue_block_editor_assets() not working.

I use enqueue_block_editor_assets() in my themes instead of pure add_editor_style() approach to distinguish between classic and Gutenberg editor stylesheets. And that's where all my issues started actually and gradually I kept finding new ones, including these !important overrides.

I was simply very confused what's going on and I had to trace my issues by looking into WordPress and Gutenberg source code, which was not ideal.

Also, I'm pretty confused as I think you guys must have experienced similar issues we experience now while testing WP5.9 with older native themes such as Twenty Twenty or similar. Yet I haven't found a mention about these potential issues (such as font size overrides) themes may experience anywhere, not even in the "Bundled theme updates with WordPress 5.9" article.

My opinion about !important has not changed though. I don't think the global CSS WP produces should override a theme styles, especially not with this "hacky" way. The theme should be able to override the WP out of the box. If the !important was used by WP specifically to fix issues with some 3rd party themes, it's the theme author's responsibility to change the theme code.

I'm in the process of updating all of my affected themes, so I will get more time to look into developer focused communication site feedback and especially into the actual WordPress 5.9 later. I really appreciate the work on WP 5.9 and was really looking forward to playing with it (especially Twenty Twenty-Two looks stunning!), but for now I'm stuck putting out the fire, which obviously deforms my WP5.9 experience, unfortunately.

webprom commented 2 years ago

The change by latest WP 5.9 of the default stylesheet is very bad, no doubt. I liked WP for back versions compatibility.

This is how I override new presets for my own styles: `/ my theme / h3, .has-large-font-size { font-size: 23px; --wp--preset--font-size--large: 23px; }

/ default wp css / .has-large-font-size { font-size: var(--wp--preset--font-size--large) !important;`

Works even when they use !important.

annezazu commented 2 years ago

@webmandesign hey! Thanks for the additional thoughts. Going to quickly try to share some ones in return as it was quite helpful to read through.

So I really must have read that article too fast and must have missed the !important in the global stylesheet code example or I've ignored it as it was not mentioned in the actual text of the article (it is only mentioned that !important is being removed from custom link color styles).

This all tells me that a developer focused communication site could really benefit from posts that apply Dev Note information and clarify how it might impact you rather than just stating concisely what's changed, how to adapt, etc. I think the conciseness of Dev Notes serve a purpose but can also create gaps like this.

Also, I'm pretty confused as I think you guys must have experienced similar issues we experience now while testing WP5.9 with older native themes such as Twenty Twenty or similar.

Ah yes! This did come up in a related way with default themes as you can see in this issue: https://github.com/WordPress/gutenberg/issues/37617 I hadn't thought about whether it should have been included with the Bundled theme updates with WordPress 5.9. That's a great call out and good to keep in mind (ie cross connecting dev notes to clarify impact and spread the word).

but for now I'm stuck putting out the fire, which obviously deforms my WP5.9 experience, unfortunately.

I hear you there. Thank you for taking the time to pause while putting out fires to share these thoughts.

johnstonphilip commented 2 years ago

@annezazu While a developer focused communication site might be beneficial, what is more important is that backwards compatibility should never ever, ever, ever, ever, be broken. If backwards compatibility is broken (like it was here), the best communications site in the world could not prevent the fallout of trust in WordPress.

chthonic-ds commented 2 years ago

While a developer focused communication site might be beneficial, what is more important is that backwards compatibility should never ever, ever, ever, ever, be broken. If backwards compatibility is broken (like it was here), the best communications site in the world could not prevent the fallout of trust in WordPress.

This is an important point and one that felt baked into WordPress pre-5.0. I share many of the experiences and frustrations in the commentary here, arising from the trade-off between the convenience of Gutenberg versus a loss of freedom to use WordPress as a blank canvas.

I love blocks and (the promise of) the new editor experience but the time investment required to keep on top of Gutenberg updates impacting theme development is extraordinary. The only way I've felt able to keep up is by reviewing open issues and PRs here on a daily basis. I'm thankful for Keeping up with Gutenberg as a broad digest of Gutenberg activity (thank you @bph! ~~to whoever compiles this!~~), even though it does feel a little like an ambulance at the bottom of the cliff.

annezazu commented 2 years ago

Thank you all for continuing to share here, including highlighting specific points. I'm listening and taking it all in.

I'm thankful for Keeping up with Gutenberg as a broad digest of Gutenberg activity (thank you to whoever compiles this!), even though it does feel a little like an ambulance at the bottom of the cliff.

This is mainly the effort of @bph at this point! <3

annezazu commented 2 years ago

Circling back here to close some loops. Specifically, want to share the following from this issue:

We've heard the conversation in the issue you point to and we'll be shipping in 5.9.1 a fix for classic themes that modify the defaults https://github.com/WordPress/wordpress-develop/pull/2233 The editor styles and the front will be the same.

I recommend reading the comment I linked for broader context too. Thank you all again for sharing feedback that helped lead to this change. There's lots to learn here for future situations, especially around how to approach preserving trust and backwards compatibility when moving features forward.

jnicol commented 2 years ago

Does the PR mentioned in the previous comment remove !important from declarations? If not then it will still cause problems for legacy themes.

annezazu commented 2 years ago

@jnicol No, it does not address that. That is being discussed in https://github.com/WordPress/gutenberg/issues/37590. The PR mentioned above addresses the issue here around font sizes. Further information about why !important is being used can be seen in this comment too: https://github.com/WordPress/gutenberg/issues/29985#issuecomment-814773798 Hope that helps clarify things!

maddisondesigns commented 2 years ago

Further information about why !important is being used can be seen in this comment too: https://github.com/WordPress/gutenberg/issues/29985#issuecomment-814773798 Hope that helps clarify things!

That comment states that...

Note that we discussed https://github.com/WordPress/gutenberg/pull/29533#discussion_r586724054 and thought using !important was simpler.

This may be simpler for the core Gutenberg devs, but it leaves it being a real pain for everyone else. By not removing the reliance on !important, it doesn't sound like 5.9.1 is going to do much to address this particular issue.

Ever since Gutenberg was added to core it's been an absolute pain trying to keep themes working properly, when every single release constantly changes the html structure and styles that it outputs. This is just another example of such. Keeping up with all these changes is near impossible, as others have clearly stated in all the previous comments.

webmandesign commented 2 years ago

Hi guys,

I've compiled the code above into a plugin https://github.com/webmandesign/global-styles-mods

After installation you can also copy the global-styles-mods--mu.php file into your ./wp-content/mu-plugins/ folder so it is used as a "must use" plugin - active on all sites in multi-site environment.

Plugin installation ZIP file can be obtained from https://github.com/webmandesign/global-styles-mods/releases

I've also submitted the plugin to WordPress plugins repository, so hopefully it will be available there too.

webmandesign commented 2 years ago

To whom it may help: plugin Global Styles Mods is out at https://wordpress.org/plugins/global-styles-mods/

maddisondesigns commented 2 years ago

This issue still wasn't fixed in 5.9.1

oandregal commented 2 years ago

The fix for this was ported to WordPress at https://github.com/WordPress/wordpress-develop/pull/2233 and it's part of 5.9.1 (core ticket).

WordPress / gutenberg