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

List/quote: unwrap inner block when pressing Backspace at start #45075

Merged
merged 4 commits into from
Dec 1, 2022
Merged

List/quote: unwrap inner block when pressing Backspace at start #45075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8bc2ad7
List/quote: unwrap inner block when pressing Backspace at start
ellatrix Oct 18, 2022
46526d8
Implement forward delete and fix tests
ellatrix Oct 18, 2022
53c1209
Add comment
ellatrix Nov 25, 2022
e1b346d
[Mobile] List/quote: Unwrap inner block when pressing Backspace at st…
Nov 30, 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
90 changes: 68 additions & 22 deletions packages/block-editor/src/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
serializeRawBlock,
switchToBlockType,
store as blocksStore,
getDefaultBlockName,
isUnmodifiedBlock,
} from '@wordpress/blocks';
import { withFilters } from '@wordpress/components';
import {
Expand Down Expand Up @@ -311,7 +313,6 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
__unstableMarkLastChangeAsPersistent,
moveBlocksToPosition,
removeBlock,
selectBlock,
} = dispatch( blockEditorStore );

// Do not add new properties here, use `useDispatch` instead to avoid
Expand Down Expand Up @@ -348,8 +349,71 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
getBlockAttributes,
getBlockName,
getBlockOrder,
getBlockIndex,
getBlockRootClientId,
canInsertBlockType,
} = registry.select( blockEditorStore );

/**
* Moves the block with clientId up one level. If the block type
* cannot be inserted at the new location, it will be attempted to
* convert to the default block type.
*
* @param {string} _clientId The block to move.
* @param {boolean} changeSelection Whether to change the selection
* to the moved block.
*/
function moveFirstItemUp( _clientId, changeSelection = true ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A high-level comment stating the goal of the function and its general algorithm would be greatly appreciated. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

const targetRootClientId = getBlockRootClientId( _clientId );
const blockOrder = getBlockOrder( _clientId );
const [ firstClientId ] = blockOrder;

if (
blockOrder.length === 1 &&
isUnmodifiedBlock( getBlock( firstClientId ) )
) {
removeBlock( _clientId );
} else {
if (
canInsertBlockType(
getBlockName( firstClientId ),
targetRootClientId
)
) {
moveBlocksToPosition(
[ firstClientId ],
_clientId,
targetRootClientId,
getBlockIndex( _clientId )
);
} else {
const replacement = switchToBlockType(
getBlock( firstClientId ),
getDefaultBlockName()
);

if ( replacement && replacement.length ) {
registry.batch( () => {
insertBlocks(
replacement,
getBlockIndex( _clientId ),
targetRootClientId,
changeSelection
);
removeBlock( firstClientId, false );
} );
}
}

if (
! getBlockOrder( _clientId ).length &&
isUnmodifiedBlock( getBlock( _clientId ) )
) {
removeBlock( _clientId, false );
}
}
}

// For `Delete` or forward merge, we should do the exact same thing
// as `Backspace`, but from the other block.
if ( forward ) {
Expand Down Expand Up @@ -400,15 +464,8 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
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 );
if ( getBlockOrder( nextBlockClientId ).length ) {
moveFirstItemUp( nextBlockClientId, false );
} else {
mergeBlocks( clientId, nextBlockClientId );
}
Expand Down Expand Up @@ -453,18 +510,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => {
}
}

// 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 );
} );
}
moveFirstItemUp( rootClientId );
}
}
},
Expand Down
90 changes: 69 additions & 21 deletions packages/block-editor/src/components/block-list/block.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
getBlockType,
__experimentalGetAccessibleBlockLabel as getAccessibleBlockLabel,
switchToBlockType,
getDefaultBlockName,
isUnmodifiedBlock,
} from '@wordpress/blocks';
import { useSetting } from '@wordpress/block-editor';

