Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Changes from 15 commits
6e3e49a
c73a190
919a2fe
fbb1c6d
da643e8
2f78d81
7f7faee
4d21f5c
5e3bfd0
954eeed
52efdf0
7d7f70a
0f564f5
f3e566b
aa65ac3
bfe97c2
edffdc1
b610c37
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
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 theget_block_templates
andget_block_file_template
filters that we've using.Furthermore, while the
render_block
filter worked fine forbefore
/after
insertion, it's less practical forfirst_child
orlast_child
insertion, which inevitably requires some parsed tree structure of blocks; in #51294, I was usingrender_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.
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.
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.
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 fromgutenberg_auto_insert_block()
's)? 🤔 Note that the empty block is passed ablockName
argument (to create an instance of the desired block for auto-insertion).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
toPHP_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 of10
, 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?