Rename "auto inserting blocks" to "block hooks" #54147
Conversation
ockham
added
[Feature] Block API
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.3/rest-api.php ❔ lib/experimental/class-gutenberg-rest-block-patterns-controller.php ❔ lib/experimental/editor-settings.php ❔ lib/experimental/rest-api.php ❔ lib/experiments-page.php ❔ lib/load.php ❔ lib/experimental/block-hooks.php |
|
Size Change: -31 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
ockham
force-pushed
the
rename/auto-inserting-blocks-to-block-hooks
branch
from
83f4bd3
to
2e4749f
Compare
|
Flaky tests detected in ca5331b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6097680830
|
|
I agree there are some things to consider with the revised name would be whether it should be Before: "__experimentalAutoInsert": {
"core/comment-template": "lastChild"
}After: "hooks": {
"core/comment-template": "lastChild"
}Would it be better to go with something more detailed and should we make the hook type the key? Example: "hooks": {
"autoInsertLastChild": "core/comment-template"
}Other than that, I think this PR isn't complex so we can always split the decision process into two steps as soon as we agree on the UI aspect, and cover the rest in a follow-up. |
I considered something like this when first working on #51449 but decided against it. The main reason was that I thought it's possible (and not even entirely implausible) for a block to be auto-inserted next to various different blocks; e.g. a Like block as a Comment Template's last block, and after Post Content: "__experimentalAutoInsert": {
"core/comment-template": "lastChild",
"core/post-content": "after"
}Now we could of course achieve the same with the key and value flipped, if we allow arrays as values: "__experimentalAutoInsert": {
"lastChild": "core/comment-template",
"after": [ "core/post-content", "core/other-block" ]
}... but that's more complex, so I thought why bother 😬 Note that the opposite scenario -- auto-inserting a block in various different relative positions next to the same block -- seems unlikely enough that I don't think we'll need to consider it: "__experimentalAutoInsert": {
"core/post-content": [ "before", "after" ]
} |
| array( | ||
| 'schema' => array( | ||
| 'description' => __( 'Block types that may be automatically inserted near this block and the associated relative position where they are inserted.', 'gutenberg' ), | ||
| 'description' => __( 'This block is automatically inserted near any occurence of the block types used as keys of this map, into a relative position given by the corresponding value.', 'gutenberg' ), |
There was a problem hiding this comment.
See #52969 (comment):
BTW while working on #54147, I discovered that my original description was inaccurate [...]: The field isn't for block types that are automatically inserted next to this block, but the other way around: we're inserting this block next to those block types 😬
I'll update, probably as part of #54147.
cc/ @dmsnell for wording 😊
ockham
requested review from
TimothyBJacobs,
spacedmonkey,
ellatrix and
ajitbohra
as code owners
| * | ||
| * @package gutenberg | ||
| */ | ||
|
|
||
| /** | ||
| * Return a function that auto-inserts a block next to a given "anchor" block. |
There was a problem hiding this comment.
Note that the two helper methods that used to be located at the top of the file were moved further down.
|
I'm confused by the term "block hooks" as it has lost its specific meaning, which is inserting blocks in relation to others. I understand some relationship to WordPress hooks, but WordPress hooks are also "fire and forget" in that they are more about running code as an announcement of activity. If anything, auto-inserting blocks is closer in semantic to filters since we're discussing modifying the content passed into the filter. @ockham you had some use of "anchor" that I think could be a clue to a more descriptive name. Using "block hooks" seems so undefined to me, and the kind of thing we could easily add later for more generic purposes than auto-inserting them. Can we find something that links back to familiar terminology without matching an overly-generic name with an over-specific behavior?
|
I should clarify that better. My only concern was that "insertHooks": {
"core/comment-template": "lastChild"
}or "hooks": {
"insert": {
"core/comment-template": "lastChild"
}
}I didn't refer to how the connection between blocks and the insertion place was shaped. |
Ah, gotcha! My apologies, I was misreading you. I see now what you mean. While I think it's a valid concern, I'm a bit worried about YAGNI: I'd rather not introduce e.g. a nested A similar argument applies to a "flat" |
| <InspectorControls> | ||
| <PanelBody | ||
| className="block-editor-hooks__block-hooks" | ||
| title={ __( 'Plugins' ) } |
There was a problem hiding this comment.
Should it be renamed to "Block Hooks"?
There was a problem hiding this comment.
I think I'd leave it as is. "Plugins" was originally suggested as the name of this panel when the feature was still called "Auto-inserting Blocks", so arguably, we have precedent that the panel name is disconnected from the feature's name. My understanding is that "Auto-inserting Blocks" or "Block Hooks" is developer-oriented terminology, which we don't want to necessarily burden the user with; for them, it's enough to know that, "Hey, there's this plugin that would like to insert a block here."
|
@ndiego Can I ask you to look this over (from a documentation POV)? 🙌 (Plus native language background and all 😬 ) |
| 'gutenberg-auto-inserting-blocks', | ||
| __( 'Auto-inserting blocks', 'gutenberg' ), | ||
| 'gutenberg-block-hooks', | ||
| __( 'Block hooks', 'gutenberg' ), |
There was a problem hiding this comment.
An important note: remember to enable the experiment again before testing these changes. It took me a while to realize that.
|
@nerrad In case you'd also like to weigh in on nomenclature 😊 For context, this is what prompted this change:
|
There was a problem hiding this comment.
I approve this PR, assuming "block hooks" is the final term. I like the changes applied to code occurrences of the old name in the variable or function names and to code comments that received a bit of clarification as part of the process. I can confirm that everything works as expected when you follow the testing instructions in the PR description.
Not totally sure I follow/agree 100% 😅 The most canonical sources I've seen seem to define hooks as the union of actions (that's what you meant with "fire and forget"/announcement of activity?) and filters: I agree however that auto-inserting blocks are closer in spirit to filters than actions (and thus, than their union). Still, they're dissimilar enough from filters to warrant different nomenclature; and since "hooks" is already used to denote the set of existing extension mechanisms, it might be okay to re-use them here. I guess my mental model would be that per their introduction, block hooks extend (heh) the set of extensions mechanisms:
Yeah, that one came rather naturally when I was looking for a way to describe the relevant function arg in some PHPDoc. I have admittedly been struggling a bit with the fact that I haven't been able to reconcile that name with "block hooks" -- I continue to call the function arguments "anchor blocks", since "block hook" doesn't quite seem to work there. The way I've been thinking about it is that if anything, a block hook is an insertion point defined by an anchor block plus a relative position; I've actually added one sentence to that effect as part of this PR.
I like "block anchor points" (which is probably no surprise since it aligns with what I landed on earlier 😅 ), and also "implied/implicit blocks" (though I'm not quite sure they express strongly enough that these blocks are actually rendered and present in the editor 🤔 The others are a bit too vague IMO or have too much potential to be confused with other concepts. Based on what I wrote above, I'll throw "block insertion points" in the mix. On the note of naming things overly generic when they have overly specific behavior: I get that concern (and have voiced a variant thereof in early discussions on the renaming), but I do think that this mechanism will indeed be one of the more generic ways to extend a block theme, so I'm not that worried that its behavior is that specific. Anyway: The main reason for this PR was to get rid of the "auto-inserting" moniker which doesn't seem to have resonated with people (maybe it sends the vibe that some blocks are forcefully inserted, and they can't do anything about it?), and to do so in time for GB 16.6 (to be released tomorrow) so that there can be some user testing of the feature before the WP 6.4 Feature Freeze (in three weeks from now). Since the name was suggested by Matías, I'm inclined to land the PR; I believe that the planned Call for Testing could also serve as a testbed for the name of the feature, and since we still have the I really appreciate your thoughts and suggestions ❤️ I'm mostly trying to bridge the gap between some urgency for changing the name in time for wider testing, and wider discussion. Maybe we can explicitly ask users how they like the name and provide some alternative suggestions as part of the Call for Testing? cc/ @annezazu |
ockham
added
the
Backport to Gutenberg RC
ockham
force-pushed
the
rename/auto-inserting-blocks-to-block-hooks
branch
from
5dafcb3
to
ca5331b
Compare
|
Thanks fro the feedback @nerrad—naming things remains one of the most challenging parts of API design :) Hooks is still a familiar term in WP parlance to signify "attach to" and it still feels adequate beyond WP. It somehow resembles the little hooks lego pieces have to combine with each other (I believe they call them studs). There might be a better name we can settle on during testing, but I want to avoid shipping with auto-insert for the initial round. |
|
Okay, thanks all for weighing in! Going to merge this so it'll make it into GB 16.6, as originally planned 😊 |
See #53987 (comment): > I've seen anecdotal feedback that `autoInsert` is not the clearest of descriptions. I'd like to propose renaming to the more familiar `hooks` terminology—and "block hooks" in more general terms—to help folks understand the mechanics and purpose more rapidly.
|
"Block hooks" sounds awfully vague and liable to be confused with a number of existing things like the block editor PHP hooks or the React hooks used by blocks. Additionally, it seems weird to leave "insert" entirely out of the naming, since that's all this feature is about. Why not call it "default insertion points" or "default insertions"? That would avoid the negative connotations of "auto" ("default" implies it can be changed) while also clarifying the "when" of the block insertion. |
Mamaduka
removed
the
Backport to Gutenberg RC
What?
Rename "auto inserting blocks" to "block hooks".
Additionally, this PR contains two minor, unrelated changes:
Why?
See #53987 (comment):
How?
Replacing strings, of course. Furthermore, by hopefully picking the right replacements for different nouns and verbs previously used for auto-inserting blocks with their block hook counterpart.
For one, while hooks is a familiar concept in WordPress, it's not really used in Core's functions names (we have
add_filterandadd_actionbut notadd_hook); we might want to consider this for our block hook related function names.For example, should
gutenberg_register_auto_inserted_blockbecomegutenberg_add_block_hook? That seems misleading, like it would actually add the block hook -- i.e. the location where it is inserted -- rather than the block. So maybe justgutenberg_add_block? That will becomeadd_block_hookin Core 😕 I ended up usinggutenberg_register_hooked_blockin this case.Or: in
block.json, should the field be calledblockHooksor justhooks-- since implicitly,block.jsonis always about blocks?Testing Instructions
Note that you have to re-enable the experiment, as its name has changed as well!
You can use v0.3.1 of the Like Button block plugin for testing (it's been updated to use
__experimentalBlockHooksinstead of__experimentalAutoInsert).Verify that
auto-inserting blocksblock hooks work as before.