I think this is part of #58298
CC @madhusudhand

Edit: @carolinan Just saw you engaged already in the specific PR and you're probably aware of the issue. Sorry, please ignore then.

I got confused because looking at #58766 it only addressed theme.json, not classic themes.

@madhusudhand
Applying the PR keeps the shadow option in classic themes, but there are no presets. How do classic themes add presets? How do classic themes opt-out completely?

Adding this to the 6.5 board for likely discussion and perhaps some quick fixes as this impacts adding appearance tools to classic themes. cc @tellthemachines and @andrewserong.

@annezazu this feature is independent from appearance-tools; as I understand it this issue is about the inability to opt out of it in classic themes. The shadow feature has been in core for about a year though, so this can't strictly be classed as a regression.

@tellthemachines, while the shadow feature has been in core for a long time, I think the control at the block level in the block inspector is new, and landed only a few weeks ago: #57654 — so it might be something that needs following up on. I'll just ping @vcanales and @mikachan for visibility, too.

Oh good point @andrewserong, then I guess this is still eligible to fix post-Beta.

Yes! Sorry for not clarifying further. Those changes landed very recently and details matter.

Sure, my main point is this issue is unrelated to appearance-tools. The shadow controls will always display in classic themes whether appearance-tools are enabled or not.

Looks like another PR landed last week that then consolidated shadow effects into the Border panel in #58466. It'd be good to confirm with @vcanales and @madhusudhand what the intended behaviour is, since there's been a bit of movement with these controls, and I wonder if that's a factor at play here 🤔

Update: I wonder if the problem is that the core lib/theme.json file has settings.shadow contain values and a set of default shadow presets:

This means that hasShadowControl will always be true, as the default state of the core theme.json is that it's (effectively) switched on because of that default value:

I'm not too sure what the fix could be here, given that default value (typically appearanceTools works by the default value being off or false, and then appearanceTools injects the expected truthy value). But hopefully that might help hone in on where the problem is 🙂

@madhusudhand Could you clarify what you see as the next steps here? 🙏

@andrewserong Thank you for the inputs. I am taking a look at theme-json-resolver for a possible fix by conditionally setting the shadow to false when there is no theme.json

I am not sure yet, but I guess the core function get_classic_theme_supports_block_editor_settings also needs to be modified to add few settings such as enableShadowPresets for classic themes, using which the default and custom shadow presets can be enabled.

Please share your thoughts on this approach.

cc: @getdave

Update: Created #58967 to test this approach.

Thanks for opening #58967 @madhusudhand! That approach looks close to me, but to neaten things up, I think it might work better if on classic themes it set settings.shadow.defaultPresets to false so that the logic is consolidated surrounding that existing flag. For that to work, though, there's a subtle UI bug that'll need to be fixed, which is that the shadow controls shouldn't be displayed if there are no presets available. Let me know what you think, happy to help however I can!

• Make shadow support opt-in by default (matches other block supports e.g border)
• Enable shadow support via appearanceTools
• Only display shadow control if the control is enabled in settings, the block has shadow support, and there are available presets.
• Do not display control for reset purposes due to edge cases.

@madhusudhand Have you managed to make any progress with this yet?

We're 1 week out from RC 1 and it seems like landing Shadow support in a manner that is inconsistent with other supports is undesirable.

@andrewserong has offered support with reviews so perhaps the work could be divided up?

@getdave I believe this one can likely be closed out now that #58766 has been merged. @madhusudhand was there anything else that needed to be done before we close this?

@getdave @andrewserong This issue is closed out by #58766. There isn't anything required for classic themes.

Marking it as resolved.