From 862d71975d72abd942c5528e142662168c7e0e7a Mon Sep 17 00:00:00 2001
From: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
Date: Fri, 30 Jun 2023 14:50:43 +0100
Subject: [PATCH] Block removal prompt: let consumers pass their own rules
(#51841)
* Block removal prompt: let consumers pass their own rules
Following up on #51145, this untangles `edit-site` from `block-editor`
by removing the hard-coded set of rules `blockTypePromptMessages` from
the generic `BlockRemovalWarningModal` component. Rules are now to be
passed to that component by whichever block editor is using it.
Names and comments have been updated accordingly and improved.
* Site editor: Add e2e test for block removal prompt
---
.../block-removal-warning-modal/index.js | 40 ++++-------
.../block-editor/src/store/private-actions.js | 57 +++++++++-------
.../src/store/private-selectors.js | 4 +-
packages/block-editor/src/store/reducer.js | 24 ++++---
.../block-editor/src/store/test/actions.js | 6 +-
.../edit-site/src/components/editor/index.js | 13 +++-
.../specs/site-editor/block-removal.spec.js | 67 +++++++++++++++++++
7 files changed, 146 insertions(+), 65 deletions(-)
create mode 100644 test/e2e/specs/site-editor/block-removal.spec.js
diff --git a/packages/block-editor/src/components/block-removal-warning-modal/index.js b/packages/block-editor/src/components/block-removal-warning-modal/index.js
index 2ed65481f68959..08f3deccb5ae08 100644
--- a/packages/block-editor/src/components/block-removal-warning-modal/index.js
+++ b/packages/block-editor/src/components/block-removal-warning-modal/index.js
@@ -16,38 +16,26 @@ import { __ } from '@wordpress/i18n';
import { store as blockEditorStore } from '../../store';
import { unlock } from '../../lock-unlock';
-// In certain editing contexts, we'd like to prevent accidental removal of
-// important blocks. For example, in the site editor, the Query Loop block is
-// deemed important. In such cases, we'll ask the user for confirmation that
-// they intended to remove such block(s).
-//
-// @see https://github.com/WordPress/gutenberg/pull/51145
-export const blockTypePromptMessages = {
- 'core/query': __( 'Query Loop displays a list of posts or pages.' ),
- 'core/post-content': __(
- 'Post Content displays the content of a post or page.'
- ),
-};
-
-export function BlockRemovalWarningModal() {
+export function BlockRemovalWarningModal( { rules } ) {
const { clientIds, selectPrevious, blockNamesForPrompt } = useSelect(
( select ) =>
unlock( select( blockEditorStore ) ).getRemovalPromptData()
);
const {
- clearRemovalPrompt,
- toggleRemovalPromptSupport,
+ clearBlockRemovalPrompt,
+ setBlockRemovalRules,
privateRemoveBlocks,
} = unlock( useDispatch( blockEditorStore ) );
- // Signalling the removal prompt is in place.
+ // Load block removal rules, simultaneously signalling that the block
+ // removal prompt is in place.
useEffect( () => {
- toggleRemovalPromptSupport( true );
+ setBlockRemovalRules( rules );
return () => {
- toggleRemovalPromptSupport( false );
+ setBlockRemovalRules();
};
- }, [ toggleRemovalPromptSupport ] );
+ }, [ rules, setBlockRemovalRules ] );
if ( ! blockNamesForPrompt ) {
return;
@@ -55,22 +43,20 @@ export function BlockRemovalWarningModal() {
const onConfirmRemoval = () => {
privateRemoveBlocks( clientIds, selectPrevious, /* force */ true );
- clearRemovalPrompt();
+ clearBlockRemovalPrompt();
};
return (
{ blockNamesForPrompt.length === 1 ? (
- { blockTypePromptMessages[ blockNamesForPrompt[ 0 ] ] }
+ { rules[ blockNamesForPrompt[ 0 ] ] }
) : (
{ blockNamesForPrompt.map( ( name ) => (
-
- { blockTypePromptMessages[ name ] }
-
+ { rules[ name ] }
) ) }
) }
@@ -80,7 +66,7 @@ export function BlockRemovalWarningModal() {
: __( 'Removing this block is not advised.' ) }
-
+
{ __( 'Cancel' ) }
diff --git a/packages/block-editor/src/store/private-actions.js b/packages/block-editor/src/store/private-actions.js
index d90aae340a92a7..6a17be50f5c8bc 100644
--- a/packages/block-editor/src/store/private-actions.js
+++ b/packages/block-editor/src/store/private-actions.js
@@ -3,11 +3,6 @@
*/
import { Platform } from '@wordpress/element';
-/**
- * Internal dependencies
- */
-import { blockTypePromptMessages } from '../components/block-removal-warning-modal';
-
const castArray = ( maybeArray ) =>
Array.isArray( maybeArray ) ? maybeArray : [ maybeArray ];
@@ -158,23 +153,22 @@ export const privateRemoveBlocks =
// confirmation that they intended to remove such block(s). However,
// the editor instance is responsible for presenting those confirmation
// prompts to the user. Any instance opting into removal prompts must
- // register using `toggleRemovalPromptSupport()`.
+ // register using `setBlockRemovalRules()`.
//
// @see https://github.com/WordPress/gutenberg/pull/51145
- if ( ! forceRemove && select.isRemovalPromptSupported() ) {
+ const rules = ! forceRemove && select.getBlockRemovalRules();
+ if ( rules ) {
const blockNamesForPrompt = new Set();
// Given a list of client IDs of blocks that the user intended to
// remove, perform a tree search (BFS) to find all block names
// corresponding to "important" blocks, i.e. blocks that require a
// removal prompt.
- //
- // @see blockTypePromptMessages
const queue = [ ...clientIds ];
while ( queue.length ) {
const clientId = queue.shift();
const blockName = select.getBlockName( clientId );
- if ( blockTypePromptMessages[ blockName ] ) {
+ if ( rules[ blockName ] ) {
blockNamesForPrompt.add( blockName );
}
const innerBlocks = select.getBlockOrder( clientId );
@@ -185,7 +179,7 @@ export const privateRemoveBlocks =
// skip any other steps (thus postponing actual removal).
if ( blockNamesForPrompt.size ) {
dispatch(
- displayRemovalPrompt(
+ displayBlockRemovalPrompt(
clientIds,
selectPrevious,
Array.from( blockNamesForPrompt )
@@ -237,7 +231,7 @@ export const ensureDefaultBlock =
* Returns an action object used in signalling that a block removal prompt must
* be displayed.
*
- * Contrast with `toggleRemovalPromptSupport`.
+ * Contrast with `setBlockRemovalRules`.
*
* @param {string|string[]} clientIds Client IDs of blocks to remove.
* @param {boolean} selectPrevious True if the previous block
@@ -245,16 +239,19 @@ export const ensureDefaultBlock =
* (if no previous block exists)
* should be selected
* when a block is removed.
- * @param {string[]} blockNamesForPrompt Names of blocks requiring user
+ * @param {string[]} blockNamesForPrompt Names of the blocks that
+ * triggered the need for
+ * confirmation before removal.
+ *
* @return {Object} Action object.
*/
-export function displayRemovalPrompt(
+function displayBlockRemovalPrompt(
clientIds,
selectPrevious,
blockNamesForPrompt
) {
return {
- type: 'DISPLAY_REMOVAL_PROMPT',
+ type: 'DISPLAY_BLOCK_REMOVAL_PROMPT',
clientIds,
selectPrevious,
blockNamesForPrompt,
@@ -268,24 +265,36 @@ export function displayRemovalPrompt(
*
* @return {Object} Action object.
*/
-export function clearRemovalPrompt() {
+export function clearBlockRemovalPrompt() {
return {
- type: 'CLEAR_REMOVAL_PROMPT',
+ type: 'CLEAR_BLOCK_REMOVAL_PROMPT',
};
}
/**
- * Returns an action object used in signalling that a removal prompt display
- * mechanism is available or unavailable in the current editor.
+ * Returns an action object used to set up any rules that a block editor may
+ * provide in order to prevent a user from accidentally removing certain
+ * blocks. These rules are then used to display a confirmation prompt to the
+ * user. For instance, in the Site Editor, the Query Loop block is important
+ * enough to warrant such confirmation.
+ *
+ * IMPORTANT: Registering rules implicitly signals to the `privateRemoveBlocks`
+ * action that the editor will be responsible for displaying block removal
+ * prompts and confirming deletions. This action is meant to be used by
+ * component `BlockRemovalWarningModal` only.
+ *
+ * The data is a record whose keys are block types (e.g. 'core/query') and
+ * whose values are the explanation to be shown to users (e.g. 'Query Loop
+ * displays a list of posts or pages.').
*
- * Contrast with `displayRemovalPrompt`.
+ * Contrast with `displayBlockRemovalPrompt`.
*
- * @param {boolean} status Whether a prompt display mechanism exists.
+ * @param {Record|false} rules Block removal rules.
* @return {Object} Action object.
*/
-export function toggleRemovalPromptSupport( status = true ) {
+export function setBlockRemovalRules( rules = false ) {
return {
- type: 'TOGGLE_REMOVAL_PROMPT_SUPPORT',
- status,
+ type: 'SET_BLOCK_REMOVAL_RULES',
+ rules,
};
}
diff --git a/packages/block-editor/src/store/private-selectors.js b/packages/block-editor/src/store/private-selectors.js
index 454f3f32ba8f70..3abc1b0b3bdfd3 100644
--- a/packages/block-editor/src/store/private-selectors.js
+++ b/packages/block-editor/src/store/private-selectors.js
@@ -205,6 +205,6 @@ export function getRemovalPromptData( state ) {
*
* @return {boolean} Whether removal prompt exists.
*/
-export function isRemovalPromptSupported( state ) {
- return state.isRemovalPromptSupported;
+export function getBlockRemovalRules( state ) {
+ return state.blockRemovalRules;
}
diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js
index b4104a69ac620b..b4750e9e261be0 100644
--- a/packages/block-editor/src/store/reducer.js
+++ b/packages/block-editor/src/store/reducer.js
@@ -1480,14 +1480,14 @@ export function isSelectionEnabled( state = true, action ) {
*/
function removalPromptData( state = false, action ) {
switch ( action.type ) {
- case 'DISPLAY_REMOVAL_PROMPT':
+ case 'DISPLAY_BLOCK_REMOVAL_PROMPT':
const { clientIds, selectPrevious, blockNamesForPrompt } = action;
return {
clientIds,
selectPrevious,
blockNamesForPrompt,
};
- case 'CLEAR_REMOVAL_PROMPT':
+ case 'CLEAR_BLOCK_REMOVAL_PROMPT':
return false;
}
@@ -1495,17 +1495,25 @@ function removalPromptData( state = false, action ) {
}
/**
- * Reducer prompt availability state.
+ * Reducer returning any rules that a block editor may provide in order to
+ * prevent a user from accidentally removing certain blocks. These rules are
+ * then used to display a confirmation prompt to the user. For instance, in the
+ * Site Editor, the Query Loop block is important enough to warrant such
+ * confirmation.
+ *
+ * The data is a record whose keys are block types (e.g. 'core/query') and
+ * whose values are the explanation to be shown to users (e.g. 'Query Loop
+ * displays a list of posts or pages.').
*
* @param {boolean} state Current state.
* @param {Object} action Dispatched action.
*
- * @return {boolean} Updated state.
+ * @return {Record} Updated state.
*/
-function isRemovalPromptSupported( state = false, action ) {
+function blockRemovalRules( state = false, action ) {
switch ( action.type ) {
- case 'TOGGLE_REMOVAL_PROMPT_SUPPORT':
- return action.status;
+ case 'SET_BLOCK_REMOVAL_RULES':
+ return action.rules;
}
return state;
@@ -1930,7 +1938,7 @@ const combinedReducers = combineReducers( {
blockVisibility,
blockEditingModes,
removalPromptData,
- isRemovalPromptSupported,
+ blockRemovalRules,
} );
function withAutomaticChangeReset( reducer ) {
diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js
index 78df4a3e131d70..fc15737eda8d2f 100644
--- a/packages/block-editor/src/store/test/actions.js
+++ b/packages/block-editor/src/store/test/actions.js
@@ -618,7 +618,7 @@ describe( 'actions', () => {
const select = {
getBlockRootClientId: () => undefined,
canRemoveBlocks: () => true,
- isRemovalPromptSupported: () => false,
+ getBlockRemovalRules: () => false,
};
const dispatch = Object.assign( jest.fn(), {
selectPreviousBlock: jest.fn(),
@@ -729,7 +729,7 @@ describe( 'actions', () => {
const select = {
getBlockRootClientId: () => null,
canRemoveBlocks: () => true,
- isRemovalPromptSupported: () => false,
+ getBlockRemovalRules: () => false,
};
const dispatch = Object.assign( jest.fn(), {
selectPreviousBlock: jest.fn(),
@@ -754,7 +754,7 @@ describe( 'actions', () => {
const select = {
getBlockRootClientId: () => null,
canRemoveBlocks: () => true,
- isRemovalPromptSupported: () => false,
+ getBlockRemovalRules: () => false,
};
const dispatch = Object.assign( jest.fn(), {
selectPreviousBlock: jest.fn(),
diff --git a/packages/edit-site/src/components/editor/index.js b/packages/edit-site/src/components/editor/index.js
index 238cad73870553..9ff576e5842da6 100644
--- a/packages/edit-site/src/components/editor/index.js
+++ b/packages/edit-site/src/components/editor/index.js
@@ -63,6 +63,15 @@ const typeLabels = {
wp_block: __( 'Pattern' ),
};
+// Prevent accidental removal of certain blocks, asking the user for
+// confirmation.
+const blockRemovalRules = {
+ 'core/query': __( 'Query Loop displays a list of posts or pages.' ),
+ 'core/post-content': __(
+ 'Post Content displays the content of a post or page.'
+ ),
+};
+
export default function Editor( { isLoading } ) {
const {
record: editedPost,
@@ -197,7 +206,9 @@ export default function Editor( { isLoading } ) {
{ showVisualEditor && editedPost && (
<>
-
+
>
) }
{ editorMode === 'text' &&
diff --git a/test/e2e/specs/site-editor/block-removal.spec.js b/test/e2e/specs/site-editor/block-removal.spec.js
new file mode 100644
index 00000000000000..66c75d722e4251
--- /dev/null
+++ b/test/e2e/specs/site-editor/block-removal.spec.js
@@ -0,0 +1,67 @@
+/**
+ * WordPress dependencies
+ */
+const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );
+
+test.describe( 'Site editor block removal prompt', () => {
+ test.beforeAll( async ( { requestUtils } ) => {
+ await requestUtils.activateTheme( 'emptytheme' );
+ } );
+
+ test.afterAll( async ( { requestUtils } ) => {
+ await requestUtils.activateTheme( 'twentytwentyone' );
+ } );
+
+ test.beforeEach( async ( { admin, editor } ) => {
+ await admin.visitSiteEditor( {
+ postId: 'emptytheme//index',
+ postType: 'wp_template',
+ } );
+ await editor.canvas.click( 'body' );
+ } );
+
+ test( 'should appear when attempting to remove Query Block', async ( {
+ page,
+ } ) => {
+ // Open and focus List View
+ const topBar = page.getByRole( 'region', { name: 'Editor top bar' } );
+ await topBar.getByRole( 'button', { name: 'List View' } ).click();
+
+ // Select and try to remove Query Loop block
+ const listView = page.getByRole( 'region', { name: 'List View' } );
+ await listView.getByRole( 'link', { name: 'Query Loop' } ).click();
+ await page.keyboard.press( 'Backspace' );
+
+ // Expect the block removal prompt to have appeared
+ await expect(
+ page.getByText( 'Query Loop displays a list of posts or pages.' )
+ ).toBeVisible();
+ } );
+
+ test( 'should not appear when attempting to remove something else', async ( {
+ editor,
+ page,
+ } ) => {
+ // Open and focus List View
+ const topBar = page.getByRole( 'region', { name: 'Editor top bar' } );
+ await topBar.getByRole( 'button', { name: 'List View' } ).click();
+
+ // Select Query Loop list item
+ const listView = page.getByRole( 'region', { name: 'List View' } );
+ await listView.getByRole( 'link', { name: 'Query Loop' } ).click();
+
+ // Reveal its inner blocks in the list view
+ await page.keyboard.press( 'ArrowRight' );
+
+ // Select and remove its Post Template inner block
+ await listView.getByRole( 'link', { name: 'Post Template' } ).click();
+ await page.keyboard.press( 'Backspace' );
+
+ // Expect the block to have been removed with no prompt
+ await expect(
+ editor.canvas.getByRole( 'document', {
+ name: 'Block: Post Template',
+ } )
+ ).toBeHidden();
+ } );
+} );