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

InnerBlocks: Support insert before/after block actions when using allowedBlocks #59162

Merged
merged 4 commits into from
Feb 22, 2024
Merged

InnerBlocks: Support insert before/after block actions when using allowedBlocks #59162

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
f77d553
Don't call selectors during the render; subscribe to the store instead
Mamaduka Feb 19, 2024
6aeb454
Get blocks on demand
Mamaduka Feb 19, 2024
95d23d4
Update 'insertBeforeBlock' and 'insertAfterBlock' actions
Mamaduka Feb 19, 2024
9010ab0
A handler 'attributesToCopy' definitions
Mamaduka Feb 20, 2024
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
4 changes: 2 additions & 2 deletions docs/reference-guides/data/data-core-block-editor.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1263,15 +1263,15 @@ Action that hides the insertion point.

### insertAfterBlock

Action that inserts an empty block after a given block.
Action that inserts a default block after a given block.

_Parameters_

- _clientId_ `string`:

### insertBeforeBlock

Action that inserts an empty block before a given block.
Action that inserts a default block before a given block.

_Parameters_

Expand Down
104 changes: 57 additions & 47 deletions packages/block-editor/src/components/block-actions/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,42 +20,55 @@ export default function BlockActions( {
children,
__experimentalUpdateSelection: updateSelection,
} ) {
const {
canInsertBlockType,
getBlockRootClientId,
getBlocksByClientId,
canMoveBlocks,
canRemoveBlocks,
} = useSelect( blockEditorStore );
const { getDefaultBlockName, getGroupingBlockName } =
useSelect( blocksStore );

const blocks = getBlocksByClientId( clientIds );
const rootClientId = getBlockRootClientId( clientIds[ 0 ] );

const canCopyStyles = blocks.every( ( block ) => {
return (
!! block &&
( hasBlockSupport( block.name, 'color' ) ||
hasBlockSupport( block.name, 'typography' ) )
);
} );

const canDuplicate = blocks.every( ( block ) => {
return (
!! block &&
hasBlockSupport( block.name, 'multiple', true ) &&
canInsertBlockType( block.name, rootClientId )
);
} );

const canInsertDefaultBlock = canInsertBlockType(
getDefaultBlockName(),
rootClientId
const selected = useSelect(
Copy link
Member

Choose a reason for hiding this comment

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

Why store in selected instead of destructuring right here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Learned this "hack" from @ellatrix. It's to avoid the already declared in the upper scope ESLint error for mapSelect.

It's no longer needed after 6aeb454. But let's keep this pattern for now.

Copy link
Member

Choose a reason for hiding this comment

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

😮 omg that's life changing

( select ) => {
const {
canInsertBlockType,
getBlockRootClientId,
getBlocksByClientId,
getDirectInsertBlock,
canMoveBlocks,
canRemoveBlocks,
} = select( blockEditorStore );

const blocks = getBlocksByClientId( clientIds );
const rootClientId = getBlockRootClientId( clientIds[ 0 ] );
const canInsertDefaultBlock = canInsertBlockType(
getDefaultBlockName(),
rootClientId
);
const directInsertBlock = rootClientId
? getDirectInsertBlock( rootClientId )
: null;

return {
canMove: canMoveBlocks( clientIds, rootClientId ),
canRemove: canRemoveBlocks( clientIds, rootClientId ),
canInsertBlock: canInsertDefaultBlock || !! directInsertBlock,
canCopyStyles: blocks.every( ( block ) => {
return (
!! block &&
( hasBlockSupport( block.name, 'color' ) ||
hasBlockSupport( block.name, 'typography' ) )
);
} ),
canDuplicate: blocks.every( ( block ) => {
return (
!! block &&
hasBlockSupport( block.name, 'multiple', true ) &&
canInsertBlockType( block.name, rootClientId )
);
} ),
};
},
[ clientIds, getDefaultBlockName ]
);
const { getBlocksByClientId, getBlocks } = useSelect( blockEditorStore );
Copy link
Member

Choose a reason for hiding this comment

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

This approach is much nicer 🙂


const canMove = canMoveBlocks( clientIds, rootClientId );
const canRemove = canRemoveBlocks( clientIds, rootClientId );
const { canMove, canRemove, canInsertBlock, canCopyStyles, canDuplicate } =
selected;

const {
removeBlocks,
Expand All @@ -75,11 +88,9 @@ export default function BlockActions( {
return children( {
canCopyStyles,
canDuplicate,
canInsertDefaultBlock,
canInsertBlock,
Copy link
Member

Choose a reason for hiding this comment

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

Just confirming that BlockActions isn't a public API, yeah?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. It's not a public API.

canMove,
canRemove,
rootClientId,
blocks,
onDuplicate() {
return duplicateBlocks( clientIds, updateSelection );
},
Expand All @@ -104,44 +115,43 @@ export default function BlockActions( {
setBlockMovingClientId( clientIds[ 0 ] );
},
onGroup() {
if ( ! blocks.length ) {
if ( ! clientIds.length ) {
return;
}

const groupingBlockName = getGroupingBlockName();

// Activate the `transform` on `core/group` which does the conversion.
const newBlocks = switchToBlockType( blocks, groupingBlockName );
const newBlocks = switchToBlockType(
getBlocksByClientId( clientIds ),
groupingBlockName
);

if ( ! newBlocks ) {
return;
}
replaceBlocks( clientIds, newBlocks );
},
onUngroup() {
if ( ! blocks.length ) {
if ( ! clientIds.length ) {
return;
}

const innerBlocks = blocks[ 0 ].innerBlocks;

const innerBlocks = getBlocks( clientIds[ 0 ] );
if ( ! innerBlocks.length ) {
return;
}

replaceBlocks( clientIds, innerBlocks );
},
onCopy() {
const selectedBlockClientIds = blocks.map(
( { clientId } ) => clientId
);
if ( blocks.length === 1 ) {
flashBlock( selectedBlockClientIds[ 0 ] );
if ( clientIds.length === 1 ) {
flashBlock( clientIds[ 0 ] );
}
notifyCopy( 'copy', selectedBlockClientIds );
notifyCopy( 'copy', clientIds );
},
async onPasteStyles() {
await pasteStyles( blocks );
await pasteStyles( getBlocksByClientId( clientIds ) );
},
} );
}
21 changes: 12 additions & 9 deletions packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ const POPOVER_PROPS = {
placement: 'bottom-start',
};

function CopyMenuItem( { blocks, onCopy, label } ) {
const ref = useCopyToClipboard( () => serialize( blocks ), onCopy );
function CopyMenuItem( { clientIds, onCopy, label } ) {
const { getBlocksByClientId } = useSelect( blockEditorStore );
const ref = useCopyToClipboard(
() => serialize( getBlocksByClientId( clientIds ) ),
onCopy
);
const copyMenuItemLabel = label ? label : __( 'Copy' );
return <MenuItem ref={ ref }>{ copyMenuItemLabel }</MenuItem>;
}
Expand Down Expand Up @@ -198,7 +202,7 @@ export function BlockSettingsDropdown( {
{ ( {
canCopyStyles,
canDuplicate,
canInsertDefaultBlock,
canInsertBlock,
canMove,
canRemove,
onDuplicate,
Expand All @@ -208,7 +212,6 @@ export function BlockSettingsDropdown( {
onCopy,
onPasteStyles,
onMoveTo,
blocks,
} ) => (
<DropdownMenu
icon={ moreVertical }
Expand Down Expand Up @@ -245,7 +248,7 @@ export function BlockSettingsDropdown( {
'core/block-editor/insert-after',
event
) &&
canInsertDefaultBlock
canInsertBlock
) {
event.preventDefault();
setOpenedBlockSettingsMenu( undefined );
Expand All @@ -255,7 +258,7 @@ export function BlockSettingsDropdown( {
'core/block-editor/insert-before',
event
) &&
canInsertDefaultBlock
canInsertBlock
) {
event.preventDefault();
setOpenedBlockSettingsMenu( undefined );
Expand Down Expand Up @@ -286,7 +289,7 @@ export function BlockSettingsDropdown( {
/>
) }
<CopyMenuItem
blocks={ blocks }
clientIds={ clientIds }
onCopy={ onCopy }
/>
{ canDuplicate && (
Expand All @@ -301,7 +304,7 @@ export function BlockSettingsDropdown( {
{ __( 'Duplicate' ) }
</MenuItem>
) }
{ canInsertDefaultBlock && (
{ canInsertBlock && (
<>
<MenuItem
onClick={ pipe(
Expand All @@ -327,7 +330,7 @@ export function BlockSettingsDropdown( {
{ canCopyStyles && (
<MenuGroup>
<CopyMenuItem
blocks={ blocks }
clientIds={ clientIds }
onCopy={ onCopy }
label={ __( 'Copy styles' ) }
/>
Expand Down
68 changes: 54 additions & 14 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 @@ -1539,7 +1539,7 @@ export const duplicateBlocks =
};

/**
* Action that inserts an empty block before a given block.
* Action that inserts a default block before a given block.
*
* @param {string} clientId
*/
Expand All @@ -1555,16 +1555,34 @@ export const insertBeforeBlock =
return;
}

const firstSelectedIndex = select.getBlockIndex( clientId );
return dispatch.insertDefaultBlock(
{},
rootClientId,
firstSelectedIndex
);
const blockIndex = select.getBlockIndex( clientId );
const directInsertBlock = rootClientId
? select.getDirectInsertBlock( rootClientId )
: null;

if ( ! directInsertBlock ) {
return dispatch.insertDefaultBlock( {}, rootClientId, blockIndex );
}

const copiedAttributes = {};
if ( directInsertBlock.attributesToCopy ) {
const attributes = select.getBlockAttributes( clientId );
directInsertBlock.attributesToCopy.forEach( ( key ) => {
if ( attributes[ key ] ) {
copiedAttributes[ key ] = attributes[ key ];
}
} );
}

const block = createBlock( directInsertBlock.name, {
...directInsertBlock.attributes,
...copiedAttributes,
} );
return dispatch.insertBlock( block, blockIndex, rootClientId );
};

/**
* Action that inserts an empty block after a given block.
* Action that inserts a default block after a given block.
*
* @param {string} clientId
*/
Expand All @@ -1580,12 +1598,34 @@ export const insertAfterBlock =
return;
}

const firstSelectedIndex = select.getBlockIndex( clientId );
return dispatch.insertDefaultBlock(
{},
rootClientId,
firstSelectedIndex + 1
);
const blockIndex = select.getBlockIndex( clientId );
const directInsertBlock = rootClientId
? select.getDirectInsertBlock( rootClientId )
: null;

if ( ! directInsertBlock ) {
return dispatch.insertDefaultBlock(
{},
rootClientId,
blockIndex + 1
);
}

const copiedAttributes = {};
if ( directInsertBlock.attributesToCopy ) {
const attributes = select.getBlockAttributes( clientId );
directInsertBlock.attributesToCopy.forEach( ( key ) => {
if ( attributes[ key ] ) {
copiedAttributes[ key ] = attributes[ key ];
}
} );
}

const block = createBlock( directInsertBlock.name, {
...directInsertBlock.attributes,
...copiedAttributes,
} );
return dispatch.insertBlock( block, blockIndex + 1, rootClientId );
Copy link
Member

Choose a reason for hiding this comment

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

Can we make insertBeforeBlock and insertAfterBlock both call into e.g. insertDefaultBlock( clientId: string, position: 'before' | 'after' ) or insertDefaultBlock( clientId: string, offset: 1 | -1 ) to reduce duplication? There's quite a lot of repetition now that both actions have more logic.

Copy link
Member Author

@Mamaduka Mamaduka Feb 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So, move most of the logic into the insertDefaultBlock action, correct?

The only issue I see here is that insertDefaultBlock is more widely used where consumers rely on it to insert an empty paragraph. Example:

gutenberg/packages/editor/src/components/post-title/index.js

Lines 77 to 79 in 09dc127

function onEnterPress() {
insertDefaultBlock( undefined, undefined, 0 );
}

P.S. It's definitely doable, but I am unsure if removing 20+ lines is worth complicating the logic of the existing insertDefaultBlock action.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I meant a new selector not insertDefaultBlock. Not sure what to to name it 😀

Up to you. I think it's worth it now that the logic isn't trivial, but respect your judgement.

};

/**
Expand Down
Loading