Layout: use CSS logical properties for margin top and bottom #38816
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
ramonjd
added
[Type] Enhancement
|
Size Change: +3 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
|
In my projects, I've been using an implementation of this in my own overrides of the Render Layout Support portion of the block supports API for months now across about a dozen or so production FSE sites in the wild and they've all worked flawlessly. It's implementation makes sense because:
|
ramonjd
force-pushed
the
try/layout-margin-css-logical-property
branch
from
112dce8
to
149a2c8
Compare
ramonjd
commented
Feb 17, 2022
ramonjd
commented
Feb 17, 2022
This was referenced Feb 18, 2022
ramonjd
force-pushed
the
try/layout-margin-css-logical-property
branch
from
e598999
to
e242394
Compare
ramonjd
commented
Feb 22, 2022
| $has_block_gap_support = _wp_array_get( $this->theme_json, array( 'settings', 'spacing', 'blockGap' ) ) !== null; | ||
| if ( $has_block_gap_support ) { | ||
| $block_rules .= '.wp-site-blocks > * { margin-block-start: 0; margin-block-end: 0; }'; | ||
| $block_rules .= '.wp-site-blocks > * + * { margin-block-start: var( --wp--style--block-gap ); }'; |
There was a problem hiding this comment.
This method was imported to redefine these styles.
ramonjd
requested review from
youknowriad,
ntsekouras,
oandregal and
andrewserong
ramonjd
requested review from
TimothyBJacobs,
spacedmonkey and
ellatrix
as code owners
There was a problem hiding this comment.
Thanks for working on this @ramonjd, this looks like a good path forward for supporting different languages and writing modes in our layout supports 👍
I couldn't find any issues immediately with this PR applied, except that when I switched writing-mode: vertical-rl on within the front end of my site running TT2, the margins then behave differently to how I'd expect. While the paragraphs and spacing rendered correctly in a vertical layout, the horizontal margins that were aligning the block centred in the page, now cause the block to display at the left side of the page:
| Before applying writing mode | After applying writing mode |
|---|---|
 |
 |
Did you encounter anything like that in your testing? I manually applied the CSS property in Dev Tools to the group block.
Update: I think that's because of margin-block-start and margin-block-end being set to 0 in the vertical writing mode. If they're auto then I can get it centred again?
So I wonder if there's a limit to how many of the rules we want to be logical (and in which circumstances), versus physical? E.g. in vertical writing mode, how much of the layout do we want it to automatically change?
Thanks for testing @andrewserong and for pointing out the inconsistency there. This one was always going to be cutting edge, given that there's no immediate requirement for CSS logical properties and seeing as we don't officially support It was really meant as a stepping stone towards getting full support if we ever refactor layout, and also to set theme authors on an easier path to their own writing mode implementations.
This makes sense to me. I didn't see this when testing, but I agree it's a bit strange. I'll have another look to try and get it working more intuitively. Thanks again! |
kevin940726
approved these changes
Mar 9, 2022
…t-to-right, vertical and so on. With CSS logical properties, block and inline margins will apply to the appropriate side depending on the direction of the document flow. This change to the layout margins ensures that margins will adhere to the logic of current flow. For example, margin-block-start (instead of margin-top) will manifest itself as a top margin for `writing-mode: horizontal-tb;` but a right margin for `writing-mode: vertical-rl;`.
…ht. Other minor formatting Taking a clumsy stab at compat files so we can load the global styles and settings.
Removing `get_stylesheet()` from WP_Theme_JSON_Gutenberg
…l page layout (left and right margins) so that any changes to the writing mode of blocks within wp-site-blocks and block containers still align themselves according to current horizontal rules.
ramonjd
force-pushed
the
try/layout-margin-css-logical-property
branch
from
f9acb32
to
0616be3
Compare
ramonjd
changed the title
|
Thanks for testing @kevin940726 !
@andrewserong The PR is now limited to swapping The right and left values I've left alone to preserve centered layouts even in alternative writing-modes.
I've rebased and tested the changes again. |
andrewserong
approved these changes
Mar 10, 2022
There was a problem hiding this comment.
@andrewserong The PR is now limited to swapping margin-top and margin-bottom for margin-block-start and margin-block-end respectively.
Thanks for the ping @ramonjd! Yes, that's fixed it up for me so the centered styling is preserved now in testing:
I've given this a re-test with a few different blocks, and tested in TT2 and looked for regressions in TT1 and TwentyTwenty themes, and couldn't find any issues 🙂
This LGTM, and we can always look into follow-ups if needed for #38319 down the track!
ramonjd
added a commit
that referenced
this pull request
Mar 31, 2022
…ECTOR` made it in. It should be static to retain the inheritance powers introduced in #38671
ramonjd
added a commit
that referenced
this pull request
Mar 31, 2022
…ECTOR` made it in. It should be static to retain the inheritance powers introduced in #38671
36 tasks
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.
Depends on
Description
The layout implementation doesn't take into account different writing modes, such as left-to-right, vertical and so on.
With CSS logical properties, block and inline margins will apply to the appropriate side of an element depending on the direction of the document flow.
This change to the layout margins ensures that margins will adhere to the logic of the current flow.
Related PR:
For example, margin-block-start (instead of margin-top) will manifest itself as a top margin for
writing-mode: horizontal-tb;but a right margin forwriting-mode: vertical-rl;.With writing-mode: horizontal-tb;
With writing-mode: vertical-rl;
Trunk:
This branch:
TODO
Why do this?
As mentioned over at #38694 (comment), this change will make it easier for theme authors to style block content using alternative
writing-modes.Testing Instructions
Test both in WordPress 5.8 and WordPress 5.9.
Open the editor and add a group block with several children. Here is some test code:
Test block code
Play around with the gap value, then publish the post and take a squiz at the frontend.
Check that the gap values between the group's child elements (margin top) appear as expected.
Using the browser inspector, add
writing-mode: vertical-rl;to the group container div.Check that the gap values between the group's child elements (logical margin top (right)) appear as expected.
Use a theme that opts into
blockGapsuch as TwentyTwentyTwo.Check that the gap between site blocks are uneffected.
For block themes,
.wp-site-blocksshould have an inline class ofwp-site-blocks > * + * { margin-block-start: var( --wp--style--block-gap );Run the tests!
Types of changes
CSS enhancement. Future compatibility.
Checklist:
*.native.jsfiles for terms that need renaming or removal).