Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#53176 closed task (blessed) (fixed)

Block Editor: Backport the FSE infrastructure required to make the template editor work for classic themes.

Reported by: youknowriad's profile youknowriad Owned by: youknowriad's profile youknowriad
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

This includes:

  • The Template CPT.
  • The Template REST API endpoint and controller.
  • The Template loader logic that allows loading custom block templates assigned to pages.
  • A change to the list of available templates for pages to include saved custom block templates.

The template-part related code can be left out of Core for WordPress 5.8.

Change History (21)

#1 @Bernhard Reiter
4 years ago

I'll start tackling this one. I'll probably use this issue to discuss potential approaches -- we might want to massage the API a bit when carrying it over from Gutenberg to Core (rather than copying it 1:1).

Some relevant discussions here:

https://github.com/WordPress/gutenberg/pull/31604#pullrequestreview-655398786
https://github.com/WordPress/gutenberg/issues/31652#issuecomment-836553438

#2 @Bernhard Reiter
4 years ago

In Gutenberg, we're currently hooking into all the `{type}_template` filters.

The overall sequence in Core looks about like this:

  • WordPress detects that the current route maps to an entity of type {type} (e.g. a page or category).
  • It calls get_{type}_template() (e.g. get_page_template()), which in turn passes an array of candidate template slugs (with .php file extension) to `get_query_template()`.
  • Internally, get_query_template() calls, in order:

Gutenberg only hooks into the latter filter, to "redirect" WordPress to its special template-canvas.php template file, which will render the best matching block template (if any). The reason for this is that WP expects a (PHP) file _path_ -- and in GB, we can't guarantee that there's always a template file, since block templates can also come from a wp_template post object.

OTOH, it means that Gutenberg is conflating concerns within that filter, rather than aligning them within the framework of e.g. get_{type}_template() and locate_template().

#3 @Bernhard Reiter
4 years ago

Some more thoughts:

