Try fixing post editor layout for wide/full Post Content blocks #49565
Conversation
tellthemachines
added
[Type] Bug
|
Size Change: +2.12 kB (0%) Total Size: 1.35 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for putting together a fix so quickly! I think the direction here is a good pragmatic fix for the reported issue, and helps get the UI closer to what's expected. As you mention, so long as the post editor is separate from the template, it'll always be a little different to how the content will appear on the site frontend, but this gets it much closer for this case 👍
Just left a comment about post content blocks that have wide alignment as well as using constrained layout, and whether we should check that we're using the default layout type before outputting the align class and styles.
Another subtle issue, that could very well not be worth attempting to fix in this PR, is that the blocks within the editor canvas report the None alignment as Max 840x wide. However in this context, everything will be at either wide width or full width.
Aside from those two issues, this is otherwise testing great for me!
| @@ -272,7 +272,8 @@ export default function VisualEditor( { styles } ) { | |||
| { | |||
| 'is-layout-flow': ! themeSupportsLayout, | |||
| }, | |||
| themeSupportsLayout && postContentLayoutClasses | |||
| themeSupportsLayout && postContentLayoutClasses, | |||
| align && `align${ align }` | |||
There was a problem hiding this comment.
I know this is likely pretty edge-casey, but what about the case where the wide post content block has the constrained layout type (or inherit: true)? In that case, in the site editor, the preview of the post content block will visually look like the content is constrained to the contentSize, but when opening up the page editor, the alignwide rules take precedence.
To avoid that, I was wondering if we could add a variable to use before outputting the classname and styles. E.g. something along the lines of:
const { layout = {}, align = '' } = newestPostContentAttributes || {};
const outputAlignmentStyles = !! ( align && ( ! layout.type || layout.type === 'default' ) );
And then use that in this line and before outputting <LayoutStyle... below?
There was a problem hiding this comment.
Well spotted! I think we should still output the alignment styles though, because if the Post Content is alignwide none of its children will ever be wider than that, so it helps to have the content max width set.
Instead, I'm going to try scoping the rules affecting Post Content children to when Post Content itself is is-layout-flow.
There was a problem hiding this comment.
Instead, I'm going to try scoping the rules affecting Post Content children to when Post Content itself is is-layout-flow.
Nice, that helps keep the code simpler, too 👍
|
Flaky tests detected in d3b8b67. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4624343665
|
|
Testing this, it looks to me to address the issue of ensuring that the post content in the post editor reflects post content block alignment. I've given then post content block in the singular post template an alignment value of "full". Here's what I see in the post editor: Before
After
I have a question, and I'm not sure if this is a valid one. I was fiddling with alignments on the post content block in the template editor. E.g., I gave the block a right alignment:
In trunk I'm seeing this in the post editor, which matches what I see on the frontend:
On this branch however, the alignment is being overridden, I think by this line of CSS
And the post editor no longer appears as it does on the frontend, that is, right-aligned. |
|
Thanks for the feedback folks!
Ah yes, that should now be fixed too! It was a consequence of the issue Andy pointed out here.
I think that might be good to address separately because it's really an existing issue: children of blocks with flow layout still get that |
There was a problem hiding this comment.
Thanks for the updates, this is testing well for me now! Nice work coming up with an elegant CSS fix 👍
✅ The original issue of displaying flow layouts for a post content block set to wide is still working well
✅ When the post content block is set to a constrained layout type, the alignments are working now
✅ The alignwide and alignfull rules are being correctly applied, as you mention, this ensures that if a post content block is set to wide that the full size cannot be larger than the wide size:
I think that might be good to address separately because it's really an existing issue: children of blocks with flow layout still get that max [content width] wide label even though their width with no alignment will be the same as that of their parent, which could be anything.
I agree, a separate PR sounds good — it sounds like hiding the description of "none" for children of flow layouts is a good way to go 👍
LGTM! ✨
andrewserong
added
the
Backport to WP Minor Release
|
@tellthemachines, @andrewserong, I'm working on backports for WP 6.2.1, but can't cleanly cherry-pick this change. I believe the fix here depends on changes from #45299, which is still experimental. How crucial is it that we ship this fix in the minor release? Can a similar fix be applied on the The WP 6.2.1-RC is scheduled for tomorrow. |
|
Thanks for the feedback, @tellthemachines! I will remove the backport label. |
Mamaduka
removed
the
Backport to WP Minor Release
| @@ -327,6 +328,12 @@ export default function VisualEditor( { styles } ) { | |||
| [ styles ] | |||
| ); | |||
|
|
|||
| // Add some styles for alignwide/alignfull Post Content and its children. | |||
| const alignCSS = `.is-root-container.alignwide { max-width: var(--wp--style--global--wide-size); margin-left: auto; margin-right: auto;} | |||
There was a problem hiding this comment.
This is so weird, I don't really understand why we need this code, any explanations?
There was a problem hiding this comment.
That was added to address this comment. Post Content layout in the front end is influenced by both its layout and alignment; this was an attempt make the post editor accurately reflect them.
There was a problem hiding this comment.
So are you saying that we try to apply the "alignment" of the post content block in the post editor? For me that seems a bit random because we don't really know the layout of its parent block, maybe it's within a small sidebar, and wide there means something else. Anyway, I'm not touching that at the moment but I feel like it's just complexity for nothing.
There was a problem hiding this comment.
True, we're not looking at the complete nesting tree. It would be possible to grab any custom wide align value from the parent (which we're not doing) and that might make things a bit more accurate, but in terms of effort vs effect the current solution will work for the majority of cases. Many themes nest Post Content inside one or more Group blocks; even those that nest it inside Columns will most likely have it in a responsive column that takes up most of the page. Having Post Content display in a narrow column on the single post template is a very unlikely scenario.
With these layout problems there are always many variables at play: nesting structure, parent block styles, theme styles, root padding, alignments... and often no approach works for all cases; I think if there's no universal solution it's best to try for a solution that works for most cases 😅
What?
Fixes #49546.
This is not a perfect fix, because it assumes that Post Content is inside a constrained layout block with the default wide content value. If the parent block has custom content/wide width values set, it won't really work.
Currently the only way to make sure the post editor layout conforms exactly to the appearance of the Post Content block would be to look at the attributes of all its parent blocks, which would require a lot of complicated logic 😅 and, given the direction #41717 sets for the future of post editing, it is likely not worth the effort. (Once the editors are unified as per #41717, the problem will be solved because the post editing interface will reproduce its template structure exactly.)
The fix in this PR should cover a bunch of common cases though, so I think it can still be useful.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Editor with full width layout: