From 4e532d0d52276262d2e3939957ddf205e5cabb21 Mon Sep 17 00:00:00 2001 From: Carlos Bravo <37012961+cbravobernal@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:59:53 +0200 Subject: [PATCH] Block bindings: Refactor passing select and dispatch instead of full Registry. (#65710) * Try use select and dispatch instead of registry * Pass `select` to `canUserEditValue` * Pass `select` to `getFieldsList` * Fix `setValues` --------- Co-authored-by: cbravobernal Co-authored-by: SantosGuillamot --- .../src/components/rich-text/index.js | 13 +- .../block-editor/src/hooks/block-bindings.js | 9 +- .../src/hooks/use-bindings-attributes.js | 129 +++++++++--------- packages/block-library/src/button/edit.js | 5 +- packages/block-library/src/image/edit.js | 5 +- packages/block-library/src/image/image.js | 9 +- .../e2e-tests/plugins/block-bindings/index.js | 4 +- .../editor/src/bindings/pattern-overrides.js | 57 ++++---- packages/editor/src/bindings/post-meta.js | 58 ++++---- 9 files changed, 138 insertions(+), 151 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 83cf5e564c68b..26cd2a97b515e 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -188,7 +188,7 @@ export function RichTextWrapper( const _disableBoundBlock = ! blockBindingsSource?.canUserEditValue?.( { - registry, + select, context: blockBindingsContext, args: relatedBinding.args, } ); @@ -206,7 +206,7 @@ export function RichTextWrapper( const { getBlockAttributes } = select( blockEditorStore ); const blockAttributes = getBlockAttributes( clientId ); const fieldsList = blockBindingsSource?.getFieldsList?.( { - registry, + select, context: blockBindingsContext, } ); const bindingKey = @@ -235,14 +235,7 @@ export function RichTextWrapper( bindingsLabel: _bindingsLabel, }; }, - [ - blockBindings, - identifier, - blockName, - blockContext, - registry, - adjustedValue, - ] + [ blockBindings, identifier, blockName, blockContext, adjustedValue ] ); const shouldDisableEditing = readOnly || disableBoundBlock; diff --git a/packages/block-editor/src/hooks/block-bindings.js b/packages/block-editor/src/hooks/block-bindings.js index ea0d4cbb7fb5b..c258927f2fd6d 100644 --- a/packages/block-editor/src/hooks/block-bindings.js +++ b/packages/block-editor/src/hooks/block-bindings.js @@ -12,7 +12,7 @@ import { __experimentalVStack as VStack, privateApis as componentsPrivateApis, } from '@wordpress/components'; -import { useRegistry, useSelect } from '@wordpress/data'; +import { useSelect } from '@wordpress/data'; import { useContext, Fragment } from '@wordpress/element'; import { useViewportMatch } from '@wordpress/compose'; @@ -186,7 +186,6 @@ function EditableBlockBindingsPanelItems( { } export const BlockBindingsPanel = ( { name: blockName, metadata } ) => { - const registry = useRegistry(); const blockContext = useContext( BlockContext ); const { removeAllBlockBindings } = useBlockBindingsUtils(); const bindableAttributes = getBindableAttributes( blockName ); @@ -194,7 +193,7 @@ export const BlockBindingsPanel = ( { name: blockName, metadata } ) => { // `useSelect` is used purposely here to ensure `getFieldsList` // is updated whenever there are updates in block context. - // `source.getFieldsList` may also call a selector via `registry.select`. + // `source.getFieldsList` may also call a selector via `select`. const _fieldsList = {}; const { fieldsList, canUpdateBlockBindings } = useSelect( ( select ) => { @@ -214,7 +213,7 @@ export const BlockBindingsPanel = ( { name: blockName, metadata } ) => { } } const sourceList = getFieldsList( { - registry, + select, context, } ); // Only add source if the list is not empty. @@ -234,7 +233,7 @@ export const BlockBindingsPanel = ( { name: blockName, metadata } ) => { .canUpdateBlockBindings, }; }, - [ blockContext, bindableAttributes, registry ] + [ blockContext, bindableAttributes ] ); // Return early if there are no bindable attributes. if ( ! bindableAttributes || bindableAttributes.length === 0 ) { diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 0fb8c38ce28d3..fdc617fda20c0 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -118,79 +118,81 @@ export const withBlockBindingSupport = createHigherOrderComponent( // there are attribute updates. // `source.getValues` may also call a selector via `registry.select`. const updatedContext = {}; - const boundAttributes = useSelect( () => { - if ( ! blockBindings ) { - return; - } + const boundAttributes = useSelect( + ( select ) => { + if ( ! blockBindings ) { + return; + } - const attributes = {}; + const attributes = {}; - const blockBindingsBySource = new Map(); + const blockBindingsBySource = new Map(); - for ( const [ attributeName, binding ] of Object.entries( - blockBindings - ) ) { - const { source: sourceName, args: sourceArgs } = binding; - const source = sources[ sourceName ]; - if ( ! source || ! canBindAttribute( name, attributeName ) ) { - continue; - } + for ( const [ attributeName, binding ] of Object.entries( + blockBindings + ) ) { + const { source: sourceName, args: sourceArgs } = binding; + const source = sources[ sourceName ]; + if ( + ! source || + ! canBindAttribute( name, attributeName ) + ) { + continue; + } - // Populate context. - for ( const key of source.usesContext || [] ) { - updatedContext[ key ] = blockContext[ key ]; - } + // Populate context. + for ( const key of source.usesContext || [] ) { + updatedContext[ key ] = blockContext[ key ]; + } - blockBindingsBySource.set( source, { - ...blockBindingsBySource.get( source ), - [ attributeName ]: { - args: sourceArgs, - }, - } ); - } + blockBindingsBySource.set( source, { + ...blockBindingsBySource.get( source ), + [ attributeName ]: { + args: sourceArgs, + }, + } ); + } - if ( blockBindingsBySource.size ) { - for ( const [ source, bindings ] of blockBindingsBySource ) { - // Get values in batch if the source supports it. - let values = {}; - if ( ! source.getValues ) { - Object.keys( bindings ).forEach( ( attr ) => { - // Default to the the source label when `getValues` doesn't exist. - values[ attr ] = source.label; - } ); - } else { - values = source.getValues( { - registry, - context: updatedContext, - clientId, - bindings, - } ); - } - for ( const [ attributeName, value ] of Object.entries( - values - ) ) { - if ( - attributeName === 'url' && - ( ! value || ! isURLLike( value ) ) - ) { - // Return null if value is not a valid URL. - attributes[ attributeName ] = null; + if ( blockBindingsBySource.size ) { + for ( const [ + source, + bindings, + ] of blockBindingsBySource ) { + // Get values in batch if the source supports it. + let values = {}; + if ( ! source.getValues ) { + Object.keys( bindings ).forEach( ( attr ) => { + // Default to the the source label when `getValues` doesn't exist. + values[ attr ] = source.label; + } ); } else { - attributes[ attributeName ] = value; + values = source.getValues( { + select, + context: updatedContext, + clientId, + bindings, + } ); + } + for ( const [ attributeName, value ] of Object.entries( + values + ) ) { + if ( + attributeName === 'url' && + ( ! value || ! isURLLike( value ) ) + ) { + // Return null if value is not a valid URL. + attributes[ attributeName ] = null; + } else { + attributes[ attributeName ] = value; + } } } } - } - return attributes; - }, [ - blockBindings, - name, - clientId, - updatedContext, - registry, - sources, - ] ); + return attributes; + }, + [ blockBindings, name, clientId, updatedContext, sources ] + ); const hasParentPattern = !! updatedContext[ 'pattern/overrides' ]; const hasPatternOverridesDefaultBinding = @@ -240,7 +242,8 @@ export const withBlockBindingSupport = createHigherOrderComponent( bindings, ] of blockBindingsBySource ) { source.setValues( { - registry, + select: registry.select, + dispatch: registry.dispatch, context: updatedContext, clientId, bindings, diff --git a/packages/block-library/src/button/edit.js b/packages/block-library/src/button/edit.js index 2749199d1092a..d7b8e6486c3c6 100644 --- a/packages/block-library/src/button/edit.js +++ b/packages/block-library/src/button/edit.js @@ -48,7 +48,7 @@ import { store as blocksStore, } from '@wordpress/blocks'; import { useMergeRefs, useRefEffect } from '@wordpress/compose'; -import { useSelect, useDispatch, useRegistry } from '@wordpress/data'; +import { useSelect, useDispatch } from '@wordpress/data'; const LINK_SETTINGS = [ ...LinkControl.DEFAULT_LINK_SETTINGS, @@ -190,7 +190,6 @@ function ButtonEdit( props ) { const colorProps = useColorProps( attributes ); const spacingProps = useSpacingProps( attributes ); const shadowProps = useShadowProps( attributes ); - const registry = useRegistry(); const ref = useRef(); const richTextRef = useRef(); const blockProps = useBlockProps( { @@ -249,7 +248,7 @@ function ButtonEdit( props ) { lockUrlControls: !! metadata?.bindings?.url && ! blockBindingsSource?.canUserEditValue?.( { - registry, + select, context, args: metadata?.bindings?.url?.args, } ), diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 454b49dfb58b3..d44dc73abfd85 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -9,7 +9,7 @@ import clsx from 'clsx'; import { isBlobURL, createBlobURL } from '@wordpress/blob'; import { store as blocksStore, createBlock } from '@wordpress/blocks'; import { Placeholder } from '@wordpress/components'; -import { useDispatch, useSelect, useRegistry } from '@wordpress/data'; +import { useDispatch, useSelect } from '@wordpress/data'; import { BlockIcon, useBlockProps, @@ -113,7 +113,6 @@ export function ImageEdit( { const [ temporaryURL, setTemporaryURL ] = useState( attributes.blob ); - const registry = useRegistry(); const containerRef = useRef(); // Only observe the max width from the parent container when the parent layout is not flex nor grid. // This won't work for them because the container width changes with the image. @@ -381,7 +380,7 @@ export function ImageEdit( { lockUrlControls: !! metadata?.bindings?.url && ! blockBindingsSource?.canUserEditValue?.( { - registry, + select, context, args: metadata?.bindings?.url?.args, } ), diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index f0d2f00a68d53..1673d36e463d5 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -17,7 +17,7 @@ import { Placeholder, } from '@wordpress/components'; import { useViewportMatch } from '@wordpress/compose'; -import { useSelect, useDispatch, useRegistry } from '@wordpress/data'; +import { useSelect, useDispatch } from '@wordpress/data'; import { BlockControls, InspectorControls, @@ -134,7 +134,6 @@ export default function Image( { const numericWidth = width ? parseInt( width, 10 ) : undefined; const numericHeight = height ? parseInt( height, 10 ) : undefined; - const registry = useRegistry(); const imageRef = useRef(); const { allowResize = true } = context; const { getBlock, getSettings } = useSelect( blockEditorStore ); @@ -497,7 +496,7 @@ export default function Image( { lockUrlControls: !! urlBinding && ! urlBindingSource?.canUserEditValue?.( { - registry, + select, context, args: urlBinding?.args, } ), @@ -512,7 +511,7 @@ export default function Image( { lockAltControls: !! altBinding && ! altBindingSource?.canUserEditValue?.( { - registry, + select, context, args: altBinding?.args, } ), @@ -526,7 +525,7 @@ export default function Image( { lockTitleControls: !! titleBinding && ! titleBindingSource?.canUserEditValue?.( { - registry, + select, context, args: titleBinding?.args, } ), diff --git a/packages/e2e-tests/plugins/block-bindings/index.js b/packages/e2e-tests/plugins/block-bindings/index.js index dfdc706b69c64..0bfc49041647d 100644 --- a/packages/e2e-tests/plugins/block-bindings/index.js +++ b/packages/e2e-tests/plugins/block-bindings/index.js @@ -14,10 +14,10 @@ const getValues = ( { bindings } ) => { } return newValues; }; -const setValues = ( { registry, bindings } ) => { +const setValues = ( { dispatch, bindings } ) => { Object.values( bindings ).forEach( ( { args, newValue } ) => { // Example of what could be done. - registry.dispatch( 'core' ).editEntityRecord( 'postType', 'post', 1, { + dispatch( 'core' ).editEntityRecord( 'postType', 'post', 1, { meta: { [ args?.key ]: newValue }, } ); } ); diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 88c6c73bdc61c..baa1f72f47694 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -7,9 +7,9 @@ const CONTENT = 'content'; export default { name: 'core/pattern-overrides', - getValues( { registry, clientId, context, bindings } ) { + getValues( { select, clientId, context, bindings } ) { const patternOverridesContent = context[ 'pattern/overrides' ]; - const { getBlockAttributes } = registry.select( blockEditorStore ); + const { getBlockAttributes } = select( blockEditorStore ); const currentBlockAttributes = getBlockAttributes( clientId ); const overridesValues = {}; @@ -32,9 +32,9 @@ export default { } return overridesValues; }, - setValues( { registry, clientId, bindings } ) { + setValues( { select, dispatch, clientId, bindings } ) { const { getBlockAttributes, getBlockParentsByBlockName, getBlocks } = - registry.select( blockEditorStore ); + select( blockEditorStore ); const currentBlockAttributes = getBlockAttributes( clientId ); const blockName = currentBlockAttributes?.metadata?.name; if ( ! blockName ) { @@ -61,12 +61,10 @@ export default { const syncBlocksWithSameName = ( blocks ) => { for ( const block of blocks ) { if ( block.attributes?.metadata?.name === blockName ) { - registry - .dispatch( blockEditorStore ) - .updateBlockAttributes( - block.clientId, - attributes - ); + dispatch( blockEditorStore ).updateBlockAttributes( + block.clientId, + attributes + ); } syncBlocksWithSameName( block.innerBlocks ); } @@ -77,27 +75,26 @@ export default { } const currentBindingValue = getBlockAttributes( patternClientId )?.[ CONTENT ]; - registry - .dispatch( blockEditorStore ) - .updateBlockAttributes( patternClientId, { - [ CONTENT ]: { - ...currentBindingValue, - [ blockName ]: { - ...currentBindingValue?.[ blockName ], - ...Object.entries( attributes ).reduce( - ( acc, [ key, value ] ) => { - // TODO: We need a way to represent `undefined` in the serialized overrides. - // Also see: https://github.com/WordPress/gutenberg/pull/57249#discussion_r1452987871 - // We use an empty string to represent undefined for now until - // we support a richer format for overrides and the block bindings API. - acc[ key ] = value === undefined ? '' : value; - return acc; - }, - {} - ), - }, + + dispatch( blockEditorStore ).updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...currentBindingValue, + [ blockName ]: { + ...currentBindingValue?.[ blockName ], + ...Object.entries( attributes ).reduce( + ( acc, [ key, value ] ) => { + // TODO: We need a way to represent `undefined` in the serialized overrides. + // Also see: https://github.com/WordPress/gutenberg/pull/57249#discussion_r1452987871 + // We use an empty string to represent undefined for now until + // we support a richer format for overrides and the block bindings API. + acc[ key ] = value === undefined ? '' : value; + return acc; + }, + {} + ), }, - } ); + }, + } ); }, canUserEditValue: () => true, }; diff --git a/packages/editor/src/bindings/post-meta.js b/packages/editor/src/bindings/post-meta.js index 267d01003b80c..9198ac0fe41e1 100644 --- a/packages/editor/src/bindings/post-meta.js +++ b/packages/editor/src/bindings/post-meta.js @@ -15,8 +15,8 @@ import { unlock } from '../lock-unlock'; * If the value is not available based on context, like in templates, * it falls back to the default value, label, or key. * - * @param {Object} registry The registry context exposed through `useRegistry`. - * @param {Object} context The context provided. + * @param {Object} select The select function from the data store. + * @param {Object} context The context provided. * @return {Object} List of post meta fields with their value and label. * * @example @@ -34,11 +34,9 @@ import { unlock } from '../lock-unlock'; * } * ``` */ -function getPostMetaFields( registry, context ) { - const { getEditedEntityRecord } = registry.select( coreDataStore ); - const { getRegisteredPostMeta } = unlock( - registry.select( coreDataStore ) - ); +function getPostMetaFields( select, context ) { + const { getEditedEntityRecord } = select( coreDataStore ); + const { getRegisteredPostMeta } = unlock( select( coreDataStore ) ); let entityMetaValues; // Try to get the current entity meta values. @@ -75,8 +73,8 @@ function getPostMetaFields( registry, context ) { export default { name: 'core/post-meta', - getValues( { registry, context, bindings } ) { - const metaFields = getPostMetaFields( registry, context ); + getValues( { select, context, bindings } ) { + const metaFields = getPostMetaFields( select, context ); const newValues = {}; for ( const [ attributeName, source ] of Object.entries( bindings ) ) { @@ -88,61 +86,61 @@ export default { } return newValues; }, - setValues( { registry, context, bindings } ) { + setValues( { dispatch, context, bindings } ) { const newMeta = {}; Object.values( bindings ).forEach( ( { args, newValue } ) => { newMeta[ args.key ] = newValue; } ); - registry - .dispatch( coreDataStore ) - .editEntityRecord( 'postType', context?.postType, context?.postId, { + + dispatch( coreDataStore ).editEntityRecord( + 'postType', + context?.postType, + context?.postId, + { meta: newMeta, - } ); + } + ); }, - canUserEditValue( { registry, context, args } ) { + canUserEditValue( { select, context, args } ) { // Lock editing in query loop. if ( context?.query || context?.queryId ) { return false; } const postType = - context?.postType || - registry.select( editorStore ).getCurrentPostType(); + context?.postType || select( editorStore ).getCurrentPostType(); // Check that editing is happening in the post editor and not a template. if ( postType === 'wp_template' ) { return false; } - const fieldValue = getPostMetaFields( registry, context )?.[ args.key ] + const fieldValue = getPostMetaFields( select, context )?.[ args.key ] ?.value; // Empty string or `false` could be a valid value, so we need to check if the field value is undefined. if ( fieldValue === undefined ) { return false; } // Check that custom fields metabox is not enabled. - const areCustomFieldsEnabled = registry - .select( editorStore ) - .getEditorSettings().enableCustomFields; + const areCustomFieldsEnabled = + select( editorStore ).getEditorSettings().enableCustomFields; if ( areCustomFieldsEnabled ) { return false; } // Check that the user has the capability to edit post meta. - const canUserEdit = registry - .select( coreDataStore ) - .canUser( 'update', { - kind: 'postType', - name: context?.postType, - id: context?.postId, - } ); + const canUserEdit = select( coreDataStore ).canUser( 'update', { + kind: 'postType', + name: context?.postType, + id: context?.postId, + } ); if ( ! canUserEdit ) { return false; } return true; }, - getFieldsList( { registry, context } ) { - return getPostMetaFields( registry, context ); + getFieldsList( { select, context } ) { + return getPostMetaFields( select, context ); }, };