The wp_template post objects are clearly the the major challenge. The rest of the logic would scale fairly well to block templates:

  • locate_template() seems suffix agnostic -- it could probably find .html files (even if located in a block-templates subfolder) as easily as .php ones.
  • We could change either the get_{type}_template methods, or use the type_template_hiearchy filter, to add the block-templates/*.html counterpart for each PHP template. E.g.
array(
        'page-slug.php',
        'page-123.php',
        'page.php',
);

becomes

array(
        'block-templates/page-slug.html',
        'page-slug.php',
        'block-templates/page-123.html',
        'page-123.php',
        'block-templates/page.html',
        'page.php',
);

The duplication might look a bit heavy-handed, but it's at least representative of what the get_{type}_template function (or the type_template_hiearchy) are supposed to do, which is nominating template file candidates.

#4 @Bernhard Reiter
4 years ago

What get_{type}_template and {type}_template_hierarchy cannot really capture is the presence of wp_template post objects as template candidates. The obvious place for this to go is probably locate_template -- which would mean changing its character rather significantly, from a rather simple check for existence of a file at a given path to something that also involves a moderately complex DB query. Combined with the ideas laid out in the previous comment(s), locate_template would check for the existence of a wp_template object for each .html file it sees in the $templates candidates array.

This ticket was mentioned in Slack in #core-editor by bernhardreiter. View the logs.


4 years ago

This ticket was mentioned in Slack in #core-editor by bernhardreiter. View the logs.


4 years ago

This ticket was mentioned in PR #1267 on WordPress/wordpress-develop by ockham.


4 years ago
#7

  • Keywords has-patch has-unit-tests added

Early draft. Details to follow.

Trac ticket: https://core.trac.wordpress.org/ticket/53176

#8 @Bernhard Reiter
4 years ago

JFTR, I'm focusing on integrating Gutenberg's current template logic as-is into Core in https://github.com/WordPress/wordpress-develop/pull/1267 (which is tricky enough). If we decide to align template resolution concerns more with locate_template et al (as discussed in the above comments), we'll need to do so in a follow-up.

youknowriad commented on PR #1267:


4 years ago
#9

Some updates here:

  • Removed all changes related to the WP-Admin menus for the CPT (these are not going to be shipped).
  • Moved some files that are not "admin" into "wp-includes".
  • Enable the block-templates support by default.

The next step for me is going to make everything work as it should in the UI before attacking the code cleaning.

youknowriad commented on PR #1267:


4 years ago
#10

@ockham I removed all the logic that fetches templates from files (block based themes) from the code and left just the part about custom template (the CPT). I wonder if resolve_block_template is still necessary. At least I think there's a few things we can remove from the resolution logic (as we don't care about theme files)

youknowriad commented on PR #1267:


4 years ago
#11

@ockham I wanted to add that I had to change the "prepare_for_database" function in the REST API controller because the create_item unit test was failing (compare it with Gutenberg). I think the failure was legitimate and we probably should fix that function similarly in Gutenberg as well, though not sure why the unit test was not failing there.

(Just wanted to note this comment somewhere so I don't forget about it :P )

ockham commented on PR #1267:


4 years ago
#12

Pushed a few changes; feel free to revert if you disagree.

This LGTM overall. I haven't reviewed the controller closely, but I think this PR should be in good enough shape to merge.

#13 @youknowriad
4 years ago

  • Owner set to youknowriad
  • Resolution set to fixed
  • Status changed from new to closed

In 51003:

Block Editor: Introduce block templates for classic themes.

With this patch, users will be able to create custom block based templates
and assign them to specific pages/posts.

Themes can also opt-out of this feature

Props bernhard-reiter, carlomanf.
Fixes #53176.

#14 @SergeyBiryukov
4 years ago

In 51014:

Block Editor: Declare the wp_template post type as built-in.

Remove redundant show_in_admin_bar property, which defaults to the value of show_in_menu.

Follow-up to [51003].

See #53176.

#15 @SergeyBiryukov
4 years ago

In 51015:

Posts, Post Types: Remove some unused strings from built-in post type declarations.

The customize_changeset, wp_block, and wp_template post types are not displayed in the admin menu or in the admin bar.

If they do get added, the labels can just fall back to name and singular_name until separate strings are required for more flexible translations.

Follow-up to [38810], [44146], [51003].

See #53176.

ockham commented on PR #1267:


4 years ago
#16

@ockham I wanted to add that I had to change the "prepare_for_database" function in the REST API controller because the create_item unit test was failing (compare it with Gutenberg). I think the failure was legitimate and we probably should fix that function similarly in Gutenberg as well, though not sure why the unit test was not failing there.

(Just wanted to note this comment somewhere so I don't forget about it :P )

Working on a fix here: https://github.com/WordPress/gutenberg/pull/32233

#17 @jorbin
4 years ago

In 51144:

Block Editor: Prevent duplicate queries

When passing args to WP_Query::__construct method (in this case, but creating a new WP_Query, this one internally executes the WP_Query::get_posts method and stores the result in the WP_Query::posts property. When calling the WP_Query::get_posts again, the same SQL query gets executed, and the result is again stored in the WP_Query::posts property.

This was introduced in [51003].

Props david.binda, jorbin.
Fixes #53280. See #53176.

#18 @youknowriad
4 years ago

Thanks for the commit @jorbin we should probably do the same thing in the Gutenberg plugin (for plugin users, as this is overriden there)

#20 @jorbin
4 years ago

In 51321:

Remove unnecessary function_exists check in get_the_block_template_html

WordPress can be confident that WordPress functions exist.

I forgot this function existed.
And I thought that it would fatal, but it didn't
And it was so nice
So peaceful and quiet
I forgot this function existed
It isn't love, it isn't hate, it's just indifference

Introduced in [51003].

Props walbo.
Fixes #53578. See #53176.

#21 @jorbin
4 years ago

In 51325:

Remove unnecessary function_exists check in get_the_block_template_html

Merges [51321] to the 5.8 branch. Reviewed by SergeyBiryukov.

WordPress can be confident that WordPress functions exist.

I forgot this function existed.
And I thought that it would fatal, but it didn't
And it was so nice
So peaceful and quiet
I forgot this function existed
It isn't love, it isn't hate, it's just indifference

Introduced in [51003].

Props walbo.
Fixes #53578. See #53176.

Note: See TracTickets for help on using tickets.