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

@youknowriad youknowriad commented Oct 13, 2021

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?
@aristath
Copy link
Member

@aristath aristath commented Oct 13, 2021

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
Contributor

@mtias 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 was marked as off-topic.

@jasmussen
Copy link
Contributor

@jasmussen 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

@youknowriad youknowriad commented Oct 14, 2021

Updated this:

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

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Oct 14, 2021

Sweet!

@jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Oct 15, 2021

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment