Don’t add <img> `height` & `width` attributes
eeeps opened this issue · 6 comments
On my test page, the following Liquid tag:
{% cloudinary /assets/test.png alt="beautiful!" %}
Produced an image with width
and height
attributes. These don't make sense for a couple of reasons.
- They represent the dimensions of the full-size image; often we will be loading one of the smaller-downscaled versions in
srcset
. - The intrinsic
width
is over-ridden by the standardmax-width: 100%;
display rule. Without theheight
, the image would be scaled proportionately, but with it, it is only scaled horizontally, and distorted to fit both the 100% width, and the 1012 height.
height
and width
are often more trouble than they're worth on responsive layouts. Get rid of them?
The aim was to let the browser know the image ratio so that it can reserve the necessary space, even before the image is loaded, which would limit content "jump".
It looks like the browser don't do it, after all, so you're right, I should remove them.
For your distortion issue, you should anyway always put these two together:
max-width: 100%;
height: auto;
I now remember why I did this.
If the image is small, c_scale
will enlarge it, so setting width
and height
attributes show it at the right size.
Which means it can be an enlarged image (with ugly artefact) shown at a reduced size. Not really efficient.
I just read the image resizing documentation once again, and it seems I should use c_limit
instead of c_scale
.
I could even remove rmagick
dependency, letting Cloudinary do its magic even more… ;-)
@eeeps any advice?
Parts of what I said in previous comment are false.
Yes, c_scale
will enlarge the image, while c_limit
won't, so the later might be safer. But I still need rmagick
anyway, because I try to prevent using superfluous Cloudinary URLs, like the ones where the required width is larger than the natural one. If I succeed, c_limit
should not be required.
The main objective of width
and height
attributes was to show an image at its natural size if it is smaller than its container, because —as @eeeps says— "naturalWidth will always report the evaluated sizes
value".
I see two options to show the image at its true natural size when the container is larger:
- keep
<width>
and<height>
attributes on the image, easy. - add a breakpoint to
sizes
for before and after the image is exactly the width of the container. Really more difficult, even maybe impossible, because it means thesizes
attribute must be different for each image, but it depends on the design, which is the reason the user currently has to set it manually in_config.yml
.
::just spent a half hour testing things and being surprised::
OK so I agree with you, leave these attributes!
I thought that width=
set the intrinsic width, and was trumped by sizes
. It doesn’t! It sets the layout width, and it + sizes
are independent. CSS layout width:
trumps HTML layout width=
for layout... but the HTML width=
, and not the intrinsic width (as I thought), provides the upper limit for max-width:
.
What a horrible and confusing paragraph. Too many “widths!” Anyways, the system works as you describe, and my mental model was all wrong.
As you say, if I just set the CSS height:
to auto
for all of my responsive images, the height=
attribute wouldn't distort my image. And, as you say, it turns out that while space isn't actually reserved on the layout until the image is loaded, the spec has an example that tells you that it is, advocating for exactly the pattern that you’re using... so who knows maybe that'll work some day too.
TL;DR if you don’t want to upscale,* max-width:
and height: auto
+ width=
and height=
is the best pattern. Use it!
* I’m weird, and in the habit of setting width:
instead of max-width:
on my images and treating layout width and resource width totally independently...but that's unusual I think
Awesome, I'll put the width
and height
back… ;-)
Maybe there's an interesting example to build, with both large and small images, and discuss with @respimg people?
After reading the discussion, I am still uncertain what is proposed here. Having width and height attributes screws up my rendering and I can't see a way of removing them.
Adding height: auto
to my CSS fixed the rendering issues, though.