-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add box shadow support to blocks (using code editor for now). #46896
Conversation
97d2369
to
acacb73
Compare
Flaky tests detected in 70a2e72. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3956915668
|
@@ -136,6 +136,17 @@ final class WP_Style_Engine { | |||
), | |||
), | |||
), | |||
'shadow' => array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this boxShadow, so we can also have textShadow later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do this in a different PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh, we should do this in a follow up as shadow
is used in a number of places that are merged already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually thinking about it a bit more, the same shadow presets can work for both text and box shadows, so I'm not sure we should rename it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like they don't work.
Text shadow doesn't have spread
prop and supplying a box-shadow value to text-shadow doesn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok let's keep shadow here and if we add text shadow in the future they will live under typography.
lib/block-supports/shadow.php
Outdated
return array(); | ||
} | ||
|
||
$has_shadow_support = _wp_array_get( $block_type->supports, array( 'shadow' ), false ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use block_has_support
here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, missed that, done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Could folks confirm if this needs a backport for 6.3? I couldn't find the relevant change in WordPress core. Should it be in kses.php? |
Yes this should be backported. |
@madhusudhand or @glendaviesnz are you able to backport this change? |
What?
Adds support for shadow via templates.
shadow
block prop.Testing Instructions
page-title
usingshadow
orstyle
props in the templates.Screenshots or screencast