Use theme mods for templates and template parts

[carlomanf]

Description

Partially resolves #31954 and partially resolves #31397 (some background to this in comments there.)

To fully resolve these issues, a UI will need to be built. This branch does not attempt the UI, but implements a necessary change in the underlying infrastructure before any such UI will be possible.

The idea behind the approach is to use theme_mods to store the "slugs" of templates and template parts, rather than the post_name of the posts themselves.

This makes it trivial to assign any existing template or template part to any newly activated theme, using set_theme_mod. It doesn't even need to have the same slug in both themes. For example, you could have the same template be single in one theme and single-post in another theme.

To do the same with the existing system is near impossible, because you quickly run into issues with slug conflicts.

The API endpoint for templates and template parts can now be used in two ways: (removed for now)

1. active theme name // slug (existing, for fetching active templates and template parts)
2. empty string // post ID (new, for fetching inactive templates and template parts)

How has this been tested?

Tested out creating new templates in the template editor and the site editor. They correctly resolve on the front end and they are stored in the database as intended.

Screenshots

Types of changes

Nothing should break, just clears the way for new features to be added later.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @carlomanf! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[aristath]

Hey there @carlomanf, thank you for working on this PR! It does simplify things a lot and from a quick look I took, the code makes sense...
There are quite a few failures in phpunit tests, could you please fix those and make sure we're OK on that front?

[carlomanf]

There are quite a few failures in phpunit tests, could you please fix those and make sure we're OK on that front?

Yes, I was just looking at that myself. I think they're failing because they are expecting the templates to be classified using the taxonomy and post_name, rather than through theme mods. I can see if I can rewrite the tests to expect the changed format.

[carlomanf]

There are quite a few failures in phpunit tests, could you please fix those and make sure we're OK on that front?

Yes, I was just looking at that myself. I think they're failing because they are expecting the templates to be classified using the taxonomy and post_name, rather than through theme mods. I can see if I can rewrite the tests to expect the changed format.

I think this test class is failing because it switches theme to tt1-blocks before it creates the template that is not intended for tt1-blocks, so the template gets stored with tt1-blocks anyway. Suggestions?

[carlomanf]

No idea why Gutenberg_REST_Templates_Controller_Test::test_update_item is failing after the merge from trunk. I tested manually and changing the title of a template works fine.

[carlomanf]

Just copying in a comment from elsewhere for reference:

I don't think the current solution is scalable because taxonomies and slugs offer too many features that are not suitable for this purpose, nor are those systems aware of the constraints that are needed here. For example, the use of get_the_terms()[0] is problematic, because if a template somehow has two theme terms, and one is deleted, it can suddenly change theme. That could then trigger a slug conflict that causes the front-end to render differently.

If the taxonomy makes it into 5.8, I can see a potentially complicated and risky database migration needing to be done in a later version, and it would be nice to avoid that.

Also see this comment for further rationale.

@youknowriad @ockham Given the feature freeze, are there any amendments that should be made to this patch to make it eligible for 5.8? (for example, temporarily disabling the additional API endpoint?)

[youknowriad]

I've added this to the 5.8 board to get it properly reviewed and considered. I need to take some time to dive here and understand it properly and its impacts. Also @ockham for thoughts here.

I'm also pinging @TimothyBJacobs here because he shared some concerns about using theme mods for the Site logo block, while it might not be the same thing here, it's always good to have more perspective.

[getdave]

Hi @carlomanf. Thank you so much for your continued work on this.

It was raised in Core Editor chat and a few folks said they'd really appreciate it if you were able to summarise the latest state of the discussion including the current key points that are requiring decisions/discussion (if any).

The threads (and related Issues) are quite long and it would really help folks to feel able to contribute here. I'd completely understand if you didn't have time but anything you could do would go a long way.

Thanks again 🙇

[carlomanf]

This comment has a good overview of the main reason for the delay.

In the past week, I re-tooled the approach such that the theme mod update is no longer being done automatically via the action hook, but can be done manually through a new function called gutenberg_customize_template. The advantage of this approach is the theme mod update is omitted by default, therefore it doesn't affect the XML import, but it can be included wherever it is required (for example, the REST API.) The function also has a few boolean parameters that can be used to vary the way it behaves in different scenarios.

There do still exist some relevant bugs that I'm aware of, but they are already present in trunk and not introduced by this branch. The purpose of this branch is not really to fix those bugs, but to make a preliminary change to the data model. In other words, when you test this branch, you should expect no behaviours to change and everything to work the same as trunk.

There will also be the need for a migration from the old data model to the new, which I guess would be best done in a separate branch.

[westi]

Dropping in here to leave some thoughts after this was surfaced up to me.

From an end-user perspective i think putting templates/template parts into theme_mods is a backwards step for users because it is too tightly joined to the current theme and switching themes will end up breaking users sites in difficult to resolve and annoying ways.

I know historically there was a move away from using post_meta but I think actually post_meta (and potentially leveraging "spare" fields in the posts table - guid/mime_type - to help with limiting the list of post_ids we query against post_meta for) is a better solution than either taxonomies/theme_mods.

[carlomanf]

@westi

From an end-user perspective i think putting templates/template parts into theme_mods is a backwards step for users because it is too tightly joined to the current theme and switching themes will end up breaking users sites in difficult to resolve and annoying ways.

