Skip to content
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

Enable theme supports automatically for FSE theme #35593

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

youknowriad
Copy link
Contributor

Related #26901

We're introducing new kind of themes so it's a good opportunity to rethink what should be the defaults for themes. The idea is that ideally block themes shouldn't need to have a functions.php file at all to work properly. And if they need some config/theme supports, theme.json should be preferred.

The latter (theme.json config) is not supported yet in this PR, it will open a new kind of issues as Right now we use theme-supports to generate the final theme.json config used as a last resort cc @oandregal while we want to do the opposite for other theme supports: theme.json informs the value of theme supports. I think the idea here would be to have a ThemeConfig object/model that is the result of both theme.json and theme supports at the same time, and that model should be used consistently in WP's code base instead of relying directly on theme.json file or theme supports.

Questions

  • What do you think of the theme supports I chose to enable for all FSE themes?
  • Are there more theme supports to add?
  • Should we remove any?

@youknowriad youknowriad added [Type] Enhancement A suggestion for improvement. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Feature] Full Site Editing labels Oct 13, 2021
@youknowriad youknowriad self-assigned this Oct 13, 2021
@aristath
Copy link
Member

These look good to me!
Could we also add support for loading separate styles for rendered-blocks only?
In WP 5.8 we didn't do it because there were no block themes, but in 5.9 where we'll also have a check about whether a theme is block-based or not, we should default to loading separate styles for block themes:

// Opt-in to only load styles for rendered blocks.
add_filter( 'should_load_separate_core_block_assets', '__return_true' );

Note that this is currently done manually in the TT2 theme for example, and it should be the default out of the box

@mtias
Copy link
Member

mtias commented Oct 13, 2021

Excellent. We should probably do an issue to keep track of what would be good defaults. Most of these features have been rolled out incrementally and opt-in, as things get settled, good defaults are crucial.

@aristath

This comment has been minimized.

@jasmussen
Copy link
Contributor

jasmussen commented Oct 13, 2021

Just personally I appreciate this, as it furthers the thing I've appreciated the most about developing a new theme: all the complexity and boilerplate code that's beeen absorbed by the system, rather than being something I have to shepard along from theme to theme.

In addition to those mentioned, I have automatic-feed-links (https://codex.wordpress.org/Automatic_Feed_Links) as well.

I do wonder if we can't remove wp-block-styles from the list, though. This one opts into the extra theme.scss styles, which provide a handy shortcut for themes being upgraded to support the myriad of new blocks without having to provide new styles for each. However as you are starting work on a new block theme, and especially as some of the properties currently stored in theme.scss can be absorbed by theme.json, this one might likely end up unnecessary. And you can easily opt into that one separately as well. What do you think?

@youknowriad
Copy link
Contributor Author

Updated this:

  • Added the styles optim.
  • Added the automatic feeds.
  • Removed the wp-block-styles.

@jasmussen
Copy link
Contributor

Sweet!

@jasmussen
Copy link
Contributor

@youknowriad I realize the following use case is not something to cater to, as I'm fully aware of how hacky and temporary it is. However in case it has legitimate side effects, I wanted to share.

Until I can fix them at the source, I'm loading a low-specificity stylesheet into the editor, like so:

add_action('init', function() {
	wp_register_style('editor-hacks', get_template_directory_uri() . '/editor-hacks.css');
 
	register_block_type('mocco/editor-hacks', [
		'style' => 'editor-hacks',
	]);
});

As of this PR, that file no longer gets loaded at all. I checked to verify that the commit before this worked fine. Can you tell if this is just me doing it wrong (very possible since I know it's hacky), or whether there's a legitimate issue here?

@youknowriad
Copy link
Contributor Author

It's definitely related to this PR, basically this #35593 (comment) ensures that we only load block styles when the block is being used and your random block is not being loaded.

I think you should consider alternatives why not just enqueue the stylesheet directly? Why does it need to be a block?

@jasmussen
Copy link
Contributor

I think you should consider alternatives why not just enqueue the stylesheet directly? Why does it need to be a block?

Oh it's super hacky which is why I couched my comment so thoroughly. The sample in question was intended to load a stylesheet into the editor with minimal specificity (without the .editor-styles-wrapper prefix) so as to fix a few issues in the editor with low specificity. Stuff I put here was stuff I always meant to come back to fix upstream, which it has been now, so I don't actually need to load it this way anymore.

Thanks for the context.

noisysocks added a commit to noisysocks/wordpress-develop that referenced this pull request Dec 8, 2021
By default, block themes should have a few theme supports enabled by
default. These are: post-thumbnails, responsive-embeds, editor-styles,
html5, automatic-feed-links.

This is a backport of WordPress/gutenberg#35593.
@paaljoachim paaljoachim added the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 8, 2021
@noisysocks noisysocks removed the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 13, 2021
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Dec 14, 2021
By default, block themes should have a few theme supports enabled by default. These are: `post-thumbnails`, `responsive-embeds`, `editor-styles`, `html5`, `automatic-feed-links`. This changeset backports the changes introduced in WordPress/gutenberg#35593.

Props noisysocks, ocean90, youknowriad, audrasjb, hellofromTonya.
Fixes #54597.

Built from https://develop.svn.wordpress.org/trunk@52369


git-svn-id: http://core.svn.wordpress.org/trunk@51961 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this pull request Dec 14, 2021
By default, block themes should have a few theme supports enabled by default. These are: `post-thumbnails`, `responsive-embeds`, `editor-styles`, `html5`, `automatic-feed-links`. This changeset backports the changes introduced in WordPress/gutenberg#35593.

Props noisysocks, ocean90, youknowriad, audrasjb, hellofromTonya.
Fixes #54597.

Built from https://develop.svn.wordpress.org/trunk@52369


git-svn-id: https://core.svn.wordpress.org/trunk@51961 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants