Automattic / Automattic/jetpack
Carousel / AMP: AMP lightbox applied only to first image, and regardless of image link
- Dominant language
- PHP
- Stars
- 1.8k
- Forks
- 898
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 774
Description
#### Steps to reproduce the issue
Start with a site where you've enabled the Carousel feature under Jetpack > Settings, and where you've added the AMP plugin and set it to Transitional mode.
1. Go to Posts > Add New
2. Add some content, and 2 single images.
3. Add a link to both of those images, linking to other posts on your site.
4. Publish your post.
5. On the frontend, you'll notice that when clicking on the images, you're redirected to the other post you linked to.
6. Add `?amp` to the post URL.
7. You'll notice that the first image (and only the first image) now uses an AMP lightbox: instead of linking out to the other post, it opens a lightbox.
This was introduced in #15398, in an attempt to try to replicate Jetpack's Single Image Carousel feature that was introduced in #5469. There are 2 distinct issues here:
- [ ] We should only apply the AMP lightbox (in `maybe_add_amp_lightbox()`) when the image links to an attachment page. In the general implementation of the single image carousel feature, this was done via the `data-permalink` attribute, added to the image markup and pointing to the attachment page. Any other link would be ignored.
- [ ] The AMP lightbox should be applied to all single images on the page, not just the first one.
*******
Original report:
- 3332333-zen
- p9F6qB-5WD-p2
Contributor guide
Research direction
Start at maybe_add_amp_lightbox() and trace how single-image markup is inspected for data-permalink. Reproduce the post with two linked images in AMP mode, then verify that only attachment links receive the lightbox and that every eligible single image does.】【。
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php, wordpress
- Domain
- frontend, web-dev
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100