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

6.5: Maximum width of wide blocks inside group set to Alignment: Full is overwritten in classic themes #60413

Closed
andersnoren opened this issue Apr 3, 2024 · 19 comments · Fixed by #60489
Assignees
Labels
[Feature] Layout Layout block support, its UI controls, and style output. [Type] Bug An existing feature does not function as intended

Comments

@andersnoren
Copy link

andersnoren commented Apr 3, 2024

Description

In classic themes in WordPress 6.5, the maximum width of alignwide blocks are overwritten when they're direct descendants of a group block set to the "Full" alignment.

Many classic themes built after the block content editor was introduced set the maximum width of alignwide blocks with CSS, like this:

.alignwide { max-width: 1200px; }

When running a classic theme without theme.json in WordPress 6.5 and including a wide block inside a full group, Core outputs the following CSS even though the --wp--style--global--wide-size variable is undefined:

body .is-layout-constrained > .alignwide {
    max-width: var(--wp--style--global--wide-size);
}

This results in the alignwide maximum width set by the theme CSS being overwritten by the undefined variable, breaking the layout. I've replicated this in Twenty Twenty and in two of my classic themes (Chaplin and Eksell).

Expected behavior

The CSS above should not be output if --wp--style--global--wide-size is undefined. Given the high specificity of the CSS generated by Core, I'd expect this to affect many classic themes that set a maximum width for alignwide with CSS.

Step-by-step reproduction instructions

  1. Activate Twenty Twenty.
  2. Create a new post or page.
  3. Insert a group set to the full alignment, and inside that group, insert a group set to the wide alignment. Give them different background colors for visibility. You can also copy the code included below.
  4. The wide group will fill the parent .wp-block-group__inner-container div, instead of maxing out at the 120rem maximum width set by the Twenty Twenty CSS.
  5. Check the inspector to see that the theme CSS setting a maximum width for .alignwide is overwritten by the CSS generated by Core, included above.

Screenshots, screen recording, code snippet

WordPress 6.5 WordPress 6.4
localhost_8888_utsikt_wide-inside-full_ (3) localhost_8888_utsikt_wide-inside-full_ (4)

In the screenshots, the black group is set to full and the red is set to wide.

Block markup

<!-- wp:group {"align":"full","backgroundColor":"primary","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull has-primary-background-color has-background"><!-- wp:paragraph {"align":"center"} -->
<p class="has-text-align-center">Full group.</p>
<!-- /wp:paragraph -->

<!-- wp:group {"align":"wide","backgroundColor":"accent","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignwide has-accent-background-color has-background"><!-- wp:paragraph {"align":"center"} -->
<p class="has-text-align-center">Wide group.</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group -->

Environment info

WordPress 6.5
With or without Gutenberg active
Twenty Twenty 2.6

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

@andersnoren andersnoren added the [Type] Bug An existing feature does not function as intended label Apr 3, 2024
@Mamaduka Mamaduka added the [Feature] Layout Layout block support, its UI controls, and style output. label Apr 3, 2024
@annezazu annezazu moved this to ❓ Triage in WordPress 6.5 Editor Tasks Apr 3, 2024
@annezazu annezazu moved this to ❓ Triage in WordPress 6.5.x Editor Tasks Apr 3, 2024
@annezazu
Copy link
Contributor

annezazu commented Apr 3, 2024

Adding this to the 6.5.x board to triage. @tellthemachines @andrewserong is this something you can look into with the work you all have done around layout? @carolinan too for your thoughts as you're a wiz at this stuff :)

@tellthemachines
Copy link
Contributor

Oooh this is was caused by #56130. Thanks for the ping! I'll see what can be done to fix it.

@andrewserong
Copy link
Contributor

andrewserong commented Apr 4, 2024

+1 thanks for the ping!

Interestingly, running a 6.4 test site without Gutenberg active, I'm seeing the issue on that WP version, too, where the .alignwide rule from the TwentyTwenty theme is being overwritten (without the CSS variable being available) 🤔

image

I very well could be missing something in my test setup though!

@tellthemachines
Copy link
Contributor

@andrewserong is your alignwide block nested inside a Group block?

@andrewserong
Copy link
Contributor

andrewserong commented Apr 4, 2024

Yes! Here's the markup from my screenshot:

<!-- wp:group {"align":"full","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull"><!-- wp:cover {"overlayColor":"accent","isUserOverlayColor":true,"align":"wide","layout":{"type":"constrained"}} -->
<div class="wp-block-cover alignwide"><span aria-hidden="true" class="wp-block-cover__background has-accent-background-color has-background-dim-100 has-background-dim"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">A cover block</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover --></div>
<!-- /wp:group -->

@tellthemachines
Copy link
Contributor

And what does the generated markup look like? I'm unable to repro the issue in 6.4, and am seeing the expected markup for that version (layout classes in the Group outer container instead of the inner):

Screenshot 2024-04-04 at 12 21 05 PM

@andrewserong
Copy link
Contributor

My bad! I thought I'd deactivated Gutenberg but it must have failed for some reason. I've just re-tested on a fresh 6.4 install, and the problem only exists with the GB plugin activated, and 6.4 without Gutenberg is fine.

Gutenberg deactivated

image

Gutenberg activated

image

Sorry for that, and thanks for helping me confirm!

@annezazu annezazu moved this from ❓ Triage to 📥 Todo in WordPress 6.5.x Editor Tasks Apr 4, 2024
@annezazu
Copy link
Contributor

annezazu commented Apr 4, 2024

Going to list this as a "to do" item for 6.5.x in hopes we can perhaps get a fix. Thanks so much for reporting btw @andersnoren ❤️

@andrewserong
Copy link
Contributor

@tellthemachines, I just had an idea for a potential fix. Would it work to skip some of the base layout output if there's no content or wide size set in the global layout settings object? I was thinking something like the following (around line 1617 of class-wp-theme-json-gutenberg.php):

	// Skip rules that reference content size or wide size if they are not defined in the theme.json.
	if (
		is_string( $css_value ) &&
		( str_contains( $css_value, '--global--content-size' ) || str_contains( $css_value, '--global--wide-size' ) ) &&
		! isset( $this->theme_json['settings']['layout']['contentSize'] ) &&
		! isset( $this->theme_json['settings']['layout']['wideSize'] )
	) {
		continue;
	}

I can put that into a draft PR if you like, but I don't feel at all strongly about it if you had other ideas for a fix 🙂

@tellthemachines
Copy link
Contributor

@andrewserong that should work, yes, thanks!

@wolffe
Copy link

wolffe commented May 8, 2024

This update broke hundreds of websites.

I have this in my :root in all my classic themes:

--content_width: <?php echo get_option( 'content_width' ); ?>px;
--global--content-size: <?php echo get_option( 'content_width' ); ?>px;
--wp--style--global--content-size: <?php echo get_option( 'content_width' ); ?>px;
--global--wide-size: 1440px;
--wp--style--global--wide-size: 1440px;

I also have these in my after_setup_theme filter:

add_theme_support( 'align-wide' );
add_theme_support( 'responsive-embeds' );

add_theme_support( 'wp-block-styles' );
add_theme_support( 'custom-line-height' );
add_theme_support( 'custom-spacing' );
add_theme_support( 'block-templates' );
add_theme_support( 'appearance-tools' );

add_theme_support( 'dark-editor-style' );

add_theme_support( 'link-color' );
add_theme_support( 'border' );

How do I fix it? What rules do I need to add to my CSS stylesheet?

@annezazu
Copy link
Contributor

annezazu commented May 8, 2024

@wolffe this should be fixed in WordPress 6.5.3 which was released yesterday. Can you upgrade and confirm if this remains a problem?

@wolffe
Copy link

wolffe commented May 8, 2024

@wolffe this should be fixed in WordPress 6.5.3 which was released yesterday. Can you upgrade and confirm if this remains a problem?

No, this was has been working for years, and WordPress 6.5.3 broke hundreds of sites I manage.

I had to manually add CSS to my theme to bring it back to the way it was before.

@andrewserong
Copy link
Contributor

Oh, what a fascinating problem, thanks for bringing this up @wolffe.

This update broke hundreds of websites.

Are there particular things that broke, that could help debug what's happened?

My understanding is that the content and wide size CSS variables are controlled ones from the perspective of WordPress' output, and so the logic that was recently introduced (to resolve other bugs) was to only output rules that depend on those variables if those variables are being output. The canonical way to ensure those rules are output is to include a small theme.json file that specifies contentSize and wideSize. For an example, see this page in the documentation: https://developer.wordpress.org/block-editor/how-to-guides/themes/global-settings-and-styles/#settings. theme.json files can happily be added to Classic themes without having to also introduce block based templates, but the decision there could depend on the requirements of your theme.

If you're looking for a short-term solution to restore the behaviour you were expecting for this particular case, another potential option could be to manually add within your theme the rules that are no longer being output. In this case, I believe that would be something like:

.is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {
    max-width: var(--wp--style--global--content-size);
    margin-left: auto !important;
    margin-right: auto !important;
}

.is-layout-constrained > .alignwide {
    max-width: var(--wp--style--global--wide-size);
}

However, if your theme depends on content and wide sizes, I'd recommend exploring adopting theme.json.

If you're looking for a reference of the kinds of rules that get output for layout styles in general (in case there are additional rules required for your theme), you might be interested in checking the layout definitions defined in core here: https://github.com/WordPress/wordpress-develop/blob/5466d3a9a72b38039ded378ae0f7331e8ba32a3f/src/wp-includes/block-supports/layout.php#L21


All that said, that's just my perspective as one contributor in this area. I'll also ping @tellthemachines for visibility.

@tellthemachines
Copy link
Contributor

Thanks for the ping! As @andrewserong mentioned above, --wp--style--global--content-size (as all variables starting with --wp--) are for internal use, and should be set via theme.json. If there is no theme.json, those styles shouldn't be output because there's a high probability of conflict with classic theme styles.

@wolffe
Copy link

wolffe commented May 9, 2024

After updating to WordPress 6.5.3, I cannot have a constrained block inside a full width cover or group. I cannot do what's illustrated in the image below:

breaking-out-of-a-central-wrapper-02

I have a classic theme, and I am not interested in switching to a block theme, or FSE, or Site Editor, or anything like that.

My only option right now is to switch back to 6.5.2, get the CSS styles and hardcode them into my theme. But I shouldn't need to do this.

Is it going to be impossible for a classic theme to have constrained blocks inside full width covers or groups?

@andrewserong
Copy link
Contributor

After updating to WordPress 6.5.3, I cannot have a constrained block inside a full width cover or group. I cannot do what's illustrated in the image below:

Unfortunately I'm not sure I understand what the underlying issue is here. From testing a few of the different core Classic themes, that kind of layout appears to be achievable with how those themes implement their styling and theme supports. E.g. here's using full and wide width Cover blocks in Twenty Twenty theme:

image

As a result, my hunch is that the theme(s) in question are depending on parts of the behaviour of global styles rather than providing their own rules (which I believe is the traditional way that Classic themes handle content widths, etc).

I have a classic theme, and I am not interested in switching to a block theme, or FSE, or Site Editor, or anything like that.

Adopting a theme.json file for some behaviour that you wish to use doesn't mean that you need to switch to a block theme. For more information on this, please see the theme handbook documentation for hybrid themes: https://developer.wordpress.org/themes/getting-started/what-is-a-theme/#hybrid-themes. Essentially, you can add a theme.json file to a Classic theme, and it remains a classic theme without needing to adopt the site editor.

For example, in the root directory of a theme, a simple theme.json file that defines layout content and wide sizes could be added that looks like the following (the appearanceTools line there is optional, but I see your example included it, so I've added it here, too):

{
    "$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 3,
	"settings": {
		"appearanceTools": true,
		"layout": {
			"contentSize": "840px",
			"wideSize": "1100px"
		}
	}
}

Adding that file wouldn't convert the theme into being a block theme, the site editor will still be inaccessible, but the layout content and wide sizes (and the styles that use them) would now be output.

While the concept of hybrid themes isn't a formal distinction, it's a pragmatic term to capture just this kind use case, where a classic theme exists, but there's an aspect of how global styles works that's still useful for such a theme. From the docs:

This is a widely agreed-upon term by the community, but it is not an “official” theme type. At the end of the day, hybrids are still classic themes.

Hope that helps!

@wolffe
Copy link

wolffe commented May 10, 2024

The problem is that the styles that were there previously (for years, I'd say) have been removed in WordPress 6.5.3.

The solution is for me to add those styles, but then what's the point of having a full-width block which is not working out of the box? Also, why was this removal needed?

@andrewserong
Copy link
Contributor

I understand the change here is causing an issue for themes that have been developed in that way. From what you've described so far, unfortunately the approach that's been used in those themes does not match how the layout support is intended to work.

The problem is that the styles that were there previously (for years, I'd say) have been removed in WordPress 6.5.3.

This is a nuanced issue, but this isn't exactly the case. The logic has changed in terms of when the styles are output, and the styles that were output depend on particular CSS variables that are generated by core. The fix in #60489 resolved a different conflict that also affected many themes.

The solution is for me to add those styles, but then what's the point of having a full-width block which is not working out of the box?

It works out of the box for classic and block themes that define content and wide sizes via a theme.json file. Other classic themes (such as TwentyTwenty, etc) provide their own rules rather than depending on the layout block support's styles.

Also, why was this removal needed?

Unfortunately, I'm not sure I have too much to add here beyond the current issue description and the linked PR (#60489) that skipped outputting the rules, as I believe they cover most of the context. WordPress 6.5 moved to applying the layout classnames where they're intended to be applied (the direct wrapper for inner blocks) in #56130, and this issue's description describes the bug that resulted. This exposed that the layout styles that reference CSS variables were being output when they shouldn't (for themes that aren't defining content and wide sizes), which suggested the path forward of #60489.

The recommended solution, if you're wishing to use the rules output by core, is to add a theme.json file as in my previous comment: #60413 (comment).

If there are further issues or logic that should tweaked, do feel free to open an issue. Getting layout styles to work properly in all expected situations is particularly challenging, and it's very helpful to have more context and details on how the features are being used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Layout Layout block support, its UI controls, and style output. [Type] Bug An existing feature does not function as intended
Projects
7 participants