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

Add __default binding for pattern overrides #60694

Merged
merged 18 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions backport-changelog/6.6/6694.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
https://github.com/WordPress/wordpress-develop/pull/6694

* https://github.com/WordPress/gutenberg/pull/60694
46 changes: 46 additions & 0 deletions lib/compat/wordpress-6.6/blocks.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php
/**
* Temporary compatibility shims for block APIs present in Gutenberg.
*
* @package gutenberg
*/

/**
* Replace the `__default` block bindings attribute with the full list of supported
* attribute names for pattern overrides.
*
* @param array $parsed_block The full block, including name and attributes.
*
* @return string The parsed block with default binding replace.
*/
function gutenberg_replace_pattern_override_default_binding( $parsed_block ) {
$supported_block_attrs = array(
'core/paragraph' => array( 'content' ),
'core/heading' => array( 'content' ),
'core/image' => array( 'id', 'url', 'title', 'alt' ),
'core/button' => array( 'url', 'text', 'linkTarget', 'rel' ),
);

$bindings = $parsed_block['attrs']['metadata']['bindings'] ?? array();
if (
isset( $bindings['__default']['source'] ) &&
'core/pattern-overrides' === $bindings['__default']['source']
) {
$updated_bindings = array();

// Build an binding array of all supported attributes.
// Note that this also omits the `__default` attribute from the
// resulting array.
foreach ( $supported_block_attrs[ $parsed_block['blockName'] ] as $attribute_name ) {
// Retain any non-pattern override bindings that might be present.
$updated_bindings[ $attribute_name ] = isset( $bindings[ $attribute_name ] )
? $bindings[ $attribute_name ]
: array( 'source' => 'core/pattern-overrides' );
}
$parsed_block['attrs']['metadata']['bindings'] = $updated_bindings;
}

return $parsed_block;
}

add_filter( 'render_block_data', 'gutenberg_replace_pattern_override_default_binding', 10, 1 );
1 change: 1 addition & 0 deletions lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ function gutenberg_is_experiment_enabled( $name ) {

// WordPress 6.6 compat.
require __DIR__ . '/compat/wordpress-6.6/admin-bar.php';
require __DIR__ . '/compat/wordpress-6.6/blocks.php';
require __DIR__ . '/compat/wordpress-6.6/compat.php';
require __DIR__ . '/compat/wordpress-6.6/resolve-patterns.php';
require __DIR__ . '/compat/wordpress-6.6/block-bindings/pattern-overrides.php';
Expand Down
53 changes: 48 additions & 5 deletions packages/block-editor/src/hooks/use-bindings-attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { store as blocksStore } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';
import { useRegistry, useSelect } from '@wordpress/data';
import { useCallback } from '@wordpress/element';
import { useCallback, useMemo } from '@wordpress/element';
import { addFilter } from '@wordpress/hooks';

/**
Expand All @@ -29,6 +29,41 @@ const BLOCK_BINDINGS_ALLOWED_BLOCKS = {
'core/button': [ 'url', 'text', 'linkTarget', 'rel' ],
};

const DEFAULT_ATTRIBUTE = '__default';

/**
* Returns the bindings with the `__default` binding for pattern overrides
* replaced with the full-set of supported attributes. e.g.:
*
* bindings passed in: `{ __default: { source: 'core/pattern-overrides' } }`
* bindings returned: `{ content: { source: 'core/pattern-overrides' } }`
*
* @param {string} blockName The block name (e.g. 'core/paragraph').
* @param {Object} bindings A block's bindings from the metadata attribute.
*
* @return {Object} The bindings with default replaced for pattern overrides.
*/
function replacePatternOverrideDefaultBindings( blockName, bindings ) {
SantosGuillamot marked this conversation as resolved.
Show resolved Hide resolved
// The `__default` binding currently only works for pattern overrides.
if (
bindings?.[ DEFAULT_ATTRIBUTE ]?.source === 'core/pattern-overrides'
) {
const supportedAttributes = BLOCK_BINDINGS_ALLOWED_BLOCKS[ blockName ];
const bindingsWithDefaults = {};
for ( const attributeName of supportedAttributes ) {
// If the block has mixed binding sources, retain any non pattern override bindings.
const bindingSource = bindings[ attributeName ]
? bindings[ attributeName ]
: { source: 'core/pattern-overrides' };
bindingsWithDefaults[ attributeName ] = bindingSource;
}

return bindingsWithDefaults;
}

return bindings;
}

/**
* Based on the given block name,
* check if it is possible to bind the block.
Expand Down Expand Up @@ -61,8 +96,15 @@ export const withBlockBindingSupport = createHigherOrderComponent(
const sources = useSelect( ( select ) =>
unlock( select( blocksStore ) ).getAllBlockBindingsSources()
);
const bindings = props.attributes.metadata?.bindings;
const { name, clientId, context } = props;
const bindings = useMemo(
() =>
replacePatternOverrideDefaultBindings(
name,
props.attributes.metadata?.bindings
),
[ props.attributes.metadata?.bindings, name ]
);
const boundAttributes = useSelect( () => {
if ( ! bindings ) {
return;
Expand Down Expand Up @@ -128,8 +170,8 @@ export const withBlockBindingSupport = createHigherOrderComponent(
continue;
}

const source =
sources[ bindings[ attributeName ].source ];
const binding = bindings[ attributeName ];
const source = sources[ binding?.source ];
if ( ! source?.setValue && ! source?.setValues ) {
continue;
}
Expand Down Expand Up @@ -157,12 +199,13 @@ export const withBlockBindingSupport = createHigherOrderComponent(
attributeName,
value,
] of Object.entries( attributes ) ) {
const binding = bindings[ attributeName ];
source.setValue( {
registry,
context,
clientId,
attributeName,
args: bindings[ attributeName ].args,
args: binding.args,
value,
} );
}
Expand Down
12 changes: 2 additions & 10 deletions packages/block-library/src/block/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ import { name as patternBlockName } from './index';
import { unlock } from '../lock-unlock';

const { useLayoutClasses } = unlock( blockEditorPrivateApis );
const { isOverridableBlock } = unlock( patternsPrivateApis );
const { isOverridableBlock, hasOverridableBlocks } =
unlock( patternsPrivateApis );

const fullAlignments = [ 'full', 'wide', 'left', 'right' ];

Expand Down Expand Up @@ -73,15 +74,6 @@ const useInferredLayout = ( blocks, parentLayout ) => {
}, [ blocks, parentLayout ] );
};

function hasOverridableBlocks( blocks ) {
return blocks.some( ( block ) => {
if ( isOverridableBlock( block ) ) {
return true;
}
return hasOverridableBlocks( block.innerBlocks );
} );
}

function setBlockEditMode( setEditMode, blocks, mode ) {
blocks.forEach( ( block ) => {
const editMode =
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/block/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function render_block_core_block( $attributes ) {
* filter so that it is available when a pattern's inner blocks are
* rendering via do_blocks given it only receives the inner content.
*/
$has_pattern_overrides = isset( $attributes['content'] );
$has_pattern_overrides = isset( $attributes['content'] ) && null !== get_block_bindings_source( 'core/pattern-overrides' );
if ( $has_pattern_overrides ) {
$filter_block_context = static function ( $context ) use ( $attributes ) {
$context['pattern/overrides'] = $attributes['content'];
Expand Down
26 changes: 21 additions & 5 deletions packages/blocks/src/api/parser/convert-legacy-block.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,25 +88,41 @@ export function convertLegacyBlockNameAndAttributes( name, attributes ) {
( name === 'core/paragraph' ||
name === 'core/heading' ||
name === 'core/image' ||
name === 'core/button' )
name === 'core/button' ) &&
newAttributes.metadata.bindings.__default?.source !==
'core/pattern-overrides'
) {
const bindings = [
'content',
'url',
'title',
'id',
Copy link
Contributor

Choose a reason for hiding this comment

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

To do this strictly correctly, we should use PARTIAL_SYNCING_SUPPORTED_BLOCKS to match attributes to specific block types. Right now, this function is duplicating what is already some temporary hardcoded information, which just seems a bit too error-prone.

That said, IMO the strictly correct way seems overkill for the purpose of this branch. We could just loop over all keys of .metadata.bindings to replace .source.name and call it a day.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree we should probably just iterate through the bindings and remove the ones with source core/pattern-overrides, and avoid having this hardcoded list.

'alt',
'text',
'linkTarget',
];
// Delete any existing individual bindings and add a default binding.
// It was only possible to add all the default attributes through the UI,
// So as soon as we find an attribute, we can assume all default attributes are overridable.
let hasPatternOverrides = false;
bindings.forEach( ( binding ) => {
if (
newAttributes.metadata.bindings[ binding ]?.source?.name ===
'pattern_attributes'
newAttributes.metadata.bindings[ binding ]?.source ===
'core/pattern-overrides'
) {
newAttributes.metadata.bindings[ binding ].source =
'core/pattern-overrides';
hasPatternOverrides = true;
newAttributes.metadata = {
...newAttributes.metadata,
bindings: { ...newAttributes.metadata.bindings },
};
delete newAttributes.metadata.bindings[ binding ];
}
} );
if ( hasPatternOverrides ) {
newAttributes.metadata.bindings.__default = {
source: 'core/pattern-overrides',
};
}
Comment on lines +121 to +125
Copy link
Contributor

@talldan talldan Apr 22, 2024

Choose a reason for hiding this comment

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

This works ok in the site editor, but for some reason isn't working in the post editor when navigating to the original pattern. It must be that convertLegacyBlockNameAndAttributes is never called when switching to edit a different entity.

Copy link
Member Author

Choose a reason for hiding this comment

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

This works ok in the site editor, but for some reason isn't working in the post editor when navigating to the original pattern.

Could you clarify the testing instructions for this? I'm seeing a consistent result on my end.

Kapture.2024-04-23.at.14.59.09.mp4

It won't automatically change from content to __default as shown in the recording, but I think it's expected as the function only runs on change. It's also backward compatible this way, I believe. Maybe there's something I'm missing though 🤔.

}
}
return [ name, newAttributes ];
Expand Down
16 changes: 16 additions & 0 deletions packages/patterns/src/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,19 @@ export function isOverridableBlock( block ) {
)
);
}

/**
* Determines whether the blocks list has overridable blocks.
*
* @param {WPBlock[]} blocks The blocks list.
*
* @return {boolean} `true` if the list has overridable blocks, `false` otherwise.
*/
export function hasOverridableBlocks( blocks ) {
return blocks.some( ( block ) => {
if ( isOverridableBlock( block ) ) {
return true;
}
return hasOverridableBlocks( block.innerBlocks );
} );
}
63 changes: 19 additions & 44 deletions packages/patterns/src/components/pattern-overrides-controls.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,66 +9,48 @@ import { __ } from '@wordpress/i18n';
/**
* Internal dependencies
*/
import {
PARTIAL_SYNCING_SUPPORTED_BLOCKS,
PATTERN_OVERRIDES_BINDING_SOURCE,
} from '../constants';
import { PATTERN_OVERRIDES_BINDING_SOURCE } from '../constants';
import {
AllowOverridesModal,
DisallowOverridesModal,
} from './allow-overrides-modal';

function removeBindings( bindings, syncedAttributes ) {
let updatedBindings = {};
for ( const attributeName of syncedAttributes ) {
// Omit any bindings that's not the same source from the `updatedBindings` object.
if (
bindings?.[ attributeName ]?.source !==
PATTERN_OVERRIDES_BINDING_SOURCE &&
bindings?.[ attributeName ]?.source !== undefined
) {
updatedBindings[ attributeName ] = bindings[ attributeName ];
}
}
function removeBindings( bindings ) {
let updatedBindings = { ...bindings };
delete updatedBindings.__default;
if ( ! Object.keys( updatedBindings ).length ) {
updatedBindings = undefined;
}
return updatedBindings;
}

function addBindings( bindings, syncedAttributes ) {
const updatedBindings = { ...bindings };
for ( const attributeName of syncedAttributes ) {
if ( ! bindings?.[ attributeName ] ) {
updatedBindings[ attributeName ] = {
source: PATTERN_OVERRIDES_BINDING_SOURCE,
};
}
}
return updatedBindings;
function addBindings( bindings ) {
return {
...bindings,
__default: { source: PATTERN_OVERRIDES_BINDING_SOURCE },
};
}

function PatternOverridesControls( { attributes, name, setAttributes } ) {
function PatternOverridesControls( { attributes, setAttributes } ) {
const controlId = useId();
const [ showAllowOverridesModal, setShowAllowOverridesModal ] =
useState( false );
const [ showDisallowOverridesModal, setShowDisallowOverridesModal ] =
useState( false );

const syncedAttributes = PARTIAL_SYNCING_SUPPORTED_BLOCKS[ name ];
const attributeSources = syncedAttributes.map(
( attributeName ) =>
attributes.metadata?.bindings?.[ attributeName ]?.source
);
const isConnectedToOtherSources = attributeSources.every(
( source ) => source && source !== 'core/pattern-overrides'
);
const hasName = !! attributes.metadata?.name;
const defaultBindings = attributes.metadata?.bindings?.__default;
const allowOverrides =
hasName && defaultBindings?.source === PATTERN_OVERRIDES_BINDING_SOURCE;
const isConnectedToOtherSources =
defaultBindings?.source &&
defaultBindings.source !== PATTERN_OVERRIDES_BINDING_SOURCE;

function updateBindings( isChecked, customName ) {
const prevBindings = attributes?.metadata?.bindings;
const updatedBindings = isChecked
? addBindings( prevBindings, syncedAttributes )
: removeBindings( prevBindings, syncedAttributes );
? addBindings( prevBindings )
: removeBindings( prevBindings );

const updatedMetadata = {
...attributes.metadata,
Expand All @@ -89,13 +71,6 @@ function PatternOverridesControls( { attributes, name, setAttributes } ) {
return null;
}

const hasName = !! attributes.metadata?.name;
const allowOverrides =
hasName &&
attributeSources.some(
( source ) => source === PATTERN_OVERRIDES_BINDING_SOURCE
);

return (
<>
<InspectorControls group="advanced">
Expand Down
3 changes: 2 additions & 1 deletion packages/patterns/src/private-apis.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
default as DuplicatePatternModal,
useDuplicatePatternProps,
} from './components/duplicate-pattern-modal';
import { isOverridableBlock } from './api';
import { isOverridableBlock, hasOverridableBlocks } from './api';
import RenamePatternModal from './components/rename-pattern-modal';
import PatternsMenuItems from './components';
import RenamePatternCategoryModal from './components/rename-pattern-category-modal';
Expand All @@ -34,6 +34,7 @@ lock( privateApis, {
CreatePatternModalContents,
DuplicatePatternModal,
isOverridableBlock,
hasOverridableBlocks,
useDuplicatePatternProps,
RenamePatternModal,
PatternsMenuItems,
Expand Down
Loading
Loading