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

Jump to bottom

Use block context to detect presence of parent pattern for overrides #62861

Merged
merged 6 commits into from
Jun 26, 2024
Merged

Use block context to detect presence of parent pattern for overrides #62861

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
320dcb9
Use block context to detect presence of parent pattern
talldan Jun 26, 2024
214a2a2
Regenerate fixtures
talldan Jun 26, 2024
318cc7b
Update image block to use context for checking a parent pattern block…
talldan Jun 26, 2024
14be2c2
Rename context to `pattern/overrides` to be consistent with php code
talldan Jun 26, 2024
b8b3733
Move pattern overrides context shim to pattern overrides hooks
talldan Jun 26, 2024
eb486e8
Remove shim
talldan Jun 26, 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
34 changes: 12 additions & 22 deletions packages/block-editor/src/hooks/use-bindings-attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { addFilter } from '@wordpress/hooks';
* Internal dependencies
*/
import { unlock } from '../lock-unlock';
import { store as blockEditorStore } from '../store';

/** @typedef {import('@wordpress/compose').WPHigherOrderComponent} WPHigherOrderComponent */
/** @typedef {import('@wordpress/blocks').WPBlockSettings} WPBlockSettings */
Expand Down Expand Up @@ -95,24 +94,12 @@ export const withBlockBindingSupport = createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const registry = useRegistry();
const { name, clientId, context } = props;
const { sources, hasParentPattern } = useSelect(
( select ) => {
const { getAllBlockBindingsSources } = unlock(
select( blocksStore )
);
const { getBlockParentsByBlockName } = unlock(
select( blockEditorStore )
);

return {
sources: getAllBlockBindingsSources(),
hasParentPattern:
getBlockParentsByBlockName( clientId, 'core/block' )
.length > 0,
};
},
[ clientId ]
const sources = useSelect(
( select ) =>
unlock( select( blocksStore ) ).getAllBlockBindingsSources(),
[]
);
const hasParentPattern = !! props.context.patternOverridesContent;
const hasPatternOverridesDefaultBinding =
props.attributes.metadata?.bindings?.[ DEFAULT_ATTRIBUTE ]
?.source === 'core/pattern-overrides';
Expand Down Expand Up @@ -254,12 +241,13 @@ export const withBlockBindingSupport = createHigherOrderComponent(
[
registry,
bindings,
name,
clientId,
context,
hasPatternOverridesDefaultBinding,
hasParentPattern,
setAttributes,
name,
sources,
hasPatternOverridesDefaultBinding,
context,
clientId,
]
);

Expand Down Expand Up @@ -292,6 +280,8 @@ function shimAttributeSource( settings, name ) {
return {
...settings,
edit: withBlockBindingSupport( settings.edit ),
// TODO - move this to be located with pattern overrides code.
usesContext: [ 'patternOverridesContent', ...settings?.usesContext ],
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move this to the usesContext property when registering the pattern overrides source?

gutenberg/lib/compat/wordpress-6.6/block-bindings/pattern-overrides.php

Line 57 in 320dcb9

'uses_context' => array( 'pattern/overrides' ),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will this also register usesContext for client side code?

Considering both pattern/overrides and patternOverridesContent are both sharing the same attribute via context, I guess they should be named the same thing. I'm not enamoured with using pattern/overrides naming in js code where the variable is often accessed via dot notation or destructuring.

I think changing it would require a backport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will this also register usesContext for client side code?

If not, I think it could still be moved to this file if we change it over to the registerBlockType hook similar to the one in the use-binding-attributes.js file:
https://github.com/WordPress/gutenberg/blob/trunk/packages/editor/src/hooks/pattern-overrides.js#L110

Copy link
Contributor

Choose a reason for hiding this comment

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

Will this also register usesContext for client side code?

Yes, it should work client-side. We might make some changes to how it is handled, but it should work.

I'm not familiar with how that context is used, but it'd be great to use just one property if possible.

I think changing it would require a backport.

Yes, I think it would need one. But it should be just one line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to change it to pattern/overrides on the client as well, so that it's consistent and we won't need a backport.

I think I've addressed all the feedback.

};
}

Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/block/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@
"type": "number"
},
"content": {
"type": "object"
"type": "object",
"default": {}
}
},
"providesContext": {
"patternOverridesContent": "content"
},
"supports": {
"customClassName": false,
"html": false,
Expand Down
19 changes: 9 additions & 10 deletions packages/editor/src/bindings/pattern-overrides.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,22 @@ const CONTENT = 'content';
export default {
name: 'core/pattern-overrides',
label: _x( 'Pattern Overrides', 'block bindings source' ),
getValue( { registry, clientId, attributeName } ) {
const { getBlockAttributes, getBlockParentsByBlockName } =
registry.select( blockEditorStore );
getValue( { registry, clientId, context, attributeName } ) {
const { patternOverridesContent } = context;
const { getBlockAttributes } = registry.select( blockEditorStore );
const currentBlockAttributes = getBlockAttributes( clientId );
const [ patternClientId ] = getBlockParentsByBlockName(
clientId,
'core/block',
true
);

if ( ! patternOverridesContent ) {
return currentBlockAttributes[ attributeName ];
}

const overridableValue =
getBlockAttributes( patternClientId )?.[ CONTENT ]?.[
patternOverridesContent?.[
currentBlockAttributes?.metadata?.name
]?.[ attributeName ];

// If there is no pattern client ID, or it is not overwritten, return the default value.
if ( ! patternClientId || overridableValue === undefined ) {
if ( overridableValue === undefined ) {
return currentBlockAttributes[ attributeName ];
}

Expand Down
Loading