Add lightbox effect doesn't work for image blocks

[rsmith4321]

Bug Description

If an image is inserted using the block editor image block, and under AMP Settings the Add lightbox effect is selected as on. The image is not loaded in a lightbox when clicked.

Expected Behaviour

The image should be loaded in an amp lightbox when clicked.

Steps to reproduce

The first 3 images on this page have Add lightbox effect on with no effect. https://www.ryansmithphotography.com/myrtle-beach-wedding-photography-prices/

Screenshots

Additional context

• WordPress version: 5.2.3
• Plugin version: 1.3.0
• Gutenberg plugin version (if applicable):
• AMP plugin template mode: standard
• PHP version: 7.3
• OS:
• Browser: chrome
• Device: MacOS Desktop

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[westonruter]

@rsmith4321 Do you have any filters on images like you had for galleries in that other issue?

[rsmith4321]

As far as I know there are no filters or special settings on my site for images. Does this work on other sites? I just assumed it was something that hasn't been looked at yet similar to the gallery lightbox issue.

[westonruter]

@kienstra Would you please investigate?

[kienstra]

Sure, @westonruter!

[kienstra]

Thanks, I also saw this

Hi @rsmith4321,
Thanks for bringing this up. It looks like this is a valid bug.

As a quick workaround, does removing the link in the Image blocks help?

I could reproduce this issue when the Image block has a link, but the lightbox looks good when it doesn't.

I'll look at a code fix in the plugin, but maybe that workaround will help in the meantime.

[rsmith4321]

Yes this works. Is there an seo disadvantage to no longer linking to the original image? Also, on a desktop I want to target these type of images with cursor:pointer css since the cursor no longer changes on hover. How can I target only lightbox supported images with css? Or perhaps this is something you can add.

[kienstra]

Hi @rsmith4321,

Yes this works.

Thanks, that sounds good. Of course, I'll also work on a fix in the plugin for that.

Is there an seo disadvantage to no longer linking to the original image?

Hm, I'm not sure.

Also, on a desktop I want to target these type of images with cursor:pointer css since the cursor no longer changes on hover. How can I target only lightbox supported images with css? Or perhaps this is something you can add.

I'll look at this also.

[kienstra]

Hi @rsmith4321,

Also, on a desktop I want to target these type of images with cursor:pointer css since the cursor no longer changes on hover. How can I target only lightbox supported images with css? Or perhaps this is something you can add.

This worked locally, though maybe you'd want to add selectors so it doesn't apply to all <amp-img> in the document:

amp-img[data-amp-lightbox] {
	cursor: pointer;
}

Images that have that 'Add lightbox effect' toggle on will have a data-amp-lightbox attribute:

[kienstra]

Request To Test

Hi @rsmith4321,
Thanks for your help with testing these PRs.

Could you please test #3460, which should fix this issue? It should now be possible select links, while still getting the lightbox (though the links won't appear on the front-end):

[kienstra]

There's probably no need to test this now until there's a response on ampproject/amphtml#25021

[westonruter]

It looks like the direction is to stop using amp-img-lightbox in favor of using amp-lightbox-gallery. See ampproject/amphtml#25021 (comment)

[kienstra]

Sounds good, thanks for 'bumping' that issue. I'll look at using the amp-lightbox-gallery for this.

[rsmith4321]

Sorry I haven't had time to test but it sounds like you are going a different way anyway. The amp-lightbox-gallery lightbox seems a lot more full featured for example the amp-img-lightbox doesn't even have a close icon to indicate how to exit the lightbox. Thanks!

[kienstra]

Sure, no problem in testing, this was dependent on the amphtml issue anyway. I'm going to work on converting this to an amp-lightbox-gallery.

[kienstra]

Request For Testing

Hi @csossi,
When you can, could you please test this, using the steps to reproduce here?

Thanks, Claudio!

[csossi]

Verified in QA