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

Disabling scroll when dialog is open #206

Closed
jerepaajanen opened this issue Apr 6, 2015 · 16 comments · Fixed by #280
Closed

Disabling scroll when dialog is open #206

jerepaajanen opened this issue Apr 6, 2015 · 16 comments · Fixed by #280
Assignees

Comments

@jerepaajanen
Copy link

Is it possible to disable scrolling when dialog is open? Overflow: hidden in body doesn't work because I'm using front-end framework that has overflow-y: scroll in html-tag.

@westonganger
Copy link

+1

@harrisrobin
Copy link

Why would you have overflow-y scroll in html tag? Can't you just remove it. You won't be able to achieve what you need if the html tag is forcing the scroll. Perhaps you can dynamically modify the html tag when the dialog is open by either changing the attribute or some class with !important at the end.

@egor-smirnov
Copy link
Member

@jerepaajanen I think it could be a bit unexpected for users if we apply what you propose by default (maybe users still want to have scrolling of body when they open the dialog). For me, it looks like this functionality could be implemented as an optional parameter called ie. disableBodyScroll which will enable disabling of scroll (by default this parameter will be false).

Your thoughts?

@jerepaajanen
Copy link
Author

As a optional parameter that could be really nice. When html/body scroll isn't disabled, it is kinda troublesome to scroll a bit longer dialogs especially when viewing in small screes or mobile. Only background site is scrolled, not the actual dialog.

@egor-smirnov
Copy link
Member

Ok, then added to the features list for future releases.

@jdcrecur
Copy link

jdcrecur commented Jul 8, 2015

Hi just wondering when this might be going live? As without it ngDialog really doesn't work well on long pages and often locks up.

@jerepaajanen
Copy link
Author

When this is going to live?

@egor-smirnov
Copy link
Member

@jerepaajanen After some diving into source code I noticed that we already have CSS class ngdialog-open which is added to body tag and has overflow: hidden. Will it be enough for your case if we'll just add the same class to html tag? If I understand correctly, this is what you need.

@johncarmichael
Copy link

@egor-smirnov That would work perfectly.

@jerepaajanen
Copy link
Author

@egor-smirnov Sounds great!

@marmotz
Copy link

marmotz commented Jul 23, 2015

👍

@davidvuong
Copy link
Contributor

@marmotz +1 looks pretty good!

There's a bit of code duplication with how you're adding/removing the ng-dialog class and the initialization of the state/location change listeners but it's fairly minimal. Can I ask that you also update the README.md to document this new forceHTMLReload function?

@egor-smirnov What do you think?

@marmotz
Copy link

marmotz commented Jul 27, 2015

@davidvuong I've updated README.md file.

@marmotz
Copy link

marmotz commented Jul 27, 2015

@davidvuong Here is a little refactoring following your advice.

@egor-smirnov
Copy link
Member

@davidvuong @marmotz let's move discussion to #280

egor-smirnov added a commit that referenced this issue Jul 28, 2015
Disable scroll when dialog is open (fix #206)
@roaks3
Copy link
Contributor

roaks3 commented Aug 26, 2015

This has the unfortunate side effect of bumping the user up to the top of a long page whenever a dialog is opened. The previous functionality opened the dialog in-place, which feels more natural to me, but will also probably surprise some people when upgrading. I'm not sure there's an ideal solution (open in-place AND disable scrolling), but this change can be overridden in the project CSS to match the old behavior:

html.ngdialog-open {
  overflow: visible;
}

@likeastore likeastore locked and limited conversation to collaborators Dec 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants