Skip to content

Commit 6163fc4

Browse files
talldantellthemachinesramonjdandrewserongscruffian
authored andcommitted
Fix contentOnly insertion, removal, and moving (#72416)
* Fix confusion between clientId and rootClientId * Fix contentOnly insertion logic for blocks at root of section * Fix movers appearing in contentOnly mode * Rename selector that mentions write mode * Fix typo * Add e2e tests for synced patterns to ensure blocks cannot be moved duplicate or deleted * Update move behavior to disable movers for templateLock contentOnly * Add contentOnly templateLock tests for block duplication, insertion and moving * Add buttons example to pattern overrides test too * Fix selector memoization * Make the selectors work in a more similar fashion * Fix test * Allow moving for template lock of `insert` * Make home link a content block * Condense identical test cases ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: scruffian <scruffian@git.wordpress.org>
1 parent be5d5fd commit 6163fc4

File tree

8 files changed

+383
-30
lines changed

8 files changed

+383
-30
lines changed

packages/block-editor/src/components/block-list/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ function Items( {
184184
getTemplateLock,
185185
getBlockEditingMode,
186186
isSectionBlock,
187-
isContainerInsertableToInWriteMode,
187+
isContainerInsertableToInContentOnlyMode,
188188
getBlockName,
189189
isZoomOut: _isZoomOut,
190190
canInsertBlockType,
@@ -223,7 +223,7 @@ function Items( {
223223
isZoomOut: _isZoomOut(),
224224
shouldRenderAppender:
225225
( ! isSectionBlock( rootClientId ) ||
226-
isContainerInsertableToInWriteMode(
226+
isContainerInsertableToInContentOnlyMode(
227227
getBlockName( selectedBlockClientId ),
228228
rootClientId
229229
) ) &&

packages/block-editor/src/store/private-selectors.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export const isBlockSubtreeDisabled = ( state, clientId ) => {
9595
* @param {string} rootClientId The client ID of the root container block.
9696
* @return {boolean} Whether the container allows insertion.
9797
*/
98-
export function isContainerInsertableToInWriteMode(
98+
export function isContainerInsertableToInContentOnlyMode(
9999
state,
100100
blockName,
101101
rootClientId
@@ -105,7 +105,7 @@ export function isContainerInsertableToInWriteMode(
105105
const isContainerContentBlock = isContentBlock( rootBlockName );
106106
const isRootBlockMain = getSectionRootClientId( state ) === rootClientId;
107107

108-
// In write mode, containers shouldn't be inserted into unless:
108+
// In contentOnly mode, containers shouldn't be inserted into unless:
109109
// 1. they are a section root;
110110
// 2. they are a content block and the block to be inserted is also content.
111111
return (

packages/block-editor/src/store/selectors.js

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import {
4343
isSectionBlock,
4444
getParentSectionBlock,
4545
isZoomOut,
46-
isContainerInsertableToInWriteMode,
46+
isContainerInsertableToInContentOnlyMode,
4747
} from './private-selectors';
4848

4949
const { isContentBlock } = unlock( blocksPrivateApis );
@@ -1682,15 +1682,7 @@ const canInsertBlockTypeUnmemoized = (
16821682
blockType = getBlockType( blockName );
16831683
}
16841684

1685-
const isLocked = !! getTemplateLock( state, rootClientId );
1686-
if ( isLocked ) {
1687-
return false;
1688-
}
1689-
const isContentRoleBlock = isContentBlock( blockName );
1690-
const isParentSectionBlock = !! isSectionBlock( state, rootClientId );
1691-
// It shouldn't be possible to insert inside a section block unless in
1692-
// some cases when the block is a content block.
1693-
if ( isParentSectionBlock && ! isContentRoleBlock ) {
1685+
if ( getTemplateLock( state, rootClientId ) ) {
16941686
return false;
16951687
}
16961688

@@ -1707,10 +1699,29 @@ const canInsertBlockTypeUnmemoized = (
17071699
return false;
17081700
}
17091701

1710-
// In write mode, check if this container allows insertion.
1702+
// It shouldn't be possible to insert inside a section block unless in
1703+
// some cases when the block is a content block.
1704+
const isContentRoleBlock = isContentBlock( blockName );
1705+
const isParentSectionBlock = !! isSectionBlock( state, rootClientId );
1706+
const isBlockWithinSection = !! getParentSectionBlock(
1707+
state,
1708+
rootClientId
1709+
);
1710+
if (
1711+
( isParentSectionBlock || isBlockWithinSection ) &&
1712+
! isContentRoleBlock
1713+
) {
1714+
return false;
1715+
}
1716+
1717+
// In content only mode, check if this container allows insertion.
17111718
if (
1712-
blockEditingMode === 'contentOnly' &&
1713-
! isContainerInsertableToInWriteMode( state, blockName, rootClientId )
1719+
( isParentSectionBlock || blockEditingMode === 'contentOnly' ) &&
1720+
! isContainerInsertableToInContentOnlyMode(
1721+
state,
1722+
blockName,
1723+
rootClientId
1724+
)
17141725
) {
17151726
return false;
17161727
}
@@ -1858,6 +1869,8 @@ export function canRemoveBlock( state, clientId ) {
18581869
return false;
18591870
}
18601871

1872+
// It shouldn't be possible to move in a section block unless in
1873+
// some cases when the block is a content block.
18611874
const isBlockWithinSection = !! getParentSectionBlock( state, clientId );
18621875
const isContentRoleBlock = isContentBlock(
18631876
getBlockName( state, clientId )
@@ -1866,21 +1879,21 @@ export function canRemoveBlock( state, clientId ) {
18661879
return false;
18671880
}
18681881

1869-
const blockEditingMode = getBlockEditingMode( state, rootClientId );
1870-
1871-
// Check if the parent container allows insertion/removal in write mode
1882+
const isParentSectionBlock = !! isSectionBlock( state, rootClientId );
1883+
const rootBlockEditingMode = getBlockEditingMode( state, rootClientId );
1884+
// Check if the parent container allows insertion/removal in contentOnly mode
18721885
if (
1873-
blockEditingMode === 'contentOnly' &&
1874-
! isContainerInsertableToInWriteMode(
1886+
( isParentSectionBlock || rootBlockEditingMode === 'contentOnly' ) &&
1887+
! isContainerInsertableToInContentOnlyMode(
18751888
state,
1876-
getBlockName( state, rootClientId ),
1889+
getBlockName( state, clientId ),
18771890
rootClientId
18781891
)
18791892
) {
18801893
return false;
18811894
}
18821895

1883-
return blockEditingMode !== 'disabled';
1896+
return rootBlockEditingMode !== 'disabled';
18841897
}
18851898

18861899
/**
@@ -1913,9 +1926,34 @@ export function canMoveBlock( state, clientId ) {
19131926
}
19141927

19151928
const rootClientId = getBlockRootClientId( state, clientId );
1916-
if ( getTemplateLock( state, rootClientId ) === 'all' ) {
1929+
const templateLock = getTemplateLock( state, rootClientId );
1930+
if ( templateLock === 'all' || templateLock === 'contentOnly' ) {
1931+
return false;
1932+
}
1933+
1934+
const isBlockWithinSection = !! getParentSectionBlock( state, clientId );
1935+
const isContentRoleBlock = isContentBlock(
1936+
getBlockName( state, clientId )
1937+
);
1938+
if ( isBlockWithinSection && ! isContentRoleBlock ) {
19171939
return false;
19181940
}
1941+
1942+
// If the parent is a section or is `contentOnly`, then check is the inner block
1943+
// should be allowed to move.
1944+
const isParentSectionBlock = !! isSectionBlock( state, rootClientId );
1945+
const rootBlockEditingMode = getBlockEditingMode( state, rootClientId );
1946+
if (
1947+
( isParentSectionBlock || rootBlockEditingMode === 'contentOnly' ) &&
1948+
! isContainerInsertableToInContentOnlyMode(
1949+
state,
1950+
getBlockName( state, clientId ),
1951+
rootClientId
1952+
)
1953+
) {
1954+
return false;
1955+
}
1956+
19191957
return getBlockEditingMode( state, rootClientId ) !== 'disabled';
19201958
}
19211959

packages/block-editor/src/store/test/selectors.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3081,7 +3081,7 @@ describe( 'selectors', () => {
30813081
byClientId: new Map(
30823082
Object.entries( {
30833083
block1: { name: 'core/test-block-ancestor' },
3084-
block2: { name: 'core/block' },
3084+
block2: { name: 'core/group' },
30853085
block3: { name: 'core/test-block-parent' },
30863086
} )
30873087
),

packages/block-editor/src/store/utils.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { parse as grammarParse } from '@wordpress/block-serialization-default-pa
1010
import { selectBlockPatternsKey } from './private-keys';
1111
import { unlock } from '../lock-unlock';
1212
import { STORE_NAME } from './constants';
13-
import { getSectionRootClientId } from './private-selectors';
13+
import { getSectionRootClientId, isSectionBlock } from './private-selectors';
1414
import { getBlockEditingMode } from './selectors';
1515
import { INSERTER_PATTERN_TYPES } from '../components/inserter/block-patterns-tab/utils';
1616

@@ -140,5 +140,6 @@ export const getInsertBlockTypeDependants = () => ( state, rootClientId ) => {
140140
state.settings.templateLock,
141141
getBlockEditingMode( state, rootClientId ),
142142
getSectionRootClientId( state ),
143+
isSectionBlock( state, rootClientId ),
143144
];
144145
};

packages/block-library/src/home-link/block.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
"textdomain": "default",
1010
"attributes": {
1111
"label": {
12-
"type": "string"
12+
"type": "string",
13+
"role": "content"
1314
}
1415
},
1516
"usesContext": [

test/e2e/specs/editor/various/content-only-lock.spec.js

Lines changed: 136 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,12 @@ test.describe( 'Content-only lock', () => {
9090
<div class="wp-block-group"><!-- wp:paragraph -->
9191
<p>Locked block a</p>
9292
<!-- /wp:paragraph -->
93-
93+
9494
<!-- wp:paragraph -->
9595
<p>Locked block b</p>
9696
<!-- /wp:paragraph --></div>
9797
<!-- /wp:group -->
98-
98+
9999
<!-- wp:heading -->
100100
<h2 class="wp-block-heading"><strong>outside block</strong></h2>
101101
<!-- /wp:heading -->` );
@@ -128,4 +128,138 @@ test.describe( 'Content-only lock', () => {
128128
page.locator( '.color-block-support-panel' )
129129
).not.toBeAttached();
130130
} );
131+
132+
test( 'blocks within content-only locked container cannot be duplicated, inserted before/after, or moved', async ( {
133+
editor,
134+
page,
135+
pageUtils,
136+
} ) => {
137+
// Add content only locked block with paragraph and list
138+
await pageUtils.pressKeys( 'secondary+M' );
139+
140+
await page.getByPlaceholder( 'Start writing with text or HTML' )
141+
.fill( `<!-- wp:group {"templateLock":"contentOnly","layout":{"type":"constrained"}} -->
142+
<div class="wp-block-group"><!-- wp:paragraph -->
143+
<p>First paragraph</p>
144+
<!-- /wp:paragraph -->
145+
146+
<!-- wp:list -->
147+
<ul class="wp-block-list"><!-- wp:list-item -->
148+
<li>List item one</li>
149+
<!-- /wp:list-item -->
150+
151+
<!-- wp:list-item -->
152+
<li>List item two</li>
153+
<!-- /wp:list-item --></ul>
154+
<!-- /wp:list --></div>
155+
<!-- /wp:group -->` );
156+
157+
await pageUtils.pressKeys( 'secondary+M' );
158+
159+
const groupBlock = editor.canvas.getByRole( 'document', {
160+
name: 'Block: Group',
161+
} );
162+
const paragraph = editor.canvas
163+
.getByRole( 'document', {
164+
name: 'Block: Paragraph',
165+
includeHidden: true,
166+
} )
167+
.filter( { hasText: 'First paragraph' } );
168+
const firstListItem = editor.canvas
169+
.getByRole( 'document', {
170+
name: 'Block: List item',
171+
includeHidden: true,
172+
} )
173+
.filter( { hasText: 'List item one' } );
174+
const secondListItem = editor.canvas
175+
.getByRole( 'document', {
176+
name: 'Block: List item',
177+
includeHidden: true,
178+
} )
179+
.filter( { hasText: 'List item two' } );
180+
181+
// Select the content-locked group block.
182+
await editor.selectBlocks( groupBlock );
183+
await test.step( 'Blocks cannot be inserted before/after or duplicated', async () => {
184+
// Test paragraph.
185+
await editor.selectBlocks( paragraph );
186+
await editor.showBlockToolbar();
187+
188+
await expect(
189+
page
190+
.getByRole( 'toolbar', { name: 'Block tools' } )
191+
.getByRole( 'button', { name: 'Options' } )
192+
).toBeHidden();
193+
194+
// Test first list item.
195+
await editor.selectBlocks( firstListItem );
196+
await editor.showBlockToolbar();
197+
198+
await expect(
199+
page
200+
.getByRole( 'toolbar', { name: 'Block tools' } )
201+
.getByRole( 'button', { name: 'Options' } )
202+
).toBeHidden();
203+
204+
// Test second list item.
205+
await editor.selectBlocks( secondListItem );
206+
await editor.showBlockToolbar();
207+
208+
await expect(
209+
page
210+
.getByRole( 'toolbar', { name: 'Block tools' } )
211+
.getByRole( 'button', { name: 'Options' } )
212+
).toBeHidden();
213+
} );
214+
215+
await test.step( 'Blocks cannot be moved', async () => {
216+
// Test paragraph.
217+
await editor.selectBlocks( paragraph );
218+
await editor.showBlockToolbar();
219+
220+
await expect(
221+
page
222+
.getByRole( 'toolbar', { name: 'Block tools' } )
223+
.getByRole( 'button', { name: 'Move up' } )
224+
).toBeHidden();
225+
226+
await expect(
227+
page
228+
.getByRole( 'toolbar', { name: 'Block tools' } )
229+
.getByRole( 'button', { name: 'Move down' } )
230+
).toBeHidden();
231+
232+
// Test first list item.
233+
await editor.selectBlocks( firstListItem );
234+
await editor.showBlockToolbar();
235+
236+
await expect(
237+
page
238+
.getByRole( 'toolbar', { name: 'Block tools' } )
239+
.getByRole( 'button', { name: 'Move up' } )
240+
).toBeHidden();
241+
242+
await expect(
243+
page
244+
.getByRole( 'toolbar', { name: 'Block tools' } )
245+
.getByRole( 'button', { name: 'Move down' } )
246+
).toBeHidden();
247+
248+
// Test second list item.
249+
await editor.selectBlocks( secondListItem );
250+
await editor.showBlockToolbar();
251+
252+
await expect(
253+
page
254+
.getByRole( 'toolbar', { name: 'Block tools' } )
255+
.getByRole( 'button', { name: 'Move up' } )
256+
).toBeHidden();
257+
258+
await expect(
259+
page
260+
.getByRole( 'toolbar', { name: 'Block tools' } )
261+
.getByRole( 'button', { name: 'Move down' } )
262+
).toBeHidden();
263+
} );
264+
} );
131265
} );

0 commit comments

Comments
 (0)