Amphtml: [amp-lightbox]: Enable scrollable lightbox

[aghassemi]

Enable lightbox content to be optionally scrollable <amp-lightbox scrollable>

[dvoytenko]

This is a dupe, I believe. /cc @sriramkrish85

Previously I decided not to pursue the fix for this because it was way to expensive. Would be great if we find a good solution. But heads up for now.

[pdufour]

🙏

[akshaylive]

Looking forward to this as well. @dvoytenko can we have an option which doesn't consume the gestures? <amp-lightbox enable-scroll="true"> or something similar? This wouldn't be too hard to implement I guess. LMK if you like the idea and want me to cut a PR! Thanks :)

[akshaylive]

Here's why we need a scrollable lightbox -

At Genius, our music community annotates lyrics of songs. When a line is clicked upon, we would like to display the annotation. Here's one of our amp pages - https://google.com/amp/genius.com/amp/Beyonce-love-drought-lyrics

Currently, we scroll the page to the annotation. We recently found out that this way of displaying annotations was broken because of https://github.com/ampproject/amphtml/issues/5309 .

As an alternative, we wanted to work with the lightbox to display annotations. However, some of our annotations is larger than the screen size, and it is necessary for us to let our users scroll the lightbox so that they can read the full annotation contents.

There are no other components that can fit our needs. Is there something else that you recommend? @ericlindley-g

[dvoytenko]

The <amp-lightbox scrollable> or <amp-lightbox scrolling=yes> would be fine. Maybe even we could rename finally amp-lightbox to amp-modal or such to avoid constant confusion.

[ebbeck]

is there a timeline when this will be fixed? It is currently causing a major headache for a page we are building where we need the lightbox to be scrollable - any workarounds at the moment to this that anyone knows of?

[aghassemi]

@muxin Let's see what we can do to enable <amp-lightbox scrollable>. Let's not worry about renaming it to amp-modal yet.

[ericlindley-g]

+1 to accelerating just the scrollability piece, as @aghassemi says

[philippklemmer]

we really need this feature, too. For desktop browsers this seems to work quite well already.
On mobile devices the behavior is quite inconsistent. iOS 10.* devices scroll ok. iOS 9.3.5 devices are kind off scrollable with two fingers, although this sometimes scrolls the body. Android seem to show the same two-finger behavior. We can supply further test results if it helps

[aghassemi]

@philippklemmer @ebbeck Fix in progress in https://github.com/ampproject/amphtml/pull/6398 which will introduce a new attribute (scrollbale) on <amp-lightbox> when set, it should behave similar to any vertically scrollable container and single-finger swipes should scroll the content.

[beck410]

fantastic! thanks for moving quickly on this 😄

[aghassemi]

@philippklemmer @ebbeck @akshaylive This will be in _Dev Channel_ this week. Please give it a try and let us know if you run into issues. You should no longer need to use amp-iframe to hack around creating a scrollable lightbox. <amp-lightbox scrollable>long stuff</amp-lightbox> should behave like any vertically scrollable sub-section. Without the scrollable attribute, lightbox behaves as it used to.

Known issue:
-The page behind will still scroll when user reaches _the end_ of the scrollable area if they keep scrolling: https://github.com/ampproject/amphtml/issues/6487

[beck410]

will this affect lightboxes that need to have an iframe?

[aghassemi]

@beck410 no, given iframe scrolling on iOS is non-trivilal, ideally the iframe itself is made non-scrollbale. If the height of iframe happens to be longer than the lightbox's height, then lightbox itself will scroll (with normal single-finger scrolling gesture) if scrollable attribute is set on the lightbox.

[beck410]

great! will the -webkit-overflow-scrolling: touch; issue also be solved with this?

[aghassemi]

@beck410 As long as the amp-iframe itself is not scrollable (e.g. scrolling=no) yes it is fixed and tested ( here is a sample test page: https://amphtml-ae5fc.firebaseapp.com/scroll/test/manual/empty.amp.html ) (mixing scrollable iframe with scrollable lightbox does not make sense as now there are two scrolling targets)

[beck410]

thanks for the example!

[beck410]

@aghassemi sorry to keep bothering you with this but i just want to check that the fix went out in the dev-channel release yesterday - i opted in on a safari browser on ios and my iframe within a light still doesnt seem to scroll

here is an example event:
https://www.evbqa.com/e/amp-test-ios-scroll-event-tickets-29597149872/amp

If you click the organge TICKETS btn you should be able to scroll to the checkout btn after choosing a ticket

any help would be appreciated

[aghassemi]

@beck410 Could you please confirm the AMP Version? The Dev Channel should be 1481235621696 (version number gets printed to Console or you can find it on the <html> tag on the amp-version attribute)

I tested your page on iPhone6/iOS10 with Dev Channel and it was scrolling fine.

[beck410]

seems i had the wrong version - i tried on another ios device and its working - thanks for your help :)

[aghassemi]

@beck410 I also put together an example that uses scrollable lightbox and iframe-resizing to handle dynamic cases where the size of the iframe is not known.

Please visit https://amphtml-ae5fc.firebaseapp.com/resize/test/manual/empty.amp.html for the example.

Few important points to observe:
1- sandbox="allow-scripts allow-same-origin" is set on the amp-iframe in this case.
2- The embedded iframe sends an embed-size event onload of the page. (see code at the end of the embedded page
3- The origin of the parent page and the embedded page are different. This is a common pitfall during dev and QA with amp-iframe and resizing. The embedded page MUST be from a different origin than the parent page ( see https://github.com/ampproject/amphtml/blob/master/spec/amp-iframe-origin-policy.md for why ) otherwise you get a error.js:112 Origin of <amp-iframe> must not be equal to container error and no resizing will happen. (This is the reason I put my embedded page on githubusercontent.com and the parent page on firebaseapp.com).

I hope this helps.

[beck410]

Thanks @aghassemi i'll have a play around with it - really appreciate the example!

[beck410]

@aghassemi I'm having some trouble resizing the iframe - i have a feeling its to do with parent page origin being the same as the embedded page except im not getting any errors and it loads fine

Where is the best place to reach you so i'm not spamming everyone following this issue? slack?

[aghassemi]

If anyone else on the thread wants direct contact, aghassemi @ google.com for email/hangout or ping me on slack.

[RymRymRym]

hello i've seen this but i still can not display the scroll bar in my site amp . Can you help me please ?

[aghassemi]

@RymRymRym Can you share a sample Url/Demo? Is <amp-lightbox scrollable just not working for you?

[smokeyfro]

@aghassemi We're experiencing the same on winefolly.com, where we're using amp-lightbox to open our feedback form, which is loaded via amp-iframe. It all works fine on desktop, but on mobile the frame gets cut off. I've tried all the suggestions that I came across, but none seem to work. What's stranger is it looks like this was fixed in amp a while ago, but it's still not working on our end. I have had to disable the button that activates the lightbox, but can re-enable if you'd like to take a look. You can see the desktop version here https://winefolly.com/tutorial/7-wines-from-chile-that-will-blow-your-freakin-mind/ and enable the mobile version by removing the uk-hidden class from the last div in the article.

Any suggestions would be much appreciated.