That already happens with the current implementation, so it's essentially what I was addressing with my earlier point:

There do still exist some relevant bugs that I'm aware of, but they are already present in trunk and not introduced by this branch. The purpose of this branch is not really to fix those bugs, but to make a preliminary change to the data model. In other words, when you test this branch, you should expect no behaviours to change and everything to work the same as trunk.

From your comment, it's unclear how satisfied/dissatisfied you are with the current implementation, since you described this branch as a "backwards step" but then you also indicated you were not in favour of taxonomies.

For me, it's less the wp_theme taxonomy that I have a problem with, and more the use of post_name to store the template identifiers. See this comment for the main 4 reasons that I don't like the current implementation. Particularly, reason number 4 shows how the use of post_name causes inconvenience with template parts, and reason number 3 shows how post_name is even more problematic for templates.

If the database schema allowed for metadata to be stored on term relationships, I could see a way forward while still using the wp_theme taxonomy, but otherwise, theme mods seem like the next best place to store the template identifiers. Potentially, the same data could be stored in term meta, but even that would still require the existing post_name data to be migrated, so I don't see any real advantage over theme mods.

[adamziel]

This is my summary of this PR:

WP has template parts and areas. Right now, template parts are the one to say "I am a footer in theme1." @carlomanf already listed many problems with this approach, like the fact that the name footer must be the same in all themes, or how switching back to a previous theme may not restore the previous arrangement of different template parts. Here's how I imagine that model:

Notice Part B can be assigned to either theme 1 or theme 2, but not to both. To me, this model seems unnatural: The template part does not have any knowledge about the nuances of all the different themes. The theme does, so the theme should be in charge. To me, this is exactly what this PR does:

Now it's the theme that chooses which template part goes where, the slugs does not have to match, and returning to a previous theme may easily preserve the last known arrangement of template parts.

I think that's the right direction to go. To me, this model is more flexible and bulletproof. In the future it could even support fancy things like having a different theme per site language. For that reason, I'm approving this PR. I'm happy to revisit my thinking if anyone is willing to share specific reasons for not moving forward here. CC @gziolo @youknowriad @mcsf @draganescu @noisysocks @talldan @ellatrix @jorgefilipecosta @kevin940726

I know you mentioned a few potential problems @westi, but I think these are the problems with the current model and not with theme mods. In other words, I have troubles thinking of specific ways this PR would contribute to breaking sites. Would you please elaborate?

[adamziel]

There is one conflict to resolve. Other than that, I have a few nitpicks related to how things are named but I don't want to block on them – this is an important PR that have been around for months and we can always tweak the naming in a follow up PR.

[draganescu]

I think @adamziel that the goal of having stricter association ability was to crete a semantic system for template parts. Your 1st drawing shows that: if theme 2's lower area si called "bottom" it is uncertain what kind of templates from other themes or plugins are available for it, except those who call themselves to be of kind bottom.

With the freedom of basically asigning anything to anything, like in your second drawing, we'll remove the semantic-ness ability of the system. The one immediate thing I worry about is that it will be nearly impossible to create a good UX that prevents users to browse through a dozen footers until they see a header.

So, if storing in theme mods does give us more flexibility, we should keep somehow the power of areas as indicators of what kind of template part one is (semantic).

[mtias]

Moving away from a wp_theme taxonomy is what I think we all generally agree. Whether that is back to a post meta key or to the theme mods mechanism I have less of a strong opinion on.

Conceptually we should not be coupling templates and template parts to themes. The current situation really puts too much weight on the theme, resulting content you created going missing when you switch themes. It's important to avoid this since one of the biggest benefits is the ability to retain parts of the site design while changing others. I should have easy access to my headers regardless of what theme I switch too.

What theme a template originated from, or are intended to be used with, can be useful information to craft the UX flows we need, so it needs to be present somewhere, but should not be a strong relationship requiring a full term taxonomy to map.

Some other requirements we have:

• It should be easy to overwrite templates or add more specific ones from a user perspective (i.e. a custom template based on "category" for "category-travel").
• It should be easy to revert things to defaults and should be clear when a template comes from a theme (the filesystem and not the user's db).
• It should be easy to use a header you created in theme A on theme B.
• All the user's templates and parts should be available with their corresponding semantics (template hierarchy and areas for parts) regardless of theme.
• Plugins should be able to add new templates, which should not be coupled to themes either.
  • A good example to have in mind would be WooCommerce template structure: it should be possible for a plugin like Woo to register some default templates and parts that map to their template hierarchy; a theme should be able to supply more specific ones; a user should be able to create their own.

[carlomanf]

With the freedom of basically asigning anything to anything, like in your second drawing, we'll remove the semantic-ness ability of the system. The one immediate thing I worry about is that it will be nearly impossible to create a good UX that prevents users to browse through a dozen footers until they see a header.

So, if storing in theme mods does give us more flexibility, we should keep somehow the power of areas as indicators of what kind of template part one is (semantic).

Areas for template parts still work the same as always, and could potentially be added for templates as well if it's deemed useful.

[carlomanf]

I have no choice but to close this because the migration of code between plugin and core particularly with #36201 is too confusing at this point, and I don't expect the long waits and accumulating merge conflicts will be alleviated at all when dealing with core, where patches have been known to take years to be merged. Discussion can move to #31954 and #31397 from this point forward.