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

[amp-lightbox-gallery] upgrade srcset on open or zoom to handle thumbnail usecase #12929

Closed
cathyxz opened this issue Jan 19, 2018 · 4 comments
Closed

Comments

@cathyxz
Copy link
Contributor

cathyxz commented Jan 19, 2018

To handle the use case of having small thumbnails show a high resolution image in lightbox gallery, we should try to upgrade the srcset / update sizes after opening the image in lightbox gallery. Some experimentation can be done with changing the sizes attribute and seeing whether browsers are able to handle this natively. Otherwise, we can revisit the logic to parse and find the correct src using srcset and upgrade on zoom. Blocked by #11575.

Here's a test case: https://codepen.io/cathyxz/full/rreWev/

@ampprojectbot
Copy link
Member

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

@cathyxz cathyxz changed the title Lightbox: handle srcsets properly in zoom [amp-lightbox-gallery] upgrade srcset on open or zoom to handle thumbnail usecase Jul 2, 2018
@cathyxz
Copy link
Contributor Author

cathyxz commented Jul 2, 2018

See description edit for updates, in short we can resume this after #11575.

@ampprojectbot
Copy link
Member

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

@cathyxz
Copy link
Contributor Author

cathyxz commented Oct 16, 2018

Already done via

updateSrc_() {
if (!this.srcset_) {
return Promise.resolve();
}
this.maxSeenScale_ = Math.max(this.maxSeenScale_, this.scale_);
const width = Math.max(
this.imageBox_.width * this.maxSeenScale_,
this.sourceWidth_
);
const src = this.srcset_.select(width, this.getDpr());
if (src == this.image_.getAttribute('src')) {
return Promise.resolve();
}
return this.mutateElement(() => {
this.image_.setAttribute('src', src);
}, this.image_);
}
, upgrades on zoom within the example.

@cathyxz cathyxz closed this as completed Oct 16, 2018
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

2 participants