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

Support (lazy) responsive images #184

Closed
simonihmig opened this issue Mar 2, 2019 · 17 comments
Closed

Support (lazy) responsive images #184

simonihmig opened this issue Mar 2, 2019 · 17 comments
Labels
help wanted We could use your help! stale

Comments

@simonihmig
Copy link
Contributor

Many of the images here are not optimized well enough, leading to rather bad Lighthouse scores. Just an example of an image, that is naturally 500px wide, but displayed only at 182px:

image

Here the Lighthouse "Opportunities" suggestions for some image heavy pages:

https://ember-beta-website.netlify.com/ember-users
image

And this for https://ember-beta-website.netlify.com/mascots:
image

We have used some home-grown addons in some projects, and are quite happy with is so far:

This would also offload the burden to create optimized images when adding new assets from the contributor to the build process.

If there's interest in this, I could put a PoC PR together for one of the image heavy pages!

@mansona
Copy link
Member

mansona commented Mar 2, 2019

@simonihmig thanks for the report 🎉 yes I'm 100% behind this plan and it is a technique that we're already investigating for other "empress" projects: empress/empress-blog-casper-template#16 😂

I would like to have a chat with you about a few things before we go ahead with this effort though 🤔 I wanted to work on a PR to ember-responsive-image to look at making it support "next-gen formats" too at some point.

Also, I would be more comfortable implementing ember-responsive-image right now but we need to discuss the lazy image implementation before we go ahead with it. Am I right in saying that it's a drop-in replacement so we won't need to change much when we decide to upgrade to it?

@simonihmig
Copy link
Contributor Author

Hey @mansona!

I'm 100% behind this plan and it is a technique that we're already investigating for other "empress" projects

Oh, that's cool!

I would like to have a chat with you about a few things before we go ahead with this effort though 🤔

Sure, feel free to ping me on Discord!

I wanted to work on a PR to ember-responsive-image to look at making it support "next-gen formats" too at some point.

Indeed, that would be cool! Seems sharp already supports WebP, so the image processing at build time would be fairly straight forward. But not sure about best practices regarding the partial browser support? If you have thoughts on that already, then please file an issue where we can discuss further...

Have a few other improvements in my mind as well:

Am I right in saying that it's a drop-in replacement so we won't need to change much when we decide to upgrade to it?

Yes! We can do that in multiple iterations...

/cc @andreasschacht

@mansona
Copy link
Member

mansona commented Mar 3, 2019

@simonihmig in fact the thing that I wanted to chat with you is mostly covered by simonihmig/responsive-image#50 (I think). Essentially when I was looking into this for empress-blog it was not 100% clear that there was a good match between where to put files and how to use them. I think we can discuss more on that existing issue 👍

At this stage I'm unlikely to be able to do much on this topic before emberconf 🙈 but I will re-engage with you on it afterwards 👍 unless you'll be going this year and we can have a chat about it over there 😉

@simonihmig
Copy link
Contributor Author

I think we can discuss more on that existing issue

Yes, we'll lay out details there. Feel free to chime in at any time!

unless you'll be going this year and we can have a chat about it over there

Unfortunately, not able to come. Yet again... 😫😭

@mansona
Copy link
Member

mansona commented Mar 4, 2019

That's a shame! But we should arrange a call for soon afterwards 👍

@stale
Copy link

stale bot commented Jul 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jul 8, 2019
@amyrlam amyrlam removed wontfix This will not be worked on labels Jul 9, 2019
@stale
Copy link

stale bot commented Nov 6, 2019

This issue has been automatically marked stale. If this issue is something that still needs work, please add a comment and it will remain open, otherwise it will close in 7 days. You are welcome to open a new issue if you miss the window. Thanks!

@stale stale bot added the stale label Nov 6, 2019
@mansona
Copy link
Member

mansona commented Nov 7, 2019

This is still valid 🙈 and I really need to catch up and have that call we discussed

@stale stale bot removed the stale label Nov 7, 2019
@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Feb 21, 2020

bad stale bot!

with the new website release, all the images bring the home page's perf score down below 50 for US (and much worse for European folks).

lazy images would be amazing here.
Also, chrome has lazy image support built in... so maybe if firefox also adds that soon, we can get away with just adding the lazy attribute to all of them, so we have built-in fallback for browsers that don't support the lazy attribute?

though, this would support even older browsers (as well as many of todays browsers): https://github.com/kaliber5/ember-responsive-image

@MelSumner MelSumner added the help wanted We could use your help! label Feb 21, 2020
@abhilashlr
Copy link
Member

Should we start adding webp optimised images as well in that case? Has better compression and optimizes for size.

@simonihmig
Copy link
Contributor Author

Should we start adding webp optimised images as well in that case? Has better compression and optimizes for size.

We cannot serve webp exclusively: https://caniuse.com/#search=webp

It would be possible to support webp with jpeg as fallback, using <picture>, but that's not currently supported by ember-responsive-image, and not sure this is easy to add...

@abhilashlr
Copy link
Member

<figure>
  <picture>
    <source srcset="webp path" type="image/webp">
     <img loading="lazy" src="jpg path" alt="alt value" />
  </picture>
</figure>

This is something that I do in my blog app to let the browser choose whichever it is compatible with.

@NullVoxPopuli
Copy link
Contributor

Does that still allow the jpg or svg to be used in case of old/legacy browsers like ie11 and Safari?

@abhilashlr
Copy link
Member

Does that still allow the jpg or svg to be used in case of old/legacy browsers like ie11 and Safari?

Yes, I've got this working for jpg in Safari. I can double-check with IE11 with SVGs. Might help: https://css-tricks.com/using-webp-images/

@alexeykostevich
Copy link

alexeykostevich commented Apr 8, 2020

@NullVoxPopuli , just released Firefox 75 adds support of image lazy loading.

Lazy loading can a good addition to responsive images but not a replacement: we can still ask a browser to load an optimized version when an image enters a viewport.

So, to sum up the discussion here, the ideal solution in terms of performance looks like this:

  1. Prefer using SVG where possible (which is inherently responsive and efficient).
  2. Use lazy loading for non-essential bitmap images.
  3. Use <picture> for bitmap images that should be responsive.

@alexeykostevich
Copy link

alexeykostevich commented Apr 8, 2020

With regards to WebP, I would not recommend using it while it is not supported by all the major browsers (e.g. Safari and iOS Safari) and supported legacy browsers (IE11).

Even if we can fallback to other bitmap formats (e.g. PNG) using <picture>, it might add extra complexity and maintenance work with no much win over using points 1-3 mentioned above. The Ember.js website is not visual content-oriented website and the impact of adding WebP for out bitmap images might be low.

@stale
Copy link

stale bot commented Aug 6, 2020

This issue has been automatically marked stale. If this issue is something that still needs work, please add a comment and it will remain open, otherwise it will close in 7 days. You are welcome to open a new issue if you miss the window. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We could use your help! stale
Projects
Development

No branches or pull requests

7 participants