Show warning on critical block removal #51145
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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'; | ||
|
|
||
| // 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() { | ||
| const { clientIds, selectPrevious, blockNamesForPrompt } = useSelect( | ||
| ( select ) => | ||
| unlock( select( blockEditorStore ) ).getRemovalPromptData() | ||
| ); | ||
|
|
||
| const { | ||
| clearRemovalPrompt, | ||
| toggleRemovalPromptSupport, | ||
| privateRemoveBlocks, | ||
| } = unlock( useDispatch( blockEditorStore ) ); | ||
|
|
||
| // Signalling the removal prompt is in place. | ||
| useEffect( () => { | ||
| toggleRemovalPromptSupport( true ); | ||
| return () => { | ||
| toggleRemovalPromptSupport( false ); | ||
| }; | ||
| }, [ toggleRemovalPromptSupport ] ); | ||
|
|
||
| if ( ! blockNamesForPrompt ) { | ||
| return; | ||
| } | ||
|
|
||
| const onConfirmRemoval = () => { | ||
| privateRemoveBlocks( clientIds, selectPrevious, /* force */ true ); | ||
| clearRemovalPrompt(); | ||
| }; | ||
|
|
||
| return ( | ||
| <Modal | ||
| title={ __( 'Are you sure?' ) } | ||
| onRequestClose={ clearRemovalPrompt } | ||
| > | ||
| { 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={ clearRemovalPrompt }> | ||
| { __( 'Cancel' ) } | ||
| </Button> | ||
| <Button variant="primary" onClick={ onConfirmRemoval }> | ||
| { __( 'Delete' ) } | ||
| </Button> | ||
| </HStack> | ||
| </Modal> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,14 @@ | |
| */ | ||
| import { Platform } from '@wordpress/element'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { blockTypePromptMessages } from '../components/block-removal-warning-modal'; | ||
|
|
||
| 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. | ||
|
|
@@ -105,3 +113,188 @@ 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. | ||
| * | ||
| * Compared to `removeBlocks`, this private interface exposes an additional | ||
| * parameter; see `forceRemove`. | ||
| * | ||
| * @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 `toggleRemovalPromptSupport()`. | ||
| // | ||
| // @see https://github.com/WordPress/gutenberg/pull/51145 | ||
| if ( | ||
| ! forceRemove && | ||
| // FIXME: Without this existence check, the unit tests for | ||
| // `__experimentalDeleteReusableBlock` in | ||
| // `packages/reusable-blocks/src/store/test/actions.js` fail due to | ||
| // the fact that the `registry` object passed to the thunk actions | ||
| // doesn't include this private action. This needs to be | ||
| // investigated to understand whether it's a real smell or if it's | ||
| // because not all store code has been updated to accommodate | ||
| // private selectors. | ||
| select.isRemovalPromptSupported && | ||
| select.isRemovalPromptSupported() | ||
|
Comment on lines
+163
to
+172
Contributor
There was a problem hiding this comment. @jsnajdr: I quickly chatted with Adam about this. Is it possible that we are missing something in our Jest setup to accommodate private selectors?
Member
There was a problem hiding this comment. It's not because of Jest, but because of the cumbersome way how we register the export const store = createReduxStore( STORE_NAME, {
...storeConfig,
persist: [ 'preferences' ],
} );
// We will be able to use the `register` function once we switch
// the "preferences" persistence to use the new preferences package.
const registeredStore = registerStore( STORE_NAME, {
...storeConfig,
persist: [ 'preferences' ],
} );
unlock( registeredStore ).registerPrivateActions( privateActions );
unlock( registeredStore ).registerPrivateSelectors( privateSelectors );This code:
The unit test that is failing is creating a new But nobody ever added the private actions and selectors to this descriptor! The short-term solution is to register the private actions/selectors also to
Contributor
|
||
| ) { | ||
| 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 ] ) { | ||
| 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( | ||
| clientIds, | ||
| selectPrevious, | ||
| 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 block removal prompt must | ||
| * be displayed. | ||
| * | ||
| * Contrast with `toggleRemovalPromptSupport`. | ||
| * | ||
| * @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 | ||
| * @return {Object} Action object. | ||
| */ | ||
| export function displayRemovalPrompt( | ||
| clientIds, | ||
| selectPrevious, | ||
| blockNamesForPrompt | ||
| ) { | ||
| return { | ||
| type: 'DISPLAY_REMOVAL_PROMPT', | ||
| clientIds, | ||
| selectPrevious, | ||
| blockNamesForPrompt, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Returns an action object used in signalling that a block removal prompt must | ||
| * be cleared, either be cause the user has confirmed or canceled the request | ||
| * for removal. | ||
| * | ||
| * @return {Object} Action object. | ||
| */ | ||
| export function clearRemovalPrompt() { | ||
| return { | ||
| type: 'CLEAR_REMOVAL_PROMPT', | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Returns an action object used in signalling that a removal prompt display | ||
| * mechanism is available or unavailable in the current editor. | ||
| * | ||
| * Contrast with `displayRemovalPrompt`. | ||
| * | ||
| * @param {boolean} status Whether a prompt display mechanism exists. | ||
| * @return {Object} Action object. | ||
| */ | ||
| export function toggleRemovalPromptSupport( status = true ) { | ||
| return { | ||
| type: 'TOGGLE_REMOVAL_PROMPT_SUPPORT', | ||
| status, | ||
| }; | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now this entangles
block-editorwithedit-site, since these are rules that only apply to the site editor. I don't want to hold up this PR any longer, but a future improvement could be to let the consumer (the component renderingBlockRemovalWarningModal) provide their own rules. Maybe:As you can tell, I'm undecided on the terminology: are these messages? Rules? Warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> #51841