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

Thought about native respimg support? #7

Closed
davidhund opened this issue Apr 17, 2015 · 8 comments
Closed

Thought about native respimg support? #7

davidhund opened this issue Apr 17, 2015 · 8 comments
Labels

Comments

@davidhund
Copy link

First: thanks for this, looks great!

I was wondering, however, if you've considered supporting Responsive images through e.g. the srcset and sizes attributes? (For a nice explainer check: https://ericportis.com/posts/2014/srcset-sizes/)

At the moment you 'force' retina img src by checking window.devicePixelRatio > 1. However, there's a lot of alternative use-cases for 'responsive' images and by forcing the src through JS you disable browser optimizations.

I am not sure how this would work—if it would work at all (since I'm not sure we can set srcset and sizes dynamically)—but you might have some thoughts?

@callmecavs
Copy link
Owner

i've been wanting to do some work with matchMedia, and the API matches (pun intended) the browser support of this project

i'll play around a bit with adding support for a data-srcset attribute. it'll likely come down to file size tho. there's a lot to consider - devicePixelDensity as well window width.

@davidhund
Copy link
Author

Does it matter though? Whether you rely on matchmedia or window.devicePixelRatio, you're still bypassing the potential browser optimisations from srcset/sizes

matchMedia would give you a bit more flexibility (more than 'retina' detection): you could add more complex media conditions in e.g. a data-media attr.

Adding data-srcset to img in the page will probably be too late (I'm not 100% sure) for the browser optimisations for srcset. I'd advise you to look at e.g. https://github.com/scottjehl/picturefill — my guess is that we'd have to resort to different markup (e.g. spans) and replace them with img.

I'm not sure if it's worthwhile altogether but my gut-feeling is that it's a pity to bypass browser 'picture' awesomesauce.

What do you mean with "it'll likely come down to file size tho. there's a lot to consider - devicePixelDensity as well window width."? Is that not exactly the point in trying to support srcset: you'd address the window-/component-width and devicePixelDensity scenario's with browser-native optimisations…

@callmecavs
Copy link
Owner

I gotcha. Considering that srcset isn't supported in modern Firefox or IE10, I can't really justify adding weight to the library to provide support for those attributes when they're not cross browser compatible to begin with

@callmecavs
Copy link
Owner

may revisit this when the use of srcset becomes more common but in its current state of compatibility, it doesn't align with the support of the rest of the lib

@JeroenReumkens
Copy link

I'm wondering if you really do need the srcset compatibility. Isn't it possible to build in a little screen size check and based on that load a different image which you can define in the data-attributes? I'm currently going to extend your layzr with this functionality, but that would mean that I'm not able to update easily in the future. So it would be way better if you would add support for that ;)

Thanks!

@callmecavs
Copy link
Owner

@JeroenReumkens the approach you describe wouldn't be too hard to implement, but it's slightly different than how the lib currently approaches the "which image to load" question currently.

right now retina images are loaded based on the pixel density of the screen, whereas the approach you describe would make it based on the screen width. srcset/sizes is an attractive solution because to my knowledge it accounts for both.

going to do some more thinking on this. thank you for your feedback

@JeroenReumkens
Copy link

@callmecavs this indeed wouldn't be hard to implement, but I do think it would absolutely be of added value to the script! This way you can not only lazy load the images, but also deliver optimized images to the devices. Which is a double win I'd think :)

Thanks for your quick reply!

@JeroenReumkens
Copy link

Hi @callmecavs,

Couldn't resist to experiment with this. I've replicated the srcset functionality in a much simpler way. You can see my fork here: https://github.com/JeroenReumkens/layzr.js/commit/f1fbdad440f6f56c90f3badaa44b97abeee19f10

It expects a parameter data-layzr-srcset which holds something like "test1.jpg 300, test3.jpg 500, xxxxl.jpg 3000". Based on that string and the retinaMultiplier it will check what the best image width is to show on the screen. I've added the multiplier because I personally don't like the 2x images for pictures, because in my opinion 1.5x is most of the time good enough.

If will run the string to check what is the image that is the first one that is bigger than the needed image. That will become the image it shows. If there isn't a srcset specified, it will of course continue to work normally.

One extra thing I think of right now, is that I'm comparing the image to the screen width, to calculate the best image. But of course that isn't true, you should calculate it to the size you'd want to display. That would mean that you need to add an extra data-attribute which tells Layzr that size you'd need. But than still those sizes might differ on different breakpoints. On desktop it might be a 500px width image, and on mobile it becomes 100% of the screen width.

I'd like to hear what you think about this :)

Thanks,
Jeroen.

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

No branches or pull requests

3 participants