-
-
Notifications
You must be signed in to change notification settings - Fork 78.8k
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
Fix unwanted body padding when a modal is opened #23718
Conversation
When the body does not overflow (achieved by hiding the QUnit container), it should not be given a margin.
Prevents the test from failing
Hmm, only one is failing on Travis. Guess I'll try pushing the jQuery outerWidth version and see what happens edit: this version doesn't work in Phantom or Chrome because it still doesn't account for margins, the two below are the ones referenced in issue #23681. |
So this is doing what I described - 8 tests are failing in PhantomJS but the same tests are all working in Chrome. And now I'll check using the first proposed solution (using |
Use a virtual scrollbar as this is simpler than having a real one (overflow: scroll doesn't seem to work in Phantom), and disable it for the new test. One test has also been altered to prevent erroneous fails when other inline styles are added to the body (e.g. overflow).
I figured out the problem! - in Phantom there are no vertical scrollbars, and this was breaking the other tests because they need vertical scrollbars to be present. However, these tests were previously working because of the buggy _checkScrollbar which thought that the 8px margin was a scrollbar - so in other words, those tests have been technically broken for a while but working thanks to this bug. To fix the tests, I've given the html element padding-right to simulate a scrollbar, and disabled this for the new test as it tests for the body element when it isn't overflowing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix thanks @TechDavid 👍
Fixes #23681
This prevents padding being added to the body when it's not overflowing but has a margin (e.g. the default 8px) by measuring its outer width (i.e. including the margin).
@Johann-S:
The problem is that this is breaking other existing PhantomJS tests... but as stated before, it works fine when running those same tests in Chrome.