Block Editor: Absorb parent block toolbar controls #33955
Conversation
gziolo
added
[Type] Technical Prototype
|
Size Change: +286 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
|
Nice! We should probably add the parent's tools as a toolbar group so it has a separation. |
|
Very nice. This is before:
This is after:
A few quick doodles. The Buttons block remains the same:
A "what if" that integrates the key parent toolbar options in the parent button selector. Doesn't feel right, but wanted to explore:
An entirely separate group. This one feels the best, but it does grow a bit wider:
The exploration might work better without the explicit drag handle as shown in the following. That isn't so much a suggestion for this PR as it is a reminder to keep thinking of how we can handle drag and drop in a more holistic way, perhaps make it a more dedicated part of the select tool:
Another thought: the Button toolbar gets pretty bordery in these mocks. Although the Link feature is technically block level and therefore separate, we migth consider curating it with the inline tools:
|
I also think a separate group can greatly help usability as seen in @jasmussen's exploration. As a user, it would be very hard to understand why sometimes a block's toolbar has some options while others don't without knowing where those options are coming from. |
gziolo
force-pushed
the
try/absorb-parent-block-controls
branch
from
8d8f561 to
fca1e35
Compare
|
Iteration 2: a new block controls group called
At the moment the parent toolbar group uses the standard class name By the way, it looks like the different distribution of children blocks is an expected feature in the Social Icons block when it's selected vs when it isn't. |
gziolo
force-pushed
the
try/absorb-parent-block-controls
branch
from
4414a30 to
0648dd3
Compare
| @@ -100,6 +100,7 @@ export function SocialLinksEdit( props ) { | |||
| const innerBlocksProps = useInnerBlocksProps( blockProps, { | |||
| allowedBlocks: ALLOWED_BLOCKS, | |||
| orientation: 'horizontal', | |||
| __experimentalCaptureToolbars: true, | |||
There was a problem hiding this comment.
@mtias, I have an idea for controls injected from the framework through supports in block.json like align. We could put also __experimentalCaptureToolbars in supports object to act as a way to decide if those general controls should also be exposed to children. An alternative would be to include __experimentalExposeToChildren setting in supports object.
I understood that we want to have it as an opt-in for every block. Otherwise, the parent's block alignment would get exposed for the huge groups of blocks 😄
There was a problem hiding this comment.
Experiment implemented in e5f1f88.
There was a problem hiding this comment.
__experimentalCaptureToolbars
I'm slightly confused by usage of this. "Capture toolbars" is for the UI space occupied by the parent toolbar to display the toolbars of any child. This is great when having deeply nested navigation blocks where having the toolbar show up on the deeply nested child gets very clunky.
If we're implementing a feature which is essentially the inverse (ie: child toolbar UI space includes controls form the parent) then it should probably have a different name/setting. __experimentalExposeToChildren seems good.
There was a problem hiding this comment.
My current thinking is that for "capture toolbars" to make sense, it also needs to expose tools to the children, otherwise it feels a bit awkward to use.
There was a problem hiding this comment.
Yes, this part is something that was only added yesterday so I'm open seek a better API here even under the experimental flag.
Let me explain my reasoning better. My assumption was that you want to combine the toolbar capturing with exposing children to have more space to display controls. Given that they both work a bit differently at the moment, I'm fine following your recommendation:
__experimentaExposeControlsToChildren- at the moment it is limited only to children, in effect, it doesn't apply to deeper descendants
Aside, I like the idea that useInnerBlocksProps would be able to read some settings from the supports object so maybe having __experimentalCaptureToolbars also there isn't that bad idea. I saw many requests where people wanted to control allowed blocks or template lock settings.
There was a problem hiding this comment.
Let me know what do you think about #33955 (comment).
gziolo
force-pushed
the
try/absorb-parent-block-controls
branch
from
0648dd3 to
4e3f7d8
Compare
Nice one! Can you visually separate the control groups with spacing, like the spacing we have between the parent button and the rest of the toolbar? Like in https://user-images.githubusercontent.com/1204802/128842093-85b815dd-a044-43dc-a77e-427eca34fc3d.png |
|
As an update to the mockups in #33955 (comment) having discussed them in chat, the motivation was to clarify their parent relationship by sharing DNA with the parent button. However that isn't necessarily any more clear in the mockups, the value of that DNA shared is perhaps also spurious, and it's less an "absorbing" of the toolbar than what is originally suggested. So perhaps we should just start with the simplest possible thing and learn from there:
|
gziolo
force-pushed
the
try/absorb-parent-block-controls
branch
from
4e3f7d8 to
e5f1f88
Compare
|
Iteration 3:
It aligns with the most recent feedback:
I also included changes that allow to opt-in to block toolbar capturing (
There are two ways to test
|
gziolo
force-pushed
the
try/absorb-parent-block-controls
branch
from
e5f1f88 to
d8c76a7
Compare
gziolo
added
[Type] New API
gziolo
added
[Feature] Block API
|
It does get pretty long. Once we land this we'll want to revisit the feature where the toolbar bumps against the right side of the screen if inside a columns block. That used to work, but has been broken for a bit, ticketed it here: #34053 |
|
🌟 I really like this idea as I frequently find it frustrating that I have to select the parent to make certain changes. Sorry to be "that guy" but looking at iteration 3 and then testing it, I am concerned that it's not obvious that the controls shown are associated with the parent. It feels very much like they need additional visual affordance (over and above that provided by a separate group with a border) to "mark" them as distinct parent controls. I worry it's going to be very surprising for a user if certain block-level controls suddenly start affecting other blocks (eg: parent) with no visual cue that they are distinct. If the user has to "learn" a UI pattern (eg: parent block controls appear to the left in a separate group) then that probably isn't going to cut the mustard. For example, when I first tested this PR I intentionally didn't read too much of the description before hand in order that I could test the new feature without prejudicing my conclusions. My experience was that I was initially confused as to which control was the parent and I had to "hunt" for it until I realised it was the justification controls. There was no clear indicator to me - that will happen for others as well. Just some ideas but have we explored...
Really hope we can find a way to land this in a manner that doesn't add additional confusion to the toolbar. |
|
Part of the thinking for absorbing toolbars is that there is no need to discriminate parent tools from child tools because the container ones are always available regardless of what is selected. Blocks that are entangled in this way will function more like a single block with contextual tools upon selecting an element within than separate child / parent blocks. |
gziolo
force-pushed
the
try/absorb-parent-block-controls
branch
from
d8c76a7 to
42e8030
Compare
|
42e8030 tries a revised approach. When
At the moment when there is the following code: <BlockControls group="block" __experimentalExposeToChildren>
<SomeControls />
</BlockControls>and the
|
gziolo
force-pushed
the
try/absorb-parent-block-controls
branch
from
42e8030 to
c80e104
Compare
|
I rebased this branch with the latest changes in
It's hard to tell if that is really a huge concern because the toolbar is rendered in the same place as the parent so maybe it's enough to have those controls in their own section. We can iterate on the design in the follow-up PRs and it's exactly how most of the features improved over time. |
gziolo
changed the title
|
@annezazu I'm curious if we can do some testing with users on this one. Start from a pattern with three buttons on the page and ask: "change the second button text to blabla"; "change the alignment of buttons to be centered". |
|
Definitely can. Do you want a full call for testing or are you okay if this is part of a larger call for testing in the future? |
Description
Exploration for #26313.
This PR explores changes for the Buttons and Social Icons blocks. The very first iteration tries passing an experimental flag
__experimentalExposeToChildreninBlockControlusage in the edit implementation of the parent block. It takes advantage of the SlotFill behavior and conditionally renders the control in the child block when it's selected.The behavior is limited to the child block at the moment, but we could also cover ancestors if we would like to go one step further. The challenge is how to handle blocks that can be nested in themselves, but I think it's doable if we feel more adventurous. The current approach is enough for the Buttons and Social Icons blocks that have a very simple parent-child relation.
Please note that I also used
__experimentalCaptureToolbarsflag for the inner block containers so the toolbar is always present in the same place like in the Navigation block.It should be possible to use the same technique for Inspector Controls.
Before
After
How has this been tested?
See the screencast:
Screen.Recording.2021-08-09.at.15.43.53.mov
I used multiple blocks of the same type to ensure that the control isn't present multiple times in the child block. I was able to ensure that the control updates only the parent block.
Knows Issues
Everything works perfectly fine for the Buttons block.
There is something strange happening with the Social Icons block that needs further investigation but it seems to be present in the
trunkas well.I still need to figure out how to handle controls that get injected through block
supportslikealign.Open Questions
Where should be parent controls located? It's the same group as in the parent in the prototype. Should we create a special group? Where should it be placed?
In the Buttons block, you can see controls from the parent (items justification) mixed with those from the child (link). If we would move parent controls to their own group we would make it more clear what we edit. We could even move this group next to the
Select Buttons(Select parent) button to make it even more glued together.Types of changes
New feature (non-breaking change which adds functionality).
Checklist:
*.native.jsfiles for terms that need renaming or removal).