__experimentalShareWithChildBlocks should apply to all children, any depth in the inner-block hierarchy
#35078
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +3 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
senadir
added
[Feature] Block API
|
It was intentional to start small since it’s an experimental API. The name used indicates that it applies only to child blocks - not all inner blocks. So it has to be a direct parent-child relationship. We can open it for deeply nested blocks but it needs to be opt-in because it will lead to unexpected issues. For instance, when you nest the same block type in itself, then you will have the same control type (example: align) from the grandparent and the parent block rendered in the toolbar next to each other. Duplicated controls that don't have a good visual reference to what they target might be very confusing for users. |
@gziolo Would this not also be the case in the direct child? Showing its own align control and the parent align control? Do you know of a block type that allows this to test this out? If the preference is a separate flag we can try updating this PR, but @senadir and I through a unified flag would be more intuitive when dealing with complex hierarchies. |
|
The issue of nested similar blocks is something we overlooked. I have no problem moving this to a separate flag and fixing it there. I think there's value in having it being nested deeply, and the issue of same blocks nested can be worked around somehow (limiting consumption to the nearest parent...) |
Buttons block and Social Icons block use that. It isn't an issue for a direct child, because all parent controls are grouped together in their toolbar group. So when both blocks have the same toolbar control, they are in different places.
It doesn't have to be a new flag. We can rename the existing one, and let it take different values so we could support both behaviors. For example, |
This was referenced Aug 24, 2022
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When using controls using
__experimentalShareWithChildBlocks, only the direct children show the controls.For example, if I have a block containing inner blocks, and have his defined:
I see the control when the top-level block is selected, and when its direct children (inner blocks) are selected:
Without this patch, any deeper inner blocks do not show the above control.
This code determines if parent controls are displayed. Notice is uses
hasSelectedInnerBlockto determine if an inner block is selected and so needs the controls to be added. This does not factor in multiple levels of inner blocks hierarchy.hasSelectedInnerBlockhas support for adeepparameter which searches inner blocks recursively. This is what we need here to solve the issue.How has this been tested?
This has been tested in WooCommerce Blocks only, with the above use case. I could find little usage of
__experimentalShareWithChildBlocksin Gutenberg (ButtonEdit block uses it).ref: woocommerce/woocommerce-blocks#4812
Types of changes
This changes the behaviour of an experimental feature only available in this plugin.
Checklist:
*.native.jsfiles for terms that need renaming or removal).cc @senadir