-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
StyleBook: Make available in all classic themes #67782
base: trunk
Are you sure you want to change the base?
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@@ -130,7 +130,7 @@ function gutenberg_styles_wp_die_handler( $default_handler ) { | |||
* @global array $submenu | |||
*/ | |||
function gutenberg_add_styles_submenu_item() { | |||
if ( ! wp_is_block_theme() && ( current_theme_supports( 'editor-styles' ) || wp_theme_has_theme_json() ) ) { | |||
if ( ! wp_is_block_theme() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave @tellthemachines to chime in with a more educated review, but I was just wondering what the implication of this comment from #66851 (comment) would be:
because if the theme has neither, the style book will be useless.
Does these mean if a theme doesn't have editor or theme.json styles, the style book would just render the default styles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does these mean if a theme doesn't have editor or theme.json styles, the style book would just render the default styles?
That's what I thought at first too. But as mentioned in this PR, it seems there's a way to apply styles for blocks on the editor side other than theme.json or editor-styles support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced we should do this, as having a condition means themes have the choice to opt in or out. If themes really want the stylebook, it costs nothing to add theme support for editor-styles. Twenty Twenty has it even though it enqueues its editor styles via enqueue_block_assets
as in your example.
Flaky tests detected in 6e47b6c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12252303353
|
Thanks for the reviews, everyone! First, should we discuss whether / how to provide a way to opt in or out of the StyleBook? Personally, I feel that it's best to enable the StyleBook for all classic themes and that the opt-in/opt-out feature itself is unnecessary. Here's why:
What do you think? |
Thanks for the PR.
This is a key discussion to have, and conceptually feels valid enough, especially for WordPress users that know what they are doing. These might already know what the customizer is, what the site editor is, what widgets are, and what the role of each of those is. But for someone entirely new, coming to a section called "Style book" and being able to do nothing at all, I just have a feeling that will be confusing. Even if we added explanatory text in the sidebar, most people in my experience, don't "read the manual" unless they have to. And even if they did, they might ask, "what's the point then?" This sounds like a strong opinion, it's not, and I'm happy to defer. But one of the opportunities we have now, is testing this in the plugin. The outreach group has already been pinged on the other PR, so hopefully this will be tested in the plugin and we'll have some broader feedback. Notably, the plugin also runs on WordPress.com, where support may hopefully gather some input. CC: @annezazu for awareness. All that is to say, it may be good to let this one sit for a bit, while we wait for what's already shipping to gather a little feedback, after which point we may be better equipped to decide. What do you think? Without data, however, my instinct would still go towards: if you allow block styles to be editable, you get the style book. Immediately this would make the point of the style book clear and it would make the tool useful. Could this happen without opt-in? Why not? |
Follow-up #66851
See this discussion: #66851 (comment)
What?
This PR enables the stylebook regardless of whether a classic theme has theme.json or supports editor styles.
Why?
I've noticed that editor-related styles can be added via code like this for example, which is not related to whether you have a theme.json or not, and whether it supports editor styles or not:
It probably makes more sense to cover all the possibilities and make it available to all classic themes, rather than adding complex conditional statements.
Testing Instructions
http://localhost:8889/wp-admin/
.