Skip to content

Commit

Permalink
Revert: Fix unable to remove empty blocks on merge (#65262)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ellatrix
ellatrix committed Oct 29, 2024
1 parent 26123cd commit c432469
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 211 deletions.
58 changes: 11 additions & 47 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,7 +25,6 @@ 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 @@ -47,8 +46,6 @@ 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 @@ -353,66 +350,34 @@ 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(
firstBlock.name,
getBlockName( firstClientId ),
targetRootClientId
)
) {
// Step 3: If the block can be moved up.
moveBlocksToPosition(
[ firstClientId ],
_clientId,
targetRootClientId,
getBlockIndex( _clientId )
);
} else {
const canLiftAndTransformToDefaultBlock =
!! replacement?.length &&
const replacement = switchToBlockType(
getBlock( firstClientId ),
getDefaultBlockName()
);

if (
replacement &&
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 @@ -421,7 +386,6 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
);
removeBlock( firstClientId, false );
} else {
// Step 5: Continue the default behavior.
switchToDefaultOrRemove();
}
}
Expand Down
4 changes: 0 additions & 4 deletions packages/blocks/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,10 +503,6 @@ _Returns_

- `Array|string`: A list of blocks or a string, depending on `handlerMode`.

### privateApis

Undocumented declaration.

### rawHandler

Converts an HTML string to known blocks.
Expand Down
9 changes: 0 additions & 9 deletions packages/blocks/src/api/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
/**
* Internal dependencies
*/
import { lock } from '../lock-unlock';
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`
// and `save`. The transforms specification allows converting one blocktype to
Expand Down Expand Up @@ -175,6 +169,3 @@ export {
__EXPERIMENTAL_ELEMENTS,
__EXPERIMENTAL_PATHS_WITH_OVERRIDE,
} from './constants';

export const privateApis = {};
lock( privateApis, { isUnmodifiedBlockContent } );
68 changes: 14 additions & 54 deletions packages/blocks/src/api/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,6 @@ 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 @@ -67,7 +43,20 @@ export function isUnmodifiedBlock( block ) {
( [ key, definition ] ) => {
const value = block.attributes[ key ];

return isUnmodifiedAttribute( definition, value );
// 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;
}
);
}
Expand All @@ -84,35 +73,6 @@ 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: 0 additions & 97 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,103 +373,6 @@ 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 c432469

Please sign in to comment.