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

Only load mobile and desktop images appropriately #361

Closed
tunetheweb opened this issue Nov 7, 2019 · 8 comments · Fixed by #604
Closed

Only load mobile and desktop images appropriately #361

tunetheweb opened this issue Nov 7, 2019 · 8 comments · Fixed by #604
Assignees
Labels
development Building the Almanac tech stack enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@tunetheweb
Copy link
Member

tunetheweb commented Nov 7, 2019

Currently we have things like this:

<img class="intro-image-2019" src="/static/images/home-hero-2019.png" alt="The Web Almanac 2019 edition">
<img class="intro-image" src="/static/images/home-hero.png" alt="The Web Almanac 2019 edition">

And then we show and hide the appropriate image on desktop and mobile using CSS.

However this causes both images to be load, even though one will not be used.

This can be mitigated by using background-images for these images and hiding the parent element appropriately. This causes the browser to realise whether the image will be displayed or not after evaluating the CSS and not load it unless it's needed.

So I think we should change this to something like this:

<span class="desktop-image">
    <span class="intro-image-2019" style="background-image: '/static/images/home-hero-2019.png'">
</span>
<span class="mobile-image">
    <span class="intro-image" style="background-image: '/static/images/home-hero.png'" alt="The Web Almanac 2019>
</span>

(though with the background-image set in CSS and not inline like this which is just done for illustrative purposes).

And then set the display:none option on the desktop-image and mobile-image styles as appropriate.

Alternatively a potentially better, more accessible option is to use the picture and srcset elements, but I'm less familair with them to be honest...

See here for more info: http://designingforperformance.com/responsive-web-design/#images

@tunetheweb
Copy link
Member Author

tunetheweb commented Nov 7, 2019

Or is browser-native lazy-loading an acceptable/recommended option here? In which case can include as part of #355.

@AymenLoukil
Copy link
Contributor

AymenLoukil commented Nov 7, 2019

Alternatively a potentially better, more accessible option is to use the picture and srcset elements,

Yes the best solution if responsive images with srcset here.
This tool is useful to generate the 3 (we just need Desktop, Tablet and Mobile) versions.
https://www.responsivebreakpoints.com/
Example :
https://res.cloudinary.com/responsivebreakpoints/image/upload/c_scale,w_385/v1573134390/home-hero-2019_x04cmk.png
https://res.cloudinary.com/responsivebreakpoints/image/upload/c_scale,w_643/v1573134390/home-hero-2019_x04cmk.png
https://res.cloudinary.com/responsivebreakpoints/image/upload/c_scale,w_715/v1573134390/home-hero-2019_x04cmk.png

@tunetheweb
Copy link
Member Author

BTW outstanding question with Chrome Dev team here: GoogleChrome/web.dev#1850

@rviscomi rviscomi added development Building the Almanac tech stack enhancement New feature or request good first issue Good for newcomers labels Nov 7, 2019
@rviscomi rviscomi added this to the SHIP IT! milestone Nov 7, 2019
@rviscomi rviscomi mentioned this issue Nov 7, 2019
5 tasks
@rviscomi
Copy link
Member

rviscomi commented Nov 7, 2019

Related: #262 and #413.

@rviscomi
Copy link
Member

<picture> helps with swapping between desktop and mobile-optimized images.

For #413 I'd also like to find a solution for swapping between an iframe on desktop and an image on mobile.

@logicalphase logicalphase self-assigned this Nov 11, 2019
@logicalphase
Copy link
Contributor

logicalphase commented Nov 11, 2019

I'd like to continue with working on <picture> method for serving up hero and others.

@rviscomi the last commit 2cc7b8b where I replaced hero img markup with picture elements and srcset for webp definitely fixes this for Chapter pages.

I can work in other optimizations and clean up some best practices, etc. I've got free time over the next few days. Does that work?

@logicalphase
Copy link
Contributor

@rviscomi just circling back. I know you've been crazy busy, but wanted see if you still want the optimized, sized, and webp images I created committed, and 2cc7b8b combined to use picture elements that serve optimized images based on screen size. I know this got rushed at the end, so let me know your thoughts and I'll proceed accordingly. Cheers.

@tunetheweb
Copy link
Member Author

@HyperPress think this got lost in the fun of the release, so I went ahead and submitted this in #603. Would be good to have your input as I implemented it slightly differently than you had.

Still considering how to do the same for the home page...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Building the Almanac tech stack enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants