From 96cd10a82b288eb17a6ceddd0e90d316eb1493da Mon Sep 17 00:00:00 2001
From: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
Date: Fri, 23 Jun 2023 11:27:00 +0100
Subject: [PATCH 1/4] 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.
---
 .../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 ++++-
 6 files changed, 79 insertions(+), 65 deletions(-)

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 2e16d2834d2ce3..ee3e6b51e5e10c 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 { __, _n } 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 (
 		<Modal
 			title={ __( 'Are you sure?' ) }
-			onRequestClose={ clearRemovalPrompt }
+			onRequestClose={ clearBlockRemovalPrompt }
 		>
 			{ blockNamesForPrompt.length === 1 ? (
-				<p>{ blockTypePromptMessages[ blockNamesForPrompt[ 0 ] ] }</p>
+				<p>{ rules[ blockNamesForPrompt[ 0 ] ] }</p>
 			) : (
 				<ul style={ { listStyleType: 'disc', paddingLeft: '1rem' } }>
 					{ blockNamesForPrompt.map( ( name ) => (
-						<li key={ name }>
-							{ blockTypePromptMessages[ name ] }
-						</li>
+						<li key={ name }>{ rules[ name ] }</li>
 					) ) }
 				</ul>
 			) }
@@ -82,7 +68,7 @@ export function BlockRemovalWarningModal() {
 				) }
 			</p>
 			<HStack justify="right">
-				<Button variant="tertiary" onClick={ clearRemovalPrompt }>
+				<Button variant="tertiary" onClick={ clearBlockRemovalPrompt }>
 					{ __( 'Cancel' ) }
 				</Button>
 				<Button variant="primary" onClick={ onConfirmRemoval }>
