fix(modal): prevent body content shifting when vertical scrollbar #4782
Conversation
| @@ -19,6 +19,7 @@ module.exports = function(config) { | |||
| 'node_modules/angular/angular.js', | |||
| 'node_modules/angular-mocks/angular-mocks.js', | |||
| 'node_modules/angular-sanitize/angular-sanitize.js', | |||
| 'node_modules/bootstrap/dist/css/bootstrap.css', | |||
There was a problem hiding this comment.
Does this affect the tests?
There was a problem hiding this comment.
Yes. Without bootstrap.css, the tests always pass. The tests rely on the modal related classes in bootstrap css which remove the scroll bar from the body when the modal is opened.
| $$scrollbarHelper.measureScrollbar(); | ||
| } | ||
|
|
||
| var body = $document.find('body').eq(0); |
There was a problem hiding this comment.
No need to do .eq(0) here, should only be one body tag.
There was a problem hiding this comment.
Removed the unnecessary .eq(0)
- If body has vertical scrollbar and appendTo element is 'body', add right padding to body equivalent to scrollbar width. Also, conditionally add right or left padding to modal window Closes angular-ui#3714
Set modal window padding after modal is rendered. Remove unnecessary null checks. Set modal padding properties only when required. Closes angular-ui#3714
| this.scrollbarWidth = width; | ||
| } | ||
| }) | ||
|
|
There was a problem hiding this comment.
I'd like to see this in the $position service as scrollbarWidth(), we might need to use it for auto/smart positioning. Would be nice to cache the result to avoid manipulating the DOM every time we need this value as it should not change.
There was a problem hiding this comment.
I agree - we have a related issue in #4317.
There was a problem hiding this comment.
$position service now has a scrollbarWidth function.
|
Can you squash then rebase this, and then change to use the |
|
Also, please fix the code style to match the rest of the file. |
|
|
||
| // start - from adjustDialog method of modal.js of bootstrap | ||
| // check if modal is overflowing | ||
| modalIsOverflowing = element[0].scrollHeight > document.documentElement.clientHeight; |
There was a problem hiding this comment.
Should use $document[0].clientHeight
|
Going to close this due to the issues raised in the code review & due to the merge conflict - feel free to open a new PR fixing this along with a cleaner implementation. |
Prevents page body content shifting to the right when modal is open (when appendTo is 'body')
Closes #3714