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

Bad scrollbar replacement on non-integer width windows #3448

Closed
kremerd opened this issue Nov 4, 2019 · 2 comments · Fixed by #3498
Closed

Bad scrollbar replacement on non-integer width windows #3448

kremerd opened this issue Nov 4, 2019 · 2 comments · Fixed by #3498

Comments

@kremerd
Copy link

kremerd commented Nov 4, 2019

Bug description:

When a modal is shown on a scrollable page, the scrollbar is hidden and the page body is padded to compensate for the lost width. Currently the decision, whether padding should be applied for that reason, is sometimes taken incorrectly if the browser window has a non-integer width. In an extreme case this can lead to the same padding being applied over and over again (cf. StackBlitz).

Non-integer widths occur when zooming in on the page with the browser, or when scaling your display in system settings like these in Windows.

Link to minimally-working StackBlitz that reproduces the issue:

https://stackblitz.com/edit/ngb-scrollbar-handling

Please make sure to follow the instructions on screen.

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 8.2.13

ng-bootstrap: 5.1.2

Bootstrap: 4.3.1

Resolution sketch:

Some rounding needs to be applied to util/scrollbar.ts.

@maxokorokov
Copy link
Member

maxokorokov commented Nov 7, 2019

@kremerd thanks for the StackBlitz
Can't reproduce on macOS in any of the browsers.
Can reproduce on Win 10 at least in Chrome.

@ymeine would you be interested in looking into it again ?

@kremerd
Copy link
Author

kremerd commented Nov 15, 2019

@maxokorokov Thanks for the confirmation. I only tested on Windows 10, but was able to reproduce the issue with Firefox, Chrome, Edge and Internet Explorer 11.

New finding: The same behavior also occurs with the default Bootstrap code (and for the same reason). I opened a ticket there as well.

ymeine added a commit to ymeine/ng-bootstrap that referenced this issue Dec 2, 2019
Now the actual width of the scrollbar is used to detect if it is present or not, instead of using the wrong hypothesis that if body's rect's width is smaller than viewport's width then there is a scrollbar. Now the difference in width is compared to the width of the scrollbar.

Closes ng-bootstrap#3448
ymeine added a commit to ymeine/ng-bootstrap that referenced this issue Dec 2, 2019
Now the actual width of the scrollbar is used to detect if it is present or not, instead of using the wrong hypothesis that if body's rect's width is smaller than viewport's width then there is a scrollbar. Now the difference in width is compared to the width of the scrollbar.

Closes ng-bootstrap#3448
ymeine added a commit to ymeine/ng-bootstrap that referenced this issue Dec 3, 2019
Adding uncertainty, because the scaling can make the gap just slightly smaller than a scrollbar size.

Closes ng-bootstrap#3448
benouat pushed a commit that referenced this issue Dec 5, 2019
)

Now the actual width of the scrollbar is used to detect if it is present or not, instead of using the wrong hypothesis that if body's rect's width is smaller than viewport's width then there is a scrollbar. Now the difference in width is compared to the width of the scrollbar.

Adding uncertainty, because the scaling can make the gap just slightly smaller than a scrollbar size.

Closes #3448
@maxokorokov maxokorokov added this to the 5.1.5 milestone Jan 3, 2020
maxokorokov pushed a commit that referenced this issue Jan 7, 2020
)

Now the actual width of the scrollbar is used to detect if it is present or not, instead of using the wrong hypothesis that if body's rect's width is smaller than viewport's width then there is a scrollbar. Now the difference in width is compared to the width of the scrollbar.

Adding uncertainty, because the scaling can make the gap just slightly smaller than a scrollbar size.

Closes #3448
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants