Skip to content

Prevent scrollbar replacement on non-integer width#30772

Merged
XhmikosR merged 2 commits intotwbs:masterfrom
kremerd:master-modal-scrollbar
May 12, 2020
Merged

Prevent scrollbar replacement on non-integer width#30772
XhmikosR merged 2 commits intotwbs:masterfrom
kremerd:master-modal-scrollbar

Conversation

@kremerd
Copy link
Copy Markdown

@kremerd kremerd commented May 10, 2020 •
edited by XhmikosR
Loading

Fixes #29681, Equals #30017, but also includes a test case.

@kremerd kremerd requested a review from a team as a code owner May 10, 2020 21:16
@kremerd kremerd force-pushed the master-modal-scrollbar branch from 6a27eb3 to 00bbb3e Compare May 10, 2020 21:29
kremerd
kremerd commented May 10, 2020
View reviewed changes
Comment thread js/tests/unit/modal.spec.js Outdated
Copy link
Copy Markdown
Author

@kremerd kremerd May 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not strictly necessary for the test to work, I believe it makes sense to include this style definition here: The values of rect.left and rect.right in the _checkScrollbar method depend on this margin, and when not set explicitly, a browser default will be used (e.g. 8px in Chrome) instead of the value actually set by Bootstrap.

@XhmikosR XhmikosR force-pushed the master-modal-scrollbar branch from 00bbb3e to 95ba418 Compare May 11, 2020 05:46
@XhmikosR
Copy link
Copy Markdown
Member

XhmikosR commented May 11, 2020

Thanks @kremerd! I rebased this to include the original patch and authorship, so please don't overwrite it.

BTW for the v4-dev backport the test will have to be adapted since we use QUnit there. But that's after this is merged.

@XhmikosR
Copy link
Copy Markdown
Member

XhmikosR commented May 11, 2020

Tested this on BrowserStack too, seems to be fine.

@Johann-S please review ASAP so that we backport it to #30768 and include it in 4.4.2 🙂

@XhmikosR XhmikosR mentioned this pull request May 11, 2020
Closed
@XhmikosR XhmikosR force-pushed the master-modal-scrollbar branch from 95ba418 to 79553c8 Compare May 11, 2020 16:51
XhmikosR
XhmikosR reviewed May 11, 2020
View reviewed changes
Comment thread js/tests/unit/modal.spec.js Outdated
@kremerd kremerd closed this May 11, 2020
@kremerd kremerd deleted the master-modal-scrollbar branch May 11, 2020 20:07
@kremerd kremerd restored the master-modal-scrollbar branch May 11, 2020 20:28
@kremerd kremerd reopened this May 11, 2020
@XhmikosR XhmikosR force-pushed the master-modal-scrollbar branch from 1d75bf4 to 136df8f Compare May 12, 2020 04:36
@XhmikosR
Copy link
Copy Markdown
Member

XhmikosR commented May 12, 2020

@kremerd thanks! Now the backport to v4-dev is left. I'll try to do it so that we include the patch in the upcoming v4.4.2, but if you have time feel free to backport it to v4-dev and ping me.

@XhmikosR XhmikosR merged commit d59de33 into twbs:master May 12, 2020
@XhmikosR
Copy link
Copy Markdown
Member

XhmikosR commented May 12, 2020

All good, I cherry picked it in #30768, so it'll be in v4.4.2. Thanks again @kremerd!

XhmikosR pushed a commit that referenced this pull request May 12, 2020
@mendozagioo @XhmikosR
Avoid bad scrollbar replacement into width values
c4be502 
Refs #30772
XhmikosR added a commit that referenced this pull request May 12, 2020
@XhmikosR
Backport d59de33 from #30772
3871b07 
Add a test about the scrollbar issue on non-integer width
XhmikosR pushed a commit that referenced this pull request May 12, 2020
@mendozagioo @XhmikosR
Avoid bad scrollbar replacement into width values
4211255 
Refs #30772
XhmikosR added a commit that referenced this pull request May 12, 2020
@XhmikosR
Backport d59de33 from #30772
24cbce6 
Add a test about the scrollbar issue on non-integer width
XhmikosR pushed a commit that referenced this pull request May 12, 2020
@mendozagioo @XhmikosR
Avoid bad scrollbar replacement into width values
f985012 
Refs #30772
XhmikosR added a commit that referenced this pull request May 12, 2020
@XhmikosR
Backport d59de33 from #30772
cf5e6a5 
Add a test about the scrollbar issue on non-integer width
XhmikosR pushed a commit that referenced this pull request May 12, 2020
@mendozagioo @XhmikosR
Avoid bad scrollbar replacement into width values
f1827ce 
Refs #30772
XhmikosR added a commit that referenced this pull request May 12, 2020
@XhmikosR
Backport d59de33 from #30772
30e7df6 
Add a test about the scrollbar issue on non-integer width
@nelsonni nelsonni mentioned this pull request Jan 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@XhmikosR XhmikosR XhmikosR approved these changes

Assignees

No one assigned

Labels

backport-to-v4 js v4 v5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bad scrollbar replacement on non-integer width windows

3 participants

@kremerd @XhmikosR @mendozagioo