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

Show warning on critical block removal #51145

Merged
merged 28 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2d11a45
Show warning on critical block removal
tellthemachines Jun 1, 2023
c1912d9
Extract prompt display logic into a hook
tellthemachines Jun 5, 2023
405c26b
Revert formatting change.
tellthemachines Jun 6, 2023
79ba8ad
Prompt for removal of all the blocks
tellthemachines Jun 6, 2023
15959b5
Move prompt state handling out of BlockList and into self
mcsf Jun 6, 2023
ce307ad
findCriticalBlocks: don't dismiss children of matching node
mcsf Jun 6, 2023
e5ddee6
findCriticalBlocks -> getBlocksToPromptFor
mcsf Jun 6, 2023
df68c00
Drop isBlockCritical()
mcsf Jun 6, 2023
042063a
Redesign removal modal
mcsf Jun 6, 2023
fb4c79d
Move prompt into site editor.
tellthemachines Jun 7, 2023
d3a12c8
Reset removalPromptExists upon listener unmount
mcsf Jun 20, 2023
c9952f1
Let action removeBlocks handle prompts and confirmations
mcsf Jun 20, 2023
33afda6
Add private action `privateRemoveBlocks` to hide extended interface
mcsf Jun 20, 2023
7482ba3
Fix unit tests
tellthemachines Jun 21, 2023
a5cce0a
Try: Dispatch setRemovalPromptStatus in edit-site init
mcsf Jun 21, 2023
b26efc4
Revert "Try: Dispatch setRemovalPromptStatus in edit-site init"
mcsf Jun 21, 2023
0e88e1a
Make all actions & selectors private. Rename things.
mcsf Jun 21, 2023
29f99c1
Make BlockRemovalWarningModal private
mcsf Jun 21, 2023
bb0cd7c
Cleanup: Remove BlockList changes from branch
mcsf Jun 22, 2023
c27ab49
Tweak removal message for Query. Tweak comments.
mcsf Jun 22, 2023
65df9cf
Split action into displayRemovalPrompt & clearRemovalPrompt
mcsf Jun 22, 2023
02df539
Rename setRemovalPromptStatus to toggleRemovalPromptSupport
mcsf Jun 22, 2023
d2b6a9f
Rename isRemovalPromptDisplayed to getRemovalPromptData
mcsf Jun 22, 2023
5d1c3e2
Add missing @return to displayRemovalPrompt
mcsf Jun 22, 2023
e180954
Tweak modal copy per feedback
mcsf Jun 22, 2023
23f034d
Turns out private selectors are attached to the thunk proxy!
mcsf Jun 22, 2023
6a0a806
Don't export the new reducers
mcsf Jun 22, 2023
628207a
Fix tests
mcsf Jun 22, 2023
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
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ function Root( { className, ...settings } ) {
);
const registry = useRegistry();
const { setBlockVisibility } = useDispatch( blockEditorStore );

const delayedBlockVisibilityUpdates = useDebounce(
useCallback( () => {
const updates = {};
Expand Down Expand Up @@ -151,6 +150,7 @@ function Items( {
getSelectedBlockClientIds,
__unstableGetVisibleBlocks,
} = select( blockEditorStore );

return {
order: getBlockOrder( rootClientId ),
selectedBlocks: getSelectedBlockClientIds(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* WordPress dependencies
*/
import { useEffect } from '@wordpress/element';
import { useDispatch, useSelect } from '@wordpress/data';
import {
Modal,
Button,
__experimentalHStack as HStack,
} from '@wordpress/components';
import { __, _n } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../../store';
import { unlock } from '../../lock-unlock';

export const blockTypePromptMessages = {
'core/query': __( 'Query Loop displays a list of posts.' ),
'core/post-content': __(
'Post Content displays the content of a post or page.'
),
};

export function BlockRemovalWarningModal() {
const { clientIds, selectPrevious, blockNamesForPrompt } = useSelect(
( select ) =>
unlock( select( blockEditorStore ) ).isRemovalPromptDisplayed()
);

const {
displayRemovalPrompt,
setRemovalPromptStatus,
privateRemoveBlocks,
} = unlock( useDispatch( blockEditorStore ) );

// Signalling the removal prompt is in place.
useEffect( () => {
setRemovalPromptStatus( true );
return () => {
setRemovalPromptStatus( false );
};
}, [ setRemovalPromptStatus ] );

if ( ! blockNamesForPrompt ) {
return;
}

const closeModal = () => displayRemovalPrompt( false );

const onConfirmRemoval = () => {
privateRemoveBlocks( clientIds, selectPrevious, /* force */ true );
closeModal();
};

return (
<Modal
title={ _n(
'Really delete this block?',
'Really delete these blocks?',
clientIds.length
) }
onRequestClose={ closeModal }
>
{ blockNamesForPrompt.length === 1 ? (
<p>{ blockTypePromptMessages[ blockNamesForPrompt[ 0 ] ] }</p>
) : (
<ul style={ { listStyleType: 'disc', paddingLeft: '1rem' } }>
{ blockNamesForPrompt.map( ( name ) => (
<li key={ name }>
{ blockTypePromptMessages[ name ] }
</li>
) ) }
</ul>
) }
<p>
{ _n(
'Removing this block is not advised.',
'Removing these blocks is not advised.',
blockNamesForPrompt.length
) }
</p>
<HStack justify="right">
<Button variant="tertiary" onClick={ closeModal }>
{ __( 'Cancel' ) }
</Button>
<Button variant="primary" onClick={ onConfirmRemoval }>
{ __( 'Confirm' ) }
</Button>
</HStack>
</Modal>
);
Copy link
Contributor

@talldan talldan Jun 22, 2023

Choose a reason for hiding this comment

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

Not a blocker, but just noting that this is pretty similar to the ConfirmDialog component.

There are possibly some differences (the title?), so it could be a later refactor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, good observation.

}
2 changes: 2 additions & 0 deletions packages/block-editor/src/private-apis.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { cleanEmptyObject } from './hooks/utils';
import { useBlockEditingMode } from './components/block-editing-mode';
import BlockQuickNavigation from './components/block-quick-navigation';
import { LayoutStyle } from './components/block-list/layout';
import { BlockRemovalWarningModal } from './components/block-removal-warning-modal';
import { useLayoutClasses, useLayoutStyles } from './hooks';

/**
Expand All @@ -31,6 +32,7 @@ lock( privateApis, {
useBlockEditingMode,
BlockQuickNavigation,
LayoutStyle,
BlockRemovalWarningModal,
useLayoutClasses,
useLayoutStyles,
} );
63 changes: 7 additions & 56 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,40 +25,17 @@ import {
retrieveSelectedAttribute,
START_OF_SELECTED_AREA,
} from '../utils/selection';
import { __experimentalUpdateSettings } from './private-actions';
import {
__experimentalUpdateSettings,
ensureDefaultBlock,
privateRemoveBlocks,
} from './private-actions';

/** @typedef {import('../components/use-on-block-drop/types').WPDropOperation} WPDropOperation */

const castArray = ( maybeArray ) =>
Array.isArray( maybeArray ) ? maybeArray : [ maybeArray ];

/**
* Action which will insert a default block insert action if there
* are no other blocks at the root of the editor. This action should be used
* in actions which may result in no blocks remaining in the editor (removal,
* replacement, etc).
*/
const ensureDefaultBlock =
() =>
( { select, dispatch } ) => {
// To avoid a focus loss when removing the last block, assure there is
// always a default block if the last of the blocks have been removed.
const count = select.getBlockCount();
if ( count > 0 ) {
return;
}

// If there's an custom appender, don't insert default block.
// We have to remember to manually move the focus elsewhere to
// prevent it from being lost though.
const { __unstableHasCustomAppender } = select.getSettings();
if ( __unstableHasCustomAppender ) {
return;
}

dispatch.insertDefaultBlock();
};

/**
* Action that resets blocks state to the specified array of blocks, taking precedence
* over any other content reflected as an edit in state.
Expand Down Expand Up @@ -1195,34 +1172,8 @@ export const mergeBlocks =
* should be selected
* when a block is removed.
*/
export const removeBlocks =
( clientIds, selectPrevious = true ) =>
( { select, dispatch } ) => {
if ( ! clientIds || ! clientIds.length ) {
return;
}

clientIds = castArray( clientIds );
const rootClientId = select.getBlockRootClientId( clientIds[ 0 ] );
const canRemoveBlocks = select.canRemoveBlocks(
clientIds,
rootClientId
);

if ( ! canRemoveBlocks ) {
return;
}

if ( selectPrevious ) {
dispatch.selectPreviousBlock( clientIds[ 0 ], selectPrevious );
}

dispatch( { type: 'REMOVE_BLOCKS', clientIds } );

// To avoid a focus loss when removing the last block, assure there is
// always a default block if the last of the blocks have been removed.
dispatch( ensureDefaultBlock() );
};
export const removeBlocks = ( clientIds, selectPrevious = true ) =>
privateRemoveBlocks( clientIds, selectPrevious );

/**
* Returns an action object used in signalling that the block with the
Expand Down
160 changes: 160 additions & 0 deletions packages/block-editor/src/store/private-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
*/
import { Platform } from '@wordpress/element';

/**
* Internal dependencies
*/
import { blockTypePromptMessages } from '../components/block-removal-warning-modal';
import { removalPromptExists } from './private-selectors';

const castArray = ( maybeArray ) =>
Array.isArray( maybeArray ) ? maybeArray : [ maybeArray ];

/**
* A list of private/experimental block editor settings that
* should not become a part of the WordPress public API.
Expand Down Expand Up @@ -105,3 +114,154 @@ export function unsetBlockEditingMode( clientId = '' ) {
clientId,
};
}

/**
* Yields action objects used in signalling that the blocks corresponding to
* the set of specified client IDs are to be removed.
*
* @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 {boolean} forceRemove Whether to force the operation,
* bypassing any checks for certain
* block types.
*/
export const privateRemoveBlocks =
( clientIds, selectPrevious = true, forceRemove = false ) =>
( { select, dispatch } ) => {
if ( ! clientIds || ! clientIds.length ) {
return;
}

clientIds = castArray( clientIds );
const rootClientId = select.getBlockRootClientId( clientIds[ 0 ] );
const canRemoveBlocks = select.canRemoveBlocks(
clientIds,
rootClientId
);

if ( ! canRemoveBlocks ) {
return;
}

// 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). However,
// the editor instance is responsible for presenting those confirmation
// prompts to the user. Any instance opting into removal prompts must
// register using `setRemovalPromptStatus()`.
//
// @see https://github.com/WordPress/gutenberg/pull/51145
if (
! forceRemove &&
// FIXME what's the best way to unlock a private selector in this
// context?
select( ( state ) => removalPromptExists( state.root ) )
mcsf 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 in ../components/block-removal-warning-modal
const queue = [ ...clientIds ];
while ( queue.length ) {
const clientId = queue.shift();
const blockName = select.getBlockName( clientId );
if ( blockTypePromptMessages[ blockName ] ) {
blockNamesForPrompt.add( blockName );
}
const innerBlocks = select.getBlockOrder( clientId );
queue.push( ...innerBlocks );
}

// If any such blocks were found, trigger the removal prompt and
// skip any other steps (thus postponing actual removal).
if ( blockNamesForPrompt.size ) {
dispatch(
displayRemovalPrompt( true, {
clientIds,
selectPrevious,
blockNamesForPrompt: Array.from( blockNamesForPrompt ),
} )
);
return;
}
}

if ( selectPrevious ) {
dispatch.selectPreviousBlock( clientIds[ 0 ], selectPrevious );
}

dispatch( { type: 'REMOVE_BLOCKS', clientIds } );

// To avoid a focus loss when removing the last block, assure there is
// always a default block if the last of the blocks have been removed.
dispatch( ensureDefaultBlock() );
};

/**
* Action which will insert a default block insert action if there
* are no other blocks at the root of the editor. This action should be used
* in actions which may result in no blocks remaining in the editor (removal,
* replacement, etc).
*/
export const ensureDefaultBlock =
() =>
( { select, dispatch } ) => {
// To avoid a focus loss when removing the last block, assure there is
// always a default block if the last of the blocks have been removed.
const count = select.getBlockCount();
if ( count > 0 ) {
return;
}

// If there's an custom appender, don't insert default block.
// We have to remember to manually move the focus elsewhere to
// prevent it from being lost though.
const { __unstableHasCustomAppender } = select.getSettings();
if ( __unstableHasCustomAppender ) {
return;
}

dispatch.insertDefaultBlock();
};

/**
* Returns an action object used in signalling that a removal prompt must be displayed.
*
* @param {boolean} displayPrompt Whether to prompt for removal.
* @param {Function} options Function to call if removal is confirmed and blockName.
*
* @return {Object} Action object.
*/
export function displayRemovalPrompt( displayPrompt, options = {} ) {
const { clientIds, selectPrevious, blockNamesForPrompt } = options;
return {
type: 'PROMPT_REMOVAL',
displayPrompt,
clientIds,
selectPrevious,
blockNamesForPrompt,
};
}

/**
* Returns an action object used in signalling that a removal prompt display
* mechanism is available or unavailable.
*
* @param {boolean} status Whether a prompt display mechanism exists.
* @return {Object} Action object.
*/
export function setRemovalPromptStatus( status = true ) {
return {
type: 'PROMPT_EXISTS',
status,
};
}
Loading