Setting default max-width and height for img tag

[mvdnbrk]

Pull request #426 sets height to auto in combination with a default value of max-width to 100% to the img tag.

This gives unexpected results.
See Codepen.

Loading a 100x200px image fills up the whole space although I am setting width and height specifically.

@adevade mentioned code from Bootsrap, but this is meant for a fluid image with class name image-fluid.

I took a look at the source code of BeardCSS.
BeardCSS is setting only a max-width of 100%, if width or height attributes are set they will be respected by resetting max-width to none on the img tag.

I think we should take the BeardCSS approach.

Example of same code as above with the BeardCSS approach gives the expected result.

[benface]

A couple things...

1. In your first Codepen, you used width: 100% instead of max-width: 100% like Tailwind, which doesn't "fill up the whole space". With max-width: 100% it behaves as expected, simply scaling the image down when its intrinsic width is larger than its container.

2. In your second Codepen, your syntax for CSS comments is incorrect (// instead of /* */), which breaks the CSS (the second rule is not applied). But even if it didn't, I don't believe that rule (the img[width], img[height] { max-width: none; } one) should be in the core. For one, it's more specific than a single class selector, meaning Tailwind's max-w- classes will not be enough to re-set the max-width to 100% or any other value. But also, I think the default max-width: 100% rule is quite useful, whether an img has width/height attributes or not. Those attributes are mostly hints to the browser anyway, they should reflect the image's intrinsic size (which is used to size the image whether the attributes are present or not, unless the width/height are set in CSS). I think it's pretty rare that we want an image to overflow its container, regardless of its intrinsic/preferred size.

3. The PR you referenced simply added height: auto, which you don't seem to be having an issue with. max-width: 100% was there from the start as far as I know.

That begs the question: what unexpected results did you experience?

[mvdnbrk]

Updated Codepen to point out the issue.

[benface]

I don't understand the problem. If I remove this from the Codepen:

img { 
    max-width: 100%;
    height: auto;
}

...the result is the same. Also, did you intentionally use a different width attribute (250 pixels) than the actual image (750 pixels)?

[mvdnbrk]

Yeah that's because of the wrapping flex container.

If you remove this line:
height: auto;
The image is in the expected width and height as set by the width and height attributes.

[benface]

Ah, my bad. The result is not the same; my browser was too small for me to notice the difference. OK, I see what's going on. I would say that this is an edge case as it's only a problem with images in flex containers. In fact you can solve it by wrapping the img in an extra div, or by adding align-items: flex-start to .flex (items-start in Tailwind). By the way, your solution of...

img[width], img[height] {
  max-width: none;
}

...doesn't work because in this specific case, the problem is not even max-width: 100%, it's that you are scaling the image down yourself with smaller width/height attributes, and height: auto overrides the height attribute. But more importantly, the problem would occur even if the img didn't have width/height attributes (try removing them and resizing your browser, you'll see).

[mvdnbrk]

Okay thanks.

Well putting the image in an extra div or adding .items-start to .flex just feels like a workaround for me.

I really needed some time to figure out where this unexpected behavior did come from.

Tailwind already provides a .h-auto utility, so maybe if you need to set that property it would be better to use .h-auto instead of adding it by default to the img tag?.

[benface]

Well putting the image in an extra div or adding .items-start to .flex just feels like a workaround for me.

It is and it isn’t. I agree that it’s unfortunate that Flexbox behaves this way with images, but I’m pretty sure it’s per the spec (i.e. not a browser bug). The default value of align-items is stretch, and I guess if there’s only one item it looks at its intrinsic height and uses that.

Tailwind already provides a .h-auto utility, so maybe if you need to set that property it would be better to use .h-auto instead of adding it by default to the img tag?

Just in case it wasn’t clear, this is not even caused by height: auto. It’s just making the problem more apparent because you immediately end up with an image with an intrinsic height (3000px) that doesn’t match the aspect ratio inferred by its display width (250px). You would have the same problem without height: auto if the flex container was smaller than 250px. I guess an easier way to show that is to use the actual image size (750 x 3000) as the display size (either by removing the width/height attributes or by setting them to 750 and 3000 respectively) and resizing the window. You’ll see that below 750px, the image starts getting stretched with no regard to its aspect ratio again.

[adamwathan]

Here's a Codepen showing the same issue without height: auto set for reference:

https://codepen.io/adamwathan/pen/dKKMJb

I'll admit I'm tempted to remove these styles completely and just let people use max-w-full and h-auto as needed, but it is pretty convenient :/ In this case I think seeing the image stretch in height is not completely unexpected at least, because flex items do stretch to fill the container unless you specify otherwise with align-items.

[benface]

Removing these styles completely would also be a good option. My point was that as long as Preflight has img { max-width: 100%; }, height: auto should be included as well (for the reasons in #426) since the issue described here is caused by something else. But yeah, it would make sense to let the user deal with that. There are (rare) cases in which you may want an image to overflow... and if not, you can easily add these rules in a custom reset on top of Preflight. I am planning on releasing a "opinionated reset" plugin for Tailwind, and I'll definitely include them if you remove them.

[benface]

In fact, I already add this to my own projects:

img, svg, video, iframe {
    display: block;
}

img, svg, video {
    max-width: 100%;
    height: auto;
}

The display: block fixes vertical alignment issues (replaced elements are inline by default so they are affected by surrounding text, line height, etc.), and I set max-width: 100% and height: auto to svg and video as well for consistency.

[mvdnbrk]

I agree with @adamwathan setting max-width: 100%; is pretty convenient.

In the Codepen by Adam the image gets another aspect ratio due to the fact it is placed in a parent div with a width of 100px. But the image stays at a height of 1000px as I would expect.

If you bring back the height: auto; property the image will stetch to a 3000px height.

I would solve the auto height issue mentioned by @adevade like this. This feels more natural to me instead by just setting it by default to the img tag.

I also agree with Adam that it's not completely unexpected because flex items do stretch to fill the container. But ending up with a weird aspect ratio of an image isn't really desirable.

[hacknug]

What about adding align-self to the img element on preflight? This would only do something in case the image it's inside a flex container.

[grund3g]

Even if the height: auto is not the main problem here, i would remove it by default because there is no way to unset height: auto with css afterwards.