Expand Down Expand Up @@ -436,8 +438,72 @@ export default compose( [
getBlockAttributes,
getBlockName,
getBlockOrder,
getBlockIndex,
getBlockRootClientId,
canInsertBlockType,
} = registry.select( blockEditorStore );

/**
* Moves the block with clientId up one level. If the block type
* cannot be inserted at the new location, it will be attempted to
* convert to the default block type.
*
* @param {string} _clientId The block to move.
* @param {boolean} changeSelection Whether to change the selection
* to the moved block.
*/
function moveFirstItemUp( _clientId, changeSelection = true ) {
const targetRootClientId =
getBlockRootClientId( _clientId );
const blockOrder = getBlockOrder( _clientId );
const [ firstClientId ] = blockOrder;

if (
blockOrder.length === 1 &&
isUnmodifiedBlock( getBlock( firstClientId ) )
) {
removeBlock( _clientId );
} else {
if (
canInsertBlockType(
getBlockName( firstClientId ),
targetRootClientId
)
) {
moveBlocksToPosition(
[ firstClientId ],
_clientId,
targetRootClientId,
getBlockIndex( _clientId )
);
} else {
const replacement = switchToBlockType(
getBlock( firstClientId ),
getDefaultBlockName()
);

if ( replacement && replacement.length ) {
registry.batch( () => {
insertBlocks(
replacement,
getBlockIndex( _clientId ),
targetRootClientId,
changeSelection
);
removeBlock( firstClientId, false );
} );
}
}

if (
! getBlockOrder( _clientId ).length &&
isUnmodifiedBlock( getBlock( _clientId ) )
) {
removeBlock( _clientId, false );
}
}
}

// For `Delete` or forward merge, we should do the exact same thing
// as `Backspace`, but from the other block.
if ( forward ) {
Expand Down Expand Up @@ -488,15 +554,8 @@ export default compose( [
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 );
if ( getBlockOrder( nextBlockClientId ).length ) {
moveFirstItemUp( nextBlockClientId, false );
} else {
mergeBlocks( clientId, nextBlockClientId );
}
Expand Down Expand Up @@ -541,18 +600,7 @@ export default compose( [
}
}

// 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 );
} );
}
moveFirstItemUp( rootClientId );
}
}
},
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/list-item/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import initBlock from '../utils/init-block';
import metadata from './block.json';
import edit from './edit';
import save from './save';
import transforms from './transforms';

const { name } = metadata;

Expand All @@ -25,6 +26,7 @@ export const settings = {
content: attributes.content + attributesToMerge.content,
};
},
transforms,
};

export const init = () => initBlock( { name, metadata, settings } );
17 changes: 17 additions & 0 deletions packages/block-library/src/list-item/transforms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* WordPress dependencies
*/
import { createBlock } from '@wordpress/blocks';

const transforms = {
to: [
{
type: 'block',
blocks: [ 'core/paragraph' ],
transform: ( attributes ) =>
createBlock( 'core/paragraph', attributes ),
},
],
};

export default transforms;
14 changes: 8 additions & 6 deletions packages/block-library/src/list/test/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ describe( 'List block', () => {
` );
} );

it( 'unwraps list items when attempting to merge with non-list block', async () => {
it( 'unwraps first item when attempting to merge with non-list block', async () => {
const initialHtml = `<!-- wp:paragraph -->
<p>A quick brown fox.</p>
<!-- /wp:paragraph -->
Expand Down Expand Up @@ -400,14 +400,16 @@ describe( 'List block', () => {
"<!-- wp:paragraph -->
<p>A quick brown fox.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>One</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Two</p>
<!-- /wp:paragraph -->"

<!-- wp:list -->
<ul><!-- wp:list-item -->
<li>Two</li>
<!-- /wp:list-item --></ul>
<!-- /wp:list -->"
` );
} );
} );
11 changes: 0 additions & 11 deletions packages/block-library/src/list/transforms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,6 @@ const transforms = {
);
},
} ) ),
{
type: 'block',
blocks: [ '*' ],
transform: ( _attributes, childBlocks ) => {
return getListContentFlat( childBlocks ).map( ( content ) =>
createBlock( 'core/paragraph', {
content,
} )
);
},
},
Comment on lines -117 to -127
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking my assumptions here: this bit that we removed — i.e. the Unwrap transform — was functionally equivalent to the Transform to Paragraph transform, right? So we don't lose anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it was a hackier way of doing list item to paragraph. It wasn't great because '*' should mean unwrap, while this is unwrapping and converting it to paragraphs.

],
};

Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/utils/transformation-categories.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const transformationCategories = {
'core/paragraph',
'core/heading',
'core/list',
'core/list-item',
'core/quote',
'core/pullquote',
'core/preformatted',
Expand Down
20 changes: 16 additions & 4 deletions packages/blocks/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,10 +529,9 @@ _Returns_

- `boolean`: Whether the given block is a template part.

### isUnmodifiedDefaultBlock
### isUnmodifiedBlock

Determines whether the block is a default block
and its attributes are equal to the default attributes
Determines whether the block's attributes are equal to the default attributes
which means the block is unmodified.

_Parameters_
Expand All @@ -541,7 +540,20 @@ _Parameters_

_Returns_

- `boolean`: Whether the block is an unmodified default block
- `boolean`: Whether the block is an unmodified block.

### isUnmodifiedDefaultBlock

Determines whether the block is a default block and its attributes are equal
to the default attributes which means the block is unmodified.

_Parameters_

- _block_ `WPBlock`: Block Object

_Returns_

- `boolean`: Whether the block is an unmodified default block.

### isValidBlockContent

Expand Down
1 change: 1 addition & 0 deletions packages/blocks/src/api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export {
unregisterBlockVariation,
} from './registration';
export {
isUnmodifiedBlock,
isUnmodifiedDefaultBlock,
normalizeIconObject,
isValidIcon,
Expand Down
Loading