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

Setting default max-width and height for img tag #506

Closed
mvdnbrk opened this issue Jun 22, 2018 · 40 comments
Closed

Setting default max-width and height for img tag #506

mvdnbrk opened this issue Jun 22, 2018 · 40 comments

Comments

@mvdnbrk
Copy link
Contributor

mvdnbrk commented Jun 22, 2018

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.

@mvdnbrk mvdnbrk changed the title Setting default width and height for img tag Setting default max-width and height for img tag Jun 22, 2018
@benface
Copy link
Contributor

benface commented Jun 22, 2018

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
Copy link
Contributor Author

mvdnbrk commented Jun 22, 2018

Updated Codepen to point out the issue.

@benface
Copy link
Contributor

benface commented Jun 22, 2018

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
Copy link
Contributor Author

mvdnbrk commented Jun 22, 2018

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
Copy link
Contributor

benface commented Jun 22, 2018

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
Copy link
Contributor Author

mvdnbrk commented Jun 22, 2018

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
Copy link
Contributor

benface commented Jun 22, 2018

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
Copy link
Member

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
Copy link
Contributor

benface commented Jun 23, 2018

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
Copy link
Contributor

benface commented Jun 23, 2018

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
Copy link
Contributor Author

mvdnbrk commented Jun 23, 2018

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
Copy link
Contributor

hacknug commented Jul 6, 2018

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
Copy link

grund3g commented Nov 6, 2018

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.

@benface
Copy link
Contributor

benface commented Jan 28, 2019

@grund3g I agree with you. And since max-width: 100% without height: auto introduces the issue described in #426, I would remove this rule altogether:

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

In fact I ended up undoing that rule (as much as possible, because like you said you can't fully undo height: auto) in my opinionated reset that I use on top of Preflight in every project: benface/tailwindcss-config#6

That being said, Preflight is completely optional and Adam has said on Twitter that he was working on a way to let plugins define custom "base styles". So you could make your own reset plugin that doesn't include the above rule, should Adam decide not to remove it from Preflight.

@adamwathan
Copy link
Member

If someone wants to open a PR to remove that from Preflight and target the next branch, I'm okay making this change for 1.0 👍

mvdnbrk added a commit to mvdnbrk/tailwindcss that referenced this issue Jan 28, 2019
@mvdnbrk mvdnbrk closed this as completed Jan 29, 2019
adamwathan pushed a commit that referenced this issue Feb 1, 2019
adamwathan pushed a commit that referenced this issue Feb 1, 2019
@adamwathan
Copy link
Member

Just for reference, here's a truly simplified demo that illustrates the core problem, which is that if you specify an explicit height using the height attribute, height: auto defeats it and there's no way we can force the browser to respect the explicitly defined height attribute:

https://codepen.io/anon/pen/GeOoJe

You can see there I am trying to force the image to stretch but I can't make it happen. Now that I'm typing this though I'm realizing the obvious solution is to use inline styles instead of the width and height attributes:

https://codepen.io/anon/pen/GeOoJe

🤦‍♂️

So I think I might revert this change since it's actually trivial to work around...

@adamwathan
Copy link
Member

This CodePen shows how inline styles fixes the original issue:

https://codepen.io/anon/pen/OqOMgJ

@himynameisphil
Copy link

This feels counter intuitive to me. Max width 100% and Height auto both exist as a class, why wouldn't you just add them as you see fit? Or declare them globally if really you want to. Forcing them means it's difficult to override, especially for content generated by authors through a CMS for example.

We've gone from..

<img src="image.png" width="170" height="30">

to

<div class="flex" style="width: 170px; height: 30px;"><img src="image.png"></div>

Please consider removing it again.

@himynameisphil
Copy link

Came across another reason to consider removing these default styles.

max-width being the culprit this time: https://codepen.io/himynameisphil/pen/poojXwr

@silvio-e
Copy link

silvio-e commented Oct 22, 2020

That just drove me nuts. It was unexpected. That I can't use the height attribute is for me too much of an opinion as it disables a default HTML behavior. And I didn't find it documented anywhere. Seems like I can't disable it, right? So you first have to figure out what happens, which wastes time and then use inline styling to overwrite it? That is kind of counter intuitive and not very elegant. :/

Edit: Sorry for the morning rant 😅. Just wanted to put an image somewhere and it didn't work like expected. Why do you think it is necessary to have that default styling?

@tordans
Copy link

tordans commented Mar 11, 2021

Update: I now realise this is all ages old; will have to debug some more it looks like :-)

@samtstern
Copy link

samtstern commented Jun 2, 2021

Just spent the better part of a day figuring out why my img tags were not working! Eventually realized it was height: auto overriding things like height="123px".

So is there any way to stop Tailwind from adding height: auto to img?

@adevade
Copy link
Contributor

adevade commented Jun 3, 2021

