Make getBlockEditingMode() return 'default' when parent is 'contentOnly' #51185
Conversation
This file contains hidden or 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
Change the behaviour of getBlockEditingMode() so that a block's children is considered "content" that is editable when the editing mode is 'contentOnly'. Therefore getBlockEditingMode( foo ) should be 'default' when getBlockEditingMode( getParent( foo ) ) is 'contentOnly'. The benefit of this is that block subtrees can be enabled by simply setting the parent container to 'contentOnly' rather than each individual block to 'default'. This is much more performant as each call to setBlockEditingMode() invalidates the cache.
noisysocks
added
[Type] Performance
14 tasks
…g-mode-performance
Member
Author
|
Just thinking out loud, it's not important right now, but I wonder if an object containing flags would be more flexible than encapsulating everything into function MyBlockEdit( { attributes } ) {
useBlockEditingMode( {
selection: false,
movers: false,
settingsMenu: false,
removeButton: false,
inserter: false,
} );
return ( ... );
} |
|
Size Change: -32 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
Contributor
tellthemachines
left a comment
tellthemachines
left a comment
There was a problem hiding this comment.
Changes LGTM, with the disclaimer that I'm no expert in these methods!
All blocks should be locked except for the Post Title, Post Featured Image and Post Content blocks.
Testing content editing with the patch applied mostly works as expected. One question: should we be able to edit the heading level of the Post Title block? That seems more like template editing functionality.
| state, | ||
| 'b3247f75-fd94-4fef-97f9-5bfd162cc416' | ||
| ) | ||
| ).toBe( 'disabled' ); |
Contributor
There was a problem hiding this comment.
Did anything change in what these tests check for, or did you just unfold the loop for better legibility?
Member
Author
There was a problem hiding this comment.
I unfolded the loop so as to change one key thing which is that if the parent block has an editing mode of 'contentOnly' then the child block will now have an editing mode of 'default' not 'contentOnly'.
Contributor
There was a problem hiding this comment.
Oh, got it, that makes sense!
Member
Author
|
Thanks @tellthemachines!
In the other PR I've got some code that disables editing the heading level. |
tellthemachines
approved these changes
Jun 2, 2023
artemiomorales
pushed a commit
to artemiomorales/gutenberg
that referenced
this pull request
Jun 2, 2023
…ly' (WordPress#51185) Change the behaviour of getBlockEditingMode() so that a block's children is considered "content" that is editable when the editing mode is 'contentOnly'. Therefore getBlockEditingMode( foo ) should be 'default' when getBlockEditingMode( getParent( foo ) ) is 'contentOnly'. The benefit of this is that block subtrees can be enabled by simply setting the parent container to 'contentOnly' rather than each individual block to 'default'. This is much more performant as each call to setBlockEditingMode() invalidates the cache.
13 tasks
noisysocks
added
the
[Feature] Page Content Focus
sethrubenstein
pushed a commit
to pewresearch/gutenberg
that referenced
this pull request
Jul 13, 2023
…ly' (WordPress#51185) Change the behaviour of getBlockEditingMode() so that a block's children is considered "content" that is editable when the editing mode is 'contentOnly'. Therefore getBlockEditingMode( foo ) should be 'default' when getBlockEditingMode( getParent( foo ) ) is 'contentOnly'. The benefit of this is that block subtrees can be enabled by simply setting the parent container to 'contentOnly' rather than each individual block to 'default'. This is much more performant as each call to setBlockEditingMode() invalidates the cache.
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.
What?
Follows #50643.
Needed to fix the failing performance tests in #50857.
Change the behaviour of
getBlockEditingMode()so that a block's children is considered "content" that is editable when the editing mode is'contentOnly'. ThereforegetBlockEditingMode( foo )should be'default'whengetBlockEditingMode( getParent( foo ) )is'contentOnly'.Why?
The benefit of this is that block subtrees can be enabled by simply setting the parent container's editing mode to
'contentOnly'rather than setting the editing mode of every descendant block to'default'. This is much more performant as each call tosetBlockEditingMode()invalidates the cache.To illustrate, this is what we had to do before to allow the full editing of nested blocks within the Post Content block:
But now we can simply do this:
It's much more performant because each nested block within Post Content (there could be thousands) doesn't need to call
setBlockEditingMode().Testing Instructions
Unit tests should cover this pretty well.
You can also check that
useBlockEditingModefunctions correctly by:pbpaste | git apply.To test the existing
contentOnlyfunctionality for regressions: