Skip to content

Commit

Permalink
fix: Native inner blocks merge where appropriate (#45048)
Browse files Browse the repository at this point in the history 
* 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]>
  • Loading branch information
@dcalhoun @fluiddot
dcalhoun and fluiddot committed Oct 19, 2022
1 parent b2132d8 commit 0513051
Show file tree
Hide file tree
Showing 4 changed files with 228 additions and 13 deletions.
1 change: 1 addition & 0 deletions packages/block-editor/src/components/block-list/block-list-compact.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function BlockListCompact( props ) {
{ blockClientIds.map( ( currentClientId ) => (
<BlockListBlock
clientId={ currentClientId }
rootClientId={ rootClientId }
key={ currentClientId }
marginHorizontal={ marginHorizontal }
marginVertical={ marginVertical }
Expand Down
132 changes: 123 additions & 9 deletions packages/block-editor/src/components/block-list/block.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -41,7 +42,7 @@ function BlockForType( {
getBlockWidth,
insertBlocksAfter,
isSelected,
mergeBlocks,
onMerge,
name,
onBlockFocus,
onChange,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 );
} );
}
}
}
},
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/list-item/edit.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export default function ListItemEdit( {
onReplace,
clientId,
style,
mergeBlocks,
} ) {
const [ contentWidth, setContentWidth ] = useState();
const { placeholder, content } = attributes;
Expand Down Expand Up @@ -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 ) {
Expand Down
105 changes: 102 additions & 3 deletions packages/block-library/src/list/test/edit.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import {
changeTextOfRichText,
changeAndSelectTextOfRichText,
fireEvent,
getEditorHtml,
initializeEditor,
Expand All @@ -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( () => {
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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 -->"
` );
} );
} );

0 comments on commit 0513051

Please sign in to comment.