Fix - Image block: Aspect ratio not responding when dimensions are not set. #66217
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
juanfra
added
[Type] Enhancement
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +30 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
ramonjd
reviewed
Oct 21, 2024
|
@juanfra are you able to reproduce the issue with any other image apart from the one in that pattern? I tried with another rectangular image in an image block, both on its own and inside a Grid block, and I can't reproduce it. Is there something about that specific image that is causing the bug? |
@talldan if you're using the image block to add these other images you're testing with, you shouldn't be able to reproduce as the image block will set the dimensions for the image. The problem should be happening with images that don't have a width/height set, which will likely occur with patterns including images. |
|
I think that was meant for @tellthemachines |
Yes, sorry. I accepted the autocomplete too early. |
ajlende
approved these changes
Oct 21, 2024
When would an image ever not be in an Image block though? Do you have some steps for reproduction? |
aaronrobertshaw
approved these changes
Oct 22, 2024
There was a problem hiding this comment.
When would an image ever not be in an Image block though? Do you have some steps for reproduction?
One scenario where there's an image that isn't an Image block is within the Media and Text block.
That said, I don't believe the issue here is with using, or not using, the image block. That was a red herring 🙂
Here's what I found while testing:
- The pattern in question in the PR description actually uses Image blocks
- As the images for the blocks in the pattern aren't uploaded to the media library, the width/height dimensions aren't known so those attributes aren't set on the
imgelement when rendering on the frontend - This issue can be tested in the editor using an image block if you use "insert from URL" rather than one from the media library
- The problem only exists on the frontend with Firefox
Simplest steps to replicate in Firefox:
- Add an image block to a post
- Insert portrait image from url
- Apply square aspect ratio and save
- View post on the frontend.
Before this PR the image will still have a portrait aspect ratio. After, the square aspect ratio is applied.
I'm not super familiar with the inner workings of the aspect ratio feature but if @ajlende is happy with it, it gets my vote too.
✅ The new style didn't appear to negatively impact other images or rendering in other browsers
✅ This PR fixed the aspect ratio issue on the frontend in Firefox
✅ Didn't spot any issues testing different sizes, aspect ratios, and container restrictions
| Before | After |
|---|---|
 |
 |
Sorry, I wasn't clear. I meant when images were not added using the image block and in the media library (via clicking items in the UI). Not that the markup wasn't using the image block. This should happen in any scenario where the image doesn't have width and/or height. There are steps for reproduction on the issue here.
Thanks, this is what I meant. Sorry for the confusion. The issue was discovered while developing Twenty Twenty-Five. So, it's pretty likely that people developing their patterns (with local images and aspect ratio set) will have the same problem. |
|
@juanfra This is labelled as |
@getdave I believe the problem was always there. It was probably discovered now as it's an edge case. Given that it's affecting at least one pattern of Twenty Twenty-Five, I'd love if it could get included in 6.7 RC2. And yes, it's probably a bug rather than an enhancement. |
getdave
added
[Type] Bug
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Oct 22, 2024
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: f73c2cc |
|
@juanfra After this commit, SVG images are no longer displayed. |
|
@EldarAgalarov thanks for sharing. Can you please include precise steps to replicate that? I believe it would be best if you can create an issue with clear instructions on how to reproduce. |
|
|
@EldarAgalarov I can't replicate. Please see the screencast: Screen.Recording.2024-10-31.at.16.15.39.movI'm on Mac, WP RC2, and I tried in Chrome, Safari, and Firefox. I had to add a custom snippet to allow the upload of SVG files as they're not allowed by default. |
|
@juanfra I tested with the same image and it works, because this image have width and height attributes. Try to test SVG image with viewBox and without width and height attributes. |
|
@EldarAgalarov can you please share the image that you're trying with here? |
|
@juanfra An example of responsive SVG:
|
|
@EldarAgalarov thanks for sharing. I've created a PR to address this issue here. |
|
@juanfra Similar to SVG, this is causing a bug with images that use |
kevin940726
added a commit
that referenced
this pull request
Nov 7, 2024
…ms in Firefox. (#66217)" (#66804) Co-authored-by: kevin940726 <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: mukeshpanchal27 <[email protected]> Co-authored-by: juanfra <[email protected]> This reverts commit 0e65adc.
gutenbergplugin
pushed a commit
that referenced
this pull request
Nov 7, 2024
…ms in Firefox. (#66217)" (#66804) Co-authored-by: kevin940726 <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: mukeshpanchal27 <[email protected]> Co-authored-by: juanfra <[email protected]> This reverts commit 0e65adc.
|
Thank you, looks like we need an alternative solution to this. |
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
…ms in Firefox. (WordPress#66217)" (WordPress#66804) Co-authored-by: kevin940726 <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: mukeshpanchal27 <[email protected]> Co-authored-by: juanfra <[email protected]> This reverts commit 0e65adc.
priethor
pushed a commit
that referenced
this pull request
Nov 18, 2024
…ms in Firefox. (#66217)" (#66804) Co-authored-by: kevin940726 <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: mukeshpanchal27 <[email protected]> Co-authored-by: juanfra <[email protected]> This reverts commit 0e65adc.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes #66175
Set image width to
fit-contentto solve aspect ratio problems in Firefox.Why?
For the aspect ratio to work, at least in Firefox, the image's width must be set. If not, the aspect ratio doesn't work.
How?
Setting the image width to
fit-contentso that it has a default.Testing Instructions
Screenshots or screencast
Screen.Recording.2024-10-17.at.18.23.39.mov