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.

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

Block Bindings: do not use useSource hook conditionally #59403

Merged
merged 81 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
81 commits
Select commit Hold shift + click to select a range
37a7f5a
replace use-binding-attributes with block-binding-support
retrofox Feb 12, 2024
09f8565
minor enhancement
retrofox Feb 10, 2024
45742ca
minor change
retrofox Feb 10, 2024
6af2517
tweak
retrofox Feb 10, 2024
f02c4ef
do not import use-binding-attributes
retrofox Feb 10, 2024
6412b7d
use isItPossibleToBindBlock() helper
retrofox Feb 10, 2024
5749a34
introduce core/entity source handler
retrofox Feb 10, 2024
25b0bb8
rename folder
retrofox Feb 11, 2024
76e0cb5
rename source name
retrofox Feb 12, 2024
dc48e09
polish post-entity source handler
retrofox Feb 12, 2024
44421f1
make core/post-entity more consistent with core-data
retrofox Feb 12, 2024
8a11812
make entity source hand;ler more generic
retrofox Feb 12, 2024
1b40bef
fix entity sour handl;er issues
retrofox Feb 12, 2024
59b26ba
remove uneeded useValue () hook (crossfingers)
retrofox Feb 12, 2024
72338e3
minor jsdoc improvement
retrofox Feb 12, 2024
f7d87a8
clean
retrofox Feb 12, 2024
b60c7ea
rename with updateValue()
retrofox Feb 12, 2024
adfd711
remove core/entity binding source handler
retrofox Feb 12, 2024
832f170
move useSource to Connector cmp
retrofox Feb 12, 2024
42fc47c
move the whole dryining logic to the Connect component
retrofox Feb 12, 2024
09c8cd3
improve jsdoc
retrofox Feb 12, 2024
b0a0310
rename to blockProps
retrofox Feb 12, 2024
c46c140
minor jsdoc improvements
retrofox Feb 13, 2024
4f4e3b5
use a single effect to update attr and value
retrofox Feb 13, 2024
5ae157e
discard useValue. Return value and setValue instead
retrofox Feb 13, 2024
6ebb240
check wheter updateValue function is defined
retrofox Feb 13, 2024
8c8c90c
check prop value is defined when updating attr
retrofox Feb 13, 2024
536bc90
handle `placerholder`
retrofox Feb 14, 2024
4063d77
ensure to put attribute in sync when onmount
retrofox Feb 14, 2024
6a28f73
remove // eslint comment
retrofox Feb 14, 2024
55c8cda
enable editing for bound with post-meta
retrofox Feb 14, 2024
a255fd5
move block bindiung processor to hooks/
retrofox Feb 14, 2024
a2b5326
ensure update bound attr once when mounting
retrofox Feb 15, 2024
ad5b1e1
Update packages/block-editor/src/hooks/block-binding-support/index.js
retrofox Feb 15, 2024
4e8f2c8
disable editing block attribute
retrofox Feb 15, 2024
f06ce6d
move changes to the use-binding-attributes file
retrofox Feb 15, 2024
de41aa0
introduce BlockBindingBridge component
retrofox Feb 15, 2024
c1119b5
update isItPossibleToBindBlock() import path
retrofox Feb 15, 2024
52c45e1
introduce hasPossibleBlockBinding() helper
retrofox Feb 15, 2024
bdc0b82
use hooks API to extened blocks with bound attts
retrofox Feb 15, 2024
f1e941a
fix propagating attr value. jsdoc
retrofox Feb 15, 2024
b944f21
minor changes
retrofox Feb 15, 2024
b9f8aef
minor code enhancement
retrofox Feb 19, 2024
e6cf0e3
not edit bound prop for now
retrofox Feb 19, 2024
8f69062
jsdoc
retrofox Feb 19, 2024
ff76966
revert using hooks API to extrend block
retrofox Feb 19, 2024
8b135ba
jsdoc
retrofox Feb 19, 2024
df35039
update internal path
retrofox Feb 19, 2024
d84bc0a
rollback hook utils chnages
retrofox Feb 19, 2024
75284b4
tidy
retrofox Feb 20, 2024
2c8edfb
wrap Connector instances with a Fragment
retrofox Feb 20, 2024
5b39024
return original Edit instance when no bindings
retrofox Feb 20, 2024
c86f3dc
check whether useSource is defined
retrofox Feb 20, 2024
967efaa
Use `useSelect` and move it out of the for loop
michalczaplinski Feb 20, 2024
3143392
check attr value type
retrofox Feb 20, 2024
f0af985
iterare when creating BindingConnector instances
retrofox Feb 21, 2024
67dd86a
rename helper functions
retrofox Feb 21, 2024
fed0e87
use useSelect to get binding sources
retrofox Feb 21, 2024
9126558
Update packages/block-editor/src/hooks/use-bindings-attributes.js
retrofox Feb 21, 2024
5179ad0
Update packages/block-editor/src/hooks/use-bindings-attributes.js
retrofox Feb 21, 2024
9837379
pass prev attr value to compare
retrofox Feb 22, 2024
cdbbda7
improve binding allowed block attributes
retrofox Feb 22, 2024
f4906b6
sync derevied updates when updating bound attr
retrofox Feb 22, 2024
c554dc5
improve getting attr source
retrofox Feb 22, 2024
9621b02
check properly bindings data
retrofox Feb 22, 2024
eb27c87
preserve the RichTextData for block attr
retrofox Feb 23, 2024
fbb24f2
comment line just for tesrting purposes
retrofox Feb 23, 2024
90c828a
rebasing changes
retrofox Feb 23, 2024
01ce180
rollback change foir testing purposes
retrofox Feb 23, 2024
f790ca7
change cmp prop name. improve jsdoc
retrofox Feb 27, 2024
46da935
simplify checking bindins value
retrofox Feb 27, 2024
1762e9c
use attr name as key instance
retrofox Feb 27, 2024
e03d861
store bound attrs values in a local state
retrofox Feb 27, 2024
8e2eeaa
collect and update bound attr in a local state
retrofox Feb 27, 2024
56a782b
Refactor block binding functionality from e55f6bc
michalczaplinski Feb 27, 2024
80e8102
pick block data from straight props
retrofox Feb 28, 2024
eb0f79a
Merge branch 'update/do-not-conditional-hook-in-binding' of github.co…
retrofox Feb 28, 2024
fa9ed05
remove conditional onPropValueChange call
retrofox Feb 28, 2024
021f87a
Merge branch 'trunk' into update/do-not-conditional-hook-in-binding
retrofox Feb 28, 2024
b6d14b5
Update e2e tests
SantosGuillamot Feb 28, 2024
879129c
Use `useLayoutEffect` instead of `useEffect`
SantosGuillamot Feb 28, 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 packages/block-editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand Down
276 changes: 214 additions & 62 deletions packages/block-editor/src/hooks/use-bindings-attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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(
Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to address the pattern override use case and not push ourselves into a corder, we should also make use of the updateValue callback.

Copy link
Contributor Author

@retrofox retrofox Feb 28, 2024

Choose a reason for hiding this comment

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

Initially, the PR handled the updateValue callback but then decided to address it in a follow-up. Do you think we should proceed to do it in the same PR? cc @SantosGuillamot

Copy link
Contributor

Choose a reason for hiding this comment

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

As this is not going to be a public API and updateValue is not going to be used yet, I would include it once we work on the pull request to use this hook for pattern overrides.

Copy link
Member

Choose a reason for hiding this comment

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

Exactly, updateValue is part of the API in the original PR #58085 and all prototypes. Post Meta source is forced to be in readonly mode for the initial rollout in WP 6.5.

Copy link
Member

Choose a reason for hiding this comment

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

I see that updateValue is even implemented for the Post Meta source. It isn't wired here as intended for now.

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 (
<BindingConnector
key={ attrName }
attrName={ attrName }
source={ source }
blockProps={ blockProps }
args={ boundAttribute.args }
onPropValueChange={ onPropValueChange }
/>
);
}
) }
</>
);
}

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 && (
<BlockBindingBridge
blockProps={ props }
bindings={ bindings }
onPropValueChange={ updateBoundAttributes }
/>
) }

<BlockEdit
key="edit"
{ ...props }
attributes={ updatedAttributes }
attributes={ { ...props.attributes, ...boundAttributes } }
/>
);
},
'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(
Expand Down
5 changes: 4 additions & 1 deletion packages/editor/src/bindings/post-meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default {
const postType = context.postType
? context.postType
: getCurrentPostType();

const [ meta, setMeta ] = useEntityProp(
'postType',
context.postType,
Expand All @@ -33,9 +34,11 @@ export default {
const updateMetaValue = ( newValue ) => {
setMeta( { ...meta, [ metaKey ]: newValue } );
};

return {
placeholder: metaKey,
useValue: [ metaValue, updateMetaValue ],
value: metaValue,
updateValue: updateMetaValue,
};
},
};
6 changes: 3 additions & 3 deletions test/e2e/specs/editor/various/block-bindings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
} );

Expand Down Expand Up @@ -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'
);
} );

Expand Down Expand Up @@ -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'
);
} );
} );
Expand Down
Loading