diff --git a/packages/block-editor/src/store/private-actions.js b/packages/block-editor/src/store/private-actions.js
index d90aae340a92a7..475628c60f0f2e 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() ) {
+		let rules;
+		if ( ! forceRemove && ( rules = select.getBlockRemovalRules() ) ) {
 			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<string,string>|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<string,string>} 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 && (
 										<>
 											<BlockEditor />
-											<BlockRemovalWarningModal />
+											<BlockRemovalWarningModal
+												rules={ blockRemovalRules }
+											/>
 										</>
 									) }
 									{ editorMode === 'text' &&

From 04d5c45f148cdbecc783d28f8b71e7702240371b Mon Sep 17 00:00:00 2001
From: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
Date: Thu, 29 Jun 2023 17:51:03 +0100
Subject: [PATCH 2/4] Site editor: Add e2e test for block removal prompt

---
 .../specs/site-editor/block-removal.spec.js   | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 test/e2e/specs/site-editor/block-removal.spec.js

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..cd0b7a29bfbe0a
--- /dev/null
+++ b/test/e2e/specs/site-editor/block-removal.spec.js
@@ -0,0 +1,66 @@
+/**
+ * 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
+		await page.getByRole( 'button', { name: 'List View' } ).click();
+
+		// Select and try to remove Query Loop
+		await page.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
+		await page.getByRole( 'button', { name: 'List View' } ).click();
+
+		// Select Query Loop list item
+		await page.getByRole( 'link', { name: 'Query Loop' } ).click();
+
+		// Reveal its inner blocks in the list view
+		await page.keyboard.press( 'ArrowRight' );
+
+		// Select its Post Template inner block
+		await page.getByRole( 'link', { name: 'Post Template' } ).click();
+
+		// Expect to remove the Post Template with no prompts
+		expect( await getQueryBlockInnerBlocks( editor ) ).toHaveLength( 1 );
+		await page.keyboard.press( 'Backspace' );
+		expect( await getQueryBlockInnerBlocks( editor ) ).toHaveLength( 0 );
+	} );
+} );
+
+async function getQueryBlockInnerBlocks( editor ) {
+	const block = await editor.getBlocks();
+	const query = block.find( ( { name } ) => name === 'core/query' );
+	return query.innerBlocks;
+}

From 56015104adfb24530cfc7adc91b7e281d3b34036 Mon Sep 17 00:00:00 2001
From: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
Date: Fri, 30 Jun 2023 11:41:16 +0100
Subject: [PATCH 3/4] Address feedback around E2E tests

* Better scoping of selectors
* No redundant assertions
* .toBeHidden instead of .getBlocks logic

Props mamaduka
---
 .../specs/site-editor/block-removal.spec.js   | 35 ++++++++++---------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/test/e2e/specs/site-editor/block-removal.spec.js b/test/e2e/specs/site-editor/block-removal.spec.js
index cd0b7a29bfbe0a..66c75d722e4251 100644
--- a/test/e2e/specs/site-editor/block-removal.spec.js
+++ b/test/e2e/specs/site-editor/block-removal.spec.js
@@ -24,10 +24,12 @@ test.describe( 'Site editor block removal prompt', () => {
 		page,
 	} ) => {
 		// Open and focus List View
-		await page.getByRole( 'button', { name: 'List View' } ).click();
+		const topBar = page.getByRole( 'region', { name: 'Editor top bar' } );
+		await topBar.getByRole( 'button', { name: 'List View' } ).click();
 
-		// Select and try to remove Query Loop
-		await page.getByRole( 'link', { name: 'Query Loop' } ).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
@@ -41,26 +43,25 @@ test.describe( 'Site editor block removal prompt', () => {
 		page,
 	} ) => {
 		// Open and focus List View
-		await page.getByRole( 'button', { name: 'List View' } ).click();
+		const topBar = page.getByRole( 'region', { name: 'Editor top bar' } );
+		await topBar.getByRole( 'button', { name: 'List View' } ).click();
 
 		// Select Query Loop list item
-		await page.getByRole( 'link', { name: 'Query Loop' } ).click();
+		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 its Post Template inner block
-		await page.getByRole( 'link', { name: 'Post Template' } ).click();
-
-		// Expect to remove the Post Template with no prompts
-		expect( await getQueryBlockInnerBlocks( editor ) ).toHaveLength( 1 );
+		// Select and remove its Post Template inner block
+		await listView.getByRole( 'link', { name: 'Post Template' } ).click();
 		await page.keyboard.press( 'Backspace' );
-		expect( await getQueryBlockInnerBlocks( editor ) ).toHaveLength( 0 );
+
+		// Expect the block to have been removed with no prompt
+		await expect(
+			editor.canvas.getByRole( 'document', {
+				name: 'Block: Post Template',
+			} )
+		).toBeHidden();
 	} );
 } );
-
-async function getQueryBlockInnerBlocks( editor ) {
-	const block = await editor.getBlocks();
-	const query = block.find( ( { name } ) => name === 'core/query' );
-	return query.innerBlocks;
-}

From 7860ad2da63bf6cd648063aa279c19a0b86fac74 Mon Sep 17 00:00:00 2001
From: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
Date: Fri, 30 Jun 2023 14:19:00 +0100
Subject: [PATCH 4/4] Address feedback around syntax choices

Props ntsekouras
---
 packages/block-editor/src/store/private-actions.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/block-editor/src/store/private-actions.js b/packages/block-editor/src/store/private-actions.js
index 475628c60f0f2e..6a17be50f5c8bc 100644
--- a/packages/block-editor/src/store/private-actions.js
+++ b/packages/block-editor/src/store/private-actions.js
@@ -156,8 +156,8 @@ export const privateRemoveBlocks =
 		// register using `setBlockRemovalRules()`.
 		//
 		// @see https://github.com/WordPress/gutenberg/pull/51145
-		let rules;
-		if ( ! forceRemove && ( rules = select.getBlockRemovalRules() ) ) {
+		const rules = ! forceRemove && select.getBlockRemovalRules();
+		if ( rules ) {
 			const blockNamesForPrompt = new Set();
 
 			// Given a list of client IDs of blocks that the user intended to