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

Try changing setBlockAttributes to update dynamicContent #54233

Closed
wants to merge 1 commit into from
Closed

Try changing setBlockAttributes to update dynamicContent #54233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion docs/reference-guides/core-blocks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Create and save content to reuse across your site. Update the pattern, and the c
- **Name:** core/block
- **Category:** reusable
- **Supports:** ~~customClassName~~, ~~html~~, ~~inserter~~
- **Attributes:** dynamicContent, patternId, ref
- **Attributes:** dynamicContent, ref

## Button

Expand Down
6 changes: 1 addition & 5 deletions lib/block-supports/pattern.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
$gutenberg_experiments = get_option( 'gutenberg-experiments' );
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-patterns', $gutenberg_experiments ) ) {
/**
* Adds `patternId` and `dynamicContent` items to the block's `usesContext`
* Adds `dynamicContent` items to the block's `usesContext`
* configuration.
*
* @param WP_Block_Type $block_type Block type.
Expand All @@ -21,10 +21,6 @@ function gutenberg_register_pattern_support( $block_type ) {
$block_type->uses_context = array();
}

if ( ! in_array( 'patternId', $block_type->uses_context, true ) ) {
$block_type->uses_context[] = 'patternId';
}

if ( ! in_array( 'dynamicContent', $block_type->uses_context, true ) ) {
$block_type->uses_context[] = 'dynamicContent';
}
Expand Down
200 changes: 30 additions & 170 deletions packages/block-editor/src/hooks/pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,163 +1,55 @@
/**
* External dependencies
*/
// eslint-disable-next-line import/no-extraneous-dependencies
import { v4 as uuid } from 'uuid';
// TODO: Fix the unique ID generation to avoid adding another dependency just for this.

/**
* WordPress dependencies
*/
import { getBlockSupport, getBlockType } from '@wordpress/blocks';
import { store as blocksStore } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';
import { useDispatch, useRegistry } from '@wordpress/data';
import { useCallback, useMemo } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';
import { addFilter } from '@wordpress/hooks';

/**
* Internal dependencies
*/
import { cleanEmptyObject } from './utils';
import { useBlockEditContext } from '../components/block-edit';
import { store as blockEditorStore } from '../store';

export const PATTERN_SUPPORT_KEY = '__experimentalPattern';

function hasPatternSupport( blockType ) {
return !! getBlockSupport( blockType, PATTERN_SUPPORT_KEY );
}

// TODO: Extract this to a custom source file?
function useSource( {
name,
context: { dynamicContent, patternId },
attributes,
setAttributes,
} ) {
const { updateBlockAttributes } = useDispatch( blockEditorStore );
const blockType = getBlockType( name );
const patternSupport = blockType.supports?.[ PATTERN_SUPPORT_KEY ];
// Generate unique id to link the block instance with the data in the
// pattern block's dynamic content.
function useSourceAttributes( attributes ) {
const { name, clientId } = useBlockEditContext();
const dynamicContent = useSelect(
( select ) => {
const hasPatternSupport = select( blocksStore ).hasBlockSupport(
name,
PATTERN_SUPPORT_KEY
);
if ( ! hasPatternSupport ) return undefined;
const parentPatternClientId = select(
blockEditorStore
).getBlockParentsByBlockName( clientId, 'core/block', true )[ 0 ];
if ( ! parentPatternClientId ) return undefined;
return select( blockEditorStore ).getBlockAttributes(
parentPatternClientId
).dynamicContent;
},
[ name, clientId ]
);

const attributesWithSourcedAttributes = useMemo( () => {
const id = attributes.metadata?.id;

if ( ! patternSupport || ! id || ! dynamicContent ) {
if ( ! id || ! dynamicContent ) {
return attributes;
}

return {
...attributes,
...Object.fromEntries(
Object.keys( patternSupport ).map( ( attributeName ) => {
return [
attributeName,
dynamicContent[ id ]?.[ attributeName ] ||
attributes[ attributeName ],
];
} )
),
...dynamicContent[ id ],
};
}, [ attributes, dynamicContent, patternSupport ] );

const updatedSetAttributes = useCallback(
( nextAttributes ) => {
const id = attributes.metadata?.id ?? uuid();

// Collect the updated dynamic content for the current block.
const updatedDynamicContent = Object.entries( nextAttributes ?? {} )
.filter(
( [ key ] ) => patternSupport && key in patternSupport
)
.map( ( [ key, value ] ) => {
if ( value === '' ) {
return [ key, undefined ];
}

return [ key, value ];
} );

// Collect the updated dynamic pattern content.
const nextDynamicContent = cleanEmptyObject( {
...dynamicContent,
[ id ]: {
...dynamicContent?.[ id ],
...Object.fromEntries( updatedDynamicContent ),
},
} );

// Filter out pattern stored attributes so they don't override the
// original attributes that act as a default or fallback.
const updatedAttributes = updatedDynamicContent.length
? Object.fromEntries(
Object.entries( nextAttributes ?? {} ).filter(
( [ key ] ) =>
! ( patternSupport && key in patternSupport )
)
)
: nextAttributes;

// Update the parent pattern instance's dynamic content attribute.
if ( updatedDynamicContent.length ) {
// If we are setting dynamic content on the parent pattern,
// ensure the block's id is saved in its metadata.
if ( ! attributes.metadata?.id ) {
updatedAttributes.metadata = {
...updatedAttributes.metadata,
id,
};
}

updateBlockAttributes( patternId, {
dynamicContent: nextDynamicContent,
} );
}

setAttributes( updatedAttributes );
},
[
attributes.metadata?.id,
dynamicContent,
patternId,
patternSupport,
setAttributes,
updateBlockAttributes,
]
);
}, [ attributes, dynamicContent ] );

return {
attributes: attributesWithSourcedAttributes,
setAttributes: updatedSetAttributes,
};
}

/**
* Filters registered block settings, extending usesContext to include the
* dynamic content and setter provided by a pattern block.
*
* @param {Object} settings Original block settings.
*
* @return {Object} Filtered block settings.
*/
function extendUsesContext( settings ) {
if ( ! hasPatternSupport( settings ) ) {
return settings;
}

if ( ! Array.isArray( settings.usesContext ) ) {
settings.usesContext = [ 'dynamicContent', 'patternId' ];
return settings;
}

if ( ! settings.usesContext.includes( 'dynamicContent' ) ) {
settings.usesContext.push( 'dynamicContent' );
}

if ( ! settings.usesContext.includes( 'patternId' ) ) {
settings.usesContext.push( 'patternId' );
}

return settings;
return attributesWithSourcedAttributes;
}

/**
Expand All @@ -172,37 +64,11 @@ function extendUsesContext( settings ) {
const createEditFunctionWithPatternSource = () =>
createHigherOrderComponent(
( BlockEdit ) =>
( { name, attributes, setAttributes, context, ...props } ) => {
if ( ! context.patternId ) {
return (
<BlockEdit
name={ name }
context={ context }
attributes={ attributes }
setAttributes={ setAttributes }
{ ...props }
/>
);
}

const registry = useRegistry();
const {
attributes: updatedAttributes,
setAttributes: updatedSetAttributes,
} = useSource( { name, attributes, setAttributes, context } );
( { attributes, ...props } ) => {
const sourceAttributes = useSourceAttributes( attributes );

return (
<BlockEdit
name={ name }
context={ context }
attributes={ updatedAttributes }
setAttributes={ ( newAttributes ) =>
registry.batch( () =>
updatedSetAttributes( newAttributes )
)
}
{ ...props }
/>
<BlockEdit { ...props } attributes={ sourceAttributes } />
Copy link
Member

Choose a reason for hiding this comment

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

This is interesting. I wonder if the override for attributes should also happen on the store level, similar to how modifications get applied to updateBlockAttributes. Otherwise, we risk that folks could omit using attributes passed to Edit and get local values for dynamic content when accessing the store directly similar to what would happen when they use updateBlockAttributes.

);
}
);
Expand All @@ -219,10 +85,4 @@ if ( window.__experimentalPatterns ) {
'core/pattern/shimAttributeSource',
shimAttributeSource
);

addFilter(
'blocks.registerBlockType',
'core/pattern/extendUsesContext',
extendUsesContext
);
}
101 changes: 90 additions & 11 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 @@ -13,6 +13,7 @@ import {
switchToBlockType,
synchronizeBlocksWithTemplate,
getBlockSupport,
store as blocksStore,
} from '@wordpress/blocks';
import { speak } from '@wordpress/a11y';
import { __, _n, sprintf } from '@wordpress/i18n';
Expand Down Expand Up @@ -157,18 +158,96 @@ export function receiveBlocks( blocks ) {
* @param {boolean} uniqueByBlock true if each block in clientIds array has a unique set of attributes
* @return {Object} Action object.
*/
export function updateBlockAttributes(
clientIds,
attributes,
uniqueByBlock = false
) {
return {
type: 'UPDATE_BLOCK_ATTRIBUTES',
clientIds: castArray( clientIds ),
attributes,
uniqueByBlock,
export const updateBlockAttributes =
( clientIds, attributes, uniqueByBlock = false ) =>
( { select, dispatch, registry } ) => {
if ( ! window.__experimentalPatterns ) {
dispatch( {
type: 'UPDATE_BLOCK_ATTRIBUTES',
clientIds: castArray( clientIds ),
attributes,
uniqueByBlock,
} );
return;
}

const updates = {};
for ( const clientId of castArray( clientIds ) ) {
const attrs = uniqueByBlock ? attributes[ clientId ] : attributes;
const parentBlocks = select.getBlocksByClientId(
select.getBlockParents( clientId )
);
const parentPattern = parentBlocks.findLast(
( parentBlock ) => parentBlock.name === 'core/block'
);
const block = select.getBlock( clientId );
if (
! parentPattern ||
! registry
.select( blocksStore )
.hasBlockSupport( block.name, '__experimentalPattern' )
) {
updates[ clientId ] = attrs;
continue;
}
Comment on lines +177 to +192
Copy link
Contributor

@talldan talldan Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I like the explorations you and @aaronrobertshaw have worked on!

Given the block bindings API is going to support a bunch of different data sources, I wonder how this can be made generic/pluggable, and done in a way where the core/block isn't hard-coded in.

Having read through the block bindings issue, it seems like the blocks inside the pattern will have some config denoting their connections (which I guess the pattern creator will define), which could be read here. Then there's the need to discover the source of the data for those connections, which is what happens here but somewhat hard-coded.

I think maybe we're close to the point where we can try modelling those connections and abstracting the discovery of the data source that happens here. It could be that the connection 'type' determines the strategy used to find the data source (the pattern block). Or alternatively the pattern block is somehow defined as source.

But we should at the same time connect back with the other data bindings work around custom fields to make sure the approach taken works in both places. Looking at the POCs so far for custom fields (#53300), the challenge is that they focus more on the PHP part as it stands.

Copy link
Member Author

@kevin940726 kevin940726 Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, exactly which blocks and their attributes should be connected was still undecided. This part was just following the existing exploration in the base branch.

The main goal for this PR is to find a performant and extendible way to allow override of setAttributes. Writing condition paths directly in updateBlockAttributes works for prototyping, but I'm currently trying to find a better approach so that it feels less like "duct-taping". I've tried maybe 6 different approaches now but each has its trade-offs and challenges. Once we solve that, and we have the connection "contract" defined, then I think we'll be in a good shape for implementing them.


const contentAttributes = registry
.select( blocksStore )
.getBlockSupport( block.name, '__experimentalPattern' );
const dynamicContent = {};
const updatedAttributes = {};
for ( const attributeKey of Object.keys( attrs ) ) {
if ( Object.hasOwn( contentAttributes, attributeKey ) ) {
dynamicContent[ attributeKey ] = attrs[ attributeKey ];
} else {
updatedAttributes[ attributeKey ] = attrs[ attributeKey ];
}
}
if ( Object.keys( dynamicContent ).length > 0 ) {
let id = block.attributes.metadata?.id;
if ( ! id ) {
// The id just has to be unique within the pattern context, so we
// use the block's clientId as a convenient unique identifier.
id = block.clientId;
updatedAttributes.metadata = {
...block.attributes.metadata,
id,
};
}

updates[ parentPattern.clientId ] = {
dynamicContent: {
...parentPattern.attributes.dynamicContent,
[ id ]: dynamicContent,
},
};
}
if ( Object.keys( updatedAttributes ).length > 0 ) {
updates[ clientId ] = updatedAttributes;
}
}

if (
Object.values( updates ).every(
( updatedAttributes, _index, arr ) =>
updatedAttributes === arr[ 0 ]
)
) {
dispatch( {
type: 'UPDATE_BLOCK_ATTRIBUTES',
clientIds: Object.keys( updates ),
attributes: Object.values( updates )[ 0 ],
uniqueByBlock: false,
} );
} else {
dispatch( {
type: 'UPDATE_BLOCK_ATTRIBUTES',
clientIds: Object.keys( updates ),
attributes: updates,
uniqueByBlock: true,
} );
}
};
}

/**
* Action that updates the block with the specified client ID.
Expand Down
Loading