Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overlay: Investigate ways of blocking body scroll #1662

Closed
crisbeto opened this issue Oct 31, 2016 · 9 comments
Closed

overlay: Investigate ways of blocking body scroll #1662

crisbeto opened this issue Oct 31, 2016 · 9 comments
Assignees
Labels
needs: discussion Further discussion with the team is needed before proceeding

Comments

@crisbeto
Copy link
Member

We need a way of blocking body scroll when something like a dialog is open. There are a few ways this can be done:

  1. An approach, similar to Material 1, where we set position: fixed on the body and we offset the scroll position. This approach is pretty reliable, but can have unexpected consequences.
  2. Using preventDefault on touch, keyboard and mousewheel events, as well as saving the scroll position and resetting it in a scroll handler. Mostly works, but can still cause jumping in certain browsers.
  3. Setting overflow: hidden on the body and adding padding-left with the size of the scrollbar to prevent content jumping. May be tricky to determine the scrollbar width. Can also conflict with user styling.

I'm open to more suggestions since none of these solutions are perfect.

Related to #1633.

@crisbeto crisbeto self-assigned this Oct 31, 2016
@fxck
Copy link
Contributor

fxck commented Nov 1, 2016

This is one of those things that simply do not have the perfect way of doing it. :) I say go with whatever "works" in material1, potentially change later.

@huygaa11
Copy link

huygaa11 commented Nov 9, 2016

Hi @crisbeto
Do you have an eta? Thanks.

@epelc
Copy link
Contributor

epelc commented Nov 10, 2016

@crisbeto Would option 2 cause double scrollbars?

One nice property about the material1 method is that if you have a sidenav and lock scrolling on the body(or equivalent content area) you do not end up with 2 scrollbars showing at once if your sidenav is tall.

@crisbeto
Copy link
Member Author

Option 2 is purely JS and can't cause multiple scrollbars @epelc, but it's not guaranteed that it'll block scrolling in all cases.

crisbeto added a commit to crisbeto/material2 that referenced this issue Nov 23, 2016
Adds a `DisableBodyScroll` injectable that can toggle whether the body is scrollable.

Fixes angular#1662.
crisbeto added a commit to crisbeto/material2 that referenced this issue Nov 24, 2016
Adds a `DisableBodyScroll` injectable that can toggle whether the body is scrollable.

Fixes angular#1662.
@MateuszG
Copy link

any news?

@crisbeto
Copy link
Member Author

There's a pending PR, it just isn't merged yet.

crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 4, 2017
Adds a `DisableBodyScroll` injectable that can toggle whether the body is scrollable.

Fixes angular#1662.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 12, 2017
Adds a `DisableBodyScroll` injectable that can toggle whether the body is scrollable.

Fixes angular#1662.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 14, 2017
Adds a `DisableBodyScroll` injectable that can toggle whether the body is scrollable.

Fixes angular#1662.
@tagazok
Copy link

tagazok commented May 8, 2017

friendly ping :)
Any plan to merge the PR soon?

@jelbourn
Copy link
Member

jelbourn commented Jun 6, 2017

This is in master now and will be part of the next release.

@jelbourn jelbourn closed this as completed Jun 6, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
@mmalerba mmalerba added the needs: discussion Further discussion with the team is needed before proceeding label Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: discussion Further discussion with the team is needed before proceeding
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants