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

Anything with a thumbnail and click to enlarge #14900

Closed
IamAlta opened this issue Apr 26, 2018 · 8 comments
Closed

Anything with a thumbnail and click to enlarge #14900

IamAlta opened this issue Apr 26, 2018 · 8 comments

Comments

@IamAlta
Copy link

IamAlta commented Apr 26, 2018

What's the issue?

Feature request.

The idea of AMP is to accelerate the page loads. However, loading an extra large image as a thumbnail defeats the purpose.

Many times, the visitor does not want to see an extra large image, and loading the extra large image results in unnecessary transit bits. Some extra large images cannot stand much compression if it's a detailed image where the customer wants to see the detailing.

amp-carousel and amp-lightbox have these issues. I am unaware of any other functions that might.

The request is to load an small thumbnail then load the larger image from the server if and when the visitor clicks the thumbnail. This can reduce page load times to give the end-user a better experience.

How do we reproduce the issue?

n/a

What browsers are affected?

All browsers

Which AMP version is affected?

Powered by AMP ⚡ HTML – Version 1524089272632

@nainar
Copy link
Contributor

nainar commented Apr 26, 2018

cc @cathyxz

@cathyxz
Copy link
Contributor

cathyxz commented Apr 26, 2018

This is blocked by #11575. For reference, we have for addressing this in lightbox specifically #12929. Ideally we should be able to achieve this with srcset and sizes, which is another reason to prioritize #11575.

@IamAlta
Copy link
Author

IamAlta commented Apr 27, 2018

Thanks for the suggestion to achieve this with srcset and sizes. It's not immediately intuitive how it'll load a small image with the page load, and then load a larger one when the small image is clicked. I'll research it. Have a wonderful day, @cathyxz!

@cathyxz
Copy link
Contributor

cathyxz commented May 5, 2018

Actually maybe I'm over-complicating this. @IamAlta there might be a few simple ways to achieve this:

  1. You can have a small image, on tap open a <amp-lightbox-gallery> of another (hidden) image.
  2. You can achieve this with vanilla <amp-lightbox> by putting the larger image in a lightbox and making the small image a tap target.

<amp-lightbox-gallery> is currently launched under an experiment.

srcset and sizes would enable you to specify a srcset and a sizes attribute, defining a src for each device width, and the browser would not load the larger image (this is subject to the browser's native logic) until it needs it. It would be up to components like <amp-lightbox-gallery> to figure out which url to choose based on the image dimensions.

@IamAlta
Copy link
Author

IamAlta commented May 5, 2018

@cathyxz,
Thank you. That will work.

I was unaware that you can put the larger image in the lightbox and it won't load until the lightbox is displayed.

REQUEST: Perhaps add that information to your Amp by Example and Amp Project - that an image can go into the amp-lightbox, and it won't load until the button is clicked.

I hesitate to try the amp-lightbox-gallery at this time because we are so close to launch (I hope).

Make it a wonderful day! You certainly deserve it.

@IamAlta
Copy link
Author

IamAlta commented May 5, 2018

Oh, @cathyxz,
I assume that I need a separate lightbox container for each thumbnail that has a large image.

@ampprojectbot ampprojectbot added this to the New FRs milestone Jun 5, 2018
@cathyxz cathyxz self-assigned this Jul 16, 2018
@ampprojectbot
Copy link
Member

This issue hasn't been updated in awhile. @cathyxz Do you have any updates?

@cathyxz
Copy link
Contributor

cathyxz commented Oct 16, 2018

This should work out of the box with <amp-lightbox-gallery> if you enter the srcsets correctly.

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

No branches or pull requests

4 participants