Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Native inner blocks merge where appropriate #45048

Merged
merged 3 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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 -->"
` );
} );
} );