-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix incorrect targeting of block wrappers for root padding in the edi… #48002
Conversation
Size Change: +531 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 1134fe7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4169875260
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, what a challenging problem! I like the direction of the fix, but a challenge is that the JS version of the global styles output only exists in the site editor and not the post editor. Within the post editor, the PHP version of the global styles rules is output in that context instead, so the fix doesn't appear to be present there:
Site editor:
Post editor:
Unfortunately I can't quite think of a universal way we could fix the site-frontend-facing rule so that it'd also work in the post editor 🤔
Maybe we could switch to using the JS version in the post editor instead? Or we could implement #6639 😂 Or we could remove Any of these solutions seems like overkill for such a small bug though. I'll try fiddling with the PHP selectors a bit more. |
I think that's fixed it. It should be fine to add the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one, that's done the trick @tellthemachines! The rules are applying consistently for me now in both the post and site editors, and on the site frontend 👍
Just left a tiny nit about whether we should ensure the rules are the same between the PHP and JS implementations, but otherwise this LGTM! ✨
packages/block-editor/src/components/global-styles/use-global-styles-output.js
Show resolved
Hide resolved
Awesome, thank you, @tellthemachines! |
While comparing theme.json classes in Gutenberg and WordPress core I've noticed this PR was missing in core. I understand we want to backport this, so I'm tagging it as such. (I see this PR requires a release of the JS packages. I don't know how we want to manage that, otherwise, I'd prepare the backport myself for timezones to play to our advantage). |
@oandregal, as the development diverges, the class won't always match in core and plugin. If the bug was introduced in WP 6.2, then we should be good to land a fix in RC1. |
Absolutely. If something is slated to 6.3, it's fine they are different. I don't know whether this is for 6.2 or 6.3, though, hence why I raised :) We missed important backports in the 6.1 cycle, and I don't want us to repeat the same mistake in 6.2. As a general note, until we can find a better (automated) mechanism, we should aim for the theme.json classes in Gutenberg and core to be the same (other than minor things such as |
This bug wasn't introduced in 6.2; it's been there since the root padding logic was built. As we were already in Beta at the point this was merged, I didn't bother adding the backport label. Though if folks feel it's important to fix for this release anyway, I'm happy to do a backport PR for the PHP changes! |
What?
Fixes #46724
Why?
In the editor, blocks are given classnames such as
wp-block
,block-editor-block-list__block
andblock-editor-block-list__layout
, that aren't carried through to the front end. Because of this, the rules added byuseRootPaddingAwareAlignments
have to be slightly different for editor and front end.Problem: in the front end, there is no generic class applied to all block wrappers, such as
wp-block
, so outer block wrappers had to be targeted with[class*="wp-block-"]:not([class*="__"])
(where thenot
part rules out any inner wrapper with a classname starting withwp-block
such aswp-block-cover__inner-container
). This creates a problem in the editor, because all block wrappers have a classname with a double underscore such asblock-editor-block-list__block
.But because the
wp-block
class is also applied to all block wrappers in the editor, we can target that instead.Testing Instructions
"useRootPaddingAwareAlignments": true
in its theme.json e.g. Twenty Twenty Three.Testing Instructions for Keyboard
Screenshots or screencast