Skip to content

Carousel / AMP: AMP lightbox applied only to first image, and regardless of image link #17484

Open
@jeherve

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

Metadata

Assignees

No one assigned

    Labels

    AMPCustomer ReportIssues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report"Triaged[Feature] CarouselA fullscreen modal appearing when clicking on an image in a gallery or tiled gallery.[Pri] Low[Type] BugWhen a feature is broken and / or not performing as intended

    Type

    No type

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions