-
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
Blocks: Try auto-inserting mechanism in the editor #49789
Conversation
Size Change: +331 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
7fec7d2
to
068449c
Compare
I did some testing with a test block registered with the following code on the client: registerBlockType( 'e2e-tests/auto-inserted', {
title: 'Auto Inserted',
description: 'Auto-inserted test block.',
category: 'widgets',
edit() {
return 'I got auto-inserted!';
},
save() {
return 'I got auto-inserted';
},
autoInsert: {
after: [ 'core/quote' ],
},
} ); I added that logic to one of the test plugins used with E2E tests that I enabled manually on the Plugins page for testing purposes. I was surprised that the auto-inserting works pretty well for such a minimal set of changes applied to the block editor store: auto-inserted-blocks-editor.movIt seems like it's possible to hook into the |
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'm excited that we're finally starting to explore this problem. A small change to get rolling
What's the plan here?
* | ||
* @return {Function} Enhanced reducer function. | ||
*/ | ||
const withAutoInsertBlocks = ( reducer ) => ( state, action ) => { |
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.
Do you think this should be in the action creator instead. I believe reducer needs to be pure functions and don't rely on things like selecting stores...
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 have no idea at this stage. I'm trying to build a prototype that works with all nested editors and properly integrates with the undo/redo mechanism 😅
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.
If we were able to modify the initial parsing to have all auto-inserted blocks in place as discussed in #49789 (comment) then we won't need this part at all. In fact, it would be as simple as extending the action creator to store the list of auto-inserted block names, so we can store them later in the database together with other changes to the edited content. This way, the next time we load the same content, we can skip the auto-inserting of blocks listed earlier because they are now controlled by the user.
save() { | ||
return 'I got auto-inserted'; | ||
}, | ||
autoInsert: { |
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 PR basically implements this as a generic block editor API. It is a decent approach, maybe the best one but I'd like us to ask the question of whether that's exactly what we want.
Basically, with this approach any blocks with this config, is going to be auto-inserted regardless of the block editor instances it's used in right? Could be a post editor, a site editor, a widgets editor...
I've heard of cases where we want to auto Insert blocks but only for specific post types for instance.
This approach can still be used if the block config is "modified" dynamically but I think it's worth asking whether we want to tie this with block registration. Can we imagine another approach? What are the pros and cons.
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.
Yes, it's a separate discussion on how to limit where the auto-inserting mechanism gets applied. I'm trying to explore what is possible today in the block editor for now.
The most important question for me was whether we can inject blocks without having the editor mark the changes in the UI. It looks like, we have full control over that, which sounds very promising. The other concern I have is how it all would work with undo/redo, and so far, it's going terribly, but I might be simply doing something wrong as I don't know the codebase.
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.
yeah I'm guessing since the list of blocks you feed the editor and the one you retrieve from it when you call "reset" initially are different, it's going to create an undo level. Not entirely sure how we can avoid that at the moment, unless the auto-insertion is part of the "initial parsing"
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 entirely sure how we can avoid that at the moment, unless the auto-insertion is part of the "initial parsing"
That would be neat to send the updated HTML from the server as it would work out of the box with the block editor. It would require changes in REST API to ensure that the post content, site templates, reusable blocks, and template parts get all tweaks. We will need exactly the same logic on the server anyway to support the requirements for site visitors who never lunch the editor. Potentially we could do most of the work in PHP.
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.
Sending from the server is a good solution too but personally I was referring to the parsing done in the client. Either way should work I think. But yeah we do need the server side logic for the frontend.
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 figured out you were referring to the client side. That works, too. We need to investigate whether applying changes on the server instead makes sense. The benefit would be that such an auto-inserting block would also integrate nicely with other REST API powered apps.
b9563eb
to
2a9506e
Compare
I recorded a narrated video to share the progress of the exploration. I had to divide it into two parts to fit into the size limit for the upload. In the first part, I'm talking about the issue and the importance of the comment #39439 (comment) from @mtias where he says:
I also explain the basic idea explored in this branch that led me to apply the current changes to the branch and how I wanted to keep track of the block names that were auto-inserted in the block editor. Auto-inserting.blocks.20-04.part.1.movIn the second part, I presented how I use the modified E2E test plugin to verify how the changes applied in this branch impact the list of blocks loaded in the block editor. Auto-inserting.blocks.20-04.part.2.movThe learning so far is that the approach taken doesn't scale well despite my initial enthusiasm shared in #49789 (comment). As briefly discussed with @youknowriad in #49789 (comment), it might be very difficult to bend the current approach to integrate seamlessly with the undo/redo, but also with other parts of the editing workflow like switching between the code and the visual editors. I'm inclined to try next the idea brought by Riad to change the list of blocks earlier in the flow – during initial block parsing on the client or even run that logic on the server. In particular, auto-inserting blocks on the server is appealing because we need it anyway for site visitors, so maybe we can integrate a similar logic with REST API for post content (that would apply to posts, pages, CPT, reusable blocks, template parts, and templates). The important note is that since we need to auto-insert blocks on the server if the site admin never opens the block editor, it can't work out of the box with static blocks that depend on the |
I'm closing this prototype with the intent to try alternative approaches that I'm going to list in #39439. |
What?
Part of #41236.
Explores possible approaches to address #39439.
Related to #37717.
For now, it's just a playground for exploring ideas.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast