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

Using width: 100% on img causes problems #17787

Closed
joyously opened this issue Oct 5, 2019 · 20 comments
Closed

Using width: 100% on img causes problems #17787

joyously opened this issue Oct 5, 2019 · 20 comments
Assignees
Labels
[Block] Media & Text Affects the Media & Text Block [Feature] Media Anything that impacts the experience of managing media [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@joyously
Copy link

joyously commented Oct 5, 2019

A style is making my image too big.
Using WP 5.3 beta2
A media-text block with a small image is stretched, making the image blurry and out of position.
The problematic CSS is (found in wp-includes/css/dist/block-library/style.min.css?ver=5.3-beta2-46391)

.wp-block-media-text > figure > img, .wp-block-media-text > figure > video {
    width: 100%;
}

To reproduce
Steps to reproduce the behavior:

  1. Add a Media-Text block
  2. Select a small image (mine is 165px wide)
  3. Add some text
  4. Duplicate the block
  5. Align the second block opposite the first
  6. (Save) View on front end to see the images stretched.

Expected behavior
I expect the images to show at no larger than the original.

Screenshots
This is how it looks in the editor. The images look stretched if I use a wide window.
media-tet-blocks-editor

This is the front end.
stretched-image-front-end

This is the front end when I disable the width: 100% rule mentioned above.
media-text-blocks-with-width-disabled

Desktop (please complete the following information):

  • OS: Ubuntu Studio 16.04
  • Browser SeaMonkey 2.49.5
@CreativeDive
Copy link
Contributor

@joyously I can't confirm with you. I think your theme causing the issue.

@joyously
Copy link
Author

joyously commented Oct 6, 2019

Well, I get the same result regardless of theme, because the offending rule is in the block library styles loaded on the front end, not in the theme. I think it's more that the test image is portrait and not very wide, than it is not very wide. By using a width of 100% on a narrow image, the height is accentuated (height:auto).

@CalumChilds
Copy link

Is there a block style with the max-width attribute set to 100%? I find that usually works and stops images from overflowing.

@joyously
Copy link
Author

joyously commented Oct 9, 2019

Yes, but it still is a problem. There is no reason to use width:100% on images and good reason to not use it.
This is an easy fix. Just remove the one line width: 100%.

@marbaque
Copy link

marbaque commented Nov 4, 2019

I am having the same issue regardless of the theme. When image is set to 100%, it overflows outside the container.

@joyously
Copy link
Author

This bug still exists in GB 7.0.0.

@CreativeDive
Copy link
Contributor

@joyously a lot of bugs still exists in GB 7.0 ... we'll have to wait a while to fix all these bugs :-)

@talldan talldan added [Block] Media & Text Affects the Media & Text Block [Feature] Media Anything that impacts the experience of managing media [Type] Bug An existing feature does not function as intended labels Dec 16, 2019
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Feb 8, 2021
@adamsilverstein
Copy link
Member

@joyously Thanks for opening this issue. I have addressed in #28822 - can you please review? I'd appreciate some additional testing/review if possible to make sure we aren't breaking something else inadvertently.

@joyously
Copy link
Author

joyously commented Feb 8, 2021

Thank you Adam!
I don't have an environment to test with. I wonder what happens with videos, with and without the responsive option enabled, since I wrote this issue just for images.
Also, there is something in the style sheet for .is-image-fill that gets height: 100%, so that needs to be tested with the width removed (I don't know how to get that class through the UI).

@adamsilverstein
Copy link
Member

I tested briefly with videos and agree this deserves closer scrutiny, especially if someone is familiar with the original purpose of the style.

I don't have an environment to test with.

That is unfortunate, I wonder how we can make this more straightforward to test?

Over on the Site Kit project, we added a GitHub action that builds a zip of the plugin for every PR. This along with a tester plugin lets you easily test the plugin code from any in progress PR. I wonder if we can add something like that to Gutenberg?

@paaljoachim
Copy link
Contributor

I was able to open the PR using gutenberg.run
http://gutenberg.run/28822

@ockham
Copy link
Contributor

ockham commented Feb 9, 2021

Over on the Site Kit project, we added a GitHub action that builds a zip of the plugin for every PR. This along with a tester plugin lets you easily test the plugin code from any in progress PR. I wonder if we can add something like that to Gutenberg?

FWIW, as of #26746, we're already building Gutenberg for every PR.

@paaljoachim has pointed out that we need to make those builds more visible.

@adamsilverstein
Copy link
Member

I was able to open the PR using gutenberg.run
http://gutenberg.run/28822

@paaljoachim Fantastic! Would love to see this link added automatically to every PR so it easier for testers like Joy to discover.

@adamsilverstein
Copy link
Member

Would love to see this link added automatically to every PR so it easier for testers like Joy to discover.

And, I see that exactly requested here: #27426

@joyously
Copy link
Author

joyously commented Feb 9, 2021

When I click the .run link provided, it sits on "Redirecting" for a long time. I have no idea what it's doing.
When I said I didn't have a test environment, I meant code and data and enough knowledge of the possibilities needing testing for the change. I don't use the block editor, and I haven't been adding code to my theme lately, so I'm out of touch with the various combinations available for an image that would match the CSS change, and I never did much with videos, so I know even less for that. I am a bit concerned that the max-width is unset still with the width:100% removed, and what I mentioned before about height.
I've waited ten minutes now since clicking the .run link and it still sits there doing nothing.

@tellthemachines
Copy link
Contributor

Also, there is something in the style sheet for .is-image-fill that gets height: 100%, so that needs to be tested with the width removed (I don't know how to get that class through the UI).

It's a setting called "Crop image to fill entire column", under "Media & test settings" in the sidebar. I checked and removing width: 100% makes no difference in this case as the image is positioned absolutely so will still occupy 100% of its container.

@jasmussen
Copy link
Contributor

From discussion in #28822 and #28922, it appears aspects of the original issue are no longer relevant and that the solutions discussed may have unintended consequences for the Media & Text block. For that reason, I'm closing this one. We can always open new tickets if adjacent issues surface or resurface. Thank you.

@joyously
Copy link
Author

I think the relevant comment was

In that case, let's close #17787 as wontfix and remove the Image size setting instead.

But you only did half (closing this one). Can you reference the other issue where the size setting is impacted?

@tellthemachines
Copy link
Contributor

Whoops, I let that one slip! Here it is: #29362

@jasmussen
Copy link
Contributor

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Media & Text Affects the Media & Text Block [Feature] Media Anything that impacts the experience of managing media [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
10 participants