Amphtml: Fix scroll freeze for scrollable lightbox

Created on 25 Feb 2017  Â·  9Comments  Â·  Source: ampproject/amphtml

First Timers Only

We know figuring out the process for contributing to an open source project can be intimidating, so we created this issue as a way for you to learn the ropes. (If you feel comfortable contributing to open source projects, please leave this issue for someone else.)

What you will need to know

  • CSS
  • Javascript

Background

Amp-lightbox is an amp-component like a modal that expands to fill the viewport, until it is closed by the user. It is a position: fixed element that fill the whole viewport.

Motivation

Unfortunately in ios safari, when a user overscrolls in the lightbox, there will be scroll freeze because safari thinks that the user is scrolling the background therefore freeze the scrolling of the foreground. And to fix that problem, one hacky solution is to set scrollTop = 1 when lightbox is overscroll (scrollTop == 0). We already had similar fix for viewport overscrolling

Step by step

  • [ ] Claim this issue by adding a comment below. Please only claim this bug if you plan on starting work in the next day or so.
  • [ ] If you aren't too familiar with Git/GitHub, see the Getting Started End-to-End Guide for an intro to Git & GitHub, and how to get a copy of the code. You can also refer to the Quick Start Guide for the necessary setup steps with less explanation than the End-to-End guide.
  • [ ] Follow the instructions for building AMP.
  • [ ] [Create a Git branch](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#create-a-git-branch) for making your changes.
  • [ ] Make scrollTop adjustment in scrollHandler_ inside amp-lightbox.js when this.element.scrollTop == 0
  • [ ] Also make sure to adjust the scrollTop when opening up the lightbox
  • [ ] [Commit your changes](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#edit-files-and-commit-them) frequently.
  • [ ] [Push your changes to GitHub](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#push-your-changes-to-your-github-fork).
  • [ ] [Create a Pull Request](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#send-a-pull-request-ie-request-a-code-review) and request a code review from me (@muxin). Mention Fixes #7800 in the description.
  • [ ] [Respond to your reviewer's comments](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#respond-to-pull-request-comments) (if any).

Once approved, your changes will be merged. ⚡⚡⚡Congrats on making your first contribution to the AMP Project!⚡⚡⚡ You'll be able to see it live across the web soon!

Thanks, and we hope to see more contributions from you soon.

Questions?

If you have questions ask in this issue or on your Pull Request (if you've created one) or see the How to get help section of the Getting Started guide.

GFI Claimed! When Possible

Most helpful comment

Great, I am on it.

All 9 comments

Hi guys ! Can I pick this one ?

Sorry this issue is reserved for write/speak/code event this week. We will have more great first issues coming.

hum ok, gonna find another one then :)

@letsila Now that the event is over, feel free to work on this.

Great, I am on it.

Hi @letsila; I've sent you an invite to join ampproject on GitHub. Once you accept we'll be able to assign this issue to you. Thanks!

Hi @mrjoro, thanks for the invitation. I've just accepted it.

Thanks for making your first contribution to AMP @letsila!

It's a pleasure @mrjoro. Thanks to @muxin also who help me a lot with this. I'll continue contributing ;)

Was this page helpful?
0 / 5 - 0 ratings