-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 Hooks: Set ignoredHookedBlocks metada attr upon insertion #58553
Block Hooks: Set ignoredHookedBlocks metada attr upon insertion #58553
Conversation
Size Change: +217 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
bb8367d
to
5ff53fc
Compare
return { | ||
...blockItemBase, | ||
initialAttributes: {}, | ||
initialAttributes, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was glad to learn that we already had an initialAttributes
arg, as that's exactly what we'd like to set here.
packages/blocks/src/api/templates.js
Outdated
const hookedBlocksForCurrentBlock = getBlockTypes()?.filter( | ||
( { blockHooks } ) => blockHooks && blockName in blockHooks | ||
); | ||
|
||
if ( hookedBlocksForCurrentBlock?.length ) { | ||
const { metadata, ...otherAttributes } = blockAttributes; | ||
blockAttributes = { | ||
metadata: { | ||
ignoredHookedBlocks: hookedBlocksForCurrentBlock.map( | ||
( hookedBlock ) => hookedBlock.name | ||
), | ||
...blockAttributes.metadata, | ||
}, | ||
...otherAttributes, | ||
}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed for blocks that aren't inserted directly from the inserter, but as inner blocks of a container block. One example would be the Comment Template hook, which is typically inserted as part of the Comments block.
Unfortunately, the initialAttributes
concept doesn't exist here 😕
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
bf92975
to
0d26833
Compare
Flaky tests detected in 9d5a9ba. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7785896196
|
Gotta look into some failing tests... |
e47ef93
to
268a7c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested using the instructions provided, and everything works as advertised. The code changes included also look good. There are some nitpicks to address before landing.
This reverts commit 6f973f9.
ea6fc04
to
c257a66
Compare
What?
Set the
metadata.ignoredHookedBlocks
attribute upon inserting a new block that has hooked blocks.Why?
To unblock WordPress/wordpress-develop#5726 (see WordPress/wordpress-develop#5712 (comment)).
In WP 6.5, we'll be enabling insertion of hooked blocks into modified templates/parts/patterns. This is implemented by adding a
metadata.ignoredHookedBlocks
attribute to the hooked block's anchor block upon editing.As a consequence, upon insertion of an anchor block, this attribute also needs to be added to that newly inserted instance.
A note on UX: It could be argued that ideally, insertion of a new anchor block should cause its hooked blocks to be inserted alongside with it. This could be a future addition but seems to be a bit more complicated to do. Regardless, the
metadata.ignoredHookedBlocks
attribute also would need to be set in that case, in order to avoid double insertion -- meaning that adding that attribute is required as a baseline anyway.How?
AFAICT, there are two mechanisms that set attributes for a newly inserted block:
buildBlockTypeItem
is used to build items that appear in the inserter; conveniently, it allows setting aninitialAttribute
.synchronizeBlocksWithTemplate
from atemplate
set in the container block.Both mechanisms are modified by this PR to set the
metadata.ignoredHookedBlocks
attribute.Testing Instructions
metadata.ignoredHookedBlocks
attribute set to include the Like Button block:<!-- wp:post-content {"metadata":{"ignoredHookedBlocks":["ockham/like-button"]}} /-->
.metadata.ignoredHookedBlocks
attribute set to include the Like Button block:<!-- wp:comment-template {"metadata":{"ignoredHookedBlocks":["ockham/like-button"]}} -->
Follow-up
Currently, only hooked blocks that were added via
block.json
will be taken into account, but not the ones added via a filter. This will be addressed by #58622.We should then also use it the selector to compute
hookedBlocksForCurrentBlock
inblock-hooks.js
(so it'll also use the new endpoint as its source of truth). (I've tried that out in 6f973f9 but decided to punt for now as it'd add needless complexity for the time being.)