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

Image Block: Target only smaller screen sizes for image block fix #1109

Merged
merged 4 commits into from
Oct 20, 2020

Conversation

laurelfulford
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

This PR fixes issues introduced in #1058 -- those changes were intended to fix visual issues in mobile Safari (gaps between images and their captions), but they are causing some visual issues on desktop with smaller images. Since the original issue only occurs on smaller screens, the best approach seems to be to target those smaller screen sizes win the original fix.

Closes #1065

How to test the changes in this Pull Request:

  1. Set up a post with eight smaller images (less than 300px wide) -- they can all be the same image. Add a caption to every other one that is longer, so it would be wider than the image. Align two center, two to the left, and two to the right -- you should end up with an image block with and without a caption for each alignment: none, center, left and right.
  2. View on desktop; note some odd alignment or spacing issues with the centered, left and right images -- the centered image caption won't be centered (and the centered image itself won't be centered if AMP is not enabled). The left and right images with captions have them stick out past the images:

Centre-aligned with AMP and caption:

image

Centre-aligned without AMP:

image

Left aligned with caption (is flush-left with AMP disabled):

image

Right aligned with caption (is flush-left with AMP disabled):

image

  1. Apply the PR and run npm run build.
  2. Refresh the page; note that the above images now display as expected:

Centred image with caption:

image

Left-aligned image with caption:

image

Right-aligned image with caption:

image

  1. View on an iPhone, with AMP enabled (since it greatly increases the odds of the original issue occuring); confirm that the gap image between images and captions can't be reproduced.

Note: There will still be some minor issues with small centred images -- the captions will extend past the edges of the image. I think this is a fair trade off, as that exact issue hasn't been reported yet with the current version of the fix in place, and it will be less likely to happen (since on smaller screen sizes you'd need a dang tiny image to notice it):

image

One work around would be to centre the caption text in that case, but I think, since it would look out of place unless the original image was quite small -- it's best to leave it left aligned.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@laurelfulford laurelfulford added the [Status] Needs Review The issue or pull request needs to be reviewed label Oct 14, 2020
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

👐

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Oct 20, 2020
@laurelfulford
Copy link
Contributor Author

🎉

@laurelfulford laurelfulford merged commit 0f8a7bc into master Oct 20, 2020
@laurelfulford laurelfulford deleted the fixes/1065-image-block-display branch October 20, 2020 15:05
matticbot pushed a commit that referenced this pull request Oct 20, 2020
# [1.21.0-alpha.1](v1.20.2...v1.21.0-alpha.1) (2020-10-20)

### Bug Fixes

* correct dropdown colors for Newspack Katharine ([#1110](#1110)) ([01ee559](01ee559))
* correct dropdown colors for Newspack Sacha ([#1111](#1111)) ([3748d6f](3748d6f))
* make pullquote citations inherit correct color ([#1116](#1116)) ([f537fd6](f537fd6))
* make sure accent header in the editor matches the front end ([#1112](#1112)) ([11e4970](11e4970))
* prevent captions from getting cut off with image-beside setting ([#1113](#1113)) ([2d779a8](2d779a8))
* target only smaller screen sizes for image block caption fix ([#1109](#1109)) ([0f8a7bc](0f8a7bc))

### Features

* add option to turn off Yoast's primary category functionality ([#1104](#1104)) ([95083c7](95083c7))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.21.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Oct 20, 2020
# [1.21.0](v1.20.2...v1.21.0) (2020-10-20)

### Bug Fixes

* correct dropdown colors for Newspack Katharine ([#1110](#1110)) ([01ee559](01ee559))
* correct dropdown colors for Newspack Sacha ([#1111](#1111)) ([3748d6f](3748d6f))
* make pullquote citations inherit correct color ([#1116](#1116)) ([f537fd6](f537fd6))
* make sure accent header in the editor matches the front end ([#1112](#1112)) ([11e4970](11e4970))
* prevent captions from getting cut off with image-beside setting ([#1113](#1113)) ([2d779a8](2d779a8))
* target only smaller screen sizes for image block caption fix ([#1109](#1109)) ([0f8a7bc](0f8a7bc))

### Features

* add option to turn off Yoast's primary category functionality ([#1104](#1104)) ([95083c7](95083c7))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.21.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Small centered images not centering
3 participants