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

Feedback on adoption approach for exposing editable UI for the Style Book for Classic themes #68036

Open
annezazu opened this issue Dec 16, 2024 · 18 comments
Labels
[Feature] Style Book [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In discussion Used to indicate that an issue is in the process of being discussed [Type] Feedback Issues that relate purely to feedback on a feature that isn't necessarily actionable

Comments

@annezazu
Copy link
Contributor

annezazu commented Dec 16, 2024

Context:

As of Gutenberg 19.9, the Style Book is now exposed to Classic themes. As it stands, support is available for Classic themes that either support editor styles via add_theme_support( 'editor-styles' ) or have a theme.json file. Without either, the Style Book doesn't display anything useful. Here's a quick demo using Twenty Twenty-One:

tt1.style.book.mov

In particular, the current thinking is that by having a theme.json file in a Classic theme that this marks an explicit opt in and, to quote @jasmussen, "edibility is progressive, insofar as if your theme.json is empty, or virtually empty, little to nothing would be editable. But for each array you add, whether that array is empty or not, would unlock part of the interface." For example, if you add settings.typography options, this would then give a user access to the UI for Typography and, potentially in the future, the font library.

With all of this in mind, another PR is open to enable the Style Book regardless of whether a classic theme has theme.json or supports editor styles! All of this begs the question and points to needing to get right the opt in and opt out approach to provide the most value when it comes to exposing editable UI to users of classic themes. This issue seeks to gather that feedback to ensure we can come to the best decision possible.

Feedback needed

From what I can see we have two main tension points:

  • Do we rely on the presence of editor styles or theme.json to opt in OR expose to all and provide a way to opt out?
  • Is there value in showing a non editable Style Book and is that an experience users will understand? In the comments on this issue, the desire for this is clear but I am noting this for comprehensiveness as there's a concern that this could be confusing for users in situations where nothing is editable.
  • Does the presence of a theme.json with settings automatically opt a theme into exposing parts of the global styles UI related to those settings or is an explicit opt in necessary to expose any of the global styles UI, in addition to a theme.json?

Please share feedback on the current approach and the desired approach you'd like to see. cc @WordPress/outreach & @WordPress/block-themers for good measure.

@annezazu annezazu added [Feature] Style Book [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] In discussion Used to indicate that an issue is in the process of being discussed [Type] Feedback Issues that relate purely to feedback on a feature that isn't necessarily actionable Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Dec 16, 2024
@ironprogrammer
Copy link
Contributor

Speaking from experience, in my agency days doing (now "classic") custom themes I typically created a private page as a style guide, which was quite similar to the Style Book. It wasn't intended for editing, but for visual reference and to ensure we had all the major elements covered. There may or may not have been liberal use of Save+Refresh while dialing in the stylesheet 🫣.

I like the idea of having this as a utility for any classic theme. It shows off some coolness that block themes get, but also reveals what elements/blocks you might have missed in your styles. It could be a subtle hint to upgrade to theme.json or a block-based theme to get all the benefits.

For those used to working with classic themes, I don't think there's much of an expectation of being able to live-edit with a design "preview" feature like this.

@tellthemachines
Copy link
Contributor

the current thinking is that by having a theme.json file in a Classic theme that this marks an explicit opt in and, #66851 (comment) @jasmussen, "edibility is progressive, insofar as if your theme.json is empty, or virtually empty, little to nothing would be editable. But for each array you add, whether that array is empty or not, would unlock part of the interface." For example, if you add settings.typography options, this would then give a user access to the UI for Typography

I think it needs to be very clear that, in its current iteration, the style book doesn't include any editing UI.

What is being discussed for future iterations is how to expose parts of the global styles UI for hybrid themes that have a theme.json. There are a couple possibilities:

  • The presence of a theme.json with settings should automatically expose parts of the global styles UI related to those settings.
  • An explicit opt in should be necessary to expose any of the global styles UI, in addition to a theme.json.

Based on the feedback in #41119, I'm more inclined towards the second option, as theme authors may want to leverage theme.json for styling purposes without necessarily exposing edit functionality. Having controlled editability is one of the reasons for creating a hybrid theme instead of a block theme in the first place.

I'm very curious to hear what other folks have to say on the topic!

@annezazu
Copy link
Contributor Author

Ah yes! Let me update to make that far clearer. Thank you.

@annezazu annezazu changed the title Feedback on adoption approach for exposing the Style Book for Classic themes Feedback on adoption approach for exposing editable UI for the Style Book for Classic themes Dec 17, 2024
@t-hamano
Copy link
Contributor

I think the important thing here is that theme.json is not taken into account to determine if the current theme is a block theme, i.e. if the global styles UI is available. If only the templates/index.html and style.css files exist, the theme is treated as a block theme and can access the site editor and global styles.

Considering that, relying on the existence of theme.json is at least a little confusing to me. Also, classic theme developers may develop themes based on theme.json but not want the global style UI to be available. In fact, I also develop many classic themes with theme.json for clients as a freelancer, but the purpose is to define block styles without adding CSS styles, and I do not want the global style UI to be automatically exposed.

Personally, I prefer the following approach:

  • Expose Style Book for all classic themes. Explain with more detailed text that the Style Book cannot be edited.
  • Add theme support for classic themes to be able to use the global style UI. For example, something like add_theme_support( 'global-sryles-ui' ).

Or, to show/hide some UI, we might be able to consider the following approach in theme.json:

{
	"version": 3,
	"settings": {
		"typography": {
			"showGlobalStylesUI": false
		},
		"color": {
			"showGlobalStylesUI": true
		}
	}
}

@jasmussen
Copy link
Contributor

I've prefixed most of my feedback on this issue that I do not feel strongly about this, and think that folks who feel more strongly about this, should decide. So I'm happy to support either direction. I did want to respond to this:

Considering that, relying on the existence of theme.json is at least a little confusing to me.

The motivation for relying on the existance of theme.json comes down to a few aspects:

  1. Followup updates to the Style Book are meant to progressively unlock features for classic themes. Add a palette array, and you get edit access to color palettes. Add a font sizes array and you get access to editing typography, and get access to the font library. Etc.
  2. Having the initial opt-in be about the presence of the theme.json file leans into the fact that this is the same file that you will need to edit, to make the style book editable.
  3. It avoids another theme support property to keep track on.
  4. It has this opt-in, as opposed to an opt-out, because I still think having Style Book as an interace that vaguely resembles the customizer, but isn't editable at all, is going to be more confusing than not having it.

Ultimately, I do feel strongly about letting users opt into editable aspects of the style book. But whether the style book itself is opted into our out of, I'll defer despite the stated not-so-strong opinion.

@carolinan
Copy link
Contributor

carolinan commented Jan 2, 2025

In my opinion, the correct way would be for the new feature to be opt-in, because the majority of older classic themes have not been tested with block styles.
It should be explicitly enabled with add_theme_support.

By not having it opt-in there is a big potential to introduce a broken feature and a confusing feature on many live websites.
Please remember that editor style for classic themes was introduced in WordPress 3.0. I think that makes it inappropriate to use for progressively unlocking new features.

(By the way, testing the feature today, with Gutenberg trunk it does not even work, there is no preview and there is a JS error on the color palette.) This has since been resolved

@carolinan
Copy link
Contributor

carolinan commented Jan 2, 2025

I think the important thing here is that theme.json is not taken into account to determine if the current theme is a block theme, i.e. if the global styles UI is available. If only the templates/index.html and style.css files exist, the theme is treated as a block theme and can access the site editor and global styles.

Correct, theme.json is not a required file for block themes.

@Mamaduka
Copy link
Member

We could also make it opt-in using extra arguments for editor-styles.

Example:

add_theme_support( 'editor-styles', array(
    'style-book' => true,
) );

@BowFarmer
Copy link

What happens if the parent classic theme does not have templates/index.html and style.css files but a child theme does?

@Mamaduka Mamaduka moved this to ❓ Triage in WordPress 6.8 Editor Tasks Jan 16, 2025
@Mamaduka Mamaduka moved this from ❓ Triage to 🗣️ In Discussion / Needs Decision in WordPress 6.8 Editor Tasks Jan 16, 2025
@carolinan
Copy link
Contributor

carolinan commented Jan 17, 2025

After re-reading the discussion in #67782,
My opinion remains that the basic stylebook that only has the preview should be enabled when:

  • A classic theme has a theme.json
  • When there is no theme.json and the theme enables a new dedicated theme support.

But not when the classic theme only has the editor style theme support, because of how long this has been available and because in my opinion, it is unlreated.

All classic themes should be able to opt-out, even if it has a theme.json. For example if the theme developer has already added their own style guide.

But the theme support as well as the check for it needs to be added in way that allows the theme developer to extend the functionality once it is implemented.

I mean as discussed above, if the theme.json file includes typography settings, the typography should be available.
If the developer used the theme support, they need to be able to add the typography as a parameter.

Because we don't want to have to change the formatting of the theme support later because all planned features were not implemented yet.

@Mamaduka
Copy link
Member

I haven't checked in a while, but we already enabled some theme support flags based on theme.json. The same can be done for style-book.

It would be nice to avoid some implicit logic based on what theme.json declares. The options are always hard to keep track of and can be confusing.

A classic theme with a theme.json file

The editor-styles theme support is augmented with style-book set to true. Themes can opt out of Style Book support.

A classic theme without a theme.json file

The style-book isn't set (considered false). Themes can opt-in the Style Book support.

@t-hamano
Copy link
Contributor

I agree with the approach of being explicitly enabled or disabled via theme support.

By the way, I remember seeing an issue somewhere considering exposing the global styles to the classic theme. Taking this into consideration, I think one of the following approaches might be the best:

add_theme_support( 'editor-styles', array(
	'style-book'    => true,
	'gloabl-styles' => true,
) );
add_theme_support( 'site-editor', array(
	'style-book'    => true,
	'gloabl-styles' => true,
) );

@carolinan
Copy link
Contributor

yeah using "site-editor" to plan for future changes sounds like a good idea

@Mamaduka
Copy link
Member

Using the site-editor keyword for non-block themes could be confusing. What are the cons of re-using existing editor-styles theme supports?

@carolinan
Copy link
Contributor

With the additional parameters, the fact that editor-styles has been used for many years is not a hindrance anymore 👍 .

But they are not editor styles. The default block styles and the global styles are applied to blocks both in the editor and front.
What other things could be considered, the font library? Also not editor styles.

@carolinan
Copy link
Contributor

carolinan commented Jan 28, 2025

Current:
https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-support/

Also:
remove_theme_support( 'block-templates');

@github-project-automation github-project-automation bot moved this from 🗣️ In Discussion / Needs Decision to ✅ Done in WordPress 6.8 Editor Tasks Jan 28, 2025
@carolinan carolinan reopened this Jan 28, 2025
@carolinan carolinan moved this from ✅ Done to 🗣️ In Discussion / Needs Decision in WordPress 6.8 Editor Tasks Jan 28, 2025
@t-hamano
Copy link
Contributor

I don't have a strong opinion, but the reason I want to avoid editor-styles is that, as defined in this code, the feature is defined as Whether theme opts in to the editor styles CSS wrapper..

If we want to be more abstract, how about add_theme_support( 'editor', array()) or add_theme_support( 'style-book' )?

@Mamaduka
Copy link
Member

Don't have a strong opinion on the name as long as it conveys the meaning. Having a single new support key + flag options seems more future-proof. I wish I'd thought about that while working on template part support.

the reason I want to avoid editor-styles is that, as defined in this code, the feature is defined as Whether theme opts in to the editor styles CSS wrapper..

@t-hamano, this came up recently in a different bug report - #68887.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Style Book [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In discussion Used to indicate that an issue is in the process of being discussed [Type] Feedback Issues that relate purely to feedback on a feature that isn't necessarily actionable
Projects
Status: 🗣️ In Discussion / Needs Decision
Development

No branches or pull requests

8 participants