Automatically add width and height attributes

kslstn commented 4 years ago

Now it’s useful to set width and height attributes to image tags again, it would be super nice of the plugin would do that automatically. Attributes could be set via the include arguments already, but doing that for every image is cumbersome.

rbuchberger commented 4 years ago

I'll have to do some research here to get it right, but it's not a bad idea. Thanks for the suggestion!

rbuchberger commented 4 years ago

Just adding an update that this is probably the most important new feature I want to add. I don't want to promise a timeline, but it's at the top of the pile.

stuntbox commented 4 years ago

Another big +1 here. It relies on modern browsers adopting this behavior, but would more or less make the feature request in #149 moot.

rbuchberger commented 4 years ago

Update: I've been working on this lately and have made good progress. The hardest part is caching image dimensions so we don't blow up the build times, and I have a solution to that working nicely. After that it's a few tweaks to the relevant output formats, settings, test, and some documentation. Shouldn't be more than a couple weeks unless something comes up

rbuchberger commented 4 years ago

It's working! All that's left are the docs. While I'm working on that, the internal changes are actually pretty extensive so I'd really like a few more people to try it on their sites before I merge it. Here's how:

In your gemfile, modify the appropriate line to read: gem 'jekyll_picture_tag', git: 'https://github.com/rbuchberger/jekyll_picture_tag', branch: '173-height'
Run bundle install.
Set dimension_attributes: true in the appropriate markup presets. Make sure that the relevant img elements have the following css rules, or your images will get stretched:

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

Try it out! On the img tag you should see width and height attributes in the correct aspect ratio. On modern browsers, if you reload while throttled you shouldn't see the page reflow as images load (assuming you don't also have to download fonts.)

Caveats:

This doesn't work if you're using art direction (aka multiple source images). This is because you can only add these attributes to the <img> tag, of which there is exactly one. Hopefully some day they let us set these attributes on <source> tags.

Here's what I want to know:

Aside from the new feature, did anything break? Did anything change, other than generated image names? Did your build times change much?
Do the width and height attributes work as expected? Could the implementation be better?
Is dimension_attributes a good name? Now's the time to change it, once it hits master it's set in stone. I'd prefer something shorter and harder to misspell, but it's descriptive and I don't want to use the word 'size' and cause confusion with the sizes attribute.

I appreciate the help!

stuntbox commented 4 years ago

Nice! Thanks, @rbuchberger!! I’ll test this out this weekend and let you know how it goes.

kslstn commented 4 years ago

Great work, it seems to work fine! Thank you!

I found that for images for which I already set a width and height attribute to auto, the result is that the value is added twice, f.e. width="auto 1000" height="auto 320". It's not a problem—I will have to remove those attributes anyway, but I'd expect the settings for a specific image to override presets.

rbuchberger commented 4 years ago

Ahh, yeah I didn't think of that! I can fix that pretty easily. Good catch, thanks.

stuntbox commented 4 years ago

Just tested this out, and it worked great! No issues for me.

rbuchberger commented 4 years ago

Thanks a ton guys. New release is up!

datapolitical commented 3 years ago

Is this supported in the current version? I don’t remember seeing it in the docs.

rbuchberger commented 3 years ago

It is! Documented here. Sorry it's not easy to find, is there a way the docs could be better?

datapolitical commented 3 years ago

Ahh I see it now. I think what would be helpful is a full example configuration file (a reference spec) with every option and a comment that explains what it does.

Because then it’s all in one place and you can see what each thing does. And one big file is more easily searchable than 20 small ones.

Right now it’s easy to get lost with so many options explained on different pages.

You could also link back to the docs for more detail.

I should add by the way that this is officially my favorite Jekyll plug-in because it does something incredibly complicated incredibly cleanly and very fast.

dylanhand commented 9 months ago

Hi @rbuchberger - first of all thanks for building JPT. I'm starting to add it to my site now and it looks like it'll replace some shell scripts I made to optimize images.

I'm having an issue with dimension_attributes though. It's generating a width and height attribute that use the source image's dimensions, rather than the generated image's dimensions.

Here's my preset:

# _data/picture.yml
presets:
  profile:
    formats: [avif, webp, jp2, original]
    base_width: 160
    fallback_width: 160
    size: 160px
    pixel_ratios: [1, 2]

Here's how I call it:

{% picture profile profile-pic.jpg %}

Here's my CSS, with width: 100%; and height: auto;

.author__img {
  img {
    display: block;
    width: 100%;
    height: auto;
    margin: 0 auto 70px;
    border-radius: 50%;
    background-color: $gray-white;
  }
}

And here's the output (note that width and height are "2500"!):

 <div class="author__img" alt="Dylan Hand">
    <picture>
        <source srcset="/generated/profile-pic-160-b5b8bc464.avif 1.0x, /generated/profile-pic-320-b5b8bc464.avif 2.0x" type="image/avif">
        <source srcset="/generated/profile-pic-160-747454f53.webp 1.0x, /generated/profile-pic-320-747454f53.webp 2.0x" type="image/webp">
        <source srcset="/generated/profile-pic-160-b5b8bc464.jp2 1.0x, /generated/profile-pic-320-b5b8bc464.jp2 2.0x" type="image/jp2">
        <source srcset="/generated/profile-pic-160-ab099fdc9.jpg 1.0x, /generated/profile-pic-320-ab099fdc9.jpg 2.0x" type="image/jpeg">
        <img src="/generated/profile-pic-160-ab099fdc9.jpg" width="2500" height="2500"> <!-- problem here -->
    </picture>
</div>

This results in the picture being HUGE and pixelated.

I can fix it by adding width/height attributes manually, but that seems like it defeats the purpose of dimension_attributes.

I've tried adding/removing size, base_width, and fallback_width, but all seem to result in using the source image's dimensions.

Am I missing something?

rbuchberger commented 9 months ago

Hi @dylanhand!

The width and height attributes in that context are there to set the aspect ratio on the first render, so content doesn't move around when the image loads in. If you constrain the width or height of the image with CSS that problem should go away.

dylanhand commented 9 months ago

Thanks for the quick reply @rbuchberger!

Wouldn't it be safer/better to use the actual width and height of the displayed image?

If I'm not mistaken, if the CSS fails to load, this is telling the browser that the profile pic is 2500x2500 instead of 160x160, which would cause a very bad CLS.

rbuchberger commented 9 months ago

I can't think of a situation where images would load but CSS wouldn't. If you like, you can inline those particular styles to be certain.

rbuchberger / jekyll_picture_tag

Automatically add width and height attributes #173