Skip to content

Commit

Permalink
Block removal prompt: let consumers pass their own rules (#51841)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mcsf committed Jun 30, 2023
1 parent d9ee487 commit 862d719
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,61 +16,47 @@ 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;
}

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>
) }
Expand All @@ -80,7 +66,7 @@ export function BlockRemovalWarningModal() {
: __( 'Removing this block is not advised.' ) }
</p>
<HStack justify="right">
<Button variant="tertiary" onClick={ clearRemovalPrompt }>
<Button variant="tertiary" onClick={ clearBlockRemovalPrompt }>
{ __( 'Cancel' ) }
</Button>
<Button variant="primary" onClick={ onConfirmRemoval }>
Expand Down
57 changes: 33 additions & 24 deletions packages/block-editor/src/store/private-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ];

Expand Down Expand Up @@ -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 );
Expand All @@ -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 )
Expand Down Expand Up @@ -237,24 +231,27 @@ 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
* or the immediate parent
* (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,
Expand All @@ -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,
};
}
4 changes: 2 additions & 2 deletions packages/block-editor/src/store/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
24 changes: 16 additions & 8 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1480,32 +1480,40 @@ 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;
}

return state;
}

/**
* 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;
Expand Down Expand Up @@ -1930,7 +1938,7 @@ const combinedReducers = combineReducers( {
blockVisibility,
blockEditingModes,
removalPromptData,
isRemovalPromptSupported,
blockRemovalRules,
} );

function withAutomaticChangeReset( reducer ) {
Expand Down
6 changes: 3 additions & 3 deletions packages/block-editor/src/store/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
13 changes: 12 additions & 1 deletion packages/edit-site/src/components/editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -197,7 +206,9 @@ export default function Editor( { isLoading } ) {
{ showVisualEditor && editedPost && (
<>
<BlockEditor />
<BlockRemovalWarningModal />
<BlockRemovalWarningModal
rules={ blockRemovalRules }
/>
</>
) }
{ editorMode === 'text' &&
Expand Down
67 changes: 67 additions & 0 deletions test/e2e/specs/site-editor/block-removal.spec.js
Original file line number Diff line number Diff line change
@@ -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();
} );
} );

1 comment on commit 862d719

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 862d719.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5423636698
📝 Reported issues:

Please sign in to comment.