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

Consider changing some theme.json settings to opt-out #36187

Closed
kjellr opened this issue Nov 3, 2021 · 20 comments · Fixed by #36246 or #36646
Closed

Consider changing some theme.json settings to opt-out #36187

kjellr opened this issue Nov 3, 2021 · 20 comments · Fixed by #36246 or #36646
Assignees
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] In Progress Tracking issues with work in progress

Comments

@kjellr
Copy link
Contributor

kjellr commented Nov 3, 2021

There are a number of useful theme.json settings that are off by default: Padding, Margin, Line Height, etc. These are generally self-contained controls: they "just work" without the theme having to provide any code other than the opt-in.

It can be difficult for themes to remember to opt-into all of these, and to keep up with the new ones that have been added. We should consider changing these sorts of settings to be opt-out instead whenever it is viable.

For reference: Twenty Twenty-Two opts into the following by default:

"link": true,
"spacing": {
    "blockGap": true,
    "customMargin": true,
    "customPadding": true,
},
"typography": {
    "customLineHeight": true,
},
"border": {
    "customColor": true,
    "customRadius": true,
    "customStyle": true,
    "customWidth": true
}

Of those, the only one that I think may be problematic to have active by default is blockGap. I'd love to hear thoughts from other theme authors on the topic.

Additionally, if the opt-out approach is not viable for some of these options, let's also consider allowing themes to opt-in to all available settings via a new theme.json entry. Maybe something like "appearance": "all", for example. This way they can implement with one line of code, and can stay up to date as new features are added.


Adding to the 11.9 milestone, since it would be good to have an answer one way or another by then.

@kjellr kjellr added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi labels Nov 3, 2021
@kjellr kjellr added this to the Gutenberg 11.9 milestone Nov 3, 2021
@leeshadle
Copy link

I like this. It would also be cool if we could opt-in/out of theme.json settings via a css variable, or some other variable mechanism, so we could provide a plugin or theme settings UI for turning these controls on/off. Maybe controls like this are slated for global styles?

@overclokk
Copy link

From my point of view I prefer to have all opt-in by default and than opt-out with theme.json.

@andersnoren
Copy link

I agree. Given that some features require blockGap to be enabled, like the columns gutter setting, I think it makes sense to have that be opt-out in 11.9 (and 5.9) as well.

@fabiankaegy
Copy link
Member

fabiankaegy commented Nov 4, 2021

Opt-in vs opt-out is a hard question. On the one hand, we should strive to make new features discoverable and get them out to the users as easily as possible. This will be the easiest with the opt-out approach. It comes with one bigger risk though.

When new features get added to Core and a Theme has not tested compatibility there is a larger risk that an update to Core causes something on your site to break.

This risk is not one that really applies to the perfect block-based theme that doesn't contain custom CSS and functionality. But when you have these customizations especially in Hybrid Themes it is more risk that each Core update brings.

I'm not saying that we should not do it. I only fear that it would contribute to the already existing fear of updates.

When a theme first needs to test whether it works correctly with the new functions that is a much safer route.

To me personally, the approach @kjellr outlined at the end with maybe a property like "appearance": "all" is a reasonable middle ground where themes that want to support everything as soon as possible can opt into that while others and especially hybrid themes have some more time to test and validate the features before they get activated.


Edit: I think my point mostly stands for new things that may get introduced further in the future. I agree that the items outlined in the original comment are things that could be enabled by default. I only worry about how this will work with future additions.

@overclokk
Copy link

@fabiankaegy I think that if the settings are opted-in by default they will not break anything because are just settings you can use in the UI, for example the setting for custom color only add a link to the color picker.

@fabiankaegy
Copy link
Member

@overclokk I may totally be misunderstanding something here. But id I look at the items being discussed here in this PR we are talking about the overall supports like wether or not a theme supports custom line heights. Or the block gap. At least right now there is no UI to change these settings and if a theme has its own custom line heights set in css it would probably cause issues with the core way or adding the custom line height. Therefore a theme may want to disable that. But if new features like it get introduces and are on by default the updating experience would be that your site may have broken things because the theme does not play well jet with the new option that was introduced.

If it were an opt in mechanism the user could update and only when the theme is ready it could roll out the feature.

@luminuu
Copy link
Member

luminuu commented Nov 4, 2021

I agree with @fabiankaegy about the "fear of updates" and breaking changes. Especially for CSS, which is often adjusted manually on a website to override the theme's default, this could be not very easy to debug if suddenly the block editor adds new stuff and the custom CSS may not be very specific, thus resulting in partly wrong display of elements.

How about defining a standard set of things that are activated by default, which can be opted-out if needed. For any new options in the future, make them opt-in so theme authors can update their code over time. Maybe the options can be revisited in the future to see if they should be opt-in instead of opt-out.

But for whatever approach we might end up with, I'd always suggest an up-to-date documentation about all options and making them clear if they're opt-in or opt-out for each WP version. Having a clear reference would probably solve many questions around this topic.

@fabiankaegy
Copy link
Member

@luminuu That is a great point. If we go with an opt-out approach where new features may be added at any time that is on by default it would be very important to have one place where all the new options that can be opted out or are listed so that themes can disable everything first to test wether it works and then rollout the updates on their own terms.

@scruffian
Copy link
Contributor

There are a couple more that might make sense to make opt-out if that's the way we go:

theme.json->settings->spacing->units
theme.json->settings->typography->customFontSize

@kjellr
Copy link
Contributor Author

kjellr commented Nov 4, 2021

For the sake of clarification: I didn't clearly mention this above, but let's discuss just the above settings (not any unknown future settings) as being opt-in by default only for themes that have theme.json. Yes, some of the settings above could theoretically wreak havoc on older themes, but let's set that aside for now and just start here.

I think the core of the issue represented here is a division between two types of themes:

  1. Themes which provide basically no additional CSS. (Like Twenty Twenty-Two)
  2. Themes which provide a lot of extra CSS. (For example, a classic theme that also uses a theme.json file)

For themes in the first category, there's very little downside to opting into all of this. If something breaks, it's not because of the theme, it's because of Gutenberg. For themes in the second category, opting into these settings might conflict with custom styles or settings the theme has implemented.

A number of the settings above are designed to be relatively self-contained. As in, they should work universally, and they do not require themes to do anything to get them to work:

  • Padding
  • Margin
  • Line Height
  • Spacing Units
  • Custom Font Sizes
  • Border

Some of these may conflict with what a theme is doing. For example, if your theme uses a ton of !importants or has some really aggressive margin rules. But in my experience, these are relatively unlikely to break things for most modern themes. (Have you run into issues with these features and your current themes? Please share!)

Also, please remember that we're mostly just talking about whether or not to show these controls in the UI by default. If a user inserts a pattern from the pattern directory that uses a heading with custom line height and padding, it's going to have custom line height and padding when they insert it into your theme today, whether or not you opt-in to those features. Those settings are baked into the block markup! If you don't opt-in, the user won't be able to see those values, and they won't be able to adjust them.

@richtabor
Copy link
Member

I'm leaning towards having the default experience consisting of what Gutenberg has to offers in full, with a extra step needed to opt-out of features (as proposed here).

For themes in the first category, there's very little downside to opting into all of this.

I agree.

@fabiankaegy
Copy link
Member

fabiankaegy commented Nov 4, 2021

As I said in my original comment. I do agree very much that the opt-in by default nature is great for pure FSE themes.

It is hybrid themes that I am more worried about. For the properties that we are discussing here in this issue, I am not opposed to changing them to opt-in by default.

For me, it really is the example we are setting for future features and how we deal with releasing new options to a very wide spectrum of use-cases.

And while it is 100% the right approach for pure FSE themes I am not sure it is the right answer for the rest of themes that have adapted a theme.json already. And that will adapt it in the future.

Some of these may conflict with what a theme is doing. For example, if your theme uses a ton of !importants or has some really aggressive margin rules. But in my experience, these are relatively unlikely to break things for most modern themes. (Have you run into issues with these features and your current themes? Please share!)

Also, please remember that we're mostly just talking about whether or not to show these controls in the UI by default. If a user inserts a pattern from the pattern directory that uses a heading with custom line-height and padding, it's going to have custom line-height and padding when they insert it into your theme today, whether or not you opt-in to those features. Those settings are baked into the block markup! If you don't opt-in, the user won't be able to see those values, and they won't be able to adjust them.

That is correct. But not all themes support the pattern directory or support all the blocks in the pattern library. For more restricted client-facing hybrid themes there may be options like custom line height that are disabled altogether because there is a need defined in a brand design guideline that a certain ratio is always used. So that this is hardcoded in the CSS and there are specific spacing rules that should not be broken. An update like this would mean exposing a new setting that does not have any effect on editors or lead to sites that break out of contractual obligations because of an update that enabled something for the editors that the theme does not support.

I know these are also edge-cases. But ones that right now are out there just as much as block-based themes. So I want to share them also :)

@joesnellpdx
Copy link

joesnellpdx commented Nov 4, 2021

My concern is for those of us who have intentionally built fully customized themes for valuable clients and want to mitigate any risk of regression or disruption to said UI/UX from an automatic "opt in" on these types of granular block options, settings and styles.

My suggestion, per @fabiankaegy, is to have a universal "opt-in" like appearance": "all" to avoid these disruptions - which an be quite a challenge.

Additionally, for those of us who fully build out custom UI based on very specific Visual Design guidance, we often have to remove many of these types features to enable more of an intentional UX and limited editing choices and UX settings for our client's editorial teams.

I'd much prefer we look to these types of items as opportunities for intentional "opt-in" choice enhancement rather than automatically include assumed output that may not fit all types of users.

@colorful-tones
Copy link
Member

From my point of view I prefer to have all opt-in by default and than opt-out with theme.json.

I endorse this endeavor 💯

"appearance": "all"sounds good to me!

@kjellr
Copy link
Contributor Author

kjellr commented Nov 4, 2021

In the near term, it sounds like there’s general agreement that an appearance: all option for theme.json would be an acceptable idea. Is there someone available who can make a quick PR for that? (@oandregal maybe?)

It would be cool to get it into 5.9 if possible!

@colorful-tones
Copy link
Member

Agree, this would be super relevant and nice to have in 5.9 🤞

@oandregal
Copy link
Member

I've prepared #36246 in case we want to go ahead with this. Also left a few questions.

@ellenbauer
Copy link

From my experience so far it would be totally fine to enable the settings by default as long as it's possible to opt-out of individual ones if needed.

I personally don't think an "appearance all" option is needed, but if it could benefit someone and add more flexibility for a variety of user cases, why not add it.

@oandregal
Copy link
Member

Second try to add this at #36646

@oandregal
Copy link
Member

#36646 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] In Progress Tracking issues with work in progress
Projects
None yet