Add scrollable attribute to amp-image-lightbox

[jaygray0919]

amp-lightbox supports a scrollable attribute https://www.ampproject.org/docs/reference/components/amp-lightbox

can the scrollable attribute also be supported in amp-image-lightbox?

/jay gray

[cathyxz]

amp-image-lightbox currently supports zoom and pan. It behaves slightly oddly for images with long aspect ratios, but that's a bug (that we are fixing for the newer iteration of lightbox, which I could probably port over). Can you tell me more about your use case and what's not working currently for you? Ideally, what would happen for a super long image is it would open and show the entire image as object-fit: contains, and then you double tap to zoom in, at which point you can pan to see the whole thing.

[jaygray0919]

we have 'long' SVGs (directed acyclic graphs) that are in an amp-carousel. the thumbnail is good; the image displayed in the carousel is good; but the light-box is truncated, particularly in landscape mode. So we're like to have a vertical scroll bar for desktop or swipe up/down for mobile when the light-box is in view.

Does that make sense?

[cathyxz]

So the current UX for super-long images isn't great (e.g. https://codepen.io/cathyxz/full/aENjmN/ ). I think this (scrollable) is reasonable for desktop, but I'm concerned with how this interacts with panning on mobile, since we override swipe on zoom.

I think the mobile experience can be mitigated by porting https://github.com/ampproject/amphtml/pull/12440 to amp-image-lightbox (which we should do anyway), and playing with the zoom upper bound is the max the current value and zoom factor needed for the smaller dimension to fit the screen. Similar to how photo galleries on phones handle double tap zoom on super long images.

Another possible solution is to enable this attribute when zoom factor is 1, and to disable it on zoom (since we have panning active).

@aghassemi what are your thoughts?

[jaygray0919]

here is a sample SVG:

flavonoids.zip

[cathyxz]

I think the way to fix this for mobile is as follows:

1. Port #12440 to amp-image-lightbox, which opens lightbox images in object fit.
2. Re-adjust the Zoom upper bound so that it is the max of our current default constant and whatever zoom factor necessary to achieve object-fit: cover.
3. Intelligently open images in zoomed in mode (object-fit: cover) for images that have aspect ratios wildly different from the viewport aspect ratio, vs zoomed out mode (object-fit: contains) for images whose aspect ratios are close to the viewport. You can use pinch / double tap to zoom in and zoom out of either one.

This will effectively give you swipe up / down / left / right via panning.

This will also apply to desktop (possibly with a different zoom factor adjustment), the only issue there is we haven't enabled zoom. On desktop, a tentative proposal is we'll map double click to double tap to enable zoom for the interaction, and click+drag will pan.

[jaygray0919]

Sounds good. Aside: we have some very large DAGs (directed acyclic graphs) that are SVGs. We have been able to construct a horizontal nav bar so a user can scroll/swipe left/right (as well as up/down). This approach enables us to put the DAG into 'window'. But we don't use carousel or lightbox for these Graphs. Your solution will allow us to put a subset of those Graphs into a carousel, and then use lightbox for displaying detail. As soon as we see the updated solution, we'll be able to decide: which DAG subet go into a carousel, and which are 'stand alone' because of their size.

Aside: we also want to treat Vega diagrams in a similar way. Have spoken with @aghassemi about converting our D3 diagrams to Vega. These diagrams often have similar properties to our DAGs - either unusually wide or unusually long. We want to put a Vega-subset into a carousel and the use lightbox to expose features.

[jaygray0919]

@cathyxz Hi cathy. Here is a carousel that needs the vertical scroll bar: https://afdsi.org/test/amp_carousel/ Will Lightbox 2.0 handle this issues? Will the amp-lib designation change from <script async custom-element='amp-carousel' src='https://cdn.ampproject.org/v0/amp-carousel-0.1.js'></script> to a new release number, or will the instance simply include a vertical scroll bar when Lightbox 2.0 is moved to production? /jay

[cathyxz]

Hi @jaygray0919 thank you for the example. I think we can find a way to make that work.

Lightbox 2.0 will have a slightly different API. It's currently under the extension <amp-lightbox-viewer>, and used with the lightbox attribute (though this API may change pending review). The way you would use it is to include

<script async custom-element='amp-lightbox-viewer' src='https://cdn.ampproject.org/v0/amp-lightbox-viewer-0.1.js'></script>

and add a lightboxed attribute to your amp-carousel.

[jaygray0919]

OK @cathyxz - we will build ~20 carousels with their lightbox SVGs in the form used in the example.

When you promote Lightbox 2.0 to production, we'll update those amp-carousel files to replace amp-image-lightbox with amp-lightbox-viewer

Please send a "flare" when we should update files like the above example.

[aghassemi]

@cathyxz can we close this in favour of #13602?

[cathyxz]

So revisting https://afdsi.org/test/amp_carousel/ after https://github.com/ampproject/amphtml/pull/12636, @jaygray0919 how do you feel about the current experience? It will always open the image in full view, and you can double tap / pinch zoom on mobile, as well as browser zoom on desktop. <amp-lightbox-gallery> will offer the same UX. Does that work for you, or do you explicitly want the scrollbar?

[cathyxz]

Closing in favor of #13602. Feel free to reopen if you feel like a different solution is necessary. =)