This issue pops up from time to time. I'm personally fine with how it works today, but I see that some people above are having issues. Mostly with content from CMS's it seems.

Could this, or some variation of it, maybe be a solution? I'm talking for the prefligh styles in the framework.
Only setting height: auto on img if the element does not have a height attribute defined.

img:not([height]) {
  height: auto;
}

https://codepen.io/adevade/pen/PopQrYZ?editors=1100

Sorry to bother you @adamwathan, but maybe you could take a quick look at this or make a new comment, since it was a couple of years ago now. Thank you.

@samtstern
Copy link

@adevade you're right, my use case is CMS-like as well. I have user-generated markdown content which I parse and then style with tailwindcss-typography so in order to reach in and add explicit height styles I have to parse the HTML with JavaScript, which is a shame because I love how hands-off it can be otherwise.

@adamwathan
Copy link
Member

Hey @adevade! That would be a great solution but it has a higher specificity than a class:

image

That means that classes like h-32 would just stop working, because this base style would override the height back to auto.

@samtstern For this problem I've typically just recommended people use inline styles instead of the height attribute, which will always work and override the height: auto we set in our base styles. Maybe it's something we can change for 3.0 but a bit hesitant because it's a pretty huge breaking change for people.

@samtstern
Copy link

Is there a simple way to make this configurable?

@adamwathan
Copy link
Member

I think you could add this to your own CSS (haven't tested):

img {
  height: unset
}

@dkodeit
Copy link

dkodeit commented Jul 20, 2021

img {
height: unset
}

This doesn't work per spec.

@natebrunette
Copy link

This behavior was extremely unexpected. I think it would be best to not add styles you can't easily undo.

@simonmaass
Copy link

I also ran into this issue today when using the nuxt/image module in combination with tailwindcss

@doxigo
Copy link

doxigo commented Apr 23, 2023

Having the same issue that the img tag height attribute doesn't apply in a react app, any solution to get rid of the height: auto and max-width: 100%; from img and video tag only?

@leozhao21
Copy link

I found one way to resolve this problem, maybe it may be bad, but it works

  1. use this command build base.css NODE_ENV=development npx tailwindcss -i ./src/styles/tailwind-base.css -o ./dist/base.css, in tailwind-base.css file, only have @tailwind base
  2. open base.css, remove img or video style
  3. Use @import ./base.css replace @tailwind base in tailwind.css file

@wtchnm
Copy link

wtchnm commented Dec 9, 2023

Here's a PNPM patch to remove these styles:

# patches/[email protected]

diff --git a/lib/css/preflight.css b/lib/css/preflight.css
index e5e52cd8f32310089aebc65cfd943248cfe7b92e..c32f29ba3b8e189b0b3471c3d0d69da64fec0a50 100644
--- a/lib/css/preflight.css
+++ b/lib/css/preflight.css
@@ -362,16 +362,6 @@ object {
   vertical-align: middle; /* 2 */
 }
 
-/*
-Constrain images and videos to the parent width and preserve their intrinsic aspect ratio. (https://github.com/mozdevs/cssremedy/issues/14)
-*/
-
-img,
-video {
-  max-width: 100%;
-  height: auto;
-}
-
 /* Make elements with the HTML hidden attribute stay hidden by default */
 [hidden] {
   display: none;

@BorysShulyak
Copy link

Having the same issue that the img tag height attribute doesn't apply in a react app, any solution to get rid of the height: auto and max-width: 100%; from img and video tag only?

The same issue, could not use a Next.js Image component

@quangle36
Copy link

quangle36 commented Jan 16, 2024

Still facing the same issue, are there any updates?

@AayushKarki714
Copy link

Having the same issue that the img tag height attribute doesn't apply in a react app, any solution to get rid of the height: auto and max-width: 100%; from img and video tag only?

The same issue, could not use a Next.js Image component

Any updates on how to use with Next js Image??

@AayushKarki714
Copy link

AayushKarki714 commented Feb 19, 2024

@adamwathan, The issue was opened in 2018 and still the issue persists. I am using Next js Image component and need to workaround the height auto by explicitly defining the height utility. Is there not any better way to do this??

Here is the workaround I have been using

<Image alt="Beautiful Scenery" priority width={40} height={40} className="rounded-full h-10 object-cover" src="https://images.unsplash.com/photo-1707345512638-997d31a10eaa?w=500&auto=format&fit=crop&q=60&ixlib=rb-4.0.3&ixid=M3wxMjA3fDF8MHxzZWFyY2h8MXx8bGFuZHNjYXBlfGVufDB8fDB8fHww" />

@Josevictor2
Copy link

up

@nezi311
Copy link

nezi311 commented Nov 16, 2024

Any updates on this?

@sunananus
Copy link

The issue persists: when using the Image component in Next.js, the width and height styles are being overridden.
img {
max-width: 100%;
height: auto;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests