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

Image and SrcSet helpers don't respect custom srcsets #104

Open
richardweaver opened this issue Sep 10, 2020 · 3 comments
Open

Image and SrcSet helpers don't respect custom srcsets #104

richardweaver opened this issue Sep 10, 2020 · 3 comments

Comments

@richardweaver
Copy link

I've been trying to optimize image sizes using the getImageManagerImageSrcset handlebars helper. Looking at the documentation here I should be able to specify custom sizes including inherent sizes and pixel densities:
https://developer.bigcommerce.com/stencil-docs/reference-docs/handlebars-helpers-reference

However, as far as I can see there's nothing in the code other than the default srcset ever applied and no parameter(s) anywhere that expect custom sizes to be applied?
https://github.com/bigcommerce/paper-handlebars/blob/master/helpers/lib/getObjectStorageImage.js

Am I misinterpreting the code or is the documentation wrong?

@bookernath
Copy link
Contributor

Hey @richardweaver - thanks for raising this.

With our first "srcset" helper which is {{getImageSrcset}}, we saw no next-to-no usage of custom sizes. So when creating {{getImageManagerImageSrcset}} and {{getContentImageSrcset}} we omitted the option to specify custom sizes.

The docs are a bit misleading in that regard - so I'll work with @bigcommerce/dev-docs to get those corrected.

Can you talk a bit more about your use case for custom sizes? I'd be happy to consider adding the option to the helper if it's useful to you.

In the interim, you could consider using {{getImageManagerImage}} and {{getContentImage}} to build your own srcset like this:

<img srcset="{{getImageManagerImage "asset.jpg" width=100}} 100w, {{getImageManagerImage "asset.jpg" width=200}} 200w" />

@richardweaver
Copy link
Author

Thanks @bookernath for getting back to me. The main use case was to have more fine-grained control on image sizes. Because of the way the responsiveness of our site works we know our maximum image size for some slots, and also the key "step" sizes, so it appeared to make sense to specify them in the srcset. Although it's only a small saving we're quite image heavy so it adds up.

For example, if we need an image of 1960px wide, because this is just over 1920w with the default size set the browser will use the 2560px wide image which could cost 100s kb bandwidth

Perhaps one alternative would be to be able to specify maximum (and maybe minimum) size for the srcset?

@steve-ross
Copy link

I'd love this feature, really annoying as I thought the docs supported this as well. Use case: there is a massive jump in the 'default sizes' and making a helper for every option is a PITA.

use case for us is the ability to deliver images at closer breakpoints instead of the huge jump between the default sizes apparently that was just for 'hero' or 'full width' images?

"srcset_sizes":{
    "thumb_sm": {
      "50w": "50w",
      "80w": "80w",
      "120w": "120w",
      "160w": "160w"
    },
    "thumb_med": {
      "100w": "100w",
      "140w": "140w",
      "200w": "200w",
      "240w": "240w",
      "280w": "280w"
    },
    "four_cols": {
      "340w": "340w",
      "360w": "360w",
      "380w": "380w",
      "420w": "420w",
      "460w": "460w",
      "500w": "500w"
    },
}

The alternative is a pain:

<img alt="{{alt}}" class="lazy placeholder{{#if class}} {{class}}{{/if}}" 
  src="{{getContentImage image width=50 }}" 
  data-src="{{getContentImage image width=360 }}"
  data-srcset="{{getContentImage image width=360 }} 360w,
  {{getContentImage image width=480 }} 480w,
  {{getContentImage image width=550 }} 550w,
  {{getContentImage image width=640 }} 640w,
  {{getContentImage image width=800 }} 800w,
  {{getContentImage image width=1024 }} 1024w,
  {{getContentImage image width=1260 }} 1260w,
  {{getContentImage image width=1420 }} 1420w"
  data-sizes="{{#if sizes}}{{sizes}}{{else}}100vw{{/if}}" />

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

No branches or pull requests

3 participants