Make WordPress Core

Opened 4 months ago

Closed 8 weeks ago

Last modified 8 weeks ago

#60784 closed enhancement (fixed)

Add __experimentalSkipSerialization support to shadow

Reported by: colind's profile ColinD Owned by: isabel_brison's profile isabel_brison
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

What?
This is a backport of the merged Gutenberg PR https://github.com/WordPress/gutenberg/pull/59887

When a dynamic block defines experimentalSkipSerialization in its block.supports.shadow the styles continue to be printed to the block wrapper element. This PR corrects that behavior so that shadow behaves like border, color, and others.

Why?
This provides the expected behavior for dynamic blocks that need to opt out of having the shadow styles printed to the block wrapper.

How?
Added a check at the start of the render function to return an empty array of the block has set experimentalSkipSerialization

(This is my first attempt at submitting a Gutenberg PR to be included back into WP Core so welcome any advice if I'm doing this wrong)

Change History (18)

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


4 months ago
#1

  • Keywords has-patch added

What?
This is a backport of the merged Gutenberg PR https://github.com/WordPress/gutenberg/pull/59887
When a dynamic block defines experimentalSkipSerialization in its block.supports.shadow the styles continue to be printed to the block wrapper element. This PR corrects that behavior so that shadow behaves like border, color, and others.
Why?
This provides the expected behavior for dynamic blocks that need to opt out of having the shadow styles printed to the block wrapper.
How?
Added a check at the start of the render function to return an empty array if the block has set
experimentalSkipSerialization

(This is my first attempt at submitting a Gutenberg PR to be included back into WP Core so welcome any advice if I'm doing this wrong)

Trac ticket: https://core.trac.wordpress.org/ticket/60784#ticket

#2 @swissspidy
4 months ago

  • Type changed from defect (bug) to enhancement
  • Version trunk deleted

#3 @ColinD
4 months ago

The wp_should_skip_block_supports_serialization() function is already in core and is used by other block supports features like border and color. This merely brings parity to shadow so core Guteberg dynamic block https://github.com/WordPress/gutenberg/pull/59616 as well as extenders' dynamic blocks can opt out of shadow style serialization.

#4 @youknowriad
4 months ago

This API (experimentalSkipSerialization) predates the experimental APIs discussion and is already in Core for other block supports. That said, we should probably stabilize this for all the block supports (and ensure the experimental flag still works but is deprecated). That said, it's probably independent work from the current ticket.

@madhudollu commented on PR #6279:


4 months ago
#5

@madhusudhand are you aware of any PRs in progress adding unit tests for the shadow support?

I have created https://github.com/WordPress/gutenberg/pull/60063 which tests these changes.

@vcanales commented on PR #6279:


2 months ago
#6

👋 @colinduwe @madhusudhand what is the status here?

It looks like https://github.com/WordPress/gutenberg/pull/60063 has been approved. Are you facing any blockers?

@ColinD commented on PR #6279:


2 months ago
#7

@vcanales I think it is probably only blocked by my ignorance of the process here. I thought this was merely awaiting approval from someone with commit access.

@aaronrobertshaw commented on PR #6279:


2 months ago
#8

It looks like https://github.com/WordPress/gutenberg/pull/60063 has been approved. Are you facing any blockers?

Once https://github.com/WordPress/gutenberg/pull/60063 is merged, this should probably also include the unit test the code change relates to.

@madhudollu commented on PR #6279:


2 months ago
#9

@colinduwe could you bring the unit tests from https://github.com/WordPress/gutenberg/pull/60063 into this?

@ColinD commented on PR #6279:


2 months ago
#10

@madhusudhand I think I'm out of my depth here. I'm not sure how to do that.

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


2 months ago
#11

  • Keywords has-unit-tests added

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

This pull request is a sub set of https://github.com/WordPress/wordpress-develop/pull/6279 and these changes are to be merged with it.

@madhudollu commented on PR #6279:


2 months ago
#12

@colinduwe I have created #6613 with the unit tests.
Please checkout #6613 to a branch and cherry pick the commit 8a4659608ba43363b4bd63fd76e99635c55141c8 into this PR

git fetch origin pull/6613/head:shadow-unit-tests
git cherry-pick 8a4659608ba43363b4bd63fd76e99635c55141c8

@ColinD commented on PR #6279:


2 months ago
#13

Your're a hero @madhusudhand. Thanks. Done.

#14 @isabel_brison
8 weeks ago

  • Milestone changed from Awaiting Review to 6.6

#15 @isabel_brison
8 weeks ago

  • Owner set to isabel_brison
  • Resolution set to fixed
  • Status changed from new to closed

In 58312:

Editor: Add __experimentalSkipSerialization support to shadow.

Checks if __experimentalSkipSerialization is set and returns early from wp_apply_shadow_support if so.

Props colind, madhudollu, aaronrobertshaw, vcanales, isabel_brison, swissspidy, youknowriad.
Fixes #60784.

@isabel_brison commented on PR #6279:


8 weeks ago
#16

Committed in r58312.

#17 @isabel_brison
8 weeks ago

In 58315:

Add ticket reference to test_wp_apply_shadow_support.

Props mukesh27.
Follows r58312.
See #60784.

#18 @isabel_brison
8 weeks ago

In 58318:

Remove trailing whitespace from test_wp_apply_shadow_support.

Follows r58315.
See #60784.

Note: See TracTickets for help on using tickets.