-
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 for blocks via theme.json #41972
add box-shadow support for blocks via theme.json #41972
Conversation
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @madhusudhand! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
Have you given any thought to the options we would offer to users to create their own shadows? |
What do you think about providing a set of different shadows that users can select from - so they'd provide a string which we interpret in the back end? |
Thanks for all the work here, there's a lot of useful exploration...
This seems like it might be worth exploring but I think it would be good to get feedback from @mtias before going too much further... |
b08bada
to
998fdc8
Compare
I think this is going in a good direction, just a couple of suggestions:
|
998fdc8
to
890766b
Compare
890766b
to
f45417c
Compare
f45417c
to
7340102
Compare
This is ready for another review. |
I see this warning: |
@scruffian Is there a filter on the Editor Frontend (react) ? inside the script block
Final transformed style
|
I think this is where we are appending that class, @madhusudhand |
7340102
to
0cd8faf
Compare
@scruffian @MaggieCabrera Fixed the issue with site-editor. This is ready for another review. |
This is looking good, but I'm having some trouble testing. Please could you rebase the branch from the latest trunk? |
0cd8faf
to
c9ef8c7
Compare
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
reduce specificity for default button shadow update schema for shadow fix lint issues
ced7f2b
to
3626d3c
Compare
What?
This change will add box-shadow support for blocks in theme.json
Why?
Currently it is not possible to add box-shadow to a block such as buttons or groups, and having the support will eliminate the need to add custom css in block themes.
How?
It is the simplest form of implementation to support
box-shadow
throughtheme.json
PROPERTIES_METADATA
VALID_STYLES
This implementation accepts box-shadow as a string as per the CSS Spec
Testing Instructions
In
color
section, add a new propertyshadow
to any block.Screenshots or screencast
Further thoughts and questions
Before this approach, gave it a try to add support for an
object
as follows instead of astring
value.However the
box-shadow
doesn't have anylonghand
properties to configure. Yet it could be solved by assigning them to css variables and constructingbox-shadow
out of them.It works, and it will be in the same style as that of border. However it supports only single box-shadow. 😞
why do we need to support multiple shadows.
multiple box shows to solve the scenarios like
Double border effect with multiple shadows
box-shadow: 8px 8px 0px -2px white, 8px 8px 0px 0px green
Or A much smoother shadow with the help of multiple shadows (instead of border and shadow)
box-shadow: 0px 4px 8px -2px rgba(9, 30, 66, 0.25), 0px 0px 0px 1px rgba(9, 30, 66, 0.08)
Any may more scenarios.
It would be simpler to use
string
to support multiple shadows. The UI (when implemented) should read the value, split it to appropriate values and render the UI, and vice versa.