Skip to content

Commit

Permalink
Fix unable to remove empty blocks on merge (#65262)
Browse files Browse the repository at this point in the history 
Co-authored-by: kevin940726 <[email protected]>
Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: talldan <[email protected]>
Co-authored-by: andrewserong <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: getdave <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: kspilarski <[email protected]>
Co-authored-by: ndiego <[email protected]>
Co-authored-by: richtabor <[email protected]>
* Fix unable to remove empty blocks on merge

* Update to the stabilized API

* Rename the utils
  • Loading branch information
@kevin940726
kevin940726 authored Sep 27, 2024
1 parent 0ed82ae commit 93323fd
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 25 deletions.
58 changes: 47 additions & 11 deletions packages/block-editor/src/components/block-list/block.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
getBlockDefaultClassName,
hasBlockSupport,
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 +47,8 @@ import { PrivateBlockContext } from './private-block-context';

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

const { isUnmodifiedBlockContent } = unlock( blocksPrivateApis );

/**
* Merges wrapper props with special handling for classNames and styles.
*
Expand Down Expand Up @@ -350,34 +353,66 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
removeBlock( _clientId );
} else {
registry.batch( () => {
const firstBlock = getBlock( firstClientId );
const isFirstBlockContentUnmodified =
isUnmodifiedBlockContent( firstBlock );
const defaultBlockName = getDefaultBlockName();
const replacement = switchToBlockType(
firstBlock,
defaultBlockName
);
const canTransformToDefaultBlock =
!! replacement?.length &&
replacement.every( ( block ) =>
canInsertBlockType( block.name, _clientId )
);

if (
isFirstBlockContentUnmodified &&
canTransformToDefaultBlock
) {
// Step 1: If the block is empty and can be transformed to the default block type.
replaceBlocks(
firstClientId,
replacement,
changeSelection
);
} else if (
isFirstBlockContentUnmodified &&
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 +421,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getBlockBindingsSource,
getBlockBindingsSources,
} from './registration';
import { isUnmodifiedBlockContent } 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 @@ -183,4 +184,5 @@ lock( privateApis, {
unregisterBlockBindingsSource,
getBlockBindingsSource,
getBlockBindingsSources,
isUnmodifiedBlockContent,
} );
68 changes: 54 additions & 14 deletions packages/blocks/src/api/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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.
*/
function isUnmodifiedAttribute( 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 @@ -43,20 +67,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 isUnmodifiedAttribute( definition, value );
}
);
}
Expand All @@ -73,6 +84,35 @@ export function isUnmodifiedDefaultBlock( block ) {
return block.name === getDefaultBlockName() && isUnmodifiedBlock( block );
}

/**
* Determines whether the block content is unmodified. A block content is
* considered unmodified if all the attributes that have a role of 'content'
* are equal to the default attributes (or undefined).
* If the block does not have any attributes with a role of 'content', it
* will be considered unmodified if all the attributes are equal to the default
* attributes (or undefined).
*
* @param {WPBlock} block Block Object
* @return {boolean} Whether the block content is unmodified.
*/
export function isUnmodifiedBlockContent( block ) {
const contentAttributes = getBlockAttributesNamesByRole(
block.name,
'content'
);

if ( contentAttributes.length === 0 ) {
return isUnmodifiedBlock( block );
}

return contentAttributes.every( ( key ) => {
const definition = getBlockType( block.name )?.attributes[ key ];
const value = block.attributes[ key ];

return isUnmodifiedAttribute( definition, value );
} );
}

/**
* Function that checks if the parameter is a valid icon.
*
Expand Down
97 changes: 97 additions & 0 deletions test/e2e/specs/editor/various/splitting-merging.spec.js
View file
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 with empty contents', 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 93323fd

Please sign in to comment.