Repository navigation
Fix contentOnly insertion, removal, and moving #72416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0604c2c
bc1601e
2e67350
af45a9f
624ec7b
05650ec
46634b6
cc56dab
21ab505
3b7642a
8390d65
9129bf2
ca00af1
4123bd5
63cec0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,7 +43,7 @@ import { | |
| isSectionBlock, | ||
| getParentSectionBlock, | ||
| isZoomOut, | ||
| isContainerInsertableToInWriteMode, | ||
| isContainerInsertableToInContentOnlyMode, | ||
| } from './private-selectors'; | ||
|
|
||
| const { isContentBlock } = unlock( blocksPrivateApis ); | ||
|
|
@@ -1682,15 +1682,7 @@ const canInsertBlockTypeUnmemoized = ( | |
| blockType = getBlockType( blockName ); | ||
| } | ||
|
|
||
| const isLocked = !! getTemplateLock( state, rootClientId ); | ||
| if ( isLocked ) { | ||
| return false; | ||
| } | ||
| const isContentRoleBlock = isContentBlock( blockName ); | ||
| const isParentSectionBlock = !! isSectionBlock( state, rootClientId ); | ||
| // It shouldn't be possible to insert inside a section block unless in | ||
| // some cases when the block is a content block. | ||
| if ( isParentSectionBlock && ! isContentRoleBlock ) { | ||
| if ( getTemplateLock( state, rootClientId ) ) { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -1707,10 +1699,29 @@ const canInsertBlockTypeUnmemoized = ( | |
| return false; | ||
| } | ||
|
|
||
| // In write mode, check if this container allows insertion. | ||
| // It shouldn't be possible to insert inside a section block unless in | ||
| // some cases when the block is a content block. | ||
| const isContentRoleBlock = isContentBlock( blockName ); | ||
| const isParentSectionBlock = !! isSectionBlock( state, rootClientId ); | ||
| const isBlockWithinSection = !! getParentSectionBlock( | ||
| state, | ||
| rootClientId | ||
| ); | ||
| if ( | ||
| ( isParentSectionBlock || isBlockWithinSection ) && | ||
| ! isContentRoleBlock | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| // In content only mode, check if this container allows insertion. | ||
| if ( | ||
| blockEditingMode === 'contentOnly' && | ||
| ! isContainerInsertableToInWriteMode( state, blockName, rootClientId ) | ||
| ( isParentSectionBlock || blockEditingMode === 'contentOnly' ) && | ||
| ! isContainerInsertableToInContentOnlyMode( | ||
| state, | ||
| blockName, | ||
| rootClientId | ||
| ) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
@@ -1858,6 +1869,8 @@ export function canRemoveBlock( state, clientId ) { | |
| return false; | ||
| } | ||
|
|
||
| // It shouldn't be possible to move in a section block unless in | ||
| // some cases when the block is a content block. | ||
| const isBlockWithinSection = !! getParentSectionBlock( state, clientId ); | ||
| const isContentRoleBlock = isContentBlock( | ||
| getBlockName( state, clientId ) | ||
|
|
@@ -1866,21 +1879,21 @@ export function canRemoveBlock( state, clientId ) { | |
| return false; | ||
| } | ||
|
|
||
| const blockEditingMode = getBlockEditingMode( state, rootClientId ); | ||
|
|
||
| // Check if the parent container allows insertion/removal in write mode | ||
| const isParentSectionBlock = !! isSectionBlock( state, rootClientId ); | ||
| const rootBlockEditingMode = getBlockEditingMode( state, rootClientId ); | ||
| // Check if the parent container allows insertion/removal in contentOnly mode | ||
| if ( | ||
| blockEditingMode === 'contentOnly' && | ||
| ! isContainerInsertableToInWriteMode( | ||
| ( isParentSectionBlock || rootBlockEditingMode === 'contentOnly' ) && | ||
| ! isContainerInsertableToInContentOnlyMode( | ||
| state, | ||
| getBlockName( state, rootClientId ), | ||
| getBlockName( state, clientId ), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line looks to be a typo, because it results in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checking if user-created unsynced patterns are supposed to be included? I can still delete child blocks. I enabled the content only experiment, created an unsynced pattern, then inserted it into a post via the inserter. The first pattern is mine, the second is a theme pattern inserted from the patterns inserter. Kapture.2025-10-21.at.11.56.38.mp4 |
||
| rootClientId | ||
| ) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| return blockEditingMode !== 'disabled'; | ||
| return rootBlockEditingMode !== 'disabled'; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1913,9 +1926,34 @@ export function canMoveBlock( state, clientId ) { | |
| } | ||
|
|
||
| const rootClientId = getBlockRootClientId( state, clientId ); | ||
| if ( getTemplateLock( state, rootClientId ) === 'all' ) { | ||
| const templateLock = getTemplateLock( state, rootClientId ); | ||
| if ( templateLock === 'all' || templateLock === 'contentOnly' ) { | ||
| return false; | ||
| } | ||
|
|
||
| const isBlockWithinSection = !! getParentSectionBlock( state, clientId ); | ||
| const isContentRoleBlock = isContentBlock( | ||
| getBlockName( state, clientId ) | ||
| ); | ||
| if ( isBlockWithinSection && ! isContentRoleBlock ) { | ||
| return false; | ||
| } | ||
|
|
||
| // If the parent is a section or is `contentOnly`, then check is the inner block | ||
| // should be allowed to move. | ||
| const isParentSectionBlock = !! isSectionBlock( state, rootClientId ); | ||
| const rootBlockEditingMode = getBlockEditingMode( state, rootClientId ); | ||
| if ( | ||
| ( isParentSectionBlock || rootBlockEditingMode === 'contentOnly' ) && | ||
| ! isContainerInsertableToInContentOnlyMode( | ||
| state, | ||
| getBlockName( state, clientId ), | ||
| rootClientId | ||
| ) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| return getBlockEditingMode( state, rootClientId ) !== 'disabled'; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3081,7 +3081,7 @@ describe( 'selectors', () => { | |
| byClientId: new Map( | ||
| Object.entries( { | ||
| block1: { name: 'core/test-block-ancestor' }, | ||
| block2: { name: 'core/block' }, | ||
| block2: { name: 'core/group' }, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| block3: { name: 'core/test-block-parent' }, | ||
| } ) | ||
| ), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ import { parse as grammarParse } from '@wordpress/block-serialization-default-pa | |
| import { selectBlockPatternsKey } from './private-keys'; | ||
| import { unlock } from '../lock-unlock'; | ||
| import { STORE_NAME } from './constants'; | ||
| import { getSectionRootClientId } from './private-selectors'; | ||
| import { getSectionRootClientId, isSectionBlock } from './private-selectors'; | ||
| import { getBlockEditingMode } from './selectors'; | ||
| import { INSERTER_PATTERN_TYPES } from '../components/inserter/block-patterns-tab/utils'; | ||
|
|
||
|
|
@@ -140,5 +140,6 @@ export const getInsertBlockTypeDependants = () => ( state, rootClientId ) => { | |
| state.settings.templateLock, | ||
| getBlockEditingMode( state, rootClientId ), | ||
| getSectionRootClientId( state ), | ||
| isSectionBlock( state, rootClientId ), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From looking at
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's simple enough not to cause an issue. |
||
| ]; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,8 @@ | |
| "textdomain": "default", | ||
| "attributes": { | ||
| "label": { | ||
| "type": "string" | ||
| "type": "string", | ||
| "role": "content" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to mention that there's a problem in In the navigation editor (in the site editor), the navigation block there is set to a In trunk it means you can't insert a 'Home Link' block at all because it's not a content block. This was caught by a test, so I'm fixing it here. While I've fixed Home Link, there might be other blocks that are affected (third party blocks too). It might be a back compat break (not to mention an issue in 6.9). We might need to consider again how that editor or the contentOnly insertion thing works. Maybe some kind of exception is needed for the navigation block, or maybe we can use a different technique to make sure the Navigation block can't be removed. cc @getdave @jeryj @scruffian to make you aware of this. I'll also make an issue.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've made an issue for this here - Navigation Editor: blocks without 'content' role can no longer be inserted/removed/moved |
||
| } | ||
| }, | ||
| "usesContext": [ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the parent is a 'section' it usually has a
blockEditingModeofdefault(so that you can move, insert, remove the section), but I think that results in skipping theisContainerInsertableToInContentOnlyModelogic, so I've added an extra check forisParentSectionBlockhere..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we maybe change
isContainerInsertableToInContentOnlyModeto remove the logic that allows inserting if a block is a section root? That made sense in write mode so blocks could be added to empty templates, but with the patterns approach we probably don't need to do that anymore?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me try it. I guess zoomed out would be the only concern, as I think it still uses section roots. Though I don't think it uses
contentOnlyat all, so I don't think it should be a problem.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still needed for zoomed out based on my testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could maybe look at removing it when we get to updating the zoom out logic (i.e. as part of #71807).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we should remove it when we simplify zoom out.