amp-wp
amp-wp copied to clipboard
ampproject
[Gallery Block] Lightbox effect displays thumbnail after click instead of showing full size image
Bug Description
As in the title. Lightbox effect shows a pixelized thumbnail on full screen instead of full image. See screenshots below.
Expected Behaviour
When you click on the thumbnail, a full quality image should show up.
Screenshots
https://user-images.githubusercontent.com/25438601/152776284-848771e3-1fe2-4b36-816d-35c3e3a11cda.mp4
PHP Version
8.0
Plugin Version
2.2.1
AMP plugin template mode
Standard, Transitional, Reader
WordPress Version
5.9
Site Health
No response
Gutenberg Version
No response
OS(s) Affected
No response
Browser(s) Affected
No response
Device(s) Affected
No response
Acceptance Criteria
No response
Implementation Brief
No response
QA Testing Instructions
No response
Demo
No response
Changelog Entry
No response
OK, thanks. It may also be related to changes in WordPress 5.9. We'll investigate. Thanks for reporting.
There doesn't seem to be a regression in v2.2.2 compared to v2.2.1 in how a gallery or a lightbox is handled.
@rafaucau I think that you get pixelated images in lightbox because the "Image size" is set to "Thumbnail" in the Gallery block settings (as per the screenshot). AMP will use "at most" this image size when opening a lightbox. If you switch to a larger size, e.g. "Full size", you'll get non-pixelated images.
I think that it's also quite safe to do so since by default <img> tags have srcset and sizes attributes defined. Thanks to that, browsers can download and serve the most appropriate image sizes based on a screen resolution, pixel density, etc. Compare the set of image sources when "Thumbnail" and when "Full size" is used in the block settings:
| Thumbnail | Full size |
|---|---|
 |
 |
Rather, it should work the same way as in the standard version, i.e. thumbnails are displayed and when I click it, the full size is loaded.
And an example from the site I work for. There are tens of thousands of posts which have the gallery set up this way. Previously, we were using another AMP plugin and there it worked fine (but other things didn't work, so we switched to the official one :D).
Maybe I could help with a pull request.
I think I found the solution, and it is to implement thumbnail previews: https://amp.dev/documentation/components/amp-lightbox-gallery/#implement-thumbnail-previews
Maybe not. I was thinking there should be some mechanism to select a different image to open in a lightbox when the small image is clicked. I thought that is what the “implement thumbnail previews” feature was for, but apparently it's for the thumbnails in ght lightbox view.
Maybe I could help with a pull request.
@rafaucau It would be super helpful if you are able to send in a Pull Request for this, contributions are always welcome! ❇️
Please do confirm if you would be able to send it over, I'll have the issue assigned to you accordingly.
@maitreyie-chavan Please do confirm if you would be able to send it over, I'll have the issue assigned to you accordingly.
I confirm, but I don't know if I can handle it and if it will be possible at all in AMP. I am new here. If you have any tips it would be helpful.
In my opinion, the best way to do it would be that if the media contains links, the lightbox should be used automatically, without checking a separate option in AMP. That way, sites that were previously using the lightbox in the standard theme (non-AMP) will automatically have the lightbox in the AMP version.
I haven't looked into the code of this plugin yet, but I added such a sanitizer on my website based on this gist:
class Sanitizer extends AMP_Base_Sanitizer {
public function sanitize() {
/**
* @var Element $img
*/
foreach ( $this->dom->xpath->query( '//figure/a[ @href ]/img' ) as $img ) {
$link = $img->parentNode;
if ( $img->getAttribute( 'width' ) === '150' && $img->getAttribute( 'height' ) === '150' && ! empty( $img->getAttribute( 'data-id' ) ) ) {
$el = simplexml_load_string( wp_get_attachment_image( (int) $img->getAttribute( 'data-id' ), 'full' ) );
$img->setAttribute( 'src', $el->attributes()->src );
$img->setAttribute( 'sizes', $el->attributes()->sizes );
$img->setAttribute( 'srcset', $el->attributes()->srcset );
}
$link->parentNode->replaceChild( $img, $link );
$img->setAttribute( 'lightbox', '' );
}
}
}
The thumnail size does not have srcset, so the sanitizer takes the original image and replaces the attributes on the current one.
Hello @rafaucau This may not be required by everyone, It will be really helpful if you can create a mini plugin with the fix, so anyone who needs it can download and use it, we can list your mini plugin on our site .
You can use this skeleton plugin for reference.
