[RNMobile] Enable child block settings to be hidden via parent block #36371
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
added 2 commits
November 10, 2021 11:48
With this commit, the logic for hiding block controls is moved to its own function in the 'use-display-block-controls/index.native.' file. For the purpose of this commit, the code is copied to a native-specific file from the web-version of the file here: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/use-display-block-controls/index.js The file will be further refined with native-specific functionality in later commits.
Folllowing the previous commit, code that isn't going to be used on the apps is removed from the file that was previously copied/pasted. As the apps don't currently support multiple selection, it's not necessary to account for it when determining whether to display settings.
|
Size Change: 0 B Total Size: 1.09 MB ℹ️ View Unchanged
|
SiobhyB
changed the title
By only displaying the settings control if '__experimentalExposeControlsToChildren' is true (which is the default), parent blocks can now hide the control by setting this block support to false.
SiobhyB
added
Mobile App - i.e. Android or iOS
SiobhyB
changed the title
SiobhyB
changed the title
Merged
2 tasks
|
👋 @gziolo, I wondered whether you might have time to check whether If not, would creating a new experimental block support make sense? For example, Thanks in advance! |
guarani
reviewed
Nov 10, 2021
There was a problem hiding this comment.
👋 I left a comment below with what the point I raised with you offline today, @SiobhyB.
Even if it turns out that __experimentalExposeControlsToChildren can't be used for the purpose defined in this PR, I'm hopeful we could create another "feature" for this purpose e.g. __experimentalHideChildrensControls. I'm looking forward to seeing whether this is a possibility.
packages/block-editor/src/components/use-display-block-controls/index.native.js
Outdated
Show resolved
Hide resolved
The '__experimentalExposeControlsToChildren' block support served a different purpose to our needs. With this commit, the support we're using has been renamed to '__experimentalDisplayChildBlockControls' to more accurately reflect its intention.
guarani
approved these changes
Nov 11, 2021
There was a problem hiding this comment.
Thanks @SiobhyB! The new __experimentalDisplayChildBlockControls block support is a great way to allow parent blocks to define the availability of their child block's settings.
I tested this on a variety of blocks using the demo app and it worked as expected. I left a suggestion above, but feel this is also good to merge as-is.
packages/block-editor/src/components/use-display-block-controls/index.native.js
Outdated
Show resolved
Hide resolved
added 2 commits
November 11, 2021 22:56
The '__experimentalDisplayChildBlockControls' is renamed to '__experimentalHideChildBlockControls' with this commit. This was suggested here: #36371 (comment) With the following reason as to why this will improve clarity: "I feel it's a bit odd introducing a new block supports that declares a feature that already exists: child block controls. It might be confusing to see blocks that don't include this particular block supports at all, but still display their child's block controls."
|
What’s the rationale behind bringing this functionality only to the mobile app? Why isn’t it an issue in the web version? In my mind, we should aim to align APIs and UX between platforms. How this flag is going to be consumed? The current proposal will impact only all direct children of the block that enables this flag. For the existing flag, there is ongoing discussion to allow block authors to use an option to propagate the behavior to deeply nested inner blocks. |
Thanks for your input here @gziolo! 🙇♀️ The context behind what led to this PR can be broken down as follows:
This is the exact scenario that we've come across while attempting to use inner blocks for a third party block, Jetpack's Tiled Gallery, and we think other third parties could also get blocked with the same issues. As the web version of a block is generally the first to be created, and we don't currently have many third party native blocks, I'm not certain if there would be as much of a use case for web developers to hide the settings control for inner blocks. Do you think there would still be a case for introducing |
|
@SiobhyB, thank you your detailed answer. I'm still thinking whether we can avoid introducing a new API and bring the same behavior based on different factors.
Can you remind me what settings control is? You mentioned the HTML editing mode so in my mind it's the same as:
Or is it related to the |
Ah, sorry for the confusion, @gziolo! Yes, I'm referring to the
The reason I mentioned HTML is that changes to those settings will also change the inner block's HTML, which we're not able to support in our use case.
I'd be really grateful if you have other ideas for hiding those settings. 🙇♀️ We'll also need to customize other parts of inner blocks in future (for example, hiding the |
SiobhyB
force-pushed
the
rnmobile/try/hide-child-block-controls
branch
from
fa74b59
to
94f4ea1
Compare
|
@SiobhyB, feel free to land it as is. I’ll keep thinking if there is an alternative possible 👍 |
|
Thank you @gziolo! I'll go ahead to merge now with a view to iterate if we come up with a better solution at a later date 👍 |
SiobhyB
pushed a commit
to Automattic/jetpack
that referenced
this pull request
Nov 16, 2021
As discussed in WordPress/gutenberg#36371, '__experimentalExposeControlsToChildren' doesn't accurately reflect what we're trying to achieve. Instead, a new block supports, '__experimentalHideChildBlockControls', as been created for accuracy and clarity.
SiobhyB
pushed a commit
to Automattic/jetpack
that referenced
this pull request
Nov 16, 2021
This PR leverages the `__experimentalHideChildBlockControls` block supports introduced in WordPress/gutenberg#36371 to hide the settings tab for the inner image blocks.
SiobhyB
pushed a commit
that referenced
this pull request
Nov 18, 2021
If a parent block is hiding child block controls, we can reasonable deduce that that would include the option to edit the child block's caption. This commit builds on the work done in #36371 to detect if a parent block is hiding child block controls via the '__experimentalHideChildBlockControls' block supports.
7 tasks
SiobhyB
pushed a commit
that referenced
this pull request
Nov 18, 2021
…ocks (#36618) This commit provides a way for parent blocks to hide the block caption component (if used) in any inner child block. The code builds on the work done in #36371, which introduced a new "__experimentalHideChildBlockControls" block support. The purpose of the new block support is to hide child block settings/controls, which is helpful in cases when native blocks need to customize inner blocks. A further outline of the use case for this support can be found here. With this PR, the block caption component now also listens for "__experimentalHideChildBlockControls". If it is set to true then any captions are hidden from child blocks.
This was referenced Nov 22, 2021
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
More and more blocks are beginning to use the Inner Block API to pull in child blocks to a parent block. As part of this, it's sometimes necessary to change or hide certain parts of a child block.
For the native apps, the settings control is included within each block. This is different to the web, where the block's settings are separated out into a sidebar. Due to this difference, the ability to hide the settings control for child blocks is likely to be a fairly common use case for parent blocks on the native side.
With this PR, a
__experimentalHideChildBlockControlsblock support, is introduced. If a parent block declares__experimentalHideChildBlockControlsto betruewith this PR applied, then the settings control for its child blocks won't be displayed. It is set tofalseby default.The support was inspired by other experimental block supports that allow developers to change aspects of child blocks, for example: Following discussions in #26313,
__experimentalExposeControlsToChildrenand__experimentalShareWithChildBlockswere introduced in #33955 and #34644.How has this been tested?
Note: The Columns block is being used as a proof of concept here and for the purposes of testing. We are not planning to actually remove the settings control from inner Column blocks. The following steps can be repurposed for any block using inner blocks.
"__experimentalHideChildBlockControls": trueto the columns' block'sblock.jsonfile. It should be added to the block supports object here.We should also test other blocks using inner blocks, such as Social Icons and Buttons, to verify there are no regressions.
Screenshots
Types of changes
This PR introduces a non-breaking change which fixes an issue, with the following high-level overview of the code:
use-display-block-controlscomponent was created in d902f41.__experimentalHideChildBlockControlsis true was added to the newly created file. This will now enable parent blocks to set the value totrueif the settings control should be hidden from child blocks.hasBlockSupportis used to achieve this.Checklist:
*.native.jsfiles for terms that need renaming or removal).