Layout: Fix has-global-padding classname for constrained layouts without contentSize #43689
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewserong
added
[Type] Bug
|
Size Change: +7 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
mikachan
approved these changes
Aug 30, 2022
There was a problem hiding this comment.
Thanks so much for the quick fix!
This is working great for me. I've walked through the testing steps:
- With
settings.useRootPaddingAwareAlignmentsset to false, I do not see thehas-global-paddingclass for the group block:
- With
settings.useRootPaddingAwareAlignmentsset to true and the inner blocks set to respect the content width (constrained layout type), I can see thehas-global-paddingclass:
- With
settings.useRootPaddingAwareAlignmentsset to true and the inner blocks set to not respect the content width, I do not see the class:
I tested the above using Twenty Twenty-Three and this PR.
|
I can confirm that I am getting the same results as @mikachan. Thanks @andrewserong for getting this together so quickly 🙌 |
|
LGTM! |
andrewserong
deleted the
fix/has-global-padding-classname-on-constrained-layouts-without-content-size
branch
andrewserong
added
the
Backport to Gutenberg Minor Release
|
Thanks for the reviews, folks! I've just merged. @MaggieCabrera it looks like you're managing the current Gutenberg release? @bgardner mentioned above that it'd be good to try to get this one in for 14.0, but I wasn't sure if there's still time to cherry pick this one in. (I'm travelling at the moment, so am a bit limited time-wise, but otherwise I'd normally jump in to help out with the cherry-picking! 🙂) |
Yes, I'm adding this to the release, don't worry |
|
Excellent, thank you Maggie! 🙇 |
MaggieCabrera
pushed a commit
that referenced
this pull request
Aug 31, 2022
…out content size (#43689)
|
Apologies, all, I just realised that I should have added an |
priethor
removed
the
Backport to Gutenberg Minor Release
andrewserong
added
the
[Feature] Layout
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes #43673
For the constrained layout type, if
settings.useRootPaddingAwareAlignmentsis set totruethen ensure thathas-global-classnameis always output irrespective of whether or notcontentSizeis explicitly set.Why?
Prior to the introduction of the constrained layout type, there was always a
contentSizevalue if content sizes were in use — however with the new constrained layout type, if no value is set, then it defaults to outputting rules using the global CSS variable, so the logic for when to output thehas-global-paddingclassname needed to be updated.How?
layout.phpalways outputhas-global-paddingif the inferred layout type isconstrainedand the root padding aware alignments flag is set totruelayout.jsupdate the logic for outputtinghas-global-paddingto include aconstrainedlayout type check.Testing Instructions
settings.useRootPaddingAwareAlignmentstofalseand ensure that thehas-global-paddingclassname is not output for the Group block, irrespective of whether or not it has a contentSize appliedsettings.useRootPaddingAwareAlignmentstotrue, and so long as theInner blocks respect content widthtoggle is switched on (the constrained layout type is in use), then thehas-global-paddingclassname should be output, irrespective of whether or not a customcontentSizeis providedScreenshots or screencast
With this PR applied, classnames should be output as expected: