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

Fix post featured image scaling on the front #46838

Merged
merged 7 commits into from
Jan 13, 2023

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented Jan 2, 2023

What?

In the block editor, the post featured image <figure> wrapper has a set height. This height was missing on the front, so the image did not look the same in the editor and front.
This pr adds the height to the front, both to the <figure> and the <a> which is used when the image is linked.

Closes #42322

Why?

The image did not look the same in the editor and front when image height and scaling was set in the block settings.

How?

Updates index.php and adds the height to the get_block_wrapper_attributes and the <a> when applicable.

Testing Instructions

When testing this we need to take extra care to make sure that the border and border radius settings still work.

  1. Make sure that your WordPress install has a few recent posts with a lot of text content and a featured image. - This is needed because we are going to use a query loop and use the long content to stretch the image.
  2. Create a new post.
  3. Copy the example code from the issue into the post:
<!-- wp:query {"queryId":33,"query":{"perPage":3,"pages":0,"offset":0,"postType":"post","order":"desc","orderBy":"date","author":"","search":"","exclude":[],"sticky":"","inherit":false}} -->
<div class="wp-block-query"><!-- wp:post-template -->
<!-- wp:columns {"backgroundColor":"cyan-bluish-gray"} -->
<div class="wp-block-columns has-cyan-bluish-gray-background-color has-background"><!-- wp:column {"width":"50%"} -->
<div class="wp-block-column" style="flex-basis:50%"><!-- wp:post-title {"isLink":true,"fontSize":"x-large"} /-->

<!-- wp:post-excerpt {"fontSize":"large"} /--></div>
<!-- /wp:column -->

<!-- wp:column {"width":"50%"} -->
<div class="wp-block-column" style="flex-basis:50%"><!-- wp:post-featured-image {"isLink":true,"height":"100%","align":"wide"} /--></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
<!-- /wp:post-template --></div>
<!-- /wp:query -->
  1. The featured image should have a 100% height and "scale" set to cover, and stretch the height of the content.

  2. Save and view the front. Confirm that the image looks the same on the front.

  3. Continue to test the remaining settings. Confirm that the image looks correct on the front for each step.
    For example:

  • Enable and disable the linked image setting
  • Set the image width to 45px
  • Change the scale to contain and fill (The scale settings only show if width or height are adjusted).
  • Change the border styles including radius
  • Add an overlay

Testing Instructions for Keyboard

Screenshots or screencast

@carolinan carolinan added [Block] Post Featured Image Affects the Post Featured Image Block [Type] Bug An existing feature does not function as intended labels Jan 2, 2023
@carolinan carolinan marked this pull request as ready for review January 2, 2023 11:04
@carolinan carolinan requested a review from ajitbohra as a code owner January 2, 2023 11:04
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Hi @carolinan, thank you for fixing this issue 👍
I left some comments that I think we need to address before the merge but other than that, the PR looks good.

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 fixing this issue @carolinan!

It tests well for me when:

  • setting various combinations of explicit or relative widths
  • adding borders, including a radius
  • adding an overlay

After adding the safecss_filter_attr calls, it looks like we have a couple of redundant esc_attr calls now that we can clean up. After that, this should be good to go, nice work ✨

Editor Frontend
Screenshot 2023-01-09 at 7 58 17 pm Screenshot 2023-01-09 at 7 58 09 pm

packages/block-library/src/post-featured-image/index.php Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jan 10, 2023

Flaky tests detected in 7aa8a0c.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3880242668
📝 Reported issues:

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 the tweaks @carolinan 👍

This is still testing well for me, and the code looks good. Nice work!

@carolinan carolinan merged commit e0f919c into trunk Jan 13, 2023
@carolinan carolinan deleted the update/post-featured-image-wrapper-height branch January 13, 2023 04:58
@github-actions github-actions bot added this to the Gutenberg 15.1 milestone Jan 13, 2023
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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Featured image: height set is not being inherited correctly on the frontend
3 participants