Skip to content
This repository has been archived by the owner on Jul 10, 2022. It is now read-only.

Change to getNativeScrollbarWidth (fixes issue #42) #45

Merged
merged 1 commit into from
Jan 10, 2018
Merged

Change to getNativeScrollbarWidth (fixes issue #42) #45

merged 1 commit into from
Jan 10, 2018

Conversation

gregpabian
Copy link
Contributor

@gregpabian gregpabian commented Dec 19, 2017

Turns out the native scrollbar width calculation fails in Safari 11 (tested with 11.0.2). As the child element's offsetWidth still returns 100px after setting wrapper's overflowY to scroll, the returned scrollbar width is 0. Setting child's width to 100% makes the browser report the correct offsetWidth, though.

Fixes: #42

@DominikSerafin
Copy link
Owner

Hey @gregpabian,

I'll need to test if this doesn't brake anything in other browsers and if everything will be good I'll merge this.

Thank you!

@DominikSerafin
Copy link
Owner

DominikSerafin commented Jan 10, 2018

Hi @gregpabian,

After investigating this it seems like original code is correct. I've tested Safari 9-11 and all have overlaying scrollbars.

The getNativeScrollbarWidth should return 0 in these cases:

  • In webkit/blink browsers where the scrollbar is hackishly "hidden" using CSS :-webkit-scrollbar pseudo selectors.
  • In IE/Edge when the scrolling element has -ms-overflow-style: none;. It basically disables scrollbar, while retaining scroll.
  • In IE/Edge when the scrolling element has -ms-overflow-style: -ms-autohiding-scrollbar;. It sets scrollbar to overlay mode.
  • If the browser uses overlaying scrollbars that doesn't affect the width of document. This behavior is visible mostly in mobile browsers, in IE/Edge as in previous point, and in macOS Safari, like I've mentioned above.

And unfortunately, in the last 2 cases, it's impossible to detect the overlaying scrollbar width and even existence. (But I would love to someone prove me wrong!)

So when Vuebar has detected overlayed/0 scrollbar then it uses the constant value of 20px to overflow content. I've chosen 20px because it feels like safe value that will work in 99% of cases.

I'll close this PR without merging, but if you don't agree with it then please comment here and I'll revisit it.

@gregpabian
Copy link
Contributor Author

Thanks for testing. Does this break anything, though? I've confirmed locally it does fix the issue on OSX/Safari, when you have the first option from the image below selected:
OSX General Settings and a mouse connected (which, surprisingly, is important here).

@DominikSerafin
Copy link
Owner

@gregpabian, didn't know there's a possibility to choose scrollbar behavior on Safari. Thank you for showing it to me!

I'll test it again soon, and get back to you with the findings.

@DominikSerafin DominikSerafin changed the title Fixed #42 Change to getNativeScrollbarWidth (fixes issue #42) Jan 10, 2018
@DominikSerafin DominikSerafin changed the title Change to getNativeScrollbarWidth (fixes issue #42) Change to getNativeScrollbarWidth (fixes issue https://github.com/DominikSerafin/vuebar/issues/42) Jan 10, 2018
@DominikSerafin DominikSerafin changed the title Change to getNativeScrollbarWidth (fixes issue https://github.com/DominikSerafin/vuebar/issues/42) Change to getNativeScrollbarWidth (fixes issue #42) Jan 10, 2018
@scherii
Copy link

scherii commented Jan 10, 2018

@gregpabian The same should apply to the "Always" option, regardless of any connected input device. I'm not entirely sure but seem to have the same problem you described in Safari. Thanks for this hint:

Setting child's width to 100% makes the browser report the correct offsetWidth, though.

@DominikSerafin
Copy link
Owner

Yup, @gregpabian you were right. When the option was to "always" then getNativeScrollbarWidth didn't calculate it properly.

I assume it was because "overflow: scroll" doesn't work on Safari as on other browsers (I think?)

I'm merging this PR for Vuebar 0.0.18.

As for 2.0 in development branch... I've changed this fix by making child taller than wrapper to force scrollbar, it seems more logical to me than setting width.
https://github.com/DominikSerafin/vuebar/blob/development/vuebar.js#L843

@DominikSerafin DominikSerafin merged commit 722445f into DominikSerafin:master Jan 10, 2018
@gregpabian
Copy link
Contributor Author

Appreciated @DominikSerafin! Can't wait to see Vuebar 2 in action!

DominikSerafin added a commit that referenced this pull request May 10, 2020
Change to getNativeScrollbarWidth (fixes issue #42)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants