Regression with theme.json `settings.shadow.defaultPresets`

[ajlende]

#60633 introduced a regression in shadow defaultPresets. Details and discussion in ​WordPress/gutenberg#59989.

// Relevant parts of theme.json
{
  "settings": {
    // `appearanceTools` is not set
    "shadow": {
      // `defaultPresets` is not set
      "presets": [
        {
          "name": "Natural",
          "slug": "natural",
          "shadow": "2px 2px 3px #000"
        }
      ]
    }
  }
}

/* Generated CSS before */
--wp--preset--shadow--natural: 6px 6px 9px rgba(0, 0, 0, 0.2);

/* Generated CSS after */
--wp--preset--shadow--natural: 2px 2px 3px #000;

• Reverts core theme.json settings.shadow.defaultPresets to true to fix the regression. (Back to working the same as color/gradient preset configuration for block themes.)
• Adds editor-shadow-presets and default-shadow-presets for classic themes to configure shadows. (Works the same way as color/gradient preset configuration in classic themes.)

Backport to Gutenberg: ​https://github.com/WordPress/gutenberg/pull/60000
Gutenberg issue: ​https://github.com/WordPress/gutenberg/issues/59989
Trac ticket: https://core.trac.wordpress.org/ticket/60815

[swissspidy]

Sounds like we've had nothing but problems with this whole change.

@get_dave @youknowriad would appreciate your thoughts as editor tech leads here.

To me there's too much last minute back and forth here and I don't feel comfortable with making yet another change here that isn't a simple revert. Also, there definitely need more unit tests here, or even an e2e test.

​https://github.com/WordPress/gutenberg/issues/59989 indicates this is more an edge case, so I'd say this is a candidate for 6.5.1.

[ajlende]

I don't feel comfortable with making yet another change here that isn't a simple revert.

The one line change in ​theme.json is sufficient to fix the regression.

The intention of setting shadow.defaultPresets to false was to allow classic themes to opt-in to them, and the rest of the changes that are being discussed are to support that the correct way.

If we only include the one line change, the effect will be that classic themes don't have the option to configure shadows and the default shadows will appear in the block editor for classic themes.

[swissspidy]

In https://core.trac.wordpress.org/ticket/60633#comment:8 it was explicitly suggested to make it true, which then was changed in [57827]. This is the back and forth I mean.

[jorbin]

I'm putting this in 6.5 for visibility, but I share @swissspidy's concern about the back and forth. I think in the end this might be better placed in 6.5.1

Alternative to #6295 without the editor-shadow-presets theme support feature.

• Reverts core theme.json settings.shadow.defaultPresets to true to fix the regression. (Back to working the same as color/gradient preset configuration for block themes.)

The color/gradient support behavior that I think we should be matching enables support for the presets in classic themes by default and allows themes to opt-out. For example, gradients can be opted-out of by setting add_theme_support( 'editor-gradient-presets', array() ).

If we're okay with shadows being opt-out in the UI like colors/gradients, we can merge just this and add the ability to opt-out with add_theme_support( 'editor-shadow-presets', array() ) that's included in #6295 as a follow-up in 6.6.

Trac ticket: https://core.trac.wordpress.org/ticket/60815

[poena]

Is this summary accurate?

If we don't do anything, then:
1) Neither classic themes or block themes will have shadows enabled by default.

• This is a change from 6.4.3 where block themes had shadows by default for buttons in the (global) Styles setting sidebar.
• This is a problem because the defaults should not change without a theme.json version change(?)

2) There is a problem in any themes that has a theme.json, when a developer registers a custom shadow presets with the same slug as the core preset.
3) Classic themes without theme.json can not set their own presets.
4) Classic themes without theme.json can opt-in with the appearance-tools theme support.
5) No action is required today for the bundled themes.

