-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try using fallback layout instead of default in post editor #54371
Conversation
Size Change: +69 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
Thanks for opening this PR! I just had an idle thought — would it be worth only applying this logic if the post content block was not at the root of the template? In theory if a post content block was at the root of the template and had My main thinking there is whether there's a way to still preserve the fix in #51431 for #51371, if folks are currently expecting that behaviour for any sites. |
Ooooh that's an interesting idea! Let's give it a try. |
@andrewserong thanks for the suggestion! I added the extra check and it seems to be working ok. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I think we should just remove the fetching of the "post content" block from the template entirely, I think in most cases, it's just wasted time and wasted performance (it has a meaningful impact on some of our metrics). In most cases, the "post content" layout is just equivalent to the default "layout" in theme.json. |
Good question. There's now a few features where it's beneficial to use the real post content block. Just linking through a few issues and PRs for the history:
At this stage, removing the fetching of the post content block could be perceived as a bug for users running themes with any of those kinds of styles or layouts applied in their templates. |
Thanks for the link round-up @andrewserong! I agree the feature is important to have and at this point it's not viable to remove it as folks are expecting it to work. @youknowriad I'd love to know more about the impact this has on our performance metrics. I know the first implementation was not great performance-wise, but we're now initially fetching the block on the server and memoising subsequent client-side fetches, which improved things a fair bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is testing great for me, and I think makes for a good balance between making a best effort to ensure full-width templates can be full-width within the post and page editors, which is a fairly common requirement in some themes, and ensuring a good writing experience for sites that use full-width content blocks within columns or multiple column layouts (or really any nesting situation).
With full-width default layout Content at root | With full-width default layout Content in nested blocks |
---|---|
I'm in favour of merging this in for WP 6.4 since fetching the post content block has been in core since 6.2. From my perspective, if we wish to remove the fetching, it'd probably be worth us looking at the current feature set as a baseline for whatever we attempt next. That is, ensure that any alternative we explore features at least the same amount of template WYSIWYG as we have now, with hopefully a pathway to increasing the resemblance between the post editor and the final site. In this respect the page editing in the site editor has been quite promising since it gets us closer to that goal.
LGTM! ✨
Last time I checked (it's been some time), the |
That's fixed now! BlockList only re-renders a couple times and in each case it's caused by parent re-renders (Layout component triggering a re-render of VisualEditor). |
What?
Fixes #52163.
Adds a check for the Post Content layout type, and if it's "default" we use the fallback layout instead, which essentially renders as if it were a "constrained" layout and uses the theme.json content width if there is one. The blocks won't have the options to align full or wide though, to avoid introducing discrepancies with the front end (if Post Content has default layout set, its child blocks won't be able to align full or wide, so we respect that)
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast