-
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
Experiment: Auto-inserting blocks on the frontend and in the editor (via REST API) #51449
Experiment: Auto-inserting blocks on the frontend and in the editor (via REST API) #51449
Conversation
lib/compat/wordpress-6.3/class-gutenberg-rest-block-patterns-controller-6-3.php
Outdated
Show resolved
Hide resolved
6a0183f
to
05f86ac
Compare
Flaky tests detected in aa65ac3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5658382077
|
665c65b
to
05bde3e
Compare
Last week, I focused on visually highlighting auto-inserted blocks in the editor. This is a bit of a harder problem: On the one hand, we need the information that a block is auto-inserted in the editor; OTOH, that information should not be persisted to the DB if a user saves a template. I explored a few different approaches: Leveraging block patterns (since they are normally "expanded" into blocks the moment they are inserted, which as a mechanism seemed like a potentially good fit), but this seemed eventually limited by all the constraints that make it hard to mark blocks as auto-inserted (as described above). I then spent a while trying out a "clever", CSS-based approach. The idea was to dynamically create CSS selectors from the auto-insert locations defined in block.json, e.g. an "__experimentalAutoInsert": {
"core/post-content": {
"position": "after",
"attrs": {},
"innerBlocks": [...]
}
} will be transformed into .wp-block[data-type="core/post-content"] + .wp-block[data-type="core/social-links"] {
/* some style to highlight the block */
} However, while this worked rather well for auto-inserted sibling blocks, it doesn't really scale to auto-inserted children, since the editor adds a lot of ancillary DOM elements that seem to make it pretty much impossible to build a CSS selector that maps a parent/child relationship between two blocks to their corresponding DOM elements. Finally, I explored an approach originally discussed with @mcsf, where I would include an I'll continue to work on a solution for this. |
f02f9c5
to
45cd6b3
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.
This is a really cool feature, @ockham! Thanks for taking the time to work on this.
The test case with the avatar in the comments worked easily out of the box. For templates which are created in the client or outside of the REST API (the product editor and checkout block in the case of Woo), this involved a bit of tweaking in those repos to get things working. I managed to get the product editor working by adding this as a template to the REST API, but this opens a new set of questions since I don't think we want this template exposed when hitting the site editor.
The simplicity of the API allowing us to add __experimentalAutoInsert
to the block metadata is really nice. I think this works pretty well for the Woo checkout block with a couple caveats:
- Allowing experimental auto insertion to happen at the block level instead of just templates would be very helpful. The checkout block is a simple parent block that is saved to a page.
- The necessity to anchor to an existing block means that third parties need to know that block's slug and if we ever deprecate a block, that field would no longer show on the page. This may be acceptable behavior, but is worth noting as it has potential to disrupt backwards compatibility as we iterate on certain blocks.
The product editor, however, breaks the current mold of templates and while it is an actual template, is likely not something we want users to be able to customize in the short-term. We do want 3PDs to be able to customize this template. There are a couple additional items that would prevent this auto-insertion API from being a feasible choice for the product editor:
- The API would also need to allow for removal of blocks. This might not be the intention of this experiment and may warrant a separate pattern entirely. I would imagine a more declarative API for insertion or addition of blocks.
- As we talked about in Auto-inserting Blocks #39439 (comment), we might need a way to avoid anchors (at least at the sibling level). This led Woo to experimenting with the usage of an
order
property. We need to be able to insert at a given point with only reference to the parent element, since it's possible a sibling could be removed by another plugin.
I think those items are must-haves in order to achieve the template accessibility we need in the product editor, but that may be outside the scope of this feature and I recognize that the product editor template is in a category of its own. I'm planning on p2ing some of those differences and clarify some of the semantics around that type of template.
All in all, I really appreciate the effort here. This is a really valuable feature and looking forward to being able to use this soon! 💯
|
||
return $query_result; | ||
} | ||
add_filter( 'get_block_templates', 'gutenberg_parse_and_serialize_block_templates', 10, 1 ); |
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.
Could we change this priority from 10
to PHP_INT_MAX
or better yet, a very high number?
WooCommerce BLocks also adds templates through the get_block_templates
filter and because it also had a priority of 10
, this never ran for that code?
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.
Hmm, identical values for priority shouldn't really cause either of the filters not to be run 🤔 (Instead, the one that was added first in terms of code execution flow should be run first.)
To clarify, you're saying that the auto-insertion filter was run, while the WooCommerce Blocks one wasn't?
45cd6b3
to
462c202
Compare
Thanks a lot for taking the time to try this out and for all your feedback @joshuatf -- this is very valuable to me! 🎉 There's plenty food for thought in your comment, and I'll try to think of potential solutions to the issues you've raised; I'll reply to them potentially in a different order than you posted them, hope that's okay.
Gotcha. The main problem of doing this at any level other than templates (and arguably block patterns) is that those other levels don't have the built-in distinction of theme/plugin-provided vs user-modified, which we leverage to bypass the issue of detecting whether a user has persisted or dismissed an auto-inserted block so we can avoid erroneously continuing to auto-insert it despite of those user modifications. However, I hear you that that's just not good enough to cover some of Woo's use cases, so we'll have to figure out how to solve that 🙂 I just tinkered a bit with the Checkout block myself. One idea that I had was that we could somehow tap into a parent block's default inner block makeup (confusingly also called "template" in Gutenberg parlance), as seen here and here for the Checkout block (with some further nesting for both the Checkout Fields and Checkout Totals blocks). The There is a limitation to this, however: Parent blocks like these are typically static, so unlike the template-level approach, we cannot auto-insert on the frontend if the parent block has not been manually inserted into a page (or wherever) via the editor in the first place. This is strictly speaking a limitation of static blocks (rather than of this mechanism), but I think it's fairly easy to construct an example where an auto-inserting blocks mechanism falls short of providing feature parity with what a "classic" (filter-based) mechanism could do: It seems like WooCommerce creates a "Checkout" page that typically contains the checkout shortcode (that I assume a filter could hook into to insert other fields). If we'd want WC to create that page to hold the Checkout block instead of the shortcode, it would have to provide the serialized markup of that (static) block -- thus offering no entry points for auto-insertion. One way out of this dilemma could be to make the Checkout block (pseudo-)dynamic; there's some precedent in the Comment Template block. Curious to hear your thoughts on this! It might still be worth implementing this idea to try it on for size (and make it a bit easier to reason about), so I'll probably give it a spin 🙂 |
Yeah, that's a fair point. Off the cuff, I'm inclined to consider it acceptable -- I'd vaguely argue that a block slug is kind of a public API, and that if it changes, the author must make some provisions to preserve backwards compat. For (future) reference, here's one example in Core (where we renamed |
Hmm, is there any chance that y'all would reconsider? The fact that there's an actual "Single Product" template that's used for this would give a lot of benefits, and allow the auto-insertion logic to work as designated. I briefly looked into this, and it seems like there's nothing strictly preventing a user from editing the template in the Site Editor. When doing so, I see that it's currently using the legacy version of the block, but it's easy enough to convert that into a collection of blocks: |
That makes sense. Maybe this is a good reason to reconsider the blocks checkout as a template instead of a parent block. The block could still consume the template so its reusable, but at least the template will be filterable and readily available via the REST API.
Ya, that makes sense and helps me understand why the I still think there may be cases though where it's ideal to add in to a certain position within a parent as opposed to a sibling anchor, especially in cases where a sibling anchor block had been removed by a user.
If we use the concept I mentioned above of fetching a template from the REST API and rendering within the block, would that solve this issue? Is this similar in how the comments block gets rendered, with the exception that it's handled on the server?
That's a great point and I agree on the block slug being public API. But besides deprecation, a block could simply be removed from the template by the user, removing the anchor point. Using the parent block is the obvious choice here, though I can see cases where developers prefer a different position than the start or end of the parent block.
I think there's a misunderstanding here. The one in your screencast is the single product template, but the one I'm primarily referring to is the product editor template which renders a form via blocks and allows the user to fill in details about their product. You can see that feature here or grab the latest WooCommerce and enable it under I don't think this type of template really fits this API well and I'm not sure that we should spend too much time trying to make it fit that use case. We'll most likely need a more declarative API to add/remove blocks from those types of templates. |
9e3c19e
to
1bc8723
Compare
Right. There might even be some middle ground -- e.g. the Checkout page could use a custom Page Template -- which nowadays can be a block template, and that latter could continue to use the Checkout block.
Yeah. I'll point out that with the current mechanism, removal of anchor blocks isn't so much of a problem, as that means that the template is modified by the user, which means that the auto-inserted block is stored as a regular block to the template when it is saved -- even though its anchor block is gone.
Yeah; I tried to think about other positions but it hard to come up with any that aren't determined by next or previous sibling. (Aside from anything order/priority-based, of course.)
Ahh yes, I definitely got that wrong; thank you for putting me on the right track here. Crucially, I didn’t realize before that WooCommerce is basically using a block template to define its UI for the Product Editor on the wp-admin side. Makes a lot of sense that that should be extensible by 3rd-party plugins, but not really by the user.
Right, makes sense! 👍 |
b3af060
to
2662a0a
Compare
I feel like this should be in good enough shape for now, so I'm opening it for review. |
I'm not sure, to be honest. Since the API is experimental and we don't have any core block using it, it might be good to wait and highlight it later, at least until #52969 is ready. 🤔 |
@ockham yes! Let's mention it since we want as much testing as possible. We should also follow up with a standalone post once things are more stable. It's a huge deal. |
} | ||
return $block; | ||
}; | ||
} |
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.
how often do we know in advance the block we want to insert and then need to create a function to insert that? what if we instead made a function that actually inserts the block and skipped the anonymous function creation?
if we know we want to always insert the same block we can create our callback with a static value or an enclosed value.
also is this function different than hooking into the existing filters? I wonder if there's reason to insert after more than one block type, in which case we start duplicating code or adding new interfaces, but we already have the ability to do this if we use something like render_block
with an appropriate callback.
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.
how often do we know in advance the block we want to insert and then need to create a function to insert that? what if we instead made a function that actually inserts the block and skipped the anonymous function creation?
if we know we want to always insert the same block we can create our callback with a static value or an enclosed value.
Not sure I'm reading you correctly, but the point of this API is to allow 3rd parties to have their blocks auto-inserted next to pretty much any other block, so we can't really know in advance what blocks will need to be inserted.
also is this function different than hooking into the existing filters? I wonder if there's reason to insert after more than one block type, in which case we start duplicating code or adding new interfaces, but we already have the ability to do this if we use something like
render_block
with an appropriate callback.
It's definitely possible to insert a block after (or next to) more than one block type, e.g. a like button after Post Content, or as Comment Template's last child.
An earlier version of this (#51294) used render_block
, but that had a number of drawbacks -- most notably, it was hard to find out whether or not the block was rendered as part of a user-modified or unmodified template. In the present version of the code, we get this information pretty much for free from the get_block_templates
and get_block_file_template
filters that we've using.
Furthermore, while the render_block
filter worked fine for before
/after
insertion, it's less practical for first_child
or last_child
insertion, which inevitably requires some parsed tree structure of blocks; in #51294, I was using render_block_data
to that effect.
That aside, I'm not sure how a callback for render_block
would look substantially different from what we're doing here that would allow it to eschew the indirection? The problem we're facing is binding, isn't it? We'd like our API to allow people to specify what block they want to auto-insert next to what other block; but pretty much all existing block filters have at best one param that communicates the block that is currently being parsed/rendered/whatever (the anchor block, in our nomenclature), so in order to pass along the "other", auto-inserted block, we need some mechanism to accommodate for that. In Core, I'd do that via a dedicated registry for auto-inserted blocks (as noted here), from which the filters could then read which block needs to be inserted next to the currently processed anchor block; but for the sake of something more self-contained that can be easily implemented within Gutenberg, the filter factories seemed like a fair compromise.
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.
inserted next to pretty much any other block, so we can't really know in advance what blocks will need to be inserted.
this was kind of what led me to ask in the first place, because this function signature implies that we have to know in advance the block we want to insert plus the block type we want to anchor it to.
so in order to pass along the "other", auto-inserted block, we need some mechanism to accommodate for that
I wasn't commenting on the mechanism to do this, but the interface we present, which immediately creates an anonymous function and hides the ability to make decisions about insertion based on the current block.
it seems like if we want to auto-insert after two block types that we have to call gutenberg_auto_insert_block
twice, one for each block type. it seems like if we only want to auto-insert based on some block attribute then there's no way to do that.
I'm wondering if we were to invert this so that the consumer passed in the logic for where to insert if it could be less constrained and less complicated. we could pass in an anonymous function which returns a relative position and block to insert, if one ought to be inserted, or a filter that does the same.
add_filter( 'block_auto_insert', function ( $block ) {
if ( $block['attrs']['show_thing'] ) {
return array( 'where' => 'after', 'block' => array( … );
}
} );
add_filter( 'gutenberg_serialize_block', function ( $block ) {
$auto_insert = apply_filter( 'block_auto_insert', $block );
if ( null === $auto_insert ) {
return $block;
}
list( 'where' => $where, 'block' => $inserted_block ) = $auto_insert;
switch ( $where ) {
case 'before':
…
…
}
} );
); | ||
|
||
$inserter = gutenberg_auto_insert_block( $inserted_block, $position, $anchor_block ); | ||
add_filter( 'gutenberg_serialize_block', $inserter, 10, 1 ); |
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.
it seems here like this is an example of where we can skip the function-creating-function and implement the behavior directly. we don't need an anonymous function in order to pass this empty block around.
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 might not be seeing the forest for the trees, but how would we do that (without changing the gutenberg_serialize_block
filter's signature -- which is different from gutenberg_auto_insert_block()
's)? 🤔 Note that the empty block is passed a blockName
argument (to create an instance of the desired block for auto-insertion).
Flagging this for @mikachan who I believe is in charge of 16.4 👋 |
Thanks @ockham! I missed those comments when I ran through the changelog for the 16.4 RC. I'll make sure this is highlighted in the release notes for 16.4. This is really cool! |
What?
Based on concepts discussed in #39439, this PR explores auto-inserting blocks in the editor by means of the REST API.
Why?
We've discussed multiple ways of auto-inserting blocks in the editor, and the REST API route seemed promising enough to try it out. For more details, see #39439.
How?
Unlike the frontend where auto-insertion can happen at the render stage (e.g. via the
render_block
andrender_block_data
filters, see #50103 and #51294), things are quite different for the REST API:/templates
endpoint, block patterns from the/patterns
endpoint, etc. This means that we need to identify all such possible sources, and make sure to implement auto-insertion for all of them.get_block_file_template
andget_block_templates
._inject_theme_attribute_in_block_template_content
and_remove_theme_attribute_in_block_template_content
. In the long run, we should try to provide some specialized hook to give filters access to the parsed blocks before reserialization to avoid the performance impact from doing that dance more than once.Note that this PR is branched from #50103.
Testing Instructions
See the video below for a walkthrough.
.wp-env.override.json
file.__experimentalAutoInsert
field to theblock.json
of a dynamic block of your liking. You have to specify the "anchor" block and the relative position for auto-insertion:(A
gutenberg_register_auto_inserted_block()
function is also provided as an alternative.)Screenshots or screencast
https://www.loom.com/share/7b2cb29672ce4173b3844ad836de09d9?sid=4357767c-9a67-470a-8820-a1eff0fbfab7
Q&A
What happens if I've modified the template prior to activating the plugin that has the auto-inserted block?
The block will indeed not be auto-inserted in that case; we’re considering showing some kind of notification in the UI though. This is covered in this comment.