Skip to content
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

Block removal prompt: let consumers pass their own rules #51841

Merged
merged 4 commits into from
Jun 30, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,61 +16,47 @@ 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;
}

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 @@ -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 }>
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() ) {
let rules;
if ( ! forceRemove && ( rules = select.getBlockRemovalRules() ) ) {
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
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();
} );
} );