Allow creating custom block templates in classic themes #30438
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +827 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
4 tasks
youknowriad
requested review from
nerrad,
spacedmonkey and
TimothyBJacobs
as code owners
|
This PR is not ready yet but it's ready to get thoughts and designs. |
youknowriad
requested review from
scruffian,
jasmussen,
jameskoster,
mtias,
aristath,
carolinan and
a team
I think that would be necessary in many situations... and it should also be filterable so users can install a plugin to enable it on their old, not-updated theme. See PR #30465 for a suggestion |
17 tasks
|
Thinking about the theme-support thing some more, perhaps it should be opt-out instead of opt-in? 🤔 I believe opt-out would make more sense... (updated the PR on #30465 to reflect that) |
youknowriad
commented
Apr 2, 2021
* Change the prefix for custom templates * Update comment
* Introduce gutenberg_is_fse_enabled * typo & reverse logic * update filter name * Remove filter * Rename function * don't use "disable"theme-supports * rename theme-support
|
What an intriguing PR! I am using the Twenty Twenty One theme. Then followed the instructions added by Riad. |
|
If there is a theme.json file with template parts assigned to template areas, I think those header and footer template parts should be included, but it is too early to guess if these themes will include a theme.json or not. I think the UX needs to be improved and that the option needs to be integrated somehow with the existing "page attribute" where you select templates, but this would work for a POC and first version. |
youknowriad
requested review from
ajitbohra,
chrisvanpatten and
ntwb
as code owners
|
I've added an e2e test and a paragraph to docs to opt-out of this and also added some site blocks (site title, logo, tagline) to the default template. I agree that the flows and design need to be iterated on. What do you think. Ship it or not? |
Yes!! |
|
Then click the green button :P |
aristath
approved these changes
Apr 8, 2021
youknowriad
force-pushed
the
add/template-support-classic-themes
branch
from
3853828
to
d64964a
Compare
|
From my perspective this feature is a good one to have. Merging a bare bones version seems like a good idea. Of course there are a lot of followups based on the tests that I shared that needs to be done, but that is something one gradually can get to through various PR's. Thanks for creating the PR, Riad! |
youknowriad
force-pushed
the
add/template-support-classic-themes
branch
from
d64964a
to
7006394
Compare
2 tasks
youknowriad
added
the
Needs Dev Note
25 tasks
vindl
reviewed
May 4, 2021
| @@ -177,3 +177,4 @@ function register_site_icon_url( $response ) { | |||
| add_filter( 'rest_index', 'register_site_icon_url' ); | |||
|
|
|||
| add_theme_support( 'widgets-block-editor' ); | |||
| add_theme_support( 'block-templates' ); | |||
There was a problem hiding this comment.
@youknowriad I find this part a bit unexpected. Why are we unconditionally declaring theme support in the plugin/core? 🤔
If we want to have this active by default, we could remove the theme support part altogether, and modify the activation checks to use the filter for opt-out, instead of:
Theme authors can opt-out of this feature by removing the
block-templatestheme support in theirfunctions.phpfile.
There was a problem hiding this comment.
This is something I explain here #30465 (comment)
Instead of having a "disable-something" theme support, Core prefer to have theme supports enabled by default and plugins/themes can do remove_theme_support as it reads better. Adding a theme support calls "disable-something" doesn't read very well.
There was a problem hiding this comment.
Thanks for clarifying! While reading the activation conditions I assumed that this will be only enabled for themes that explicitly declare support, so I was a bit surprised afterward when I found out in testing that it actually works for all themes.
There was a problem hiding this comment.
yes, I the moment, this is opt-out because we want people to experience this, depending how the betas of WP go though, we might reconsider.
There was a problem hiding this comment.
To close the loop, #32858 makes it opt-in on classic themes and opt-out for themes using theme.json.
15 tasks
youknowriad
removed
the
Needs Dev Note
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea of this PR is to allow creating custom block templates (starting blank) for a given page in classic themes. It's a way for classic themes to enable some FSE features without any change.
How to test it
It's very rough at the moment, more a POC, but here's how you do it:
Now, on the frontend, your page is using its own custom template and not the regular
page.phporsingular.phpthat comes from the theme itself.Notes
This PR can be seen as a migration strategy for classic themes, it highlights a few things in terms of code/architecture:
Todo