Add layout styles from Post Content block to post editor #44258
Conversation
tellthemachines
added
[Type] Bug
|
Size Change: +5.78 kB (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
tellthemachines
force-pushed
the
fix/post-editor-layout
branch
from
bb436e7
to
68c57b3
Compare
There was a problem hiding this comment.
I love the direction here @tellthemachines, this looks very promising!
When testing manually in my local environment, it looks like it wasn't able to find a post content block, even though templateBlocks contains a nested one:
I've left a comment on findPostContent — I'm pretty sure it might be a fairly simple change in that function to get it working.
Happy to do further testing!
packages/block-editor/README.md
Outdated
| Generates the utility classnames for the given blocks layout attributes. | ||
| This method was primarily added to reintroduce classnames that were removed | ||
| in the 5.9 release (<https://github.com/WordPress/gutenberg/issues/38719>), rather | ||
| than providing an extensive list of all possible layout classes. The plan is to | ||
| have the style engine generate a more extensive list of utility classnames which | ||
| will then replace this method. |
There was a problem hiding this comment.
This feedback is probably low priority, but just jotting down some thoughts:
When this function was originally written, I believe we were thinking that the style engine might be responsible for more of the layout support that we do now (I think this function was written prior to the layout refactor landing). It might be worth us updating the description here, as I believe it probably makes sense for useLayoutClasses to be the entry point for getting layout classnames — if in some point in the future we do use the style engine for constructing the classnames, we might wind up doing it as an internal implementation detail, so we'd still probably want this as the main function for getting those classnames?
In terms of stability / API signature, the function currently accepts the layout object, and a layoutDefinitions object. However, in useLayoutStyles the signature is a bit different in that it accepts a block object and a selector. Would it be worth coming up with a signature for both of these hooks that is pretty much the same, to create some consistency there? I like the use of the block object in the latter — we might wind up needing to access other aspects of the block object in useLayoutClasses in the future, too, so it could be worth making some changes there?
There was a problem hiding this comment.
Thanks for thinking this through! I'll update the description to remove references to hypothetical future style engine work 😅
Would it be worth coming up with a signature for both of these hooks that is pretty much the same, to create some consistency there?
We wouldn't need the selector for useLayoutClasses though, would we? Other than that I'm happy to change it to take the block object.
There was a problem hiding this comment.
We wouldn't need the selector for useLayoutClasses though, would we?
Ah, good point. Yeah, I can't think of a circumstance where we'd need the selector for getting the classnames. We might have use for it eventually for where we output the classnames, but not for retrieving them in the first place?
There was a problem hiding this comment.
Yeah I don't think we'd ever need the selector in useLayoutClasses.
I'm also struggling to think of a use for the whole block in this function; we only really need the layout object. We could potentially change it to get the layout definitions from useSetting( 'layout' ) instead of passing them as a parameter. Would that work everywhere?
In useLayoutStyles we need both layout and styles from attributes, as well as the block name to pass to the layout type's useLayoutStyles function, so it can work out whether the block skips serialisation or not.
It's not looking like we can make the signatures very consistent here 😅
Regarding the doc comment for useLayoutClasses, could we simply shorten it to "Generates the utility classnames for the given blocks layout attributes.", or is there any additional info that might be useful here?
Also, I'm thinking we should make these experimental given Layout itself has that status.
There was a problem hiding this comment.
we only really need the layout object
Right now that's definitely the case. My only thought was if we wind up eventually needing to use parts of the spacing object for potential future classnames (e.g. if we needed to add a has-gap classname further down the track, which I think was discussed a little in the spacing presets discussions), then it'd be easier if we already had the object there.
We could potentially change it to get the layout definitions from useSetting( 'layout' ) instead of passing them as a parameter. Would that work everywhere?
That very well could. I think passing it around is probably a result of some of these functions originally being pure functions instead of React hooks? Now that they're hooks, it's probably better to keep the API signature simpler instead of passing stuff around 🤔
"Generates the utility classnames for the given blocks layout attributes."
Sounds good to me!
Also, I'm thinking we should make these experimental given Layout itself has that status.
Good idea, there's still plenty that we're moving around, so would be good to maintain that flexibility 👍
| @@ -89,6 +89,24 @@ function useLayoutClasses( layout, layoutDefinitions ) { | |||
| return layoutClassnames; | |||
| } | |||
|
|
|||
| export function useLayoutStyles( block = {}, selector ) { | |||
There was a problem hiding this comment.
Nice abstraction here!
| const templateBlocks = parse( templateContent ); | ||
| const postContentBlock = findPostContent( templateBlocks ); | ||
| const postContentLayoutClasses = useLayoutClasses( | ||
| postContentBlock?.attributes?.layout, | ||
| defaultLayout?.definitions | ||
| ); | ||
|
|
||
| const blockListLayoutClass = classnames( | ||
| { | ||
| 'is-layout-flow': ! themeSupportsLayout, | ||
| 'wp-container-visual-editor': themeSupportsLayout, | ||
| }, | ||
| postContentLayoutClasses | ||
| ); | ||
|
|
||
| const postContentLayoutStyles = useLayoutStyles( | ||
| postContentBlock, | ||
| '.wp-container-visual-editor' | ||
| ); |
There was a problem hiding this comment.
Will we ultimately want to wrap this in a useMemo so that we only run it when templateContent changes?
There was a problem hiding this comment.
Good question! I never know when it's best to useMemo or not 😅 Another option would be to return null from useLayoutStyles if there's no block.
There was a problem hiding this comment.
Another option would be to return null from useLayoutStyles if there's no block.
Actually I don't think we can do that because it's using hooks underneath 😕
There was a problem hiding this comment.
I never know when it's best to useMemo or not 😅
Me either, it's often a bit tricky to work out when they're useful. In principle if we're doing potentially expensive things like block parsing on a component that might get rendered multiple times, I think it makes sense to add a useMemo. If I add a wrapper findPostContentWrapper function that calls findPostContent and log out the number of times it's called, in my local environment it looks like we call it 9 times (not sure how valid that is, since there's only 5 log lines there, but it at least suggests we might benefit from the useMemo caching 🤔):
From memory we can't call a hook from within another hook, so the optimisation might only be to wrap the block containing the two lines parse and findPostContent in a useMemo, so that we only parse / find blocks when the template content changes?
But this can totally be something to look at in the final stages of the PR.
There was a problem hiding this comment.
Ok I tried what you recommended and it seems to be working well! We can still see a jump in layout when the page first loads, I think because the initial data from useSelect takes a while to appear, but I don't think there's much to be done about that.
There was a problem hiding this comment.
I think because the initial data from useSelect takes a while to appear, but I don't think there's much to be done about that.
Probably outside the scope of this PR, but I wonder if there's a way to preload the data for this selector? I see that the Navigation block has a preloading function here and sets apiFetch to use it here. There's similar logic in gutenberg_initialise_editor here, so I wonder if there's a PHP filter of some kind to add in a desired API endpoint to pre-fetch or something? Might be a bit of a rabbithole 😅
I don't at all understand the logic there, but just in case something along those lines sparks any ideas 🙂
| return blocks[ i ]; | ||
| } | ||
| if ( blocks[ i ].innerBlocks.length ) { | ||
| return findPostContent( blocks[ i ].innerBlocks ); |
There was a problem hiding this comment.
I think we only want to return here if we've found a post content block? Otherwise it looks like findPostContent bails early without having found something if the first depth-first discovery does not match a post content block.
In my current template, it bails after examining the deepest separator block, rather than moving up a level to eventually find the post content block.
In the below screenshot, I logged out looking at for each instance of the for loop, and it appears to stop at the core/separator, where it should have moved up a level and found core/spacer and then moved down one to core/post-content.
So, maybe something like the following might work?
let foundPostContent;
...
foundPostContent = findPostContent( blocks[ i ].innerBlocks );
if ( foundPostContent ) {
return foundPostContent;
}There was a problem hiding this comment.
Ahhhh well spotted! Yeah it'll have to be something along those lines.
|
Nice one, that's fixed up locating the I noticed that if I go to make a change in the post content block of the current template it crashes the editor. Not too sure why, though!
|
tellthemachines
force-pushed
the
fix/post-editor-layout
branch
from
0d193ca
to
2178750
Compare
|
I think I found another instance of
|
There was a problem hiding this comment.
This is working pretty well so far, I'm really excited about the progress here!
I was wondering if it's also worth us hooking in the style engine to gather CSS for the other block supports that might be in use in the Post Content block instance in the current template. I'm mostly thinking of Typography supports like font-size, font family, and the like. It'd be cool if we could output those styles in addition to the layout styles (but I also totally understand if you think that's better for a separate PR). We might be able to get away with passing the block instance's style object to this function to get the CSS we need:
Aside from the other couple of comments, I ran into another issue with TwentyTwentyTwo which is that it looks like its editor styles appear to override the layout content justification rules (it renders correctly on the site frontend). If I set the Post Content block in my current template to left content justification with custom content and wide sizes, here's how the Cover block looks in the editor:
| // if it doesn't have one. If no Post Content this must be a classic theme, | ||
| // in which case we use the fallback layout. | ||
| const postContentLayout = postContentBlock | ||
| ? postContentBlock?.attributes?.layout || { type: 'default' } |
There was a problem hiding this comment.
For this one, do we need the fallback logic for legacy markup (e.g. inherit) here, too? I noticed in TwentyTwentyTwo, the single template appears to still be using inherit so the view in the post editor for me was full width because it was assuming default (flow) layout instead of constrained:
There was a problem hiding this comment.
Ooooh good point! On it.
| @@ -286,9 +356,9 @@ export default function VisualEditor( { styles } ) { | |||
| className={ | |||
| isTemplateMode | |||
| ? 'wp-site-blocks' | |||
| : `${ blockListLayoutClass } wp-block-post-content` // Ensure root level blocks receive default/flow blockGap styling rules. | |||
There was a problem hiding this comment.
I was wondering if we might still need the wp-block-post-content classname to be added, so that we can hook into styles that are set at the core/post-content block in global styles? (It looks like the layout classnames don't output the block classname, so I think we might still need to add it in manually 🤔)
There was a problem hiding this comment.
I'm a bit hazy on how the global styles stuff works in the post editor, so if you think it's needed I'll just add it back. Won't do any harm to have the class there 😄
There was a problem hiding this comment.
I'm a bit hazy on how the global styles stuff works in the post editor
I was looking at this the other day while reviewing other PRs, so in case the breadcrumbs here are helpful: it's pretty naive when it comes to global styles. The server-rendered global styles are added to the block editor settings in PHP here, and then the post editor figures out whether or not to use them here. The styles are then passed down to the post editor's Layout component on this line. That component then passes the styles to VisualEditor here. And in VisualEditor, the styles are passed to MaybeIframe which finally passes it along to EditorStyles, which ultimately outputs styles in a style tag on this line: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/editor-styles/index.js#L81
The TL;DR is that it's nearly as if the global styles stylesheet had been enqueued, only those styles get transformed a bit by the EditorStyles component to play nicely in the editor.
I mostly just wanted to write that out somewhere because it finally clicked for me how it's hooked together 😀
There was a problem hiding this comment.
Thanks, that's really helpful!
|
Thanks for the continued feedback @andrewserong! I think I've addressed everything so far; the only remaining todoes are adding tests for the experimental functions we're exposing and working out why e2e are failing on this branch 🤔
Definitely let's do that, but I'd rather it be a separate PR to keep things simple for this one. I'm also unsure of the urgency of that work 6.1-wise, as so far we've only had reports about the missing layout styles. |
tellthemachines
requested review from
ellatrix and
ajitbohra
as code owners
|
Sounds good 👍
I imagine it's most likely lower priority because themes can target |
|
Looking into adding tests for |
There was a problem hiding this comment.
Thanks for all the updates again @tellthemachines, this feels much closer. I left a comment where I think we might be able to fix up the styling issue with the Cover block in TwentyTwentyTwo.
It looks like there's still an issue with going to the template editor and then exiting out after making an unsaved change. While the editor no longer crashes, it looks like only dealing with a string value results in the editor forgetting which layout type to render. In my local env, after making a change and exiting out, the layout defaults to flow again stretching everything full width:
|
|
||
| const postContentLayoutStyles = useLayoutStyles( | ||
| postContentBlock, | ||
| '.wp-container-visual-editor' |
There was a problem hiding this comment.
I'm wondering if this selector needs to be a higher specificity selector such as .block-editor-block-list__layout.is-root-container, which I think was used prior to this PR? If I use that one on this line, the layout styles appear to win out over the TwentyTwentyTwo styles as expected. But I wasn't sure if you changed this selector for another reason?
| Current PR (TwentyTwentyTwo's margins override the left content justification) | When updating to the higher specificity selector |
|---|---|
 |
 |
There was a problem hiding this comment.
Not really! Must be my subconscious drive to lower specificity everywhere 😅
It's a good idea to use the selector we had before though.
| const usedLayout = | ||
| layout && | ||
| ( layout?.type === 'constrained' || | ||
| layout?.inherit || | ||
| layout?.contentSize || | ||
| layout?.wideSize ) | ||
| ? { ...layout, type: 'constrained' } | ||
| : { ...layout, type: 'default' }; |
There was a problem hiding this comment.
Will we ever need to use this hook for flex layouts? With the current version, it looks like layout.type set to flex would result in output for the default layout.
There was a problem hiding this comment.
Ooooooh right, that needs to change
I agree, so long as we're manually testing each of the cases we're dealing with in these hooks, I think we can leave writing tests for them to a follow-up if need be 👍 |
tellthemachines
force-pushed
the
fix/post-editor-layout
branch
from
7631b33
to
08fe809
Compare
There was a problem hiding this comment.
A bit of a fly-by review, sorry! I don't have a lot of context, but the code reads well and changes I make to the post content block in the singular template are reflect in the post editor. 👍
Not sure what's going on with the inline template editing errors yet... 🤔
| @@ -82,20 +85,40 @@ function MaybeIframe( { | |||
| ); | |||
| } | |||
|
|
|||
| function findPostContent( blocks ) { | |||
There was a problem hiding this comment.
Could we help our future selves here and add a JS doc comment?
| const postContentBlock = useMemo( () => { | ||
| // When in template editing mode, we can access the blocks directly. | ||
| if ( editedPostTemplate?.blocks ) { | ||
| return findPostContent( editedPostTemplate?.blocks ); |
There was a problem hiding this comment.
I can reproduce error that @andrewserong noticed but only sometimes.
Most of the times it works for me.
When it does fail I suspect it's because postContentBlock is undefined (and therefore trying to access the attributes below fails).
Replacing with return findPostContent( editedPostTemplate?.blocks ) || {}; seems to get around that,
But it exposes another error triggered by the component 🤣
Uncaught TypeError: Cannot read properties of null (reading 'removeEventListener')
Not sure why yet. I can't reproduce either error in trunk.
There was a problem hiding this comment.
Hmm, I've only managed to repro the error once on TT2, and not at all on Empty theme. But I think something's messed up because previous customisations are now appearing as the default template state 😅
Will continue investigating.
There was a problem hiding this comment.
Ok I fixed the failure if Post Content block is removed from the template by accessing layout from attributes via optional chaining instead of destructuring. The reason returning empty object from findPostContent won't work correctly is some logic further down is dependent on checking if postContentBlock exists, and empty object will always be true 😅
I managed to reproduce the issue on trunk with Empty theme. Steps to reproduce:
I'm not yet sure what's behind it; the error is due to something null being passed to either |
There was a problem hiding this comment.
I'm not yet sure what's behind it; the error is due to something null being passed to either BlockParentSelector or BlockPopoverInbetween (in my testing errors alternate between the two 🤔)
In any case it seems unlikely to be related to this PR.
Thanks for looking into that! One of the usages of BlockPopoverInbetween is in the ZoomOutModeInserters — I noticed that if I switch off that experimental feature, I can't seem to replicate the issue immediately, so perhaps it's related to that implementation? In any case, I agree, it sounds like it's unrelated to this change, maybe we just noticed it here.
Separately, I think I found another styling rule issue with the middle content justification, so I've added another comment. Not too sure if it's the right place?
| const postContentLayoutStyles = useLayoutStyles( | ||
| postContentBlock, | ||
| '.block-editor-block-list__layout.is-root-container' | ||
| ); |
There was a problem hiding this comment.
I think there appears to be a difference in the rules that are being output here versus trunk 🤔 I noticed that there's an issue with the middle content justification with the Cover block set to wide alignment in TwentyTwentyTwo:
| Trunk | This PR |
|---|---|
 |
 |
This appears to be the rule that isn't present / winning out:
There was a problem hiding this comment.
I wonder if the issue is that useLayoutStyles generates the layout styles for the individual post content block, but not the base layout styles that need to be attached to this more specific selector in order for it to win out? Or to put it differently, prior to this PR, it looks like we might have depended on the output of base layout rules with this more specific selector for this middle content justification. Apologies if I'm way off!
There was a problem hiding this comment.
Those styles that are overriding the middle justification are the TT2 root padding logic that I thought were going to be removed after the Core root padding styles became available.
I'm reluctant to either increase specificity of the default layout styles or add wp-container rules for the default justification state, because any theme can conceivably still override them with higher specificity rules 😅
This should probably be fixed in TT2 instead cc @carolinan @scruffian
There was a problem hiding this comment.
I'm reluctant to either increase specificity of the default layout styles or add wp-container rules for the default justification state
Agreed, it'd be better not to have to add container rules, or explicit rules for the default justification state 🤔
This should probably be fixed in TT2 instead
It's a tricky one... because TT2 has been used as a base for many other themes, it might not just TT2 out in the wild that has this issue. (E.g. the rule appears to be part of BlockBase https://github.com/Automattic/themes/blob/23b9090090279c95788357bb634fd2e57c3973db/blockbase/sass/base/_alignment.scss#L58 which many themes have been built on top of, too).
Just trying to think of other options... would it work to update the change to this line in this PR
to include.block-editor-block-list__layout.is-root-container as well (as on trunk)? Or did we we need to remove it for the postContentLayoutStyles to work? Since they already have the selector attached, I was wondering if they can coexist. From quickly adding it back in in my local environment, it appears to get all three content justifications working in TT2 for me, but it's highly likely I'm missing some context 😅
There was a problem hiding this comment.
Just trying to think of other options... would it work to update the change to this line in this PR to include
.block-editor-block-list__layout.is-root-containeras well (as on trunk)? Or did we we need to remove it for thepostContentLayoutStylesto work?
I did remove it initially so that custom content widths would work, but because we since increased the specificity of the default rules it seems to be working ok. I just tried pushing that change now, hopefully it doesn't break anything else 😅
It's a tricky one... because TT2 has been used as a base for many other themes, it might not just TT2 out in the wild that has this issue.
I'm not sure how much of a concern that should be to us, because the temporary status of these rules is explicitly called out in the TT2 stylesheet.
There was a problem hiding this comment.
Nice one @tellthemachines, I think adding that rule back in did the trick. This is all testing nicely for me now. Also, adding back that rule seems to have fixed a flash of full width alignment on blocks when using the centre content justification in the post editor, so the UI feels a little more stable on a full page reload to me, now 👍
Tested with the following:
✅ TwentyTwentyTwo with left, center, right alignments, with a variety of content and wide size widths
✅ Tove blocks-based theme
✅ Emptytheme with the above, and useRootPaddingAwareAlignments to check that the new global padding settings still work correctly
✅ Classic themes TT1, TT, TwentyNineteen
I just left a comment where I think we can now remove the <LayoutStyle> component for classic themes since it'll already be covered in the earlier instance. But otherwise, this LGTM! ✨
| { | ||
| // For classic themes using theme.json we want a default content width in the editor. | ||
| ! postContentBlock && ( | ||
| <LayoutStyle | ||
| selector=".block-editor-block-list__layout.is-root-container" | ||
| layout={ fallbackLayout } | ||
| layoutDefinitions={ | ||
| globalLayoutSettings?.definitions | ||
| } | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Now that the above <LayoutStyle> also includes .block-editor-block-list__layout.is-root-container, it looks like we no longer need this conditional for classic themes, because it'll already be covered by the one above?
There was a problem hiding this comment.
Oh yes, well spotted!
tellthemachines
added
the
Backport to WP 6.7 Beta/RC
|
Just closing the loop that in #44506 we addressed a performance regression introduced by this PR. |
andrewserong
added
the
[Feature] Layout
* Add layout styles from Post Content block to post editor * Fix post content finding logic. * Remove extra styles when Post Content block exists. * templateContent should always be a string * Memoise post content block. * Change function signature and mark experimental * _Really_ make sure templateContent is a string * Update layout type for legacy layouts. * Fix selector specificity and legacy layout logic. * Fix 404s on fetch and only parse content if needed * Fix acces to layout when no Post Content block * Add doc comment to findPostContent * Increase specificity * Remove unneeded LayoutStyle
michalczaplinski
removed
the
Backport to WP 6.7 Beta/RC
|
Cherry-picked for the 6.1 release in #44656 |
* Add layout styles from Post Content block to post editor * Fix post content finding logic. * Remove extra styles when Post Content block exists. * templateContent should always be a string * Memoise post content block. * Change function signature and mark experimental * _Really_ make sure templateContent is a string * Update layout type for legacy layouts. * Fix selector specificity and legacy layout logic. * Fix 404s on fetch and only parse content if needed * Fix acces to layout when no Post Content block * Add doc comment to findPostContent * Increase specificity * Remove unneeded LayoutStyle
Package updates for bug and regression fixes: * @wordpress/annotations: 2.17.3 * @wordpress/block-directory: 3.15.4 * @wordpress/block-editor: 10.0.4 * @wordpress/block-library: 7.14.4 * @wordpress/blocks: 11.16.4 * @wordpress/components: 21.0.4 * @wordpress/core-data: 5.0.4 * @wordpress/customize-widgets: 3.14.4 * @wordpress/data: 7.1.3 * @wordpress/data-controls: 2.17.3 * @wordpress/edit-post: 6.14.4 * @wordpress/edit-site: 4.14.5 * @wordpress/edit-widgets: 4.14.4 * @wordpress/editor: 12.16.4 * @wordpress/format-library: 3.15.4 * @wordpress/interface: 4.16.3 * @wordpress/keyboard-shortcuts: 3.15.3 * @wordpress/list-reusable-blocks: 3.15.4 * @wordpress/notices: 3.17.3 * @wordpress/nux: 5.15.4 * @wordpress/preferences: 2.9.4 * @wordpress/reusable-blocks: 3.15.4 * @wordpress/rich-text: 5.15.3 * @wordpress/server-side-render: 3.15.4 * @wordpress/style-engine: 1.0.3 * @wordpress/viewport: 4.15.3 * @wordpress/widgets: 2.15.4 References: * [WordPress/gutenberg#44634 Gutenberg PR 44634] – Quote block: stop slash inserter popup showing in citation * [WordPress/gutenberg#44630 Gutenberg PR 44630] – Query Loop: Fix condition for displaying 'parents' control * [WordPress/gutenberg#44554 Gutenberg PR 44554] – Hide the Classic block in the Site Editor * [WordPress/gutenberg#44594 Gutenberg PR 44594] – Fix navigation block console error * [WordPress/gutenberg#44555 Gutenberg PR 44555] – Theme export: Fix broken spacingScale export * [WordPress/gutenberg#44580 Gutenberg PR 44580] – Code Block: Add box-sizing to fix inconsistent layout * [WordPress/gutenberg#44556 Gutenberg PR 44556] – Remove border from Global Styles previews * [WordPress/gutenberg#44141 Gutenberg PR 44141] – Spacing presets: Modify the styling of the input controls when in unlinked mode in order to better differentiate sides * [WordPress/gutenberg#44453 Gutenberg PR 44453] – Preserve the generic signature of getEntityRecord and getEntityRecords through currying * [WordPress/gutenberg#44504 Gutenberg PR 44504] – Theme.json: fix some outline properties doesn't work properly on the editor * [WordPress/gutenberg#44516 Gutenberg PR 44516] – Add style engine to editor tsconfig references * [WordPress/gutenberg#44523 Gutenberg PR 44523] – Query Loop Block: Rename Query Loop variations allowControls to allowedControls * [WordPress/gutenberg#44520 Gutenberg PR 44520] – Post Featured Image: Fix application of default border style in editor * [WordPress/gutenberg#44286 Gutenberg PR 44286] – Post Featured Image: Fix borders after addition of overlay feature * [WordPress/gutenberg#44482 Gutenberg PR 44482] – Template Editor: Fix crashes due to undefined variables * [WordPress/gutenberg#44480 Gutenberg PR 44480] – Template Parts: Prevent adding block in post editor or inside post template or content blocks * [WordPress/gutenberg#44425 Gutenberg PR 44425] – Fix rotated image crop area aspect ratio * [WordPress/gutenberg#44485 Gutenberg PR 44485] – Fix padding/margin visualizer accuracy * [WordPress/gutenberg#44569 Gutenberg PR 44569] – Theme.json: Fix some shadow properties that do not work properly in the site editor * [WordPress/gutenberg#44575 Gutenberg PR 44575] – ToggleGroupControl: Fix unselected icon color * [WordPress/gutenberg#44526 Gutenberg PR 44526] – TokenInput Field: Try alternative approach to fix screen reader focus issue * [WordPress/gutenberg#44506 Gutenberg PR 44506] – Edit Post: Optimize legacy post content layout * [WordPress/gutenberg#44258 Gutenberg PR 44258] – Add layout styles from Post Content block to post editor Follow-up to [54257] and [54335]. Props czapla, isabel_brison, wildworks, bernhard-reiter, hellofromTonya. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54383 602fd350-edb4-49c9-b593-d223f7449a82
Package updates for bug and regression fixes: * @wordpress/annotations: 2.17.3 * @wordpress/block-directory: 3.15.4 * @wordpress/block-editor: 10.0.4 * @wordpress/block-library: 7.14.4 * @wordpress/blocks: 11.16.4 * @wordpress/components: 21.0.4 * @wordpress/core-data: 5.0.4 * @wordpress/customize-widgets: 3.14.4 * @wordpress/data: 7.1.3 * @wordpress/data-controls: 2.17.3 * @wordpress/edit-post: 6.14.4 * @wordpress/edit-site: 4.14.5 * @wordpress/edit-widgets: 4.14.4 * @wordpress/editor: 12.16.4 * @wordpress/format-library: 3.15.4 * @wordpress/interface: 4.16.3 * @wordpress/keyboard-shortcuts: 3.15.3 * @wordpress/list-reusable-blocks: 3.15.4 * @wordpress/notices: 3.17.3 * @wordpress/nux: 5.15.4 * @wordpress/preferences: 2.9.4 * @wordpress/reusable-blocks: 3.15.4 * @wordpress/rich-text: 5.15.3 * @wordpress/server-side-render: 3.15.4 * @wordpress/style-engine: 1.0.3 * @wordpress/viewport: 4.15.3 * @wordpress/widgets: 2.15.4 References: * [WordPress/gutenberg#44634 Gutenberg PR 44634] – Quote block: stop slash inserter popup showing in citation * [WordPress/gutenberg#44630 Gutenberg PR 44630] – Query Loop: Fix condition for displaying 'parents' control * [WordPress/gutenberg#44554 Gutenberg PR 44554] – Hide the Classic block in the Site Editor * [WordPress/gutenberg#44594 Gutenberg PR 44594] – Fix navigation block console error * [WordPress/gutenberg#44555 Gutenberg PR 44555] – Theme export: Fix broken spacingScale export * [WordPress/gutenberg#44580 Gutenberg PR 44580] – Code Block: Add box-sizing to fix inconsistent layout * [WordPress/gutenberg#44556 Gutenberg PR 44556] – Remove border from Global Styles previews * [WordPress/gutenberg#44141 Gutenberg PR 44141] – Spacing presets: Modify the styling of the input controls when in unlinked mode in order to better differentiate sides * [WordPress/gutenberg#44453 Gutenberg PR 44453] – Preserve the generic signature of getEntityRecord and getEntityRecords through currying * [WordPress/gutenberg#44504 Gutenberg PR 44504] – Theme.json: fix some outline properties doesn't work properly on the editor * [WordPress/gutenberg#44516 Gutenberg PR 44516] – Add style engine to editor tsconfig references * [WordPress/gutenberg#44523 Gutenberg PR 44523] – Query Loop Block: Rename Query Loop variations allowControls to allowedControls * [WordPress/gutenberg#44520 Gutenberg PR 44520] – Post Featured Image: Fix application of default border style in editor * [WordPress/gutenberg#44286 Gutenberg PR 44286] – Post Featured Image: Fix borders after addition of overlay feature * [WordPress/gutenberg#44482 Gutenberg PR 44482] – Template Editor: Fix crashes due to undefined variables * [WordPress/gutenberg#44480 Gutenberg PR 44480] – Template Parts: Prevent adding block in post editor or inside post template or content blocks * [WordPress/gutenberg#44425 Gutenberg PR 44425] – Fix rotated image crop area aspect ratio * [WordPress/gutenberg#44485 Gutenberg PR 44485] – Fix padding/margin visualizer accuracy * [WordPress/gutenberg#44569 Gutenberg PR 44569] – Theme.json: Fix some shadow properties that do not work properly in the site editor * [WordPress/gutenberg#44575 Gutenberg PR 44575] – ToggleGroupControl: Fix unselected icon color * [WordPress/gutenberg#44526 Gutenberg PR 44526] – TokenInput Field: Try alternative approach to fix screen reader focus issue * [WordPress/gutenberg#44506 Gutenberg PR 44506] – Edit Post: Optimize legacy post content layout * [WordPress/gutenberg#44258 Gutenberg PR 44258] – Add layout styles from Post Content block to post editor Follow-up to [54257] and [54335]. Props czapla, isabel_brison, wildworks, bernhard-reiter, hellofromTonya. See #56467. Built from https://develop.svn.wordpress.org/trunk@54383 git-svn-id: http://core.svn.wordpress.org/trunk@53942 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Package updates for bug and regression fixes: * @wordpress/annotations: 2.17.3 * @wordpress/block-directory: 3.15.4 * @wordpress/block-editor: 10.0.4 * @wordpress/block-library: 7.14.4 * @wordpress/blocks: 11.16.4 * @wordpress/components: 21.0.4 * @wordpress/core-data: 5.0.4 * @wordpress/customize-widgets: 3.14.4 * @wordpress/data: 7.1.3 * @wordpress/data-controls: 2.17.3 * @wordpress/edit-post: 6.14.4 * @wordpress/edit-site: 4.14.5 * @wordpress/edit-widgets: 4.14.4 * @wordpress/editor: 12.16.4 * @wordpress/format-library: 3.15.4 * @wordpress/interface: 4.16.3 * @wordpress/keyboard-shortcuts: 3.15.3 * @wordpress/list-reusable-blocks: 3.15.4 * @wordpress/notices: 3.17.3 * @wordpress/nux: 5.15.4 * @wordpress/preferences: 2.9.4 * @wordpress/reusable-blocks: 3.15.4 * @wordpress/rich-text: 5.15.3 * @wordpress/server-side-render: 3.15.4 * @wordpress/style-engine: 1.0.3 * @wordpress/viewport: 4.15.3 * @wordpress/widgets: 2.15.4 References: * [WordPress/gutenberg#44634 Gutenberg PR 44634] – Quote block: stop slash inserter popup showing in citation * [WordPress/gutenberg#44630 Gutenberg PR 44630] – Query Loop: Fix condition for displaying 'parents' control * [WordPress/gutenberg#44554 Gutenberg PR 44554] – Hide the Classic block in the Site Editor * [WordPress/gutenberg#44594 Gutenberg PR 44594] – Fix navigation block console error * [WordPress/gutenberg#44555 Gutenberg PR 44555] – Theme export: Fix broken spacingScale export * [WordPress/gutenberg#44580 Gutenberg PR 44580] – Code Block: Add box-sizing to fix inconsistent layout * [WordPress/gutenberg#44556 Gutenberg PR 44556] – Remove border from Global Styles previews * [WordPress/gutenberg#44141 Gutenberg PR 44141] – Spacing presets: Modify the styling of the input controls when in unlinked mode in order to better differentiate sides * [WordPress/gutenberg#44453 Gutenberg PR 44453] – Preserve the generic signature of getEntityRecord and getEntityRecords through currying * [WordPress/gutenberg#44504 Gutenberg PR 44504] – Theme.json: fix some outline properties doesn't work properly on the editor * [WordPress/gutenberg#44516 Gutenberg PR 44516] – Add style engine to editor tsconfig references * [WordPress/gutenberg#44523 Gutenberg PR 44523] – Query Loop Block: Rename Query Loop variations allowControls to allowedControls * [WordPress/gutenberg#44520 Gutenberg PR 44520] – Post Featured Image: Fix application of default border style in editor * [WordPress/gutenberg#44286 Gutenberg PR 44286] – Post Featured Image: Fix borders after addition of overlay feature * [WordPress/gutenberg#44482 Gutenberg PR 44482] – Template Editor: Fix crashes due to undefined variables * [WordPress/gutenberg#44480 Gutenberg PR 44480] – Template Parts: Prevent adding block in post editor or inside post template or content blocks * [WordPress/gutenberg#44425 Gutenberg PR 44425] – Fix rotated image crop area aspect ratio * [WordPress/gutenberg#44485 Gutenberg PR 44485] – Fix padding/margin visualizer accuracy * [WordPress/gutenberg#44569 Gutenberg PR 44569] – Theme.json: Fix some shadow properties that do not work properly in the site editor * [WordPress/gutenberg#44575 Gutenberg PR 44575] – ToggleGroupControl: Fix unselected icon color * [WordPress/gutenberg#44526 Gutenberg PR 44526] – TokenInput Field: Try alternative approach to fix screen reader focus issue * [WordPress/gutenberg#44506 Gutenberg PR 44506] – Edit Post: Optimize legacy post content layout * [WordPress/gutenberg#44258 Gutenberg PR 44258] – Add layout styles from Post Content block to post editor Follow-up to [54257] and [54335]. Props czapla, isabel_brison, wildworks, bernhard-reiter, hellofromTonya. See #56467. Built from https://develop.svn.wordpress.org/trunk@54383 git-svn-id: https://core.svn.wordpress.org/trunk@53942 1a063a9b-81f0-0310-95a4-ce76da25c4cd
| // if not, this must be a classic theme, in which case we use the fallback layout. | ||
| const blockListLayout = postContentBlock | ||
| ? postContentLayout | ||
| : fallbackLayout; |
There was a problem hiding this comment.
Hi there! I stumbled on this code while doing some performance debugging. While right now, it's not clear to me that this code (I mean all the figuring out of the layout) is introducing performance regression (specially loading) but I noticed that the value of the layout changes three times for a regular post loaded in the post editor.
- A few things stand out to me here. Can we just compute the right layout only once to avoid unnecessary re-renders?
- Why does the layout object contains this "definitions" key? Seems weird? (probably unrelated to this PR but maybe you have an idea)
- Also the logic here is over-complexifying this component. Is there a possibility of some kind of cleanup like a dedicated hook
useBlockEditorLayoutor something (I have no idea, just wild suggesting)?
Thanks
There was a problem hiding this comment.
We don't have access to the post template straightaway on load, so we need to do a little checking to see when it becomes available. But we should probably be checking whether postContentBlock is an empty object rather than just checking whether it exists, as then blockListLayout would only change once.
The definitions object appeared in #40875; it's where we store base styles for each layout type. When we get the layout settings with useSetting the definitions will always be included.
Is there a possibility of some kind of cleanup like a dedicated hook useBlockEditorLayout or something
That's a good point, I can't say offhand but it's worth looking into!
There was a problem hiding this comment.
The definitions object appeared in #40875; it's where we store base styles for each layout type. When we get the layout settings with useSetting the definitions will always be included.
What's seems not correct here is that the "definition" are included in the "default layout" object. I'm fine having "definitions" but I don't they should be part of the default layout object.
There was a problem hiding this comment.
we should probably be checking whether postContentBlock is an empty object rather than just checking whether it exists, as then blockListLayout would only change once.
Yeah, is there a way to have no changes at all, this is to avoid rendering anything if the correct layout object is not available yet. As I guess otherwise, we'd just be doing unnecessary computation and potentially causing layout shifts in the UI.
There was a problem hiding this comment.
I'm fine having "definitions" but I don't they should be part of the default layout object.
I'm not sure I understand. The default layout type has default styles too, so we need to access those definitions sometimes. Do you mean we shouldn't be returning the definitions from useSetting( 'layout' ) or am I missing your point altogether? 😅
Yeah, is there a way to have no changes at all, this is to avoid rendering anything if the correct layout object is not available yet. As I guess otherwise, we'd just be doing unnecessary computation and potentially causing layout shifts in the UI.
Yeah, there will be a layout shift if the layout is anything other than center-aligned and constrained to default theme width. The problem is, if we wait for the post template data to be available before rendering the post content, users will be looking at a white screen for a little bit, which seems to me a worse experience than the layout shift. Unless we can find a faster way to access the Post Content block layout? The thing is we need the Post Content block from the specific template being used with the post, because it might have custom settings. It's not enough to get the data from global styles.
There was a problem hiding this comment.
Thinking through this a bit, also in light of #45262, would it be terrible to get the Post Content layout and add it to the block editor settings here? I haven't tried it out yet but happy to do so as I think we're going to need a solution that renders the correct layout for all types of users, not just the ones with the right permissions 😬
There was a problem hiding this comment.
I think in general we're trying to move away from server-side provided settings in order to make the editor more portable (outside wordpress, other contexts...) but in this case, I wouldn't mind it. The original idea of the "layout" properly in theme.json was to avoid this problem exactly: the layout in theme.json would always correspond (as a convention) to the post content block one and we'd avoid any complex computations. (statically defined)
There was a problem hiding this comment.
Thanks for the context! Yeah, the issue is precisely that folks wanted to customize the Post Content layout, and with features such as #44065 it makes sense for the post editor to reflect whatever layout is set.
Package updates for bug and regression fixes: * @wordpress/annotations: 2.17.3 * @wordpress/block-directory: 3.15.4 * @wordpress/block-editor: 10.0.4 * @wordpress/block-library: 7.14.4 * @wordpress/blocks: 11.16.4 * @wordpress/components: 21.0.4 * @wordpress/core-data: 5.0.4 * @wordpress/customize-widgets: 3.14.4 * @wordpress/data: 7.1.3 * @wordpress/data-controls: 2.17.3 * @wordpress/edit-post: 6.14.4 * @wordpress/edit-site: 4.14.5 * @wordpress/edit-widgets: 4.14.4 * @wordpress/editor: 12.16.4 * @wordpress/format-library: 3.15.4 * @wordpress/interface: 4.16.3 * @wordpress/keyboard-shortcuts: 3.15.3 * @wordpress/list-reusable-blocks: 3.15.4 * @wordpress/notices: 3.17.3 * @wordpress/nux: 5.15.4 * @wordpress/preferences: 2.9.4 * @wordpress/reusable-blocks: 3.15.4 * @wordpress/rich-text: 5.15.3 * @wordpress/server-side-render: 3.15.4 * @wordpress/style-engine: 1.0.3 * @wordpress/viewport: 4.15.3 * @wordpress/widgets: 2.15.4 References: * [WordPress/gutenberg#44634 Gutenberg PR 44634] – Quote block: stop slash inserter popup showing in citation * [WordPress/gutenberg#44630 Gutenberg PR 44630] – Query Loop: Fix condition for displaying 'parents' control * [WordPress/gutenberg#44554 Gutenberg PR 44554] – Hide the Classic block in the Site Editor * [WordPress/gutenberg#44594 Gutenberg PR 44594] – Fix navigation block console error * [WordPress/gutenberg#44555 Gutenberg PR 44555] – Theme export: Fix broken spacingScale export * [WordPress/gutenberg#44580 Gutenberg PR 44580] – Code Block: Add box-sizing to fix inconsistent layout * [WordPress/gutenberg#44556 Gutenberg PR 44556] – Remove border from Global Styles previews * [WordPress/gutenberg#44141 Gutenberg PR 44141] – Spacing presets: Modify the styling of the input controls when in unlinked mode in order to better differentiate sides * [WordPress/gutenberg#44453 Gutenberg PR 44453] – Preserve the generic signature of getEntityRecord and getEntityRecords through currying * [WordPress/gutenberg#44504 Gutenberg PR 44504] – Theme.json: fix some outline properties doesn't work properly on the editor * [WordPress/gutenberg#44516 Gutenberg PR 44516] – Add style engine to editor tsconfig references * [WordPress/gutenberg#44523 Gutenberg PR 44523] – Query Loop Block: Rename Query Loop variations allowControls to allowedControls * [WordPress/gutenberg#44520 Gutenberg PR 44520] – Post Featured Image: Fix application of default border style in editor * [WordPress/gutenberg#44286 Gutenberg PR 44286] – Post Featured Image: Fix borders after addition of overlay feature * [WordPress/gutenberg#44482 Gutenberg PR 44482] – Template Editor: Fix crashes due to undefined variables * [WordPress/gutenberg#44480 Gutenberg PR 44480] – Template Parts: Prevent adding block in post editor or inside post template or content blocks * [WordPress/gutenberg#44425 Gutenberg PR 44425] – Fix rotated image crop area aspect ratio * [WordPress/gutenberg#44485 Gutenberg PR 44485] – Fix padding/margin visualizer accuracy * [WordPress/gutenberg#44569 Gutenberg PR 44569] – Theme.json: Fix some shadow properties that do not work properly in the site editor * [WordPress/gutenberg#44575 Gutenberg PR 44575] – ToggleGroupControl: Fix unselected icon color * [WordPress/gutenberg#44526 Gutenberg PR 44526] – TokenInput Field: Try alternative approach to fix screen reader focus issue * [WordPress/gutenberg#44506 Gutenberg PR 44506] – Edit Post: Optimize legacy post content layout * [WordPress/gutenberg#44258 Gutenberg PR 44258] – Add layout styles from Post Content block to post editor Follow-up to [54257] and [54335]. Props czapla, isabel_brison, wildworks, bernhard-reiter, hellofromTonya. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54383 602fd350-edb4-49c9-b593-d223f7449a82
Why?
Fixes #42911, #44110, #44208.
How?
Gets layout classnames and styles from Post Content block so post editor layout can match the front end in all respects.
WIP
Todo:
useLayoutClasses,useLayoutStylesshould probably be experimental or unstable?Testing Instructions
Screenshots or screencast