-
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
Add static block previews experiment #57977
Conversation
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/experimental/class-gutenberg-render-blocks-controller.php ❔ lib/experimental/editor-settings.php ❔ lib/experimental/rest-api.php ❔ lib/experiments-page.php ❔ lib/load.php |
Size Change: +355 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
Why a new PR? How come we didn't just change #55850 to be experimental? Seems like fewer files were changed in this PR so perhaps it's a subset? |
7e10610
to
66f7efb
Compare
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
@getdave we made a new PR b/c the previous one was more exploratory and had lots of duplication in place. Here it's parts of the previous learning applied. Plus the experiment. I think this is in a good state now to land as an experiment. If the experiment is enabled:
We still need to iterate to:
... but iteration will be simpler with the experimental code merged. |
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.
Let's get this in and iterate on it 🚀
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. |
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.
Can we wait for a bit here? I'm investigating the pages that you are testing and there's a lot of room for improvement on the editor side.
I also want to make sure everyone is in alignement whether this shuld go into WordPress 6.5 without having gone through a few testing cycles in the Gutenberg plugin. Just want to make sure we are not rushing something into core unless it is ready |
I agree, the intention is to merge this as an experiment in the plugin only so that we can find ways to get it working with block bindings. |
I don't understand blocking an experiment from landing. @ellatrix @fabiankaegy did you find any leak in the guards we put in place for the experiment? If not why block this? This makes iterating on the server side rendering API easy and possible. With the incoming block bindings and the advancements in the HTML API one should be able to entirely render patterns and templates on the server, not needing to instance a whole editor clientside. So this work is independent of the attempts to make the editor faster. It just happens that today it is faster, hence the experimental nature. |
@draganescu sorry I didn't look at the code when I left the comment. So I for one didn't recognize this was an experimental API. Knowing that there is no concern from me. |
As this is not targetted for 6.5 and it an experiment I:
|
…ered representation of the block list
acdb897
to
1c95865
Compare
I have many thoughts on this.
|
@ellatrix where do we have these guidelines about what merging an experiment means? What is it that holds anyone back from improving the client side rendering by working on any branch with this experiment turned off? How would we test the server rendering experiment without a use case to see how it behaves? |
I don't know if there are any guidelines for experiments. But I think it would be good to know what the plan is for the experiment. Is it just for GB contributors to try, or GB plugin users?
But what are the use cases where it's valuable outside the editor? If that's the primary purpose, could we test with those use cases instead? All I'm asking here is some more time to investigate, think about it, discuss etc. If it's not for 6.5, there's no need to rush it? |
Absolutely no need to rush :) |
Closing this as we're not convinced it is useful. |
@scruffian Are you able to point me to where the usefullness of this was discussed? I know @ellatrix had shared concerns but I'd love to figure out what the final rational / resolution of this is :) |
I discussed it in person with @draganescu and @ellatrix, but I'm happy to give an overview of our thinking... Static block previews could improve rendering speeds in both
For the first of these, we aren't convinced that this approach will ever be more efficient than the current approach because it relies on an API request. Instead we would rather put more effort into speeding up the BlockPreview component by removing more unnecessary processes from it. For the second, we aren't convinced that Gutenberg is the right place to work on this. The code in this PR could still be valuable to provide static block previews in a plugin, but we couldn't see a use case for this in core. I hope that is clear. Happy to answer more questions. |
Thanks for adding this :) I think there is a ton of value in having the reasons of why something was abandoned / decided documented for our future selves so we can remember / evaluate against that. So I really appreciate you adding those details :) |
Thanks for pushing me to elaborate :) |
What?
This is a followup to #55850.
Why?
The idea is to get the basic version of this into the repo as an experiment, so that more people can use it.
The idea works well, but there are some problems with placeholder content. To solve this we need to make use of things like the block bindings API, which aren't fully fleshed out. By making this available as an experiment we hope that the shortcomings will be clearer to more people and also it will be possible to iterate on these ideas without needing to keep a long running branch in sync with trunk.
How?
Testing Instructions
Static Block Previews
experiment inwp-admin/admin.php?page=gutenberg-experiments
.