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

One hook to rule them all: preparation for a block supports API #56862

Merged
merged 8 commits into from
Dec 7, 2023
Merged

One hook to rule them all: preparation for a block supports API #56862

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f13bd13
One hook to rule them all: preparation for a block supports API
ellatrix Dec 7, 2023
35242d7
more
ellatrix Dec 7, 2023
575fac5
more
ellatrix Dec 7, 2023
70d34c4
Avoid dynamic import
ellatrix Dec 7, 2023
4ab0f93
null return
ellatrix Dec 7, 2023
e629659
Fix prop rename issue
ellatrix Dec 7, 2023
738c07a
Fix mobile
ellatrix Dec 7, 2023
aa5e8f5
Fix share controls with children
ellatrix Dec 7, 2023
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
53 changes: 8 additions & 45 deletions packages/block-editor/src/hooks/align.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { createHigherOrderComponent, pure } from '@wordpress/compose';
import { createHigherOrderComponent } from '@wordpress/compose';
import { addFilter } from '@wordpress/hooks';
import {
getBlockSupport,
Expand Down Expand Up @@ -109,7 +109,7 @@ export function addAttribute( settings ) {
}

function BlockEditAlignmentToolbarControlsPure( {
blockName,
name: blockName,
align,
setAttributes,
} ) {
Expand Down Expand Up @@ -152,45 +152,13 @@ function BlockEditAlignmentToolbarControlsPure( {
);
}

// We don't want block controls to re-render when typing inside a block. `pure`
// will prevent re-renders unless props change, so only pass the needed props
// and not the whole attributes object.
const BlockEditAlignmentToolbarControls = pure(
BlockEditAlignmentToolbarControlsPure
);

/**
* Override the default edit UI to include new toolbar controls for block
* alignment, if block defines support.
*
* @param {Function} BlockEdit Original component.
*
* @return {Function} Wrapped component.
*/
export const withAlignmentControls = createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const hasAlignmentSupport = hasBlockSupport(
props.name,
'align',
false
);

return (
<>
{ hasAlignmentSupport && (
<BlockEditAlignmentToolbarControls
blockName={ props.name }
// This component is pure, so only pass needed props!
align={ props.attributes.align }
setAttributes={ props.setAttributes }
/>
) }
<BlockEdit key="edit" { ...props } />
</>
);
export default {
edit: BlockEditAlignmentToolbarControlsPure,
attributeKeys: [ 'align' ],
Copy link
Member Author

@ellatrix ellatrix Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@youknowriad I'm using this to "subscribe" to specific attributes only. But on the other hand, extra useSelect subscription these control components are not bad per se because they are only rendered when a block is selected. If we allow that, we wouldn't need to do this declaration of which attributes need to be passed down. Either way, I'm fine with it, they're not mutually exclusive.

Copy link
Member Author

Choose a reason for hiding this comment

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

And we can decide this later when we actually make the real API.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is totally fine for me and to be honest, I'm not entirely sold on opening this API to the public yet but it's definitely a great improvement for us.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh sure, no public API for now :D

hasSupport( name ) {
return hasBlockSupport( name, 'align', false );
},
'withAlignmentControls'
);
};

function BlockListBlockWithDataAlign( { block: BlockListBlock, props } ) {
const { name, attributes } = props;
Expand Down Expand Up @@ -273,11 +241,6 @@ addFilter(
'core/editor/align/with-data-align',
withDataAlign
);
addFilter(
'editor.BlockEdit',
'core/editor/align/with-toolbar-controls',
withAlignmentControls
);
addFilter(
'blocks.getSaveContent.extraProps',
'core/editor/align/addAssignedAlign',
Expand Down
51 changes: 12 additions & 39 deletions packages/block-editor/src/hooks/anchor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { addFilter } from '@wordpress/hooks';
import { PanelBody, TextControl, ExternalLink } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { hasBlockSupport } from '@wordpress/blocks';
import { createHigherOrderComponent, pure } from '@wordpress/compose';
import { Platform } from '@wordpress/element';

/**
Expand Down Expand Up @@ -52,7 +51,11 @@ export function addAttribute( settings ) {
return settings;
}

function BlockEditAnchorControlPure( { blockName, anchor, setAttributes } ) {
function BlockEditAnchorControlPure( {
name: blockName,
anchor,
setAttributes,
} ) {
const blockEditingMode = useBlockEditingMode();

const isWeb = Platform.OS === 'web';
Expand Down Expand Up @@ -116,38 +119,13 @@ function BlockEditAnchorControlPure( { blockName, anchor, setAttributes } ) {
);
}

// We don't want block controls to re-render when typing inside a block. `pure`
// will prevent re-renders unless props change, so only pass the needed props
// and not the whole attributes object.
const BlockEditAnchorControl = pure( BlockEditAnchorControlPure );

/**
* Override the default edit UI to include a new block inspector control for
* assigning the anchor ID, if block supports anchor.
*
* @param {Component} BlockEdit Original component.
*
* @return {Component} Wrapped component.
*/
export const withAnchorControls = createHigherOrderComponent( ( BlockEdit ) => {
return ( props ) => {
return (
<>
<BlockEdit { ...props } />
{ props.isSelected &&
hasBlockSupport( props.name, 'anchor' ) && (
<BlockEditAnchorControl
blockName={ props.name }
// This component is pure, so only pass needed
// props!
anchor={ props.attributes.anchor }
setAttributes={ props.setAttributes }
/>
) }
</>
);
};
}, 'withAnchorControls' );
export default {
edit: BlockEditAnchorControlPure,
attributeKeys: [ 'anchor' ],
hasSupport( name ) {
return hasBlockSupport( name, 'anchor' );
},
};

/**
* Override props assigned to save component to inject anchor ID, if block
Expand All @@ -169,11 +147,6 @@ export function addSaveProps( extraProps, blockType, attributes ) {
}

addFilter( 'blocks.registerBlockType', 'core/anchor/attribute', addAttribute );
addFilter(
'editor.BlockEdit',
'core/editor/anchor/with-inspector-controls',
withAnchorControls
);
addFilter(
'blocks.getSaveContent.extraProps',
'core/editor/anchor/save-props',
Expand Down
78 changes: 22 additions & 56 deletions packages/block-editor/src/hooks/block-hooks.js
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

👋 I might be missing some context, so I'm not sure it matters, but I just came to say that Block Hooks are not using the Block Supports mechanism; they're separate.

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't matter, we can just turn on support for all blocks. The point is that controls are managed in one place and only rendered and re-rendered when necessary. In this case, since the controls don't depend on any attributes, we can prevent unnecessary re-renders.

Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { addFilter } from '@wordpress/hooks';
import { Fragment, useMemo } from '@wordpress/element';
import {
__experimentalHStack as HStack,
PanelBody,
ToggleControl,
} from '@wordpress/components';
import { createHigherOrderComponent, pure } from '@wordpress/compose';
import { createBlock, store as blocksStore } from '@wordpress/blocks';
import { useDispatch, useSelect } from '@wordpress/data';

Expand All @@ -21,7 +19,7 @@ import { store as blockEditorStore } from '../store';

const EMPTY_OBJECT = {};

function BlockHooksControlPure( props ) {
function BlockHooksControlPure( { name, clientId } ) {
const blockTypes = useSelect(
( select ) => select( blocksStore ).getBlockTypes(),
[]
Expand All @@ -30,10 +28,9 @@ function BlockHooksControlPure( props ) {
const hookedBlocksForCurrentBlock = useMemo(
() =>
blockTypes?.filter(
( { blockHooks } ) =>
blockHooks && props.blockName in blockHooks
( { blockHooks } ) => blockHooks && name in blockHooks
),
[ blockTypes, props.blockName ]
[ blockTypes, name ]
);

const { blockIndex, rootClientId, innerBlocksLength } = useSelect(
Expand All @@ -42,13 +39,12 @@ function BlockHooksControlPure( props ) {
select( blockEditorStore );

return {
blockIndex: getBlockIndex( props.clientId ),
innerBlocksLength: getBlock( props.clientId )?.innerBlocks
?.length,
rootClientId: getBlockRootClientId( props.clientId ),
blockIndex: getBlockIndex( clientId ),
innerBlocksLength: getBlock( clientId )?.innerBlocks?.length,
rootClientId: getBlockRootClientId( clientId ),
};
},
[ props.clientId ]
[ clientId ]
);

const hookedBlockClientIds = useSelect(
Expand All @@ -65,8 +61,7 @@ function BlockHooksControlPure( props ) {
return clientIds;
}

const relativePosition =
block?.blockHooks?.[ props.blockName ];
const relativePosition = block?.blockHooks?.[ name ];
let candidates;

switch ( relativePosition ) {
Expand All @@ -83,12 +78,12 @@ function BlockHooksControlPure( props ) {
// Any of the current block's child blocks (with the right block type) qualifies
// as a hooked first or last child block, as the block might've been automatically
// inserted and then moved around a bit by the user.
candidates = getBlock( props.clientId ).innerBlocks;
candidates = getBlock( clientId ).innerBlocks;
break;
}

const hookedBlock = candidates?.find(
( { name } ) => name === block.name
( candidate ) => name === candidate.name
Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that this broke the toggle 😬

I've filed a fix: #57956.

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, sorry!

);

// If the block exists in the designated location, we consider it hooked
Expand Down Expand Up @@ -118,12 +113,7 @@ function BlockHooksControlPure( props ) {

return EMPTY_OBJECT;
},
[
hookedBlocksForCurrentBlock,
props.blockName,
props.clientId,
rootClientId,
]
[ hookedBlocksForCurrentBlock, name, clientId, rootClientId ]
);

const { insertBlock, removeBlock } = useDispatch( blockEditorStore );
Expand Down Expand Up @@ -169,7 +159,7 @@ function BlockHooksControlPure( props ) {
block,
// TODO: It'd be great if insertBlock() would accept negative indices for insertion.
relativePosition === 'first_child' ? 0 : innerBlocksLength,
props.clientId, // Insert as a child of the current block.
clientId, // Insert as a child of the current block.
false
);
break;
Expand Down Expand Up @@ -207,9 +197,7 @@ function BlockHooksControlPure( props ) {
if ( ! checked ) {
// Create and insert block.
const relativePosition =
block.blockHooks[
props.blockName
];
block.blockHooks[ name ];
insertBlockIntoDesignatedLocation(
createBlock( block.name ),
relativePosition
Expand All @@ -218,11 +206,12 @@ function BlockHooksControlPure( props ) {
}

// Remove block.
const clientId =
removeBlock(
hookedBlockClientIds[
block.name
];
removeBlock( clientId, false );
],
false
);
} }
/>
);
Expand All @@ -235,32 +224,9 @@ function BlockHooksControlPure( props ) {
);
}

// We don't want block controls to re-render when typing inside a block. `pure`
// will prevent re-renders unless props change, so only pass the needed props
// and not the whole attributes object.
const BlockHooksControl = pure( BlockHooksControlPure );

export const withBlockHooksControls = createHigherOrderComponent(
( BlockEdit ) => {
return ( props ) => {
return (
<>
<BlockEdit key="edit" { ...props } />
{ props.isSelected && (
<BlockHooksControl
blockName={ props.name }
clientId={ props.clientId }
/>
) }
</>
);
};
export default {
edit: BlockHooksControlPure,
hasSupport() {
return true;
},
'withBlockHooksControls'
);

addFilter(
'editor.BlockEdit',
'core/editor/block-hooks/with-inspector-controls',
withBlockHooksControls
);
};
Loading
Loading