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 <[email protected]>

packages/block-editor/src/components/block-list/block-list-compact.native.js

@@ -50,6 +50,7 @@ function BlockListCompact( props ) {
  { blockClientIds.map( ( currentClientId ) => (
  <BlockListBlock
  clientId={ currentClientId }
+ rootClientId={ rootClientId }
  key={ currentClientId }
  marginHorizontal={ marginHorizontal }
  marginVertical={ marginVertical }

packages/block-editor/src/components/block-list/block.native.js

@@ -20,6 +20,7 @@ import { compose, withPreferredColorScheme } from '@wordpress/compose';
 import {
  getBlockType,
  __experimentalGetAccessibleBlockLabel as getAccessibleBlockLabel,
+ switchToBlockType,
 } from '@wordpress/blocks';
 import { useSetting } from '@wordpress/block-editor';
 
@@ -41,7 +42,7 @@ function BlockForType( {
  getBlockWidth,
  insertBlocksAfter,
  isSelected,
- mergeBlocks,
+ onMerge,
  name,
  onBlockFocus,
  onChange,
@@ -87,7 +88,7 @@ function BlockForType( {
  onFocus={ onBlockFocus }
  onReplace={ onReplace }
  insertBlocksAfter={ insertBlocksAfter }
- mergeBlocks={ mergeBlocks }
+ mergeBlocks={ onMerge }
  // Block level styles.
  wrapperProps={ wrapperProps }
  // Inherited styles merged with block level styles.
@@ -407,31 +408,144 @@ export default compose( [
  ),
  };
  } ),
- withDispatch( ( dispatch, ownProps, { select } ) => {
+ 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 );
+ } );
+ }
  }
  }
  },

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 ) {

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', () => {
  <!-- /wp:list-item -->
  <!-- wp:list-item -->
  <li>Item 2</li>
- <!-- /wp:list-item --> 
+ <!-- /wp:list-item -->
  <!-- wp:list-item -->
  <li>Item 3</li>
  <!-- /wp:list-item --></ul>
@@ -238,7 +240,7 @@ describe( 'List block', () => {
  <!-- /wp:list-item -->
  <!-- wp:list-item -->
  <li>Item 2</li>
- <!-- /wp:list-item --> 
+ <!-- /wp:list-item -->
  <!-- wp:list-item -->
  <li>Item 3</li>
  <!-- /wp:list-item --></ul>
@@ -277,7 +279,7 @@ describe( 'List block', () => {
  <!-- /wp:list-item -->
  <!-- wp:list-item -->
  <li>Item 2</li>
- <!-- /wp:list-item --> 
+ <!-- /wp:list-item -->
  <!-- wp:list-item -->
  <li>Item 3</li>
  <!-- /wp:list-item --></ul>
@@ -311,4 +313,101 @@ describe( 'List block', () => {
 
  expect( getEditorHtml() ).toMatchSnapshot();
  } );
+
+ it( 'merges with other lists', async () => {
+ const initialHtml = `<!-- wp:list -->
+ <ul><!-- wp:list-item -->
+ <li>One</li><!-- /wp:list-item --></ul>
+ <!-- /wp:list --><!-- wp:list -->
+ <ul><!-- wp:list-item -->
+ <li>Two</li><!-- /wp:list-item --></ul>
+ <!-- /wp:list -->`;
+
+ 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( `
+ "<!-- wp:list -->
+ <ul><!-- wp:list-item -->
+ <li>One</li>
+ <!-- /wp:list-item -->
+
+ <!-- wp:list-item -->
+ <li>Two</li>
+ <!-- /wp:list-item --></ul>
+ <!-- /wp:list -->"
+ ` );
+ } );
+
+ it( 'unwraps list items when attempting to merge with non-list block', async () => {
+ const initialHtml = `<!-- wp:paragraph -->
+ <p>A quick brown fox.</p>
+ <!-- /wp:paragraph -->
+ <!-- wp:list -->
+ <ul><!-- wp:list-item -->
+ <li>One</li><!-- /wp:list-item --><!-- wp:list-item -->
+ <li>Two</li><!-- /wp:list-item --></ul>
+ <!-- /wp:list -->`;
+
+ 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( `
+ "<!-- wp:paragraph -->
+ <p>A quick brown fox.</p>
+ <!-- /wp:paragraph -->
+
+ <!-- wp:paragraph -->
+ <p>One</p>
+ <!-- /wp:paragraph -->
+
+ <!-- wp:paragraph -->
+ <p>Two</p>
+ <!-- /wp:paragraph -->"
+ ` );
+ } );
 } );