Skip to content

Commit

Permalink
Fix unable to remove empty blocks on merge
Browse files Browse the repository at this point in the history
  • Loading branch information
kevin940726 committed Sep 22, 2024
1 parent f073eb1 commit 4639098
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 26 deletions.
71 changes: 59 additions & 12 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import {
isReusableBlock,
getBlockDefaultClassName,
hasBlockSupport,
__experimentalGetBlockAttributesNamesByRole,
store as blocksStore,
privateApis as blocksPrivateApis,
} from '@wordpress/blocks';
import { withFilters } from '@wordpress/components';
import { withDispatch, useDispatch, useSelect } from '@wordpress/data';
Expand All @@ -46,6 +48,8 @@ import { PrivateBlockContext } from './private-block-context';

import { unlock } from '../../lock-unlock';

const { isAttributeUnmodified } = unlock( blocksPrivateApis );

/**
* Merges wrapper props with special handling for classNames and styles.
*
Expand Down Expand Up @@ -307,6 +311,20 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
canInsertBlockType,
} = registry.select( blockEditorStore );

function isBlockEmpty( block ) {
const blockType = getBlockType( block.name );
const contentAttributes =
__experimentalGetBlockAttributesNamesByRole(
block.name,
'content'
);
return contentAttributes.every( ( attribute ) => {
const definition = blockType.attributes[ attribute ];
const value = block.attributes[ attribute ];
return isAttributeUnmodified( definition, value );
} );
}

function switchToDefaultOrRemove() {
const block = getBlock( clientId );
const defaultBlockName = getDefaultBlockName();
Expand Down Expand Up @@ -350,34 +368,62 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
removeBlock( _clientId );
} else {
registry.batch( () => {
if (
const firstBlock = getBlock( firstClientId );
const isFirstBlockEmpty = isBlockEmpty( firstBlock );
const defaultBlockName = getDefaultBlockName();
const replacement = switchToBlockType(
firstBlock,
defaultBlockName
);
const canTransformToDefaultBlock =
!! replacement?.length &&
replacement.every( ( block ) =>
canInsertBlockType( block.name, _clientId )
);

if ( isFirstBlockEmpty && canTransformToDefaultBlock ) {
// Step 1: If the block is empty and can be transformed to the default block type.
replaceBlocks(
firstClientId,
replacement,
changeSelection
);
} else if (
isFirstBlockEmpty &&
firstBlock.name === defaultBlockName
) {
// Step 2: If the block is empty and is already the default block type.
removeBlock( firstClientId );
const nextBlockClientId =
getNextBlockClientId( clientId );
if ( nextBlockClientId ) {
selectBlock( nextBlockClientId );
}
} else if (
canInsertBlockType(
getBlockName( firstClientId ),
firstBlock.name,
targetRootClientId
)
) {
// Step 3: If the block can be moved up.
moveBlocksToPosition(
[ firstClientId ],
_clientId,
targetRootClientId,
getBlockIndex( _clientId )
);
} else {
const replacement = switchToBlockType(
getBlock( firstClientId ),
getDefaultBlockName()
);

if (
replacement &&
replacement.length &&
const canLiftAndTransformToDefaultBlock =
!! replacement?.length &&
replacement.every( ( block ) =>
canInsertBlockType(
block.name,
targetRootClientId
)
)
) {
);

if ( canLiftAndTransformToDefaultBlock ) {
// Step 4: If the block can be transformed to the default block type and moved up.
insertBlocks(
replacement,
getBlockIndex( _clientId ),
Expand All @@ -386,6 +432,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
);
removeBlock( firstClientId, false );
} else {
// Step 5: Continue the default behavior.
switchToDefaultOrRemove();
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/blocks/src/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getBlockBindingsSource,
getBlockBindingsSources,
} from './registration';
import { isAttributeUnmodified } from './utils';

// The blocktype is the most important concept within the block API. It defines
// all aspects of the block configuration and its interfaces, including `edit`
Expand Down Expand Up @@ -182,4 +183,5 @@ lock( privateApis, {
unregisterBlockBindingsSource,
getBlockBindingsSource,
getBlockBindingsSources,
isAttributeUnmodified,
} );
39 changes: 25 additions & 14 deletions packages/blocks/src/api/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,30 @@ extend( [ namesPlugin, a11yPlugin ] );
*/
const ICON_COLORS = [ '#191e23', '#f8f9f9' ];

