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.

Sign up for GitHub

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

Jump to bottom

Merging blocks: allow x to be merged into wrapper blocks (quote, list, group...) #42780

Merged
merged 5 commits into from
Aug 12, 2022
Merged

Merging blocks: allow x to be merged into wrapper blocks (quote, list, group...) #42780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
095df17
Merging blocks: allow x to be merged into wrapper blocks (quote, list…
ellatrix Jul 28, 2022
e9ec61f
Polish
ellatrix Jul 28, 2022
051d9dc
Restore select
ellatrix Jul 29, 2022
18bf944
Restore comments
ellatrix Jul 29, 2022
97a77f2
Add e2e test
ellatrix Jul 29, 2022
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
36 changes: 32 additions & 4 deletions packages/block-editor/src/store/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,21 +1002,49 @@ export const __unstableExpandSelection =
*/
export const mergeBlocks =
( firstBlockClientId, secondBlockClientId ) =>
( { select, dispatch } ) => {
( { registry, select, dispatch } ) => {
const blocks = [ firstBlockClientId, secondBlockClientId ];
dispatch( { type: 'MERGE_BLOCKS', blocks } );

const [ clientIdA, clientIdB ] = blocks;
const blockA = select.getBlock( clientIdA );
const blockAType = getBlockType( blockA.name );

// Only focus the previous block if it's not mergeable.
if ( ! blockAType ) return;

const blockB = select.getBlock( clientIdB );

if ( blockAType && ! blockAType.merge ) {
dispatch.selectBlock( blockA.clientId );
// If there's no merge function defined, attempt merging inner
// blocks.
const blocksWithTheSameType = switchToBlockType(
blockB,
blockAType.name
);
// Only focus the previous block if it's not mergeable.
if ( blocksWithTheSameType?.length !== 1 ) {
dispatch.selectBlock( blockA.clientId );
return;
}
const [ blockWithSameType ] = blocksWithTheSameType;
if ( blockWithSameType.innerBlocks.length < 1 ) {
dispatch.selectBlock( blockA.clientId );
return;
}
registry.batch( () => {
dispatch.insertBlocks(
blockWithSameType.innerBlocks,
undefined,
clientIdA
);
dispatch.removeBlock( clientIdB );
dispatch.selectBlock(
blockWithSameType.innerBlocks[ 0 ].clientId
);
} );
return;
}

const blockB = select.getBlock( clientIdB );
const blockBType = getBlockType( blockB.name );
const { clientId, attributeKey, offset } = select.getSelectionStart();
const selectedBlockType =
Expand Down
4 changes: 4 additions & 0 deletions packages/e2e-tests/specs/editor/blocks/__snapshots__/quote.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ exports[`Quote can be split at the end 2`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><!-- wp:paragraph -->
<p>1</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ellatrix I think there should be a '2' here. The test types it, but it's missing. It looks like you may have committed the result of a failed test.

I've pushed a PR that updates the snapshot to its correct version - #43407. I expect the test is flakey and will still need to be fixed, but at least it won't be a false positive.

<!-- /wp:paragraph --></blockquote>
<!-- /wp:quote -->"
`;
3 changes: 2 additions & 1 deletion packages/e2e-tests/specs/editor/blocks/quote.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,9 @@ describe( 'Quote', () => {
expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.press( 'Backspace' );
await page.keyboard.type( '2' );

// Expect the paragraph to be deleted.
// Expect the paragraph to be merged into the quote block.
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

Expand Down
9 changes: 9 additions & 0 deletions ...pecs/editor/blocks/__snapshots__/Group-can-merge-into-group-with-Backspace-1-chromium.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!-- wp:group -->
<div class="wp-block-group"><!-- wp:paragraph -->
<p>1</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->

<!-- wp:paragraph -->
<p>2</p>
<!-- /wp:paragraph -->
9 changes: 9 additions & 0 deletions ...pecs/editor/blocks/__snapshots__/Group-can-merge-into-group-with-Backspace-2-chromium.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!-- wp:group -->
<div class="wp-block-group"><!-- wp:paragraph -->
<p>1</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>2</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
17 changes: 17 additions & 0 deletions test/e2e/specs/editor/blocks/group.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,21 @@ test.describe( 'Group', () => {

expect( await editor.getEditedPostContent() ).toMatchSnapshot();
} );

test( 'can merge into group with Backspace', async ( { editor, page } ) => {
await page.keyboard.press( 'Enter' );
await page.keyboard.type( '1' );
await editor.transformBlockTo( 'core/group' );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( '2' );

// Confirm last paragraph is outside of group.
expect( await editor.getEditedPostContent() ).toMatchSnapshot();

// Merge the last paragraph into the group.
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'Backspace' );

expect( await editor.getEditedPostContent() ).toMatchSnapshot();
} );
} );