From 05130518a6061018b52e46927b6ac22694c879d2 Mon Sep 17 00:00:00 2001 From: David Calhoun <438664+dcalhoun@users.noreply.github.com> Date: Wed, 19 Oct 2022 09:26:03 -0500 Subject: [PATCH] fix: Native inner blocks merge where appropriate (#45048) * fix: Native inner blocks merge where appropriate An error was thrown when attempting to merge inner blocks via pressing the backward delete key. This applies the remainder of a recent web refactor to avoid errors and crashes. The spirit of the web refactor was to improve the experience when interacting with List V2 blocks. * test: Add List block merge test - List blocks merge into other list blocks. - List blocks unwrap list items when merging into non-list blocks. Co-authored-by: Carlos Garcia --- .../block-list/block-list-compact.native.js | 1 + .../src/components/block-list/block.native.js | 132 ++++++++++++++++-- .../src/list-item/edit.native.js | 3 +- .../src/list/test/edit.native.js | 105 +++++++++++++- 4 files changed, 228 insertions(+), 13 deletions(-) diff --git a/packages/block-editor/src/components/block-list/block-list-compact.native.js b/packages/block-editor/src/components/block-list/block-list-compact.native.js index 832ea2a81bb45..ad9d0c7301eed 100644 --- a/packages/block-editor/src/components/block-list/block-list-compact.native.js +++ b/packages/block-editor/src/components/block-list/block-list-compact.native.js @@ -50,6 +50,7 @@ function BlockListCompact( props ) { { blockClientIds.map( ( currentClientId ) => ( { + withDispatch( ( dispatch, ownProps, registry ) => { const { insertBlocks, mergeBlocks, replaceBlocks, selectBlock, updateBlockAttributes, + moveBlocksToPosition, + removeBlock, } = dispatch( blockEditorStore ); return { - mergeBlocks( forward ) { - const { clientId } = ownProps; - const { getPreviousBlockClientId, getNextBlockClientId } = - select( blockEditorStore ); - + onMerge( forward ) { + const { clientId, rootClientId } = ownProps; + const { + getPreviousBlockClientId, + getNextBlockClientId, + getBlock, + getBlockAttributes, + getBlockName, + getBlockOrder, + } = registry.select( blockEditorStore ); + + // For `Delete` or forward merge, we should do the exact same thing + // as `Backspace`, but from the other block. if ( forward ) { + if ( rootClientId ) { + const nextRootClientId = + getNextBlockClientId( rootClientId ); + + if ( nextRootClientId ) { + // If there is a block that follows with the same parent + // block name and the same attributes, merge the inner + // blocks. + if ( + getBlockName( rootClientId ) === + getBlockName( nextRootClientId ) + ) { + const rootAttributes = + getBlockAttributes( rootClientId ); + const previousRootAttributes = + getBlockAttributes( nextRootClientId ); + + if ( + Object.keys( rootAttributes ).every( + ( key ) => + rootAttributes[ key ] === + previousRootAttributes[ key ] + ) + ) { + registry.batch( () => { + moveBlocksToPosition( + getBlockOrder( nextRootClientId ), + nextRootClientId, + rootClientId + ); + removeBlock( nextRootClientId, false ); + } ); + return; + } + } else { + mergeBlocks( rootClientId, nextRootClientId ); + return; + } + } + } + const nextBlockClientId = getNextBlockClientId( clientId ); - if ( nextBlockClientId ) { + + if ( ! nextBlockClientId ) { + return; + } + + // Check if it's possibile to "unwrap" the following block + // before trying to merge. + const replacement = switchToBlockType( + getBlock( nextBlockClientId ), + '*' + ); + + if ( replacement && replacement.length ) { + replaceBlocks( nextBlockClientId, replacement ); + } else { mergeBlocks( clientId, nextBlockClientId ); } } else { const previousBlockClientId = getPreviousBlockClientId( clientId ); + if ( previousBlockClientId ) { mergeBlocks( previousBlockClientId, clientId ); + } else if ( rootClientId ) { + const previousRootClientId = + getPreviousBlockClientId( rootClientId ); + + // If there is a preceding block with the same parent block + // name and the same attributes, merge the inner blocks. + if ( + previousRootClientId && + getBlockName( rootClientId ) === + getBlockName( previousRootClientId ) + ) { + const rootAttributes = + getBlockAttributes( rootClientId ); + const previousRootAttributes = + getBlockAttributes( previousRootClientId ); + + if ( + Object.keys( rootAttributes ).every( + ( key ) => + rootAttributes[ key ] === + previousRootAttributes[ key ] + ) + ) { + registry.batch( () => { + moveBlocksToPosition( + getBlockOrder( rootClientId ), + rootClientId, + previousRootClientId + ); + removeBlock( rootClientId, false ); + } ); + return; + } + } + + // Attempt to "unwrap" the block contents when there's no + // preceding block to merge with. + const replacement = switchToBlockType( + getBlock( rootClientId ), + '*' + ); + if ( replacement && replacement.length ) { + registry.batch( () => { + replaceBlocks( rootClientId, replacement ); + selectBlock( replacement[ 0 ].clientId, 0 ); + } ); + } } } }, diff --git a/packages/block-library/src/list-item/edit.native.js b/packages/block-library/src/list-item/edit.native.js index dcd20f11c9d5e..5f365dd9a5816 100644 --- a/packages/block-library/src/list-item/edit.native.js +++ b/packages/block-library/src/list-item/edit.native.js @@ -35,6 +35,7 @@ export default function ListItemEdit( { onReplace, clientId, style, + mergeBlocks, } ) { const [ contentWidth, setContentWidth ] = useState(); const { placeholder, content } = attributes; @@ -119,7 +120,7 @@ export default function ListItemEdit( { const preventDefault = useRef( false ); const { onEnter } = useEnter( { content, clientId }, preventDefault ); const onSplit = useSplit( clientId ); - const onMerge = useMerge( clientId ); + const onMerge = useMerge( clientId, mergeBlocks ); const onSplitList = useCallback( ( value ) => { if ( ! preventDefault.current ) { diff --git a/packages/block-library/src/list/test/edit.native.js b/packages/block-library/src/list/test/edit.native.js index eab2ac4690694..3ce4c8e549449 100644 --- a/packages/block-library/src/list/test/edit.native.js +++ b/packages/block-library/src/list/test/edit.native.js @@ -3,6 +3,7 @@ */ import { changeTextOfRichText, + changeAndSelectTextOfRichText, fireEvent, getEditorHtml, initializeEditor, @@ -18,6 +19,7 @@ import { */ import { getBlockTypes, unregisterBlockType } from '@wordpress/blocks'; import { registerCoreBlocks } from '@wordpress/block-library'; +import { BACKSPACE } from '@wordpress/keycodes'; describe( 'List block', () => { beforeAll( () => { @@ -210,7 +212,7 @@ describe( 'List block', () => {
  • Item 2
    • - +
  • Item 3
    • @@ -238,7 +240,7 @@ describe( 'List block', () => {
  • Item 2
    • - +
  • Item 3
    • @@ -277,7 +279,7 @@ describe( 'List block', () => {
  • Item 2
    • - +
  • Item 3
    • @@ -311,4 +313,101 @@ describe( 'List block', () => { expect( getEditorHtml() ).toMatchSnapshot(); } ); + + it( 'merges with other lists', async () => { + const initialHtml = ` + + + + `; + + const screen = await initializeEditor( { + initialHtml, + } ); + + // Select List block + const listBlock = screen.getByA11yLabel( /List Block\. Row 2/ ); + fireEvent.press( listBlock ); + + // Select List Item block + const listItemBlock = within( listBlock ).getByA11yLabel( + /List item Block\. Row 1/ + ); + fireEvent.press( listItemBlock ); + + // With cursor positioned at the beginning of the first List Item, press + // backward delete + const listItemField = + within( listItemBlock ).getByA11yLabel( /Text input. .*Two.*/ ); + changeAndSelectTextOfRichText( listItemField, 'Two' ); + fireEvent( listItemField, 'onKeyDown', { + nativeEvent: {}, + preventDefault() {}, + keyCode: BACKSPACE, + } ); + + expect( getEditorHtml() ).toMatchInlineSnapshot( ` + " + + " + ` ); + } ); + + it( 'unwraps list items when attempting to merge with non-list block', async () => { + const initialHtml = ` +

    A quick brown fox.

    + + + + `; + + const screen = await initializeEditor( { + initialHtml, + } ); + + // Select List block + const listBlock = screen.getByA11yLabel( /List Block\. Row 2/ ); + fireEvent.press( listBlock ); + + // Select List Item block + const listItemBlock = within( listBlock ).getByA11yLabel( + /List item Block\. Row 1/ + ); + fireEvent.press( listItemBlock ); + + // With cursor positioned at the beginning of the first List Item, press + // backward delete + const listItemField = + within( listItemBlock ).getByA11yLabel( /Text input. .*One.*/ ); + changeAndSelectTextOfRichText( listItemField, 'One' ); + fireEvent( listItemField, 'onKeyDown', { + nativeEvent: {}, + preventDefault() {}, + keyCode: BACKSPACE, + } ); + + expect( getEditorHtml() ).toMatchInlineSnapshot( ` + " +

    A quick brown fox.

    + + + +

    One

    + + + +

    Two

    + " + ` ); + } ); } );