Make WordPress Core

Opened 6 months ago

Closed 6 months ago

Last modified 5 months ago

#60981 closed defect (bug) (fixed)

6.5 adds the "is-layout-constrained" class to the wrong place for classic themes

Reported by: evanltd's profile evanltd Owned by:
Milestone: 6.5.3 Priority: normal
Severity: normal Version: 6.6
Component: Editor Keywords: gutenberg-merge has-patch has-unit-tests fixed-major dev-reviewed
Focuses: css Cc:

Description

Prior to 6.5, when using a classic theme that does not include a theme.json file, the frontend html output of a group block looked like this:

<div class="wp-block-group is-layout-constrained wp-block-group-is-layout-constrained">
     <div class="wp-block-group__inner-container">
</div>
</div>

After 6.5, the frontend output of a group block looks like this:

<div class="wp-block-group">
     <div class="wp-block-group__inner-container is-layout-constrained wp-block-group-is-layout-constrained">     </div>
</div>

Note how the is-layout-constrained classes moved from the outer div to the inner div.
Yes I have verified the difference with a fresh install of WP.

This causes a cascade of problems:

This WP global inline CSS rule, which perviously only applied to the inner container itself, now gets undesirably applied to EVERY (non-aligned) regular child element inside a group div:

body .is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {
margin-left: auto !important;
margin-right: auto !important;

The CSS rule is awkward and difficult to override due to the !important flags.
Completely disabling the global inline CSS output is not a great solution, as we still want to retain parts of it.
We have no option to remove the is-layout-constrained class in the post editor. (Themes that do not include a theme.json + layout data, do not display the Layout settings pane at all in the editor).

This 6.5 change has broken layouts in all of my sites that use the group block. I have not verified whether other block types are affected.

Change History (27)

#1 @poena
6 months ago

  • Component changed from Themes to Editor
  • Focuses css added

Hi @evanltd and welcome to WordPress Trac.

I believe moving the class names was an intentional change to support block gap in classic themes, added in https://github.com/WordPress/gutenberg/pull/56130

This should ideally have been mentioned in a dev note for 6.5, because it was known that it could be a breaking change for themes with custom CSS.

I am going to ping @isabel_brison and @andrewserong because it may also be related to
https://core.trac.wordpress.org/ticket/60936.

#2 @isabel_brison
6 months ago

The classnames moving shouldn't have had an impact on classic themes at all, outside of fixing the issue that stopped appearance tools from working correctly, but unfortunately that rule was still being output.

I believe this should be fixed on trunk, because the fix for #60936 has been committed already. @evanltd would you be able to confirm that? The fix is also due to be released with the next minor version.

#3 @evanltd
6 months ago

It does sound somewhat related to 60936, but that one appears to be focused around issues with the max-width declaration, whereas my issue here is specifically with the margin-left and margin-right declarations. I tried out 6.5.3-alpha-57980 and do not see a difference in the CSS or HTML output for the group block (sorry if that's not what you meant for me to test, I'm new at this). Thanks.

#4 @isabel_brison
6 months ago

  • Version changed from 6.5 to trunk

@evanltd you're right, I can still see the margins being output. I think we should try to remove that rule altogether for classic themes without a theme.json. Looking into it!

#5 @isabel_brison
6 months ago

  • Keywords gutenberg-merge added
  • Milestone changed from Awaiting Review to 6.5.3

This ticket was mentioned in Slack in #core by jorbin. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


6 months ago

This ticket was mentioned in PR #6410 on WordPress/wordpress-develop by @isabel_brison.


6 months ago
#8

  • Keywords has-patch has-unit-tests added

… theme.json.

Trac ticket: https://core.trac.wordpress.org/ticket/60981

Syncs the changes from https://github.com/WordPress/gutenberg/pull/60764 to core and updates the relevant test string.

#9 @isabel_brison
6 months ago

In 58028:

Editor: limit layout rules on themes without theme.json.

Removes output of base rules for flow and constrained layout types on themes without theme.json.

Props evanltd, poena, isabel_brison, andrewserong, oandregal.
See #60981.

@isabel_brison commented on PR #6410:


6 months ago
#10

Committed to trunk in r58028.

#11 @isabel_brison
6 months ago

  • Keywords dev-feedback added

Leaving open for commit to the release branch pending second committer review.

#13 @isabel_brison
6 months ago

In 58030:

Editor: fix spacing in function doc.

Correctly formats spacing in get_layout_styles docblock.

Props mukesh27, sabernhardt.
See #60981.

@isabel_brison commented on PR #6425:


6 months ago
#14

Thanks for the fix! Committed in r58030.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


6 months ago

#16 @jorbin
6 months ago

  • Keywords fixed-major added

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


6 months ago

#19 @jorbin
6 months ago

  • Keywords dev-reviewed added; dev-feedback removed

[58028] and [58030] look good for backport.

That said, one minor nit that doesn't affect backporting or the actual implemention. test_get_stylesheet_generates_base_fallback_gap_layout_styles should get an additional @ticket for this ticket. That can go in trunk along though

Version 0, edited 6 months ago by jorbin (next)

This ticket was mentioned in PR #6458 on WordPress/wordpress-develop by @isabel_brison.


6 months ago
#20

Trac ticket: https://core.trac.wordpress.org/ticket/60981

Follow-up to this commit; adds ticket number to the updated test.

#21 @isabel_brison
6 months ago

In 58057:

Editor: limit layout rules on themes without theme.json.

Removes output of base rules for flow and constrained layout types on themes without theme.json.

Props evanltd, poena, isabel_brison, andrewserong, oandregal.
Reviewed by jorbin.
Merges [58028] to the 6.5 branch.
See #60981.

#22 @isabel_brison
6 months ago

In 58058:

Editor: fix spacing in function doc.

Correctly formats spacing in get_layout_styles docblock.

Props mukesh27, sabernhardt.
Reviewed by jorbin.
Merges [58030] to the 6.5 branch.
See #60981.

#23 @isabel_brison
6 months ago

In 58059:

Editor: add ticket number to test after string update.

Follow-up to [58028]; adds ticket number to the updated test.

Props jorbin, isabel_brison.
See #60981.

@isabel_brison commented on PR #6458:


6 months ago
#24

Committed to trunk in r58059.

#25 @jorbin
6 months ago

  • Resolution set to fixed
  • Status changed from new to closed

With the backports complete, marking this as fixed.

#26 @evanltd
5 months ago

The fix is working well on my sites. Thank you all for your quick attention and effort on this!

#27 @rickiwylder
5 months ago

What's the opinion on hybrid themes? We're using a hybrid setup on all sites which leverages the power of building with blocks but uses custom width settings and a classic theme setup in other aspects. Which means we're using theme.json without the layout setting. A common use case is using a group with a custom width setting inside another group. Now we have to deal with

body .is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {
    margin-left: auto !important;
    margin-right: auto !important;
}

on these nested groups everywhere. Can we opt out of this in any way? Disabling layout styles isn't really an option as evanltd mentioned because it seems that we lose to much functionality. But forcing margins, especially with !important is not nice. Maybe what I'm looking for is a more granular approach of disable/enable styles or a setting on blocks to disable "constrained" (or set default layout type to default or custom)?

This piece of CSS is impossible to get around (even if we could add CSS with !important to maybe overwrite the !importants we can't remove the margins, which is needed) so our current solution is to use both PHP (render_block()) and JS (editor) to force the alignfull class on all group blocks. But it feels cumbersome and should not be needed (?).

Note: See TracTickets for help on using tickets.