Skip to content
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 featured image #59616

Conversation

colinduwe
Copy link
Contributor

What?

This PR adds box shadow support to the Post Featured Image Block so it has the same controls as the core Image block.

Why?

This PR resolves #59067 and brings more parity between the Image and Post Featured Image blocks.

How?

This PR adds block support for box shadows. The shadow style needs to be applied to the image tag inside the figure tag like the border styles. The selectors have been specified so that global styles from theme.json and elsewhere are also applied to the image rather than the figure.

In order for this to work, the shadow.php needed to be updated to support __experimentalSkipSerialization. Without it, the box shadow style is printed to the figure.

NOTE: This PR would potentially affect any themes that were providing a shadow style definition to the core/post-featured-image. Currently, that style is applied to the figure. With this PR it is applied to the image. This allows the box shadow to work in conjunction with a border radius.

Testing Instructions

  1. Open a post or page
  2. Add a featured image
  3. Add a Featured Image Block
  4. Add a box shadow style. It should be visible in the editor.
  5. Adjust any of the other style controls. They should all work in harmony.
  6. Test the result on the front end.

Testing Instructions for Keyboard

  1. Open a post or page
  2. Add a featured image
  3. Add a Featured Image Block
  4. Navigate to the sidebar with the keyboard
  5. Navigate to the Border and & Shadow section
  6. Open the kabob menu to enable the shadow with the keyboard
  7. Add a box shadow style. It should be visible in the editor.
  8. Adjust any of the other style controls. They should all work in harmony.
  9. Test the result on the front end.

Screenshots or screencast

https://github.com/WordPress/gutenberg/assets/2152870/f833d4ff-56d3-4bbe-a3de-22aeb2bae4cc
(Note that I've added a shadow to my theme.json to demonstrate that users can override global styles)

…support for __experimentalSkipSerialization to the shadow support
Copy link

github-actions bot commented Mar 5, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: colinduwe <colind@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: bph <bph@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Mar 5, 2024
Copy link

github-actions bot commented Mar 5, 2024

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @colinduwe! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@bph
Copy link
Contributor

bph commented Mar 7, 2024

Thank you, @colinduwe, for tackling this issue with your PR!
I tested on my local site, setting box shadow for Feature images on Archive Pages and for the Single post template. Both displayed the selected box shadow on the front end and in the editor.

@aaronrobertshaw would you have time to review on the PR?

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this PR together @colinduwe, nice work! 👍

I have a couple of quick thoughts and suggestions I've added as inline comments below.

I'm also currently away at WordCamp Asia until later next week when I can take this for a proper spin and dig a bit deeper.

In the meantime, hopefully, the comments below help! 🤞

lib/block-supports/shadow.php Outdated Show resolved Hide resolved
packages/block-library/src/post-featured-image/block.json Outdated Show resolved Hide resolved
@colinduwe
Copy link
Contributor Author

@aaronrobertshaw Updated the PR to removed the changes to the /supports/shadow.php. That's now a separate PR #59887 and I removed the extraneous selector.

Note that this PR will not work correctly without the other PR.

@aaronrobertshaw aaronrobertshaw added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label Mar 15, 2024
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for iterating on this @colinduwe 🙇

Note that this PR will not work correctly without the other PR.

In cases like this it can be a good idea to base this PR's branch off the new PR's so it all works or update the PR description so the test instructions to highlight how to test it.

That said, it's trivial to cherry pick the other PR's commit, rebase, or checkout an earlier commit on this branch. So just something to keep in the back of your mind.


I've rebased this PR locally for testing after merging #59887 and it works nicely for me. Great work! ✨

✅ Box shadow is correctly applied to inner img in the editor
✅ Box shadow is applied to inner img on the frontend
✅ Theme.json applied box shadows for featured images works
✅ Global Styles applied box shadows work for featured images
✅ Feature image placeholder displays box shadow in the editor

LGTM 🚢

@aaronrobertshaw aaronrobertshaw added the [Type] Enhancement A suggestion for improvement. label Mar 15, 2024
@aaronrobertshaw aaronrobertshaw merged commit 1f2bb25 into WordPress:trunk Mar 15, 2024
59 of 60 checks passed
@github-actions github-actions bot added this to the Gutenberg 18.0 milestone Mar 15, 2024
@jasmussen
Copy link
Contributor

Perhaps unintentionally, this PR changed how border and border-radius are applied to the Featured Image block. Before this PR, values were applied to the img element:

img element code

This meant you could apply a border and radius to the image, and it would properly affect the apeparance:

before the pr

This PR changed that so the styles are now applied to the wrapping figure element instead:

figure element code

That means the border radius doesn't affect the image inside at all:

after this pr

CC: @aaronrobertshaw @colinduwe @bph

It's not clear to me whether there's an easy fix, hopefully we can just point the selector to the img again.

@colinduwe
Copy link
Contributor Author

I can't seem to reproduce what you're showing. When I run Gutenberg/trunk locally the border and shadow styles are correctly applied to the image element as inline styles. That behavior is controlled in

"__experimentalBorder": {
"color": true,
"radius": true,
"width": true,
"__experimentalSelector": "img, .block-editor-media-placeholder, .wp-block-post-featured-image__overlay",

Because this PR adds
"selectors": {
"shadow": ".wp-block-post-featured-image img, .wp-block-post-featured-image .components-placeholder"
},

It might be possible that it's interfering. The fix would perhaps be to move that selector stuff from inside the supports section for borders into the separate selectors section.
@jasmussen since I can't replicate on my end, would you please try that and let me know if it changes anything?

On the front end, border and shadow styles should be removed from the figure element by "__experimentalSkipSerialization" and then added to the image element in index.php. Can you confirm that all's well on the front end?

@aaronrobertshaw
Copy link
Contributor

aaronrobertshaw commented Mar 26, 2024

There's a fix up for the border issue in #60184. If you have time to review and test it @colinduwe, that would be great! 🙏

Apologies for being a bit slow to note the fix here, while it was a simple tweak, I found another issue with the overlay being broken that isn't so straightforward.

@jasmussen
Copy link
Contributor

Looks all solved. Thank you both!

@colinduwe
Copy link
Contributor Author

Looks good to me as well. Thanks @aaronrobertshaw

carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Featured Image Affects the Post Featured Image Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Box Shadow support for featured image
5 participants