/**
* Determines whether the block's attribute is equal to the default attribute
* which means the attribute is unmodified.
* @param {Object} attributeDefinition The attribute's definition of the block type.
* @param {*} value The attribute's value.
* @return {boolean} Whether the attribute is unmodified.
*/
export function isAttributeUnmodified( attributeDefinition, value ) {
// Every attribute that has a default must match the default.
if ( attributeDefinition.hasOwnProperty( 'default' ) ) {
return value === attributeDefinition.default;
}

// The rich text type is a bit different from the rest because it
// has an implicit default value of an empty RichTextData instance,
// so check the length of the value.
if ( attributeDefinition.type === 'rich-text' ) {
return ! value?.length;
}

// Every attribute that doesn't have a default should be undefined.
return value === undefined;
}

/**
* Determines whether the block's attributes are equal to the default attributes
* which means the block is unmodified.
Expand All @@ -42,20 +66,7 @@ export function isUnmodifiedBlock( block ) {
( [ key, definition ] ) => {
const value = block.attributes[ key ];

// Every attribute that has a default must match the default.
if ( definition.hasOwnProperty( 'default' ) ) {
return value === definition.default;
}

// The rich text type is a bit different from the rest because it
// has an implicit default value of an empty RichTextData instance,
// so check the length of the value.
if ( definition.type === 'rich-text' ) {
return ! value?.length;
}

// Every attribute that doesn't have a default should be undefined.
return value === undefined;
return isAttributeUnmodified( definition, value );
}
);
}
Expand Down
97 changes: 97 additions & 0 deletions test/e2e/specs/editor/various/splitting-merging.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,103 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => {
);
} );

// Fix for https://github.com/WordPress/gutenberg/issues/65174.
test( 'should handle unwrapping and merging blocks', async ( {
editor,
page,
} ) => {
const emptyAlignedParagraph = {
name: 'core/paragraph',
attributes: { content: '', align: 'center', dropCap: false },
innerBlocks: [],
};
const emptyAlignedHeading = {
name: 'core/heading',
attributes: { content: '', textAlign: 'center', level: 2 },
innerBlocks: [],
};
const headingWithContent = {
name: 'core/heading',
attributes: { content: 'heading', level: 2 },
innerBlocks: [],
};
const placeholderBlock = { name: 'core/separator' };
await editor.insertBlock( {
name: 'core/group',
innerBlocks: [
emptyAlignedParagraph,
emptyAlignedHeading,
headingWithContent,
placeholderBlock,
],
} );
await editor.canvas
.getByRole( 'document', { name: 'Empty block' } )
.focus();

await page.keyboard.press( 'Backspace' );
await expect
.poll( editor.getBlocks, 'Remove the default empty block' )
.toEqual( [
{
name: 'core/group',
attributes: { tagName: 'div' },
innerBlocks: [
emptyAlignedHeading,
headingWithContent,
expect.objectContaining( placeholderBlock ),
],
},
] );

// Move the caret to the beginning of the empty heading block.
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'Backspace' );
await expect
.poll(
editor.getBlocks,
'Convert the non-default empty block to a default block'
)
.toEqual( [
{
name: 'core/group',
attributes: { tagName: 'div' },
innerBlocks: [
emptyAlignedParagraph,
headingWithContent,
expect.objectContaining( placeholderBlock ),
],
},
] );
await page.keyboard.press( 'Backspace' );
await expect.poll( editor.getBlocks ).toEqual( [
{
name: 'core/group',
attributes: { tagName: 'div' },
innerBlocks: [
headingWithContent,
expect.objectContaining( placeholderBlock ),
],
},
] );

// Move the caret to the beginning of the "heading" heading block.
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'Backspace' );
await expect
.poll( editor.getBlocks, 'Lift the non-empty non-default block' )
.toEqual( [
headingWithContent,
{
name: 'core/group',
attributes: { tagName: 'div' },
innerBlocks: [
expect.objectContaining( placeholderBlock ),
],
},
] );
} );

test.describe( 'test restore selection when merge produces more than one block', () => {
const snap1 = [
{
Expand Down

0 comments on commit 4639098

Please sign in to comment.