Edit Site: Remove templateIds state #22893
Conversation
|
Size Change: -112 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
ockham
requested review from
spacedmonkey,
TimothyBJacobs and
youknowriad
as code owners
ockham
changed the title
lib/edit-site-page.php
Outdated
| $home_template = gutenberg_find_template_post_and_parts( 'front-page' ); | ||
| if ( isset( $home_template ) ) { | ||
| $home_template_id = $home_template['template_post']->ID; | ||
| } |
There was a problem hiding this comment.
What happens if there is no template here? That means that not even an index.html is provided. Should we create an auto-draft as a fallback?
There was a problem hiding this comment.
Yeah, maybe. An empty one I guess? I'll implement a fix tomorrow.
There was a problem hiding this comment.
Upon closer look, I'm no longer totally sure that this is the right solution 🤔
When rendering to the frontend, we fall back to an error message. Creating an emtpy auto-draft here would thus change the frontend output -- I don't think we want that.
$home_template_id is really just used in the editor to highlight which template is the home template -- so if there are simply no templates at all, it's probably fine to set it to null.
(The issue is also present in current master, where it affects both homeTemplateId and templateId, so I should fix it with a separate PR. What I said above applies to that scenario too, though.)
There was a problem hiding this comment.
This is needed so people can access the site editor without these templates. Otherwise, they just get errors.
We can just make that error show on the front-end if the template is an empty auto-draft.
There was a problem hiding this comment.
Makes sense. It's basically the 'user opens word processor and needs to see empty document' case. I think I have a fix. PR coming up...
## Description This PR fixes two issues with template lookup: ### 1. Templates with `index.html` block template fallback When using a theme that came with an `index.html` but not with a `front-page.html` block template, the site editor would show the following `Notice: Undefined index: front-page in /var/www/html/wp-content/plugins/gutenberg/lib/edit-site-page.php on line 162`. The reason is that I was setting the wrong key here: https://github.com/WordPress/gutenberg/blob/710373b254fbcd15d524afdeb31da0d93c4defd4/lib/edit-site-page.php#L158 This would use the name of the template that was _found_, not the name of the key that was looked for. So if we're looking up `front-page`, but don't find any template, and [thus fall](https://github.com/WordPress/gutenberg/blob/710373b254fbcd15d524afdeb31da0d93c4defd4/lib/template-loader.php#L226) back to `index`, the template will be stored under the `index` key, rather than the front-end one. The fix for this is to use the template we were looking for as key (rather than the name of the template we actually found). ### 2. Templates without `index.html` block template fallback When using a theme that doesn't come with any block templates at all, we need to provide a blank `index` template when the user first opens the site editor (see [discussion](#22893 (comment))). Co-authored-by: Enrique Piqueras <epiqueras@users.noreply.github.com>
## Description This PR fixes two issues with template lookup: ### 1. Templates with `index.html` block template fallback When using a theme that came with an `index.html` but not with a `front-page.html` block template, the site editor would show the following `Notice: Undefined index: front-page in /var/www/html/wp-content/plugins/gutenberg/lib/edit-site-page.php on line 162`. The reason is that I was setting the wrong key here: https://github.com/WordPress/gutenberg/blob/710373b254fbcd15d524afdeb31da0d93c4defd4/lib/edit-site-page.php#L158 This would use the name of the template that was _found_, not the name of the key that was looked for. So if we're looking up `front-page`, but don't find any template, and [thus fall](https://github.com/WordPress/gutenberg/blob/710373b254fbcd15d524afdeb31da0d93c4defd4/lib/template-loader.php#L226) back to `index`, the template will be stored under the `index` key, rather than the front-end one. The fix for this is to use the template we were looking for as key (rather than the name of the template we actually found). ### 2. Templates without `index.html` block template fallback When using a theme that doesn't come with any block templates at all, we need to provide a blank `index` template when the user first opens the site editor (see [discussion](#22893 (comment))). Co-authored-by: Enrique Piqueras <epiqueras@users.noreply.github.com>
ockham
force-pushed
the
remove/edit-site-template-ids-state
branch
from
f463ea2 to
1a3ab30
Compare
|
TODO: Check what to do about this: gutenberg/lib/template-loader.php Lines 424 to 425 in 36a9ff5 |
What needs to be done about it? |
I think that we can change the comment to diff --git a/lib/template-loader.php b/lib/template-loader.php
index 9718b22313..d537178b46 100644
--- a/lib/template-loader.php
+++ b/lib/template-loader.php
@@ -422,7 +422,7 @@ function gutenberg_template_loader_filter_block_editor_settings( $settings ) {
}
// If this is the Site Editor, auto-drafts for template parts have already been generated
- // through `gutenberg_find_template_post_and_parts` in `gutenberg_edit_site_init`.
+ // through `gutenberg_find_template_post_and_parts`, when called via the REST API.
if ( isset( $settings['editSiteInitialState'] ) ) {
return $settings;
} |
Done in f96d403. |
| @@ -78,13 +78,6 @@ describe( 'actions', () => { | |||
| actionName: 'setPage', | |||
There was a problem hiding this comment.
This needs to check for done: true.
There was a problem hiding this comment.
I actually had it check for done: true but removed it after unit tests were failing, and I couldn't quite work out why 😬
I'll try reverting that commit 🤔
There was a problem hiding this comment.
Failing again. I don't quite see why this isn't working, do you have any ideas?
There was a problem hiding this comment.
I un-reverted that commit. AFAICS, this is failing since we're returning a dispatch action here, which is a generator itself.
There was a problem hiding this comment.
dispatch just returns an object. Something else is wrong here.
There was a problem hiding this comment.
master: 274 queries, 148,8 ms
This branch: 28 queries, 5.4 ms
🎤 💧
There was a problem hiding this comment.
That's a pretty nice performance improvement <3
There was a problem hiding this comment.
I might be missing something, but could you elaborate on how you were able to access those perf numbers, Bernie?
I'm trying to access the debug bar locally but can't seem to get the debug dashboard to display. (I'm using query params like so http://localhost:8888/wp-admin/admin.php?page=gutenberg-edit-site&debug-bar=1)
There was a problem hiding this comment.
You need the debug bar plugin installed and a few config constants set. You can use a .wp-env.override.json in your local Gutenberg source checkout dir to achieve both. This is mine (also links theme-experiments themes).
{
"core": "../wordpress-develop/build",
"plugins": [ ".", "https://downloads.wordpress.org/plugin/debug-bar.zip" ],
"themes": [ "WordPress/theme-experiments" ],
"config": { "WP_DEBUG": true, "SCRIPT_DEBUG": true, "SAVEQUERIES": true }
}
ockham
force-pushed
the
remove/edit-site-template-ids-state
branch
from
e3b7e08 to
82bea1b
Compare
|
This should be ready for another look. |
There was a problem hiding this comment.
As someone without a lot of context, it looks like we're able to remove some state management of templates parts because we no longer inject template part data on initialization. Information is now hydrated from an API (I'm guessing the WordPress REST API?).
Does that sound right?
ockham
force-pushed
the
remove/edit-site-template-ids-state
branch
from
8c3e40f to
738cd2d
Compare
ockham
force-pushed
the
remove/edit-site-template-ids-state
branch
from
e8bfbaa to
7642660
Compare
This reverts commit 894d7e0.
Remove some now-obsolete state, and corresponding initial state passed from the server. We've found that the now-obsolete `gutenberg_find_template_post_and_parts()` calls in `lib/edit-site-page.php` were causing a considerable slowdown of the initial page render (see #23662).
Description
Remove some now-obsolete state.
How has this been tested?
Verify that site editing still works 🙂
Types of changes
Janitorial.
Checklist: