Template Parts & Reusable Blocks - try overlay element for clickthrough to edit pattern. #31109
Conversation
|
Right now my main blocker is finding a way to get the overlay element to re-render/resize when the content resizes, this can be easily seen when selecting the block and opening sidebars:
On the first selection, the overlay takes up the original size of the content. However, the inline inserter is added into the content and the overlay does not update resulting in the bottom half of the template part not having an overlay. This is also observable above when the sidebar is closed. 🤔 Beside this, the rest of the functionality should be working well. |
|
Size Change: +3.6 kB (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
Addison-Stavlo
requested review from
jameskoster,
vindl,
jeyip,
mattwiebe,
Copons,
creativecoder and
david-szabo97
|
🎉 I know this is still a draft, but wanted to report two small issues I spotted: For some reason I am able to bypass the click-through in the Twenty Twentyone Header to select the spacer: bypass.mp4Also, if you nest a template part inside another template part, a similar issue occurs – I can bypass the parent template part and directly select the child: nest.mp4 |
|
Thanks for pointing those out! That spacer thing is weird, it seems to happen when you hover/click just above the actual boundary of the block 🤦♀️ . I think I can fix both of those issues with some css. I just realized a bit of a contradiction in this colored-overlay design though @jameskoster. If we apply the color overlay when we click and select the template part block, then it gets in the way of applying/viewing background colors etc from that block. So if we try to use background colors, we cannot preview the actual color until we deselect the template part, or select an inner block:
If we get the hover/selection interactions from this working well (hovering highlights the entire TP and inner contents hover effects are disabled until the template part is selected / click selection always targets the template part block itself), I think that on its own would be a large step forward. Thoughts on where to go given the color issue? |
|
Hmm yeah that color issue is a tricky one :D I'm not sure it's worth diving too deeply in to it right now, as it still isn't 100% clear whether this action will be possible in this context, or whether you will need to engage the isolated template part editing view to access them. Let's create a follow-up issue if/when we merge this PR? |
Yes for follow ups... but Im not sure if we should introduce any colored overlay on that specific block selection state if it hinders pre-existing functionality for choosing background color. 🤔 There was a project thread about a year back regarding feature parity for FSE blocks, and the spec for the template part block was to have feature parity with the group block (for managing alignment, colors, etc). If we are re-thinking that spec that is fine, but I don't think we should add anything to hinder it until a new spec is determined. On that topic I would also argue it should always be controllable from the block inspector here regardless of whether or not it is a block level or entity level setting that it controls. Having it in the stand-alone editor I would see as an additional control but not a replacement. |
|
Another option could be to hide the overlay when color inputs in the Inspector are focussed. Perhaps that's worth a try? |
|
The issue with the spacer block selection and nested template parts should be fixed now:
Thats not a bad idea.. it may be a little more complicated to get working. Or similar but easier to implement idea would be to only show that colored state on combination of selection + hover? 🤔 My main Q at this point is what does the color really do for us? It seems unnecessary in that on selection there is already a handful of UI in place that makes it clear what is selected and where its boundaries are. I feel the real upgrade in behavior here is the hover and selection interactions making it impossible to interact with or select inner blocks until the main parent becomes selected first. |
Thinking about it more / adding onto that... if the goal of the color is to distinguish what 'type of thing' it is (template part vs. reusable block), maybe that color would be better suited on the hover interaction instead (what is currently gray). In that hover flow, there is nothing in the interface really distinguishing what type of entity we are about to select when we click.. while once the item is selected there are already various UI elements in place that can distinguish this making it less necessary. If we move the color to the hover flow, then we would only need an overlay when hovered+unselected which would get rid of the resizing problems noted above. |
|
I updated this to be a bit more simplified, everything should behave the same but I would appreciate re-testing/reviewing when able. |
|
I went to test using this approach: https://make.wordpress.org/design/2021/03/03/testing-a-gutenberg-pull-request-pr/ |
|
Newest version. 1- Hover over a Reusable block and notice the overlay. Click-through-Reusable-block.mp4I believe at this stage the click through is finished and should be merged. Various blocks that have inner content such as Buttons, Social Icons, Columns, Groups and other blocks could benefit well from using the Click through approach. -level 1- -level 2- For Reusable blocks an added measure of protection is needed, because these are a kind of global blocks potentially used in many pages/posts. Editing one instance has an impact on all other places where it is used. |
|
Another test. 1- Moving block below Reusable block using arrow icons. Moves it from below and to above Reusable block. The important part is that a user can accidental drag and drop a block into the Reusable block. Drag-block-into-Reusable-block.mp4 |
Yeah im not entirely sure what we should do about that here. We have disabled the overlay on drag-and-drop so the user is still able to drag-and-drop into these containers, and have given them a border to help try to outline that container to make its boundaries more apparent in that drag-and-drop process. Disabling drag-and-drop altogether into these containers seems like it would be pretty heavy handed and limiting. 🤔 |
|
Hey all, this is working as expected for me in the post editor (having some trouble trying to open the Site Editor so I haven't re-tested there).
Agree, I would err towards leaving drag and drop enabled as is because it might be confusing to remove the option entirely. One other option that could introduce a bit more friction around this interaction could be to use a long hover to bypass the overlay. So when the user activates the drag motion and hovers the block area, the overlay would be visible. After a long hover, it could "unlock" and switch to the current behavior (outlining the parent with the ability to insert anywhere amongst the children). But I don't think this is an interaction pattern that exists elsewhere in the editor so it would probably require more consideration. It might make sense to explore the drag and drop behavior further in a follow-up PR but curious to hear what others think! |
This sounds very interesting @critterverse! I do agree that is always something one can followup with in another PR. Are there any other considerations we need to look at before we can merge this PR? |
Correct, I think we need some code reviews here again to move forward. I think folks have been a bit busy with 5.8 related items, so its completely understandable if that takes a little bit of time. |
Addison-Stavlo
deleted the
try/template-part-clickthrough-overlay-element
branch
|
@peterwilsoncc , @desrosj and @youknowriad Should this PR with the click through approach be targeted for WP 5.8 as it does in a small way improve the protection of accidental changes with Reusable blocks? |
|
I'd have loved for this to land sooner to consider it for 5.8. At this point, I have doubts, I don't feel like we have enough testing time to get the necessary confidence. |
|
Good point! |
I 100% agree that its best to play it safe and not rush things there. |
|
Great work @Addison-Stavlo ! |
|
Now that we have been testing out the overlay for Reusable blocks for a while in the Gutenberg plugin. I am also adding @desrosj Jonathan. |
| onMouseLeave={ () => setIsHovered( false ) } | ||
| > | ||
| { isOverlayActive && ( | ||
| <div |
There was a problem hiding this comment.
Is there on way to do this with less elements involved? Maybe pointer-events: none and enable when the block is selected?
There was a problem hiding this comment.
I'm pretty sure we did this already in #34012
Description
Addresses #29337 by adding an overlay requiring a click-through to template parts and reusable blocks. When the block is not selected, hovering the block will result in a blue overlay. Clicking anywhere inside the block will act to select the block itself, preventing its children from being selected before the parent.
How has this been tested?
Tested with template parts (and nested template parts) as well as reusable blocks.
Tested to ensure drag and drop functionality still works as expected.
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).