feat: picture partial #693

stereobooster · 2023-10-31T16:54:15Z

Example to demonstrate what is discussed here #649 (comment)

Example of generated code:

<figure class="m-auto mt-6 max-w-prose">
  <picture class="m-auto mt-6 max-w-prose">
    <img src="/congo/festivities.svg" width="1086" height="811.37" class="m-auto mt-6 max-w-prose" loading="lazy" decoding="async">
  </picture>
</figure>

It can detect width and height for SVGs. There is branch for LQIP in code, but it's not used now.

netlify · 2023-10-31T16:54:57Z

✅ Deploy Preview for hugo-congo ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`c8b33ae`
🔍 Latest deploy log	https://app.netlify.com/sites/hugo-congo/deploys/65634dacd77a750008962cce
😎 Deploy Preview	https://deploy-preview-693--hugo-congo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

stereobooster · 2023-10-31T16:55:49Z

layouts/_default/_markup/render-image.html

+    {{ partial "pictureDefaults.html" (dict "img" . "alt" $altText "class" $class) }}
+  {{- else -}}
+    <img src="{{ .Destination | safeURL }}" alt="{{ $altText }}" class="{{ $class }}"/>


there is no usage of lazy param for this case, and it's intentional. I think it's better to not use lazy for images without dimensions

wolfspyre · 2023-10-31T20:46:46Z

would it make more sense to include this as a well documented shortcode as initial implementation?
thus making its' usage optional?

further work would likely involve documentation changes to point out the integration as it gets rolled into default page templates?

stereobooster · 2023-10-31T21:26:56Z

would it make more sense to include this as a well documented shortcode as initial implementation?
thus making its' usage optional?

Which scenario do you have in mind? This partial just replaces similar functionality which is already there (in image render hook, figure shortcode and "single" template). Can't see how it would be useful as shortcode

wolfspyre · 2023-10-31T23:15:49Z

the thought was:

introduce the new component in a way that it can be evaluated without forcing it to be used..

this looks like a pretty slick wrapping of figure/img.

I could see this being introduced as a picture shortcode which invokes your partials.

That would get the partials available, and allow tire-kicking without a hard requirement to use it in the default page templates all in one go....
IE building comfortability with the new featureset and hunting for weird rough edges or unanticipated side effects
¯\_(ツ)_/¯

stereobooster · 2023-11-01T05:06:15Z

Oh I see. I didn't get the idea from the first message, sorry. Anyway it's up to the author

jpanther · 2023-11-25T07:44:16Z

I've finally had a little more time to have a look at this PR - sorry it's taken a while. Having the option to disable webp in the config is good and I think defaulting it to true is the right move. I like that this is removing all the code duplication as this has started to creep into more places than it needs to. Can we add the screenshot shortcode in so it uses the same logic too?

I don't see any reason why this couldn't be used across all the templates... it doesn't seem to reduce the functionality in those places and makes everything consistent. The only thing we need to be mindful of is to not lazy load the feature image.

stereobooster · 2023-11-25T22:08:24Z

Can we add the screenshot shortcode in so it uses the same logic too?

What about this part?

width="100%"
height="auto"
style="max-width:{{ div $image.Width 2 }}px; max-height:{{ div $image.Height 2 }}px;"

It is different compared to all other places. Shall I remove it?

wolfspyre · 2023-11-25T22:10:50Z

is that in the screenshot shortcode? it's hard to know without referencing the file in question

jpanther · 2023-11-25T22:17:38Z

The screenshot shortcode exists mainly because when I post HiDPI screenshots taken on my Mac they would all end up output at double their actual size. This was my way of solving that issue... but maybe there's a better way that doesn't require this?

stereobooster · 2023-11-25T23:13:49Z

The screenshot shortcode exists mainly because when I post HiDPI screenshots taken on my Mac they would all end up output at double their actual size. This was my way of solving that issue... but maybe there's a better way that doesn't require this?

I think yes - add css class with max-width: 100%; height: auto. Which is already the case:

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

browser will calculate ratio based on width, height from the image and will make image as wide as container. Like any other picture in the content. Does this fit?

jpanther · 2023-11-25T23:18:39Z

The problem here is it makes the screenshots appear blurry when they are scaled up and I like to maintain consistency between the sizes of the screenshots when there are multiple in the same article. Otherwise you end up with some that are larger than others.

I guess I'm just too picky about this one and that's why it exists as its own shortcode. This is one of the only things left in the theme that is selfishly there to solve my own use case. 😆 This is also why it's not referenced in the documentation. Perhaps it doesn't need to exist at all and I could just move it to my own project.

stereobooster · 2023-11-25T23:27:24Z

The problem here is it makes the screenshots appear blurry when they are scaled up and I like to maintain consistency between the sizes of the screenshots when there are multiple in the same article.

It's fair point. We should not upscale images (at least they should not be blurry). I need to test how it works, for some reason I think it should be ok, because max-width: 100%;, not width. But let me check

stereobooster · 2023-11-25T23:38:57Z

yes I see what you are talking about

![](lighthouse.jpg)

{{< screenshot src="lighthouse.jpg" >}}

I wonder if Hugo has a way to detect DPI and based on that we can adjust to calculation of width, height.

stereobooster · 2023-11-25T23:55:56Z

Wait a second. At first I thought issue is DPI (and browser upscales image), but really problem is in this part:

        {{ else }}
          srcset="
          ...
          {{- (.Resize "1320x").RelPermalink }} 2x"
          src="{{ (.Resize "660x").RelPermalink }}"
        {{ end }}

Image size is 1200px, but resized image is 1320px, so it's not browser. It's the code (Hugo) upscales image. We need to add some kind of condition to not produce images larger than they are.

This problem exists in original code. And I knew about it

sets HTML attribute srcset and resizes the image

but doesn't upscale image (not implemented)

UPD: nope it doesn't really fixes the issue, but it is definitely improvement:

jpanther · 2023-11-26T00:58:27Z

I'm not sure simply preventing upscaling solves the issue though. For instance, if I had a 600px wide HiDPI screenshot, that should be displayed at 300px wide in the HTML, not 600px. That's why I divide the dimensions by 2 in the screenshot shortcode.

stereobooster · 2023-11-26T01:00:41Z

I'm not sure simply preventing upscaling solves the issue though. For instance, if I had a 600px wide HiDPI screenshot, that should be displayed at 300px wide in the HTML, not 600px. That's why I divide the dimensions by 2 in the screenshot shortcode.

Yes, you are right. In fact there were both problems. Upscaling in Hugo and upscaling in the browser. I fixed issue with Hugo. And it makes things look better, but not ideal

jpanther · 2023-11-26T01:17:17Z

I don't think the screenshot use case is going to be easy to solve with the existing functions available because there's no easy way to determine if the image is a HiDPI screenshot and not just a photo or some other asset. I think it makes sense to just keep it separate for now. I don't think many people are as pedantic as me when it comes to that particular display issue.

I also wonder if this new partial should try to solve the EXIF rotation issue described in #600 too? It has been raised a few times by users and is a silly oddity in the way that Hugo handles the EXIF data (that is to say it seems it just ignores the EXIF orientation).

stereobooster · 2023-11-26T01:25:25Z

Here are 2 potential solutions to screenshot issue:

![](lighthouse.jpg?x2=true)

{{< screenshot src="lighthouse.jpg" >}}

Give it a try (latest commit in this PR)

stereobooster · 2023-11-26T01:26:47Z

I also wonder if this new partial should try to solve the EXIF rotation issue described in #600 too?

Sounds like a separate PR to me

jpanther · 2023-11-26T05:00:23Z

![](lighthouse.jpg?x2=true)

I think this is a good solution and it makes everything uniform across the various image types. It also allows me to get rid of the screenshot shortcode. The only thing I would change is to flip the parameter name around to be 2x as those assets are often named using the @2x suffix.

stereobooster · 2023-11-26T13:21:46Z

If use code like this:

{{ $2x := .2x | default false }}

Hugo complains:

 parse failed: template: partials/picture.html:7: bad number syntax: ".2x"

I can change it in query params though. Or we can come up with different name for internal variables

stereobooster · 2023-11-26T13:50:09Z

I didn't remove screenshot shortcode - don't want to blow up size of PR. Can do a separate PR

jpanther · 2023-11-26T20:38:21Z

This is looking good now. I'll merge it in at this point and we can make future adjustments as necessary.

I think the next logical step is to remove the screenshot shortcode and we just bundle it all into figure with the 2x param. It makes no sense keeping it as two different shortcodes because you could have screenshots that aren't HiDPI and in that case it makes no sense. Much easier to just let the user specify what they want and keep it all uniform.

jpanther · 2023-11-26T20:53:44Z

Thanks for your work on this PR @stereobooster, it's much appreciated! This is a great refactor and it removed a whole bunch of code duplication.

stereobooster commented Oct 31, 2023

View reviewed changes

jpanther added the enhancement New feature or request label Nov 25, 2023

stereobooster force-pushed the picture-new branch from a6fde7c to 6f0d70e Compare November 25, 2023 23:31

stereobooster mentioned this pull request Nov 26, 2023

Picture + webp #649

Closed

stereobooster added 5 commits November 26, 2023 14:14

Example of integration of picture partial

c237e04

integrate in single.html

9787918

do not upscale images in picture shortcode

3ffe2b1

use original image as backup for srcset

8cfe339

potential solution to screenshot

11fd48f

stereobooster force-pushed the picture-new branch from 954fd7f to 11fd48f Compare November 26, 2023 13:14

stereobooster added 2 commits November 26, 2023 14:23

use 2x instead of x2 in query params

6eb34e7

do not lazy load the feature image

0c6c30f

stereobooster force-pushed the picture-new branch from 171cdba to 0c6c30f Compare November 26, 2023 13:42

refactoring

c8b33ae

stereobooster force-pushed the picture-new branch from c160d9d to c8b33ae Compare November 26, 2023 13:52

stereobooster changed the title ~~Example of integration of picture partial~~ feat: picture partial Nov 26, 2023

jpanther merged commit a431f46 into jpanther:dev Nov 26, 2023
6 checks passed

stereobooster deleted the picture-new branch November 26, 2023 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: picture partial #693

feat: picture partial #693

stereobooster commented Oct 31, 2023 •

edited

Loading

netlify bot commented Oct 31, 2023 •

edited

Loading

stereobooster Oct 31, 2023

wolfspyre commented Oct 31, 2023 •

edited

Loading

stereobooster commented Oct 31, 2023

wolfspyre commented Oct 31, 2023 •

edited

Loading

stereobooster commented Nov 1, 2023

jpanther commented Nov 25, 2023

stereobooster commented Nov 25, 2023

wolfspyre commented Nov 25, 2023

jpanther commented Nov 25, 2023

stereobooster commented Nov 25, 2023

jpanther commented Nov 25, 2023

stereobooster commented Nov 25, 2023

stereobooster commented Nov 25, 2023 •

edited

Loading

stereobooster commented Nov 25, 2023 •

edited

Loading

jpanther commented Nov 26, 2023

stereobooster commented Nov 26, 2023 •

edited

Loading

jpanther commented Nov 26, 2023

stereobooster commented Nov 26, 2023 •

edited

Loading

stereobooster commented Nov 26, 2023

jpanther commented Nov 26, 2023

stereobooster commented Nov 26, 2023 •

edited

Loading

stereobooster commented Nov 26, 2023

jpanther commented Nov 26, 2023

jpanther commented Nov 26, 2023

feat: picture partial #693

feat: picture partial #693

Conversation

stereobooster commented Oct 31, 2023 • edited Loading

netlify bot commented Oct 31, 2023 • edited Loading

✅ Deploy Preview for hugo-congo ready!

stereobooster Oct 31, 2023

Choose a reason for hiding this comment

wolfspyre commented Oct 31, 2023 • edited Loading

stereobooster commented Oct 31, 2023

wolfspyre commented Oct 31, 2023 • edited Loading

stereobooster commented Nov 1, 2023

jpanther commented Nov 25, 2023

stereobooster commented Nov 25, 2023

wolfspyre commented Nov 25, 2023

jpanther commented Nov 25, 2023

stereobooster commented Nov 25, 2023

jpanther commented Nov 25, 2023

stereobooster commented Nov 25, 2023

stereobooster commented Nov 25, 2023 • edited Loading

stereobooster commented Nov 25, 2023 • edited Loading

jpanther commented Nov 26, 2023

stereobooster commented Nov 26, 2023 • edited Loading

jpanther commented Nov 26, 2023

stereobooster commented Nov 26, 2023 • edited Loading

stereobooster commented Nov 26, 2023

jpanther commented Nov 26, 2023

stereobooster commented Nov 26, 2023 • edited Loading

stereobooster commented Nov 26, 2023

jpanther commented Nov 26, 2023

jpanther commented Nov 26, 2023

stereobooster commented Oct 31, 2023 •

edited

Loading

netlify bot commented Oct 31, 2023 •

edited

Loading

wolfspyre commented Oct 31, 2023 •

edited

Loading

wolfspyre commented Oct 31, 2023 •

edited

Loading

stereobooster commented Nov 25, 2023 •

edited

Loading

stereobooster commented Nov 25, 2023 •

edited

Loading

stereobooster commented Nov 26, 2023 •

edited

Loading

stereobooster commented Nov 26, 2023 •

edited

Loading

stereobooster commented Nov 26, 2023 •

edited

Loading