Site Editor: Apply the right post content layout in the "post only" mode #56542
Conversation
youknowriad
added
[Type] Enhancement
youknowriad
requested review from
ramonjd,
tellthemachines,
andrewserong and
aaronrobertshaw
| import { useMemo } from '@wordpress/element'; | ||
| import { useSettings } from '@wordpress/block-editor'; | ||
|
|
||
| export function usePostEditorLayout( templatePostContentBlock ) { |
There was a problem hiding this comment.
@tellthemachines I'm not entirely sure about the logic to compute the right layout in this hook. Maybe you can confirm, I just tried to copy the logic from the VisualEditor component of the edit-post package which I think you implemented (at least most of it)
There was a problem hiding this comment.
@tellthemachines might be able to confirm, but the logic looks good to me! (Attempts to use constrained if available, while accounting for legacy inherit property, and falls back to default).
There was a problem hiding this comment.
The logic is correct, but I don't think we need it because, unless I'm quite mistaken 😅, what we're doing here is just grabbing the Post Content layout from the template and applying it to a newly-created Post Content or Group block.
The convoluted layering of fallbacks is necessary in the post editor because we have to manually determine the correct layout classnames to pass to BlockList so they get added at root level, and also add the correct layout styles for them to the page. Plus we're dealing with support for classic themes and all sorts of intermediate scenarios.
But if here we're just using blocks as containers for the post, the block editor already knows how to interpret their attributes, including the legacy ones 🙂 so we could just grab all the Post Content attributes from the template and add them directly instead, which would incidentally preserve any color or typography changes as @andrewserong commented above.
|
Size Change: +784 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
Nice one. For folks testing, the template preview toggle exists in the document inspector under the "Template > Page" toggle:
We might want to find a better space for that toggle, especially after we wrap up refinements like here, and whatever pattern we end up with can hopefully also be repurposed for show/hide title in post/page editors. This is separate. In testing this, the page editor before/after states look identical to me. Page editor before:
After:
Site editors with template preview do as well. Before:
After:
The big change happens when you disable template preview. Before, the width is incorrect, the main column is supposed to be wide here:
After, that aspect is solved, but it would be nice if we could keep some margin above the title, that's missing now:
|
|
@jasmussen I was wondering why we had that margin, I added it back. |
|
I'm happy to 👍 👍 from the visual behavior being corrected now, but I'm assuming this needs a code sanity check? |
There was a problem hiding this comment.
Nice follow-up! This is generally testing pretty well for me, and uses the right layout attributes for me in testing. The following screengrab has a post content block with custom content and wide sizes:
2023-11-28.11.27.36.mp4
One issue I ran into: an editor error when toggling the layout on the post content block within the template:
2023-11-28.11.36.07.mp4
This is the error:
Other than that, it's looking pretty good to me!
The ultimate goal is to actually unify the site editor and post editor (make the post editor use this code instead of its custom code).
Something that the post editor does is fetch the post content block's attributes from the server, which is necessary for users that don't have access to the raw template markup. We don't need to worry about that for the site editor, since only users who have access to the templates can use it. But if we're looking at consolidation, is that something to consider now, or something to leave for follow-ups?
| ]; | ||
| const postContentBlock = | ||
| extractPostContentBlockFromTemplateBlocks( templateBlocks ); | ||
| const postContentLayout = usePostEditorLayout( postContentBlock ); |
There was a problem hiding this comment.
Just thinking, what about other style-based block attributes that could be set within a template, e.g. typography, min height, background color, and eventually background image support? Is it worth considering grabbing all the other attributes set on the postContentBlock in addition to figuring out the right layout attributes to use?
| import { useMemo } from '@wordpress/element'; | ||
| import { useSettings } from '@wordpress/block-editor'; | ||
|
|
||
| export function usePostEditorLayout( templatePostContentBlock ) { |
There was a problem hiding this comment.
@tellthemachines might be able to confirm, but the logic looks good to me! (Attempts to use constrained if available, while accounting for legacy inherit property, and falls back to default).
| return createBlock( | ||
| 'core/group', | ||
| { | ||
| layout: postContentLayout, | ||
| }, | ||
| [ block ] | ||
| ); |
There was a problem hiding this comment.
Oh, nice, so this wraps any other content blocks in a Group block that uses the same layout as the post content block 👍
I suppose one thing we might want to look into separately (not a blocker for this PR) could be to also attempt to grab the styles or wrapper for the post title block in case a template is centering the post title or doing other styling to it? For the moment, I think going with making the layout consistent with the post content layout is a good way to go.
packages/editor/src/components/provider/use-post-editor-layout.js
Outdated
Show resolved
Hide resolved
| return undefined; | ||
| } | ||
| for ( let i = 0; i < blocks.length; i++ ) { | ||
| // Since the Query Block could contain PAGE_CONTENT_BLOCK_TYPES block types, |
There was a problem hiding this comment.
Might be better to specify here that Query could contain a Post Content block as we're not looking for all PAGE_CONTENT_BLOCK_TYPES in this function.
| import { useMemo } from '@wordpress/element'; | ||
| import { useSettings } from '@wordpress/block-editor'; | ||
|
|
||
| export function usePostEditorLayout( templatePostContentBlock ) { |
There was a problem hiding this comment.
The logic is correct, but I don't think we need it because, unless I'm quite mistaken 😅, what we're doing here is just grabbing the Post Content layout from the template and applying it to a newly-created Post Content or Group block.
The convoluted layering of fallbacks is necessary in the post editor because we have to manually determine the correct layout classnames to pass to BlockList so they get added at root level, and also add the correct layout styles for them to the page. Plus we're dealing with support for classic themes and all sorts of intermediate scenarios.
But if here we're just using blocks as containers for the post, the block editor already knows how to interpret their attributes, including the legacy ones 🙂 so we could just grab all the Post Content attributes from the template and add them directly instead, which would incidentally preserve any color or typography changes as @andrewserong commented above.
|
@tellthemachines @andrewserong I think preserving styles from the post title and post content blocks could be a good idea. That said, I don't want to do that at the moment because it might have some consequences that we don't comprehend fully yet. I would like this mode to be used in the post editor and this might make it harder. I think it's something we should definitely try in its dedicated PR. @tellthemachines Is there a place where I can learn what "constrained" mean and what's the difference with "default"? |
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
Sure! Then I guess was can just copy over the layout object from the block attributes. All the same, we don't need the complicated fallback logic because whatever layout is copied will be processed correctly once it's applied to the new block.
This doc explains the different layout types. Basically constrained supports content width, and default doesn't. |
| if ( ! blocks ) { | ||
| return undefined; | ||
| } | ||
| for ( let i = 0; i < blocks.length; i++ ) { |
There was a problem hiding this comment.
You could use for ... of if you're only ever using the value blocks[ i ]?
|
closing in favor of #56671 |
Related #52632
Follow-up to #56509
What?
In the post editor, for the root level layout, we try to guess the "layout" from the page/post template and otherwise fallback to the global styles layout if no layout is found on the post content block.
In the site editor however, we have a mode where we try to show the post/page in isolation mimicking the post editor but we we were not trying to find the right layout. We were just using the default constrained layout. In this PR I'm trying to bring the logic that exists in the post editor to this mode as well.
The ultimate goal is to actually unify the site editor and post editor (make the post editor use this code instead of its custom code).
Testing Instructions
1- Open a page in both the site and post editors
2- in the site editor, disable "template preview" to switch to "post only" mode.
3- Notice that you have the same alignment options for top level blocks within the post content block.