From ee6eeb7c651a1fd9a6a833e0c79a094a58947bdb Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Thu, 1 Jun 2023 14:12:16 +1000 Subject: [PATCH] Improvements to how blocks with a 'disabled' editing mode behave - Prevent DefaultAppender from appearing in a disabled block. - Disable selection (using user-select: none) in disabled blocks. - Prevent blocks from being inserted into a disabled block via global inserter. - Prevent disabled blocks from being removed via keyboard shortcut. - Prevent disabled blocks from being moved via List View drag and drop. - Prevent block overlay from appearing on a disabled block. --- .../components/block-list-appender/index.js | 5 +- .../src/components/block-list/content.scss | 20 ++-- .../src/store/private-selectors.js | 60 ++++++----- packages/block-editor/src/store/selectors.js | 51 ++++++---- .../src/store/test/private-selectors.js | 17 +++- .../block-editor/src/store/test/selectors.js | 99 +++++++++++++++++++ 6 files changed, 183 insertions(+), 69 deletions(-) diff --git a/packages/block-editor/src/components/block-list-appender/index.js b/packages/block-editor/src/components/block-list-appender/index.js index 56c044b6c5a8ab..99d0fc56374e42 100644 --- a/packages/block-editor/src/components/block-list-appender/index.js +++ b/packages/block-editor/src/components/block-list-appender/index.js @@ -15,6 +15,7 @@ import { getDefaultBlockName } from '@wordpress/blocks'; import DefaultBlockAppender from '../default-block-appender'; import ButtonBlockAppender from '../button-block-appender'; import { store as blockEditorStore } from '../../store'; +import { unlock } from '../../lock-unlock'; function DefaultAppender( { rootClientId } ) { const canInsertDefaultBlock = useSelect( ( select ) => @@ -46,13 +47,15 @@ function useAppender( rootClientId, CustomAppender ) { getTemplateLock, getSelectedBlockClientId, __unstableGetEditorMode, - } = select( blockEditorStore ); + getBlockEditingMode, + } = unlock( select( blockEditorStore ) ); const selectedBlockClientId = getSelectedBlockClientId(); return { hideInserter: !! getTemplateLock( rootClientId ) || + getBlockEditingMode( rootClientId ) === 'disabled' || __unstableGetEditorMode() === 'zoom-out', isParentSelected: rootClientId === selectedBlockClientId || diff --git a/packages/block-editor/src/components/block-list/content.scss b/packages/block-editor/src/components/block-list/content.scss index dab07ced873e9f..a163ddaa789551 100644 --- a/packages/block-editor/src/components/block-list/content.scss +++ b/packages/block-editor/src/components/block-list/content.scss @@ -161,15 +161,6 @@ padding: 0; } -.block-editor-block-list__layout, -.block-editor-block-list__block { - pointer-events: initial; - - &.is-editing-disabled { - pointer-events: none; - } -} - .block-editor-block-list__layout .block-editor-block-list__block { // With `position: static`, Safari marks a full-width selection rectangle, including margins. // With `position: relative`, Safari marks an inline selection rectangle, similar to that of @@ -178,12 +169,17 @@ // We choose relative, as that matches the multi-selection, which is limited to the block footprint. position: relative; - // Re-enable text-selection on editable blocks. - user-select: text; - // Break long strings of text without spaces so they don't overflow the block. overflow-wrap: break-word; + pointer-events: auto; + user-select: text; + + &.is-editing-disabled { + pointer-events: none; + user-select: none; + } + .reusable-block-edit-panel * { z-index: z-index(".block-editor-block-list__block .reusable-block-edit-panel *"); } diff --git a/packages/block-editor/src/store/private-selectors.js b/packages/block-editor/src/store/private-selectors.js index ce7802036184e5..bcc032a4ee9037 100644 --- a/packages/block-editor/src/store/private-selectors.js +++ b/packages/block-editor/src/store/private-selectors.js @@ -6,7 +6,7 @@ import createSelector from 'rememo'; /** * WordPress dependencies */ -import { createRegistrySelector } from '@wordpress/data'; +import { select } from '@wordpress/data'; import { store as blocksStore } from '@wordpress/blocks'; /** @@ -71,37 +71,35 @@ export function getLastInsertedBlocksClientIds( state ) { * @return {BlockEditingMode} The block editing mode. One of `'disabled'`, * `'contentOnly'`, or `'default'`. */ -export const getBlockEditingMode = createRegistrySelector( - ( select ) => - ( state, clientId = '' ) => { - const explicitEditingMode = getExplcitBlockEditingMode( - state, - clientId - ); - const rootClientId = getBlockRootClientId( state, clientId ); - const templateLock = getTemplateLock( state, rootClientId ); - const name = getBlockName( state, clientId ); - const isContent = - select( blocksStore ).__experimentalHasContentRoleAttribute( - name - ); - if ( - explicitEditingMode === 'disabled' || - ( templateLock === 'contentOnly' && ! isContent ) - ) { - return 'disabled'; - } - if ( - explicitEditingMode === 'contentOnly' || - ( templateLock === 'contentOnly' && isContent ) - ) { - return 'contentOnly'; - } - return 'default'; - } -); +export const getBlockEditingMode = ( state, clientId = '' ) => { + const explicitEditingMode = getExplicitBlockEditingMode( state, clientId ); + const rootClientId = getBlockRootClientId( state, clientId ); + const templateLock = getTemplateLock( state, rootClientId ); + const name = getBlockName( state, clientId ); + // TODO: Terrible hack! We're calling the global select() function here + // instead of using createRegistrySelector(). The problem with using + // createRegistrySelector() is that then the public block-editor selectors + // (e.g. canInsertBlockTypeUnmemoized) can't call this private block-editor + // selector due to a bug in @wordpress/data. See + // https://github.com/WordPress/gutenberg/pull/50985. + const isContent = + select( blocksStore ).__experimentalHasContentRoleAttribute( name ); + if ( + explicitEditingMode === 'disabled' || + ( templateLock === 'contentOnly' && ! isContent ) + ) { + return 'disabled'; + } + if ( + explicitEditingMode === 'contentOnly' || + ( templateLock === 'contentOnly' && isContent ) + ) { + return 'contentOnly'; + } + return 'default'; +}; -const getExplcitBlockEditingMode = createSelector( +const getExplicitBlockEditingMode = createSelector( ( state, clientId = '' ) => { while ( ! state.blockEditingModes.has( clientId ) && diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 487d13db811d84..5b615f67defbd4 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -26,6 +26,7 @@ import deprecated from '@wordpress/deprecated'; */ import { mapRichTextSettings } from './utils'; import { orderBy } from '../utils/sorting'; +import { getBlockEditingMode } from './private-selectors'; /** * A block selection object. @@ -1539,6 +1540,10 @@ const canInsertBlockTypeUnmemoized = ( return false; } + if ( getBlockEditingMode( state, rootClientId ?? '' ) === 'disabled' ) { + return false; + } + const parentBlockListSettings = getBlockListSettings( state, rootClientId ); // The parent block doesn't have settings indicating it doesn't support @@ -1633,6 +1638,7 @@ export const canInsertBlockType = createSelector( state.blocks.byClientId.get( rootClientId ), state.settings.allowedBlockTypes, state.settings.templateLock, + state.blockEditingModes, ] ); @@ -1663,21 +1669,19 @@ export function canInsertBlocks( state, clientIds, rootClientId = null ) { */ export function canRemoveBlock( state, clientId, rootClientId = null ) { const attributes = getBlockAttributes( state, clientId ); - - // attributes can be null if the block is already deleted. if ( attributes === null ) { return true; } - - const { lock } = attributes; - const parentIsLocked = !! getTemplateLock( state, rootClientId ); - // If we don't have a lock on the blockType level, we defer to the parent templateLock. - if ( lock === undefined || lock?.remove === undefined ) { - return ! parentIsLocked; + if ( attributes.lock?.remove ) { + return false; } - - // When remove is true, it means we cannot remove it. - return ! lock?.remove; + if ( getTemplateLock( state, rootClientId ) ) { + return false; + } + if ( getBlockEditingMode( state, rootClientId ) === 'disabled' ) { + return false; + } + return true; } /** @@ -1709,16 +1713,16 @@ export function canMoveBlock( state, clientId, rootClientId = null ) { if ( attributes === null ) { return; } - - const { lock } = attributes; - const parentIsLocked = getTemplateLock( state, rootClientId ) === 'all'; - // If we don't have a lock on the blockType level, we defer to the parent templateLock. - if ( lock === undefined || lock?.move === undefined ) { - return ! parentIsLocked; + if ( attributes.lock?.move ) { + return false; } - - // When move is true, it means we cannot move it. - return ! lock?.move; + if ( getTemplateLock( state, rootClientId ) === 'all' ) { + return false; + } + if ( getBlockEditingMode( state, rootClientId ) === 'disabled' ) { + return false; + } + return true; } /** @@ -2812,6 +2816,13 @@ export function __unstableGetTemporarilyEditingAsBlocks( state ) { } export function __unstableHasActiveBlockOverlayActive( state, clientId ) { + // Prevent overlay on disabled blocks. It's redundant since disabled blocks + // can't be selected, and prevents non-disabled nested blocks from being + // selected. + if ( getBlockEditingMode( state, clientId ) === 'disabled' ) { + return false; + } + // If the block editing is locked, the block overlay is always active. if ( ! canEditBlock( state, clientId ) ) { return true; diff --git a/packages/block-editor/src/store/test/private-selectors.js b/packages/block-editor/src/store/test/private-selectors.js index 954c8c94c13799..f9bde867b1032e 100644 --- a/packages/block-editor/src/store/test/private-selectors.js +++ b/packages/block-editor/src/store/test/private-selectors.js @@ -1,3 +1,8 @@ +/** + * WordPress dependencies + */ +import { select } from '@wordpress/data'; + /** * Internal dependencies */ @@ -7,6 +12,10 @@ import { getBlockEditingMode, } from '../private-selectors'; +jest.mock( '@wordpress/data/src/select', () => ( { + select: jest.fn(), +} ) ); + describe( 'private selectors', () => { describe( 'isBlockInterfaceHidden', () => { it( 'should return the true if toggled true in state', () => { @@ -92,11 +101,9 @@ describe( 'private selectors', () => { }; const __experimentalHasContentRoleAttribute = jest.fn( () => false ); - getBlockEditingMode.registry = { - select: jest.fn( () => ( { - __experimentalHasContentRoleAttribute, - } ) ), - }; + select.mockReturnValue( { + __experimentalHasContentRoleAttribute, + } ); it( 'should return default by default', () => { expect( diff --git a/packages/block-editor/src/store/test/selectors.js b/packages/block-editor/src/store/test/selectors.js index 60d90d80b9d41e..5cc17fc08b3142 100644 --- a/packages/block-editor/src/store/test/selectors.js +++ b/packages/block-editor/src/store/test/selectors.js @@ -2693,11 +2693,13 @@ describe( 'selectors', () => { blocks: { byClientId: new Map(), attributes: new Map(), + parents: new Map(), }, blockListSettings: {}, settings: { allowedBlockTypes: [ 'core/test-block-a' ], }, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-a' ) ).toBe( true @@ -2720,14 +2722,36 @@ describe( 'selectors', () => { ); } ); + it( 'should deny blocks when the editor has a disabled editing mode', () => { + const state = { + blocks: { + byClientId: new Map(), + attributes: new Map(), + parents: new Map(), + }, + blockListSettings: {}, + settings: {}, + blockEditingModes: new Map( + Object.entries( { + '': 'disabled', + } ) + ), + }; + expect( canInsertBlockType( state, 'core/test-block-a' ) ).toBe( + false + ); + } ); + it( 'should deny blocks that restrict parent from being inserted into the root', () => { const state = { blocks: { byClientId: new Map(), attributes: new Map(), + parents: new Map(), }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-c' ) ).toBe( false @@ -2747,9 +2771,11 @@ describe( 'selectors', () => { block1: {}, } ) ), + parents: new Map(), }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-c', 'block1' ) @@ -2769,11 +2795,13 @@ describe( 'selectors', () => { block1: {}, } ) ), + parents: new Map(), }, blockListSettings: { block1: {}, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-c', 'block1' ) @@ -2793,11 +2821,13 @@ describe( 'selectors', () => { block1: {}, } ) ), + parents: new Map(), }, blockListSettings: { block1: {}, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-c', 'block1' ) @@ -2817,6 +2847,7 @@ describe( 'selectors', () => { block1: {}, } ) ), + parents: new Map(), }, blockListSettings: { block1: { @@ -2824,6 +2855,7 @@ describe( 'selectors', () => { }, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-b', 'block1' ) @@ -2843,6 +2875,7 @@ describe( 'selectors', () => { block1: {}, } ) ), + parents: new Map(), }, blockListSettings: { block1: { @@ -2850,12 +2883,41 @@ describe( 'selectors', () => { }, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-b', 'block1' ) ).toBe( true ); } ); + it( 'should deny blocks from being inserted into a block that has a disabled editing mode', () => { + const state = { + blocks: { + byClientId: new Map( + Object.entries( { + block1: { name: 'core/test-block-a' }, + } ) + ), + attributes: new Map( + Object.entries( { + block1: {}, + } ) + ), + parents: new Map(), + }, + blockListSettings: {}, + settings: {}, + blockEditingModes: new Map( + Object.entries( { + block1: 'disabled', + } ) + ), + }; + expect( + canInsertBlockType( state, 'core/test-block-b', 'block1' ) + ).toBe( false ); + } ); + it( 'should prioritise parent over allowedBlocks', () => { const state = { blocks: { @@ -2869,6 +2931,7 @@ describe( 'selectors', () => { block1: {}, } ) ), + parents: new Map(), }, blockListSettings: { block1: { @@ -2876,6 +2939,7 @@ describe( 'selectors', () => { }, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/test-block-c', 'block1' ) @@ -2895,9 +2959,11 @@ describe( 'selectors', () => { block1: {}, } ) ), + parents: new Map(), }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/post-content-child', 'block1' ) @@ -2909,9 +2975,11 @@ describe( 'selectors', () => { blocks: { byClientId: new Map(), attributes: new Map(), + parents: new Map(), }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( state, 'core/post-content-child' ) @@ -2944,6 +3012,7 @@ describe( 'selectors', () => { block2: {}, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( @@ -2984,6 +3053,7 @@ describe( 'selectors', () => { block3: {}, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( @@ -3023,6 +3093,7 @@ describe( 'selectors', () => { block3: {}, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( @@ -3062,6 +3133,7 @@ describe( 'selectors', () => { block3: {}, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( @@ -3100,6 +3172,7 @@ describe( 'selectors', () => { }, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( @@ -3136,6 +3209,7 @@ describe( 'selectors', () => { block2: {}, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlockType( @@ -3165,6 +3239,7 @@ describe( 'selectors', () => { 3: {}, } ) ), + parents: new Map(), }, blockListSettings: { 1: { @@ -3175,6 +3250,7 @@ describe( 'selectors', () => { }, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlocks( state, [ '2', '3' ], '1' ) ).toBe( true ); } ); @@ -3196,6 +3272,7 @@ describe( 'selectors', () => { 3: {}, } ) ), + parents: new Map(), }, blockListSettings: { 1: { @@ -3203,6 +3280,7 @@ describe( 'selectors', () => { }, }, settings: {}, + blockEditingModes: new Map(), }; expect( canInsertBlocks( state, [ '2', '3' ], '1' ) ).toBe( false ); } ); @@ -3241,6 +3319,7 @@ describe( 'selectors', () => { // See: https://github.com/WordPress/gutenberg/issues/14580 preferences: {}, blockListSettings: {}, + blockEditingModes: new Map(), }; const items = getInserterItems( state ); const testBlockAItem = items.find( @@ -3349,6 +3428,7 @@ describe( 'selectors', () => { block3: {}, block4: {}, }, + blockEditingModes: new Map(), }; const stateSecondBlockRestricted = { @@ -3436,6 +3516,7 @@ describe( 'selectors', () => { }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; const items = getInserterItems( state ); const testBlockBItem = items.find( @@ -3460,6 +3541,7 @@ describe( 'selectors', () => { }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; const items = getInserterItems( state ); const reusableBlock2Item = items.find( @@ -3551,6 +3633,7 @@ describe( 'selectors', () => { settings: {}, preferences: {}, blockListSettings: {}, + blockEditingModes: new Map(), }; const blocks = [ { name: 'core/with-tranforms-a' } ]; const items = getBlockTransformItems( state, blocks ); @@ -3591,6 +3674,7 @@ describe( 'selectors', () => { settings: {}, preferences: {}, blockListSettings: {}, + blockEditingModes: new Map(), }; const block = { name: 'core/with-tranforms-a' }; const items = getBlockTransformItems( state, block ); @@ -3629,6 +3713,7 @@ describe( 'selectors', () => { }, block2: {}, }, + blockEditingModes: new Map(), }; const blocks = [ { clientId: 'block2', name: 'core/with-tranforms-a' }, @@ -3676,6 +3761,7 @@ describe( 'selectors', () => { }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; const blocks = [ { name: 'core/with-tranforms-a' } ]; const items = getBlockTransformItems( state, blocks ); @@ -3709,6 +3795,7 @@ describe( 'selectors', () => { }, blockListSettings: {}, settings: {}, + blockEditingModes: new Map(), }; const blocks = [ { name: 'core/with-tranforms-c' } ]; const items = getBlockTransformItems( state, blocks ); @@ -4121,6 +4208,12 @@ describe( 'selectors', () => { block2: {}, } ) ), + parents: new Map( + Object.entries( { + block1: '', + block2: '', + } ) + ), }, blockListSettings: { block1: { @@ -4152,6 +4245,7 @@ describe( 'selectors', () => { }, ], }, + blockEditingModes: new Map(), }; it( 'should return all patterns for root level', () => { @@ -4249,6 +4343,7 @@ describe( 'selectors', () => { block1: { name: 'core/test-block-a' }, } ) ), + parents: new Map(), }, blockListSettings: { block1: { @@ -4279,6 +4374,7 @@ describe( 'selectors', () => { }, ], }, + blockEditingModes: new Map(), }; it( 'should return empty array if no block name is provided', () => { expect( getPatternsByBlockTypes( state ) ).toEqual( [] ); @@ -4329,6 +4425,7 @@ describe( 'selectors', () => { block2: { name: 'core/test-block-b' }, } ) ), + parents: new Map(), controlledInnerBlocks: { 'block2-clientId': true }, }, blockListSettings: { @@ -4371,6 +4468,7 @@ describe( 'selectors', () => { }, ], }, + blockEditingModes: new Map(), }; describe( 'should return empty array', () => { it( 'when no blocks are selected', () => { @@ -4591,6 +4689,7 @@ describe( 'getInserterItems with core blocks prioritization', () => { settings: {}, preferences: {}, blockListSettings: {}, + blockEditingModes: new Map(), }; const items = getInserterItems( state ); const expectedResult = [