Make WordPress Core

Opened 3 months ago

Closed 2 months ago

#60633 closed defect (bug) (fixed)

Shadow: add shadow control to appearanceTools opt-ins

Reported by: madhudollu's profile madhudollu Owned by: swissspidy's profile swissspidy
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch needs-testing fixed-major dev-reviewed
Focuses: Cc:

Description

This is a ticket to track the backport of the following Gutenberg PR to WordPress 6.5:

https://github.com/WordPress/gutenberg/pull/58766

Description:

It enables to control default preset shadows in the UI via appearanceTools

this change adds array( 'shadow', 'defaultPresets' ) to APPEARANCE_TOOLS_OPT_INS

Change History (23)

This ticket was mentioned in PR #6178 on WordPress/wordpress-develop by @madhudollu.


3 months ago
#1

  • Keywords has-patch has-unit-tests added

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

It is a backport change for the following Gutenberg change:

https://github.com/WordPress/gutenberg/pull/58766

It adds array( 'shadow', 'defaultPresets' ) to appearance tools opt ins. and is also expected to merge in WordPress 6.5 (Beta 3)

#2 @swissspidy
3 months ago

  • Component changed from General to Editor

The PR and ticket are labelled as an enhancement and we're past the enhancement stage for 6.5. Can you elaborate why it is important to have this in 6.5 still?

#3 @swissspidy
3 months ago

  • Milestone changed from Awaiting Review to 6.5
  • Type changed from enhancement to defect (bug)

#4 @swissspidy
3 months ago

  • Owner set to swissspidy
  • Status changed from new to reviewing

#5 @youknowriad
3 months ago

  • Keywords commit added

This is not really an enhancement.

All new customization tools that are added to blocks are meant to be opt-in behind the appearanceTools flag which ensures that themes that use this flag will get these tools but the ones that choose to control manually all the available tools won't.

So I think we should commit this PR.

#6 @swissspidy
3 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 57717:

Editor: add shadow.defaultPresets to appearance tools opt-ins.

Props madhudollu.
Fixes #60633.

#8 @wildworks
2 months ago

I noticed that the Gutenberg PR was not fully backported. In the original PR, defaultPresets was changed from true to false in the default theme.json:

https://github.com/WordPress/gutenberg/pull/58766/files#diff-ba57fd27dc15a6079df31427f442f760cbe4035a941cfb1a4763c7cf90766b15R194

But [57717] does not include any theme.json changes, so defaultPresets remains true:

https://github.com/WordPress/wordpress-develop/blob/697bd4efd63941847e53e43daa3a23cd81c08cd3/src/wp-includes/theme.json#L194

This means that in the Classic theme, blocks are inadvertently provided with shadow control by default.

I think this issue should be resolved in WP6.5, but if I want to submit a patch/PR, should I check out the trunk branch of wordpress-develop?

#9 @swissspidy
2 months ago

Okay that is unfortunate. If you could open a PR agains trunk that would be great!

#12 @swissspidy
2 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#13 @swissspidy
2 months ago

  • Keywords needs-testing added; has-unit-tests commit removed

@swissspidy commented on PR #6255:


2 months ago
#14

What are the testing instructions for this?

@youknowriad commented on PR #6255:


2 months ago
#15

The testing instructions should be something like:

  • Use a classic theme that doesn't have a theme.json or doesn't have appearanceTools: true in its theme.json.
  • The shadow controls shouldn't appear on the inspector control (sidebar) of the button block.

@swissspidy commented on PR #6255:


2 months ago
#16

Thanks @youknowriad, I can confirm this then 👍

#17 @swissspidy
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 57827:

Editor: disable shadow.defaultPresets in default theme.json.

This is a follow-up to [57717] to ensure that classic themes without a theme.json file or without appearanceTools: true in theme.json do not have shadow controls by default.

Props wildworks, vcanales, madhudollu, swissspidy, youknowriad.
Fixes #60633

@wildworks commented on PR #6255:


2 months ago
#18

  • Use a classic theme that doesn't have a theme.json or doesn't have appearanceTools: true in its theme.json.
  • The shadow controls shouldn't appear on the inspector control (sidebar) of the button block.

Thank you. This is correct.

@swissspidy commented on PR #6255:


2 months ago
#19

Committed to trunk in https://core.trac.wordpress.org/changeset/57827

Needs double sign-off (dev-reviewed) from another committer for backporting to 6.5

#20 @swissspidy
2 months ago

  • Keywords fixed-major dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport to 6.5, sign-off required.

#21 @youknowriad
2 months ago

  • Keywords dev-reviewed added; dev-feedback removed

@youknowriad commented on PR #6255:


2 months ago
#22

Added the "dev-reviewed" tag. This LGTM

#23 @swissspidy
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 57828:

Editor: disable shadow.defaultPresets in default theme.json.

This is a follow-up to [57717] to ensure that classic themes without a theme.json file or without appearanceTools: true in theme.json do not have shadow controls by default.

Reviewed by youknowriad.
Merges [57827] to the to the 6.5 branch.

Props wildworks, vcanales, madhudollu, swissspidy, youknowriad.
Fixes #60633

Note: See TracTickets for help on using tickets.