Try to make Reusable blocks inherit alignment #31082
Conversation
|
This is an interesting approach, that on the face of it seems simple and reasonably functional. It'll need a great deal of testing, and I'd love another code gut check, but this might just work. |
getdave
changed the title
But only if we find a way to have the blocks within the reusable block inherit the same styles as if they were inserted into the main editor canvas. |
|
Size Change: -45.7 kB (-4%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
|
I'm going to say something that could be controvertial. I feel reusable blocks should have a wrapper like template parts... That way, no issues. It does have impact though: You can't insert reusable blocks in places where groups are not allowed. Alignment are specific to the default layout we have now but imagine a world where we have more layouts: Imagine we have a "grid" container: wide/full/left alignments don't make any sense there, what would you assign to the group block? |
I will call the wrapper a lid. A type of overlay (lid) is being added for Reusable blocks in this PR by @Addison-Stavlo . This is in relation to the click-through approach. Clicking a template first selects the parent container. Clicking again selects the inner/child blocks. |
I have been exploring a click-through pattern for template parts, but haven't looked at reusable blocks specifically yet. In the end it sounds like the same click-through pattern should be implemented for both. Im not very familiar with Reusable blocks in general and uncertain about whether or not this missing wrapper (is it missing entirely? or lacking functionality?) @youknowriad mentions would be any issue there or not. |
I could personally see that working. What would be the consequences for existing content if a change such as this were to be explored? |
If we go that route, we'll have to create some kind of attribute in the reusable block to indicate that it's v2 and avoid touching the old ones. |
Should it be an option on the block, denoting the downsides of not having a wrapper? I seem to recall there being some benefits to the reusable block interface not adding a wrapper, though I can't immediately recall them. |
getdave
added
Needs Dev
getdave
force-pushed
the
try/reusable-blocks-inherit-alignment
branch
from
828f0a8
to
17f3058
Compare
|
@paaljoachim This now seems to be working much better. Only problem I can see is this:
Seems that the alignment is not persisted on the reusable block when saving. |
|
Moving to Ready for Review so we get the Plugin build and contributors can test it. cc @paaljoachim |
|
The first test. Using WordPress 5.8. I made a Group block with 3 columns inside. Set it at Full Width. Duplicated the Group block. The result Frontend: Using the Gutenberg PR build (How to test with the Gutenberg PR build can be seen here.). Duplicated the original Group block with columns. Full width. Backend (shows the correct alignment.) Frontend: |
This avoids having the toolbar show up or clashing with existing alignments
There was a problem hiding this comment.
Thanks for giving this a try.
The fundamental issue here is that the reusable block doesn't have any wrapper in the frontend but does have one in the editor constraining its content.
I don't think there's a perfect solution for this because alignments styles depend on the presence of wrappers meaning if you define a layout on a container block, that layout will apply its styles on its children, and the children are different between frontend and backend.
The current PR tries to be "smart" by guessing the widest alignment applied in the children of the reusable block. Though, there several main problem here:
- this is going to add an "align" attribute to the reusable block and that attribute is persisted but useless since its value should ideally always be computed based on the children, if I align a children (add "full") after the creation of the reusable block, it won't have any impact on the parent's attribute.
- The other fundamental problem is that this solution only works in "classic themes" where alignments works the same way regardless of the container (aka applying align full in a root block or in a child of a group block has the same effect), in the context of block themes, this doesn't work at all because the layout (aka alignments styles) is defined by the direct parent, in the case of the reusable block's content, the direct parent shouldn't be the reusable block itself but it should be the parent of the reusable block (currently broken in trunk too and this solution doesn't solve that)
I personally don't have a perfect solution for this problem though, my crazy ideas (not sure if acceptable or realistic) tend to be around:
- Forbid any alignments in the reusable block's children (remove the attributes once the block is made reusable and forbid them in the UI)
- Support wrapper-less blocks in the editor (I don't think it's realistic because I don't see how we can render a block UI in the editor without rendering any Dom node for it)
|
I am not fully certain what you are saying Riad..... Creating a Reusable block should automatically take on the regular blocks alignment. I would assume this: I believe this issue is also very much associated: |
I might have missed something here, but I don't fully understand why that is an issue. The reusable block is - as you say - a transparent wrapper. It just "holds" existing blocks. If I have some blocks with a "full" alignment, then when I make a reusable block they should remain visually aligned exactly as they were before I made them reusable. Here's how I see it:
I just figured that one out 😆 I've adjusted the code so that now when the Screen.Capture.on.2021-08-10.at.10-55-45.1.mp4Note: I've not used Also note: the attribute on the This also works perfectly on the front of the site:
My new solution accounts for that. Change the width of a child and the (editor only) attribute of the Reusable block adapts as necessary. Screen.Capture.on.2021-08-10.at.11-06-36.mov
Yeh this bit I'm confused about... |
|
Also to be clear, I do not support the idea that you should be able to set an alignment on the Reusable block itself and have the wrapped blocks take on that alignment. No no no. The reusable block is a transparent wrapper. It should adapt to its children and do nothing else. IMHO... 😄 |
|
Here's the code adding the alignments styles: gutenberg/packages/block-editor/src/layouts/flow.js Lines 111 to 123 in 984160d As you can see, the styles apply only to the direct children of the container block. Meaning If you detect the widest alignments of a list of blocks, apply it to a newly added element container of the blocks in the editor (like this PR does), here's what happens: The width of that newly added container will be correct but since that newly added container doesn't have any layout definition itself, the blocks inside it will all just take the full width of that block. (alignments of the inner blocks won't have any effect). This is in an FSE theme. |
|
The reason I think alignments should be forbidden in a reusable block is simple: Alignments don't make sense on their own, they depend on the context of where the block is used and since this block is "reusable", it should be independent of its context. |
Ok this is the piece of the puzzle I was missing. |
|
Closing this as there doesn't seem to be consensus on an approach. |
Description
Addresses #8288.
Currently, when reusable blocks are created from a set of blocks which have various alignments set these are "lost". This is because the reusable block is always "centered" within the editor and has no concept of "wide" or "full".
This PR:
This sort of works, but this issue is that the blocks within reusable block that do not have an alignment set are no longer centered within the viewport. This is because they are missing the special CSS rules that are based on the root container which cause them to be centered.@jasmussen mentioned that @youknowriad is working on some Layout controls which might help to solve this.Essentially what we need is a way to make the alignment of blocks contained within the reusable block behave (lay out) as they do within the normal editor canvas.Update: this layout centering issue now seems to be fixed.
Closes #8288
How has this been tested?
Screenshots
Screen.Capture.on.2021-08-05.at.14-54-43.mp4
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).Uploading Screen Capture on 2021-04-22 at 12-32-58.mov…