VisualEditor: Always output has-global-padding classname when in post only mode #66626
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
… only mode and the site supports root padding aware alignments
andrewserong
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +8 B (0%) Total Size: 1.81 MB
ℹ️ View Unchanged
|
ramonjd
reviewed
Oct 31, 2024
| : `${ blockListLayoutClass } wp-block-post-content` // Ensure root level blocks receive default/flow blockGap styling rules. | ||
| : `${ blockListLayoutClass } wp-block-post-content`, // Ensure root level blocks receive default/flow blockGap styling rules. | ||
| { | ||
| 'has-global-padding': |
There was a problem hiding this comment.
Just to confirm, this is to make things prettier when writing posts (without template preview?)
Things are working as expected for me.
I can see that has-global-padding is on the root container in the editor, even if I switched "Inner blocks use content width" off for the content block:
| Before | After |
|---|---|
 |
 |
I tested this also on TT4 and could see the same thing.
There was a problem hiding this comment.
Thanks for taking this for a spin!
Just to confirm, this is to make things prettier when writing posts (without template preview?)
Yep, that's the goal! Feedback from @jasmussen has been that we should never (or possibly almost never) have edge-to-edge content in the abstracted editor (without template preview), so this PR helps address a particular case where there was missing padding.
Ever since the editors were unified and folks can toggle template preview, there's a little less pressure for the editor to try to be WYSIWYG, as that's what template preview is for. So, we can afford to be a little less WYSIWYG to optimise for the writing experience / things looking neat.
But let's see what the designers think 🙂
There was a problem hiding this comment.
Great thanks, LGTM in that case
|
Nice one! This fixes it for me:
I'll defer to @ramonjd on the technical nuance. But for me this makes it match editor and frontend. Now the only next thing I'd love fixed, if we can in 6.7, for making TT5 shine, is this one: #65969, I know there's a wonderful PR underway, and anything we can do to help that land would help. 🙏 |
ramonjd
approved these changes
Oct 31, 2024
|
Thanks folks! I'll merge this PR in now. I haven't flagged it for backporting to 6.7 — my weakly held opinion is that it can be tricky to make changes to some of these rules surrounding the visual editor, and this PR addresses a fairly particular concern that was flagged, but the issue has been around since prior to 6.7. At the current point in the 6.7 cycle, I think it'd make sense to try this out in the plugin and see if we run into any issues, rather than attempt to get it in during the RC cycle of 6.7. Do let me know if anyone feels differently, though!
Thanks for flagging! That PR was looking good to me, too, and I think is less likely to result in unintentional side effects. I'll see what we can do to get it merged 👍 |
andrewserong
deleted the
fix/always-output-global-padding-classname-on-post-content-area-in-abstracted-editor
branch
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
… only mode and the site supports root padding aware alignments (WordPress#66626) Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
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?
In the abstracted post editor (i.e. editing a post or page with template preview switched off), always output the
has-global-paddingclassname for the post content area, irrespective of the layout type in use (but only if the theme supports it).Why?
This is kind of similar to #66352 in some ways but addresses a slightly different problem.
Something that @jasmussen ran into while testing TT5 theme is that depending on the template, you can be in a situation where the template uses the global padding on a wrapper Group block, and the Content block is set to flow/default alignment. On the site frontend, this will mean that visually, there is some global padding being applied around the content area.
However in the abstracted post editor, there is no such wrapper block, and so the content can render to the very edge of the viewport, unexpectedly.
This PR proposes always applying the
has-global-paddingclassname so that if the site uses the global padding it applies in the abstracted post editor. Given that folks can switch on the template preview mode if they want a more WYSIWYG look, the assumption here is that it's better to optimise for things looking good for the writing experience.How?
In the
VisualEditor, output thehas-global-paddingclass name if the theme supports root padding aware alignments, we're in the post only mode, and the entity being edited is not a design one (pattern, template part, etc)Testing Instructions
This could be a little tricky to reproduce! It took me a while before I ran into the issue 😄
trunkif you use the preview in the top right and select Mobile, you'll likely see that the content unexpectedly goes edge-to-edge in the previewScreenshots or screencast