From 19185ff683d7144ca00177c871759304f078bdf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dami=C3=A1n=20Su=C3=A1rez?= Date: Wed, 28 Feb 2024 12:56:02 -0300 Subject: [PATCH] Block Bindings: do not use useSource hook conditionally (#59403) * replace use-binding-attributes with block-binding-support * minor enhancement * minor change * tweak * do not import use-binding-attributes * use isItPossibleToBindBlock() helper * introduce core/entity source handler * rename folder * rename source name * polish post-entity source handler * make core/post-entity more consistent with core-data * make entity source hand;ler more generic * fix entity sour handl;er issues * remove uneeded useValue () hook (crossfingers) * minor jsdoc improvement * clean * rename with updateValue() * remove core/entity binding source handler * move useSource to Connector cmp * move the whole dryining logic to the Connect component * improve jsdoc * rename to blockProps * minor jsdoc improvements * use a single effect to update attr and value * discard useValue. Return value and setValue instead * check wheter updateValue function is defined * check prop value is defined when updating attr * handle `placerholder` * ensure to put attribute in sync when onmount * remove // eslint comment * enable editing for bound with post-meta * move block bindiung processor to hooks/ * ensure update bound attr once when mounting * Update packages/block-editor/src/hooks/block-binding-support/index.js Co-authored-by: Michal * disable editing block attribute * move changes to the use-binding-attributes file * introduce BlockBindingBridge component * update isItPossibleToBindBlock() import path * introduce hasPossibleBlockBinding() helper * use hooks API to extened blocks with bound attts * fix propagating attr value. jsdoc * minor changes * minor code enhancement * not edit bound prop for now * jsdoc * revert using hooks API to extrend block * jsdoc * update internal path * rollback hook utils chnages * tidy * wrap Connector instances with a Fragment * return original Edit instance when no bindings * check whether useSource is defined * Use `useSelect` and move it out of the for loop * check attr value type * iterare when creating BindingConnector instances * rename helper functions * use useSelect to get binding sources * Update packages/block-editor/src/hooks/use-bindings-attributes.js Co-authored-by: Michal * Update packages/block-editor/src/hooks/use-bindings-attributes.js Co-authored-by: Michal * pass prev attr value to compare * improve binding allowed block attributes * sync derevied updates when updating bound attr * improve getting attr source * check properly bindings data * preserve the RichTextData for block attr * comment line just for tesrting purposes * rebasing changes * rollback change foir testing purposes * change cmp prop name. improve jsdoc * simplify checking bindins value * use attr name as key instance * store bound attrs values in a local state * collect and update bound attr in a local state * Refactor block binding functionality from e55f6bc * pick block data from straight props * remove conditional onPropValueChange call * Update e2e tests * Use `useLayoutEffect` instead of `useEffect` --------- Co-authored-by: Michal Co-authored-by: Mario Santos --- .../src/components/rich-text/index.js | 4 +- .../src/hooks/use-bindings-attributes.js | 276 ++++++++++++++---- packages/editor/src/bindings/post-meta.js | 5 +- .../editor/various/block-bindings.spec.js | 6 +- 4 files changed, 223 insertions(+), 68 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index aa870dee073977..3f896702d4d170 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -47,7 +47,7 @@ import { getAllowedFormats } from './utils'; import { Content } from './content'; import { withDeprecations } from './with-deprecations'; import { unlock } from '../../lock-unlock'; -import { BLOCK_BINDINGS_ALLOWED_BLOCKS } from '../../hooks/use-bindings-attributes'; +import { canBindBlock } from '../../hooks/use-bindings-attributes'; export const keyboardShortcutContext = createContext(); export const inputEventContext = createContext(); @@ -161,7 +161,7 @@ export function RichTextWrapper( ( select ) => { // Disable Rich Text editing if block bindings specify that. let _disableBoundBlocks = false; - if ( blockBindings && blockName in BLOCK_BINDINGS_ALLOWED_BLOCKS ) { + if ( blockBindings && canBindBlock( blockName ) ) { const blockTypeAttributes = getBlockType( blockName ).attributes; const { getBlockBindingsSource } = unlock( diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 0e5b6614f07cbf..5cd8cb46b3b7e7 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -4,12 +4,13 @@ import { getBlockType, store as blocksStore } from '@wordpress/blocks'; import { createHigherOrderComponent } from '@wordpress/compose'; import { useSelect } from '@wordpress/data'; +import { useLayoutEffect, useCallback, useState } from '@wordpress/element'; import { addFilter } from '@wordpress/hooks'; +import { RichTextData } from '@wordpress/rich-text'; + /** * Internal dependencies */ -import { store as blockEditorStore } from '../store'; -import { useBlockEditContext } from '../components/block-edit/context'; import { unlock } from '../lock-unlock'; /** @typedef {import('@wordpress/compose').WPHigherOrderComponent} WPHigherOrderComponent */ @@ -22,87 +23,238 @@ import { unlock } from '../lock-unlock'; * @return {WPHigherOrderComponent} Higher-order component. */ -export const BLOCK_BINDINGS_ALLOWED_BLOCKS = { +const BLOCK_BINDINGS_ALLOWED_BLOCKS = { 'core/paragraph': [ 'content' ], 'core/heading': [ 'content' ], 'core/image': [ 'url', 'title', 'alt' ], 'core/button': [ 'url', 'text', 'linkTarget' ], }; -const createEditFunctionWithBindingsAttribute = () => - createHigherOrderComponent( - ( BlockEdit ) => ( props ) => { - const { clientId, name: blockName } = useBlockEditContext(); - const blockBindingsSources = unlock( - useSelect( blocksStore ) - ).getAllBlockBindingsSources(); - const { getBlockAttributes } = useSelect( blockEditorStore ); - - const updatedAttributes = getBlockAttributes( clientId ); - if ( updatedAttributes?.metadata?.bindings ) { - Object.entries( updatedAttributes.metadata.bindings ).forEach( - ( [ attributeName, settings ] ) => { - const source = blockBindingsSources[ settings.source ]; - - if ( source && source.useSource ) { - // Second argument (`updateMetaValue`) will be used to update the value in the future. - const { - placeholder, - useValue: [ metaValue = null ] = [], - } = source.useSource( props, settings.args ); - - if ( placeholder && ! metaValue ) { - // If the attribute is `src` or `href`, a placeholder can't be used because it is not a valid url. - // Adding this workaround until attributes and metadata fields types are improved and include `url`. - const htmlAttribute = - getBlockType( blockName ).attributes[ - attributeName - ].attribute; - if ( - htmlAttribute === 'src' || - htmlAttribute === 'href' - ) { - updatedAttributes[ attributeName ] = null; - } else { - updatedAttributes[ attributeName ] = - placeholder; - } - } - - if ( metaValue ) { - updatedAttributes[ attributeName ] = metaValue; - } - } - } - ); +/** + * Based on the given block name, + * check if it is possible to bind the block. + * + * @param {string} blockName - The block name. + * @return {boolean} Whether it is possible to bind the block to sources. + */ +export function canBindBlock( blockName ) { + return blockName in BLOCK_BINDINGS_ALLOWED_BLOCKS; +} + +/** + * Based on the given block name and attribute name, + * check if it is possible to bind the block attribute. + * + * @param {string} blockName - The block name. + * @param {string} attributeName - The attribute name. + * @return {boolean} Whether it is possible to bind the block attribute. + */ +export function canBindAttribute( blockName, attributeName ) { + return ( + canBindBlock( blockName ) && + BLOCK_BINDINGS_ALLOWED_BLOCKS[ blockName ].includes( attributeName ) + ); +} + +/** + * This component is responsible for detecting and + * propagating data changes from the source to the block. + * + * @param {Object} props - The component props. + * @param {string} props.attrName - The attribute name. + * @param {Object} props.blockProps - The block props with bound attribute. + * @param {Object} props.source - Source handler. + * @param {Object} props.args - The arguments to pass to the source. + * @param {Function} props.onPropValueChange - The function to call when the attribute value changes. + * @return {null} Data-handling component. Render nothing. + */ +const BindingConnector = ( { + args, + attrName, + blockProps, + source, + onPropValueChange, +} ) => { + const { placeholder, value: propValue } = source.useSource( + blockProps, + args + ); + + const { name: blockName } = blockProps; + const attrValue = blockProps.attributes[ attrName ]; + + const updateBoundAttibute = useCallback( + ( newAttrValue, prevAttrValue ) => { + /* + * If the attribute is a RichTextData instance, + * (core/paragraph, core/heading, core/button, etc.) + * compare its HTML representation with the new value. + * + * To do: it looks like a workaround. + * Consider improving the attribute and metadata fields types. + */ + if ( prevAttrValue instanceof RichTextData ) { + // Bail early if the Rich Text value is the same. + if ( prevAttrValue.toHTMLString() === newAttrValue ) { + return; + } + + /* + * To preserve the value type, + * convert the new value to a RichTextData instance. + */ + newAttrValue = RichTextData.fromHTMLString( newAttrValue ); + } + + if ( prevAttrValue === newAttrValue ) { + return; } - return ( + onPropValueChange( { [ attrName ]: newAttrValue } ); + }, + [ attrName, onPropValueChange ] + ); + + useLayoutEffect( () => { + if ( typeof propValue !== 'undefined' ) { + updateBoundAttibute( propValue, attrValue ); + } else if ( placeholder ) { + /* + * Placeholder fallback. + * If the attribute is `src` or `href`, + * a placeholder can't be used because it is not a valid url. + * Adding this workaround until + * attributes and metadata fields types are improved and include `url`. + */ + const htmlAttribute = + getBlockType( blockName ).attributes[ attrName ].attribute; + + if ( htmlAttribute === 'src' || htmlAttribute === 'href' ) { + updateBoundAttibute( null ); + return; + } + + updateBoundAttibute( placeholder ); + } + }, [ + updateBoundAttibute, + propValue, + attrValue, + placeholder, + blockName, + attrName, + ] ); + + return null; +}; + +/** + * BlockBindingBridge acts like a component wrapper + * that connects the bound attributes of a block + * to the source handlers. + * For this, it creates a BindingConnector for each bound attribute. + * + * @param {Object} props - The component props. + * @param {Object} props.blockProps - The BlockEdit props object. + * @param {Object} props.bindings - The block bindings settings. + * @param {Function} props.onPropValueChange - The function to call when the attribute value changes. + * @return {null} Data-handling component. Render nothing. + */ +function BlockBindingBridge( { blockProps, bindings, onPropValueChange } ) { + const blockBindingsSources = unlock( + useSelect( blocksStore ) + ).getAllBlockBindingsSources(); + + return ( + <> + { Object.entries( bindings ).map( + ( [ attrName, boundAttribute ] ) => { + // Bail early if the block doesn't have a valid source handler. + const source = + blockBindingsSources[ boundAttribute.source ]; + if ( ! source?.useSource ) { + return null; + } + + return ( + + ); + } + ) } + + ); +} + +const withBlockBindingSupport = createHigherOrderComponent( + ( BlockEdit ) => ( props ) => { + /* + * Collect and update the bound attributes + * in a separate state. + */ + const [ boundAttributes, setBoundAttributes ] = useState( {} ); + const updateBoundAttributes = useCallback( + ( newAttributes ) => + setBoundAttributes( ( prev ) => ( { + ...prev, + ...newAttributes, + } ) ), + [] + ); + + /* + * Create binding object filtering + * only the attributes that can be bound. + */ + const bindings = Object.fromEntries( + Object.entries( props.attributes.metadata?.bindings || {} ).filter( + ( [ attrName ] ) => canBindAttribute( props.name, attrName ) + ) + ); + + return ( + <> + { Object.keys( bindings ).length > 0 && ( + + ) } + - ); - }, - 'useBoundAttributes' - ); + + ); + }, + 'withBlockBindingSupport' +); /** * Filters a registered block's settings to enhance a block's `edit` component * to upgrade bound attributes. * - * @param {WPBlockSettings} settings Registered block settings. - * + * @param {WPBlockSettings} settings - Registered block settings. + * @param {string} name - Block name. * @return {WPBlockSettings} Filtered block settings. */ -function shimAttributeSource( settings ) { - if ( ! ( settings.name in BLOCK_BINDINGS_ALLOWED_BLOCKS ) ) { +function shimAttributeSource( settings, name ) { + if ( ! canBindBlock( name ) ) { return settings; } - settings.edit = createEditFunctionWithBindingsAttribute()( settings.edit ); - return settings; + return { + ...settings, + edit: withBlockBindingSupport( settings.edit ), + }; } addFilter( diff --git a/packages/editor/src/bindings/post-meta.js b/packages/editor/src/bindings/post-meta.js index a9a00599b68037..0d0c737d0eaf77 100644 --- a/packages/editor/src/bindings/post-meta.js +++ b/packages/editor/src/bindings/post-meta.js @@ -19,6 +19,7 @@ export default { const postType = context.postType ? context.postType : getCurrentPostType(); + const [ meta, setMeta ] = useEntityProp( 'postType', context.postType, @@ -33,9 +34,11 @@ export default { const updateMetaValue = ( newValue ) => { setMeta( { ...meta, [ metaKey ]: newValue } ); }; + return { placeholder: metaKey, - useValue: [ metaValue, updateMetaValue ], + value: metaValue, + updateValue: updateMetaValue, }; }, }; diff --git a/test/e2e/specs/editor/various/block-bindings.spec.js b/test/e2e/specs/editor/various/block-bindings.spec.js index 419a70faeaf9be..ca19c06beff01a 100644 --- a/test/e2e/specs/editor/various/block-bindings.spec.js +++ b/test/e2e/specs/editor/various/block-bindings.spec.js @@ -1245,7 +1245,7 @@ test.describe( 'Block bindings', () => { const postId = await editor.publishPost(); await page.goto( `/?p=${ postId }` ); await expect( page.locator( '#paragraph-binding' ) ).toHaveText( - 'non_existing_custom_field' + 'fallback value' ); } ); @@ -1276,7 +1276,7 @@ test.describe( 'Block bindings', () => { const postId = await editor.publishPost(); await page.goto( `/?p=${ postId }` ); await expect( page.locator( '#paragraph-binding' ) ).toHaveText( - '_protected_field' + 'fallback value' ); } ); @@ -1309,7 +1309,7 @@ test.describe( 'Block bindings', () => { const postId = await editor.publishPost(); await page.goto( `/?p=${ postId }` ); await expect( page.locator( '#paragraph-binding' ) ).toHaveText( - 'show_in_rest_false_field' + 'fallback value' ); } ); } );