If we merge 6303, then:
1) Both classic themes and block themes will have shadows enabled by default.
2) The problem with the duplicate slugs would be solved(?)
3) Any themes can opt out or set custom presets via theme.json.
4) Classic themes without theme.json can not set their own presets (this will be fixed in 6.6).
5) Classic themes without theme.json can not opt-out (this will be fixed in 6.6).
6) The shadows must be tested with the classic bundled themes. There should not be conflicts but this needs to be confirmed with testing.

If we merge 6295, then:
1) Both classic themes and block themes will have shadows enabled by default.
2) The problem with the duplicate slugs would be solved(?)
3) Any themes can opt out or set custom presets via theme.json.
4) Classic themes without theme.json can set their own presets.
5) Classic themes without theme.json can opt-out.
6) The shadows must be tested with the classic bundled themes. There should not be conflicts but this needs to be confirmed with testing.

[poena]

Classic themes only:
With both 6303 and 6295, there is a problem with the label of the border and shadow control for the image block and button block. These two blocks have a border radius control that is not dependant on border theme support.

The label says "Shadow" when it also contains the border radius.
This is also confusing because the default control is the radius, the shadow control has to be toggled on in the panel settings.

Is there a Gutenberg PR that addresses this and will it be ready in time?

[poena]

Shadows applied in Twenty Ten

[poena]

Twenty Eleven with shadows applied, front

[poena]

Twenty Twelve, front

[poena]

Twenty Thirteen, front

[poena]

Twenty Fourteen, front

[poena]

Twenty Fifteen

[poena]

Twenty Sixteen, front

[poena]

Twenty Seventeen, front

[poena]

Twenty Nineteen, front

[poena]

Twenty Twenty, front

[poena]

Twenty Twenty-One, front

[poena]

Twenty Twenty-Two, front

[poena]

Twenty Twenty-Three, front

[poena]

Twenty Twenty-Four, front

[poena]

Because of the spacing issues, I wouldn't say that it was good experience using the shadows. They even make some of the blocks overlap.

But, the styling can be improved in any release, I don't consider it a blocker for 6.5.

Alternative to ​https://github.com/WordPress/wordpress-develop/pull/6295

This does not enable default shadow presets when the theme does not provide their own shadow presets.

Instead default shadow presets can be explicitly opted into with the default-shadow-presets or appearance-tools theme supports in classic themes.

Trac ticket: https://core.trac.wordpress.org/ticket/60815

Flagging this to @swissspidy and @dream-encode as a PR that may needing a commit prior to WP 6.5 release.

@getdave see the discussion on the trac ticket. To me there's too much back and forth on this issue and expected behavior seems unclear at this point. That's why I suggested 6.5.1 for this change. But let's discuss on the ticket please.

[ajlende]

But let's discuss on the ticket please.

The Github issue predates the trac ticket, it has more context, and I asked for discussion in the GH issue at the top of the trac ticket when I opened it. I think it makes more sense as the place for discussion.

​https://github.com/WordPress/gutenberg/issues/59989

[swissspidy]

@ajlende GH issue is fine too, but not a PR.

Looks like we're converging towards focusing on #6309, so closing this one.

Looks like we're converging towards focusing on #6309, so closing this one.

[swissspidy]

In 57885:

Editor: disable shadow.defaultPresets for classic themes.

With this change default shadow presets are never shown for classic themes, and classic themes have no options for adding custom ones.
This essentially reverts [57717] and [57827] / [57828], which had unintended consequences.

Props ajlende, oandregal, madhudollu, swissspidy, get_dave, andrewserong, desrosj.
Fixes #60815.

Committed in https://core.trac.wordpress.org/changeset/57885

[audrasjb]

Approving this for 6.5 backport 👍

[swissspidy]

In 57889:

Editor: disable shadow.defaultPresets for classic themes.

With this change default shadow presets are never shown for classic themes, and classic themes have no options for adding custom ones.
This essentially reverts [57717] and [57827] / [57828], which had unintended consequences.

Reviewed by audrasjb.
Merges [57885] to the 6.5 branch.

Props ajlende, oandregal, madhudollu, swissspidy, get_dave, andrewserong, desrosj.
Fixes #60815.