calculate scrollbar width correctly

[buschtoens]

Fixes #64

Scrollbar width is still calculated repeatedly; once for every component.

[buschtoens]

I don't have internet access on our Windows VM for testing IE, so I can't verify this, but my best guess is that the new jQuery version calculates .width() and .innerWidth() differently, as they both seem to exclude the scrollbar width. I changed the logic a bit and now it should work regardless of the version used.

[buschtoens]

I improved the logic even more. We now don't even need to sniff for Firefox.

[buschtoens]

The scrollbar thickness is now mesaured once in a service and cached from there on.

[offirgolan]

@buschtoens can you resolve conflicts and fix any failing test cases?

[buschtoens]

Yeppa. Did this as the last thing before going home. 😅

I'll fix this as the first thing tomorrow. :)

[buschtoens]

@offirgolan Ready for your eyes. 👀

I'm tempted to say that this one failing test actually isn't this PR's fault.

[buschtoens]

This is the only test failing and only for the ember-release scenario.

https://travis-ci.org/alphasights/ember-scrollable/jobs/214383533#L921-L931

not ok 31 Chrome 57.0 - Integration | Component | scroll content element: Horizontal: Initial offset triggers a scroll event
    ---
        expected: >
            Max depth.,Max depth.
        stack: >
                at Object.<anonymous> (http://localhost:7357/assets/tests.js:532:14)
                at fn (http://localhost:7357/assets/vendor.js:11483:18)
                at invokeWithOnError (http://localhost:7357/assets/vendor.js:10914:16)
                at Queue.flush (http://localhost:7357/assets/vendor.js:10973:9)
                at DeferredActionQueues.flush (http://localhost:7357/assets/vendor.js:11097:15)
                at Backburner.end (http://localhost:7357/assets/vendor.js:11167:23)

I'll see if I can replicate this locally.

[buschtoens]

The test results appear to be pretty inconsistent. When I run yarn test, the tests run in Chrome and randomly fail. 😟

Ironically, the test case that failed in Travis, never fails locally. The test that fails locally:

not ok 22 Chrome 57.0 - Integration | Component | ember scrollbar: Horizontal: onJumpTo first argument is true when click to the left of handle
    ---
        actual: >
            false
        expected: >
            true
        stack: >
                at Object.<anonymous> (http://localhost:7357/assets/tests.js:329:14)
                at Backburner.join (http://localhost:7357/assets/vendor.js:20057:31)
                at Function.run$1.join (http://localhost:7357/assets/vendor.js:39007:28)
                at http://localhost:7357/assets/vendor.js:29015:37
                at flaggedInstrument (http://localhost:7357/assets/vendor.js:38434:14)
                at http://localhost:7357/assets/vendor.js:29014:48
        message: >
            towardsAnchor should be true if going towards anchor
        Log: |
    ...

[buschtoens]

Running the tests manually in Chrome succeeds consistently. I guess this is some very unfortunate race condition.

[alexander-alvarez]

@buschtoens do we need to update https://github.com/alphasights/ember-scrollable/blob/master/tests/integration/components/ember-scrollbar-test.js#L169 ?

[buschtoens]

@buschtoens do we need to update https://github.com/alphasights/ember-scrollable/blob/master/tests/integration/components/ember-scrollbar-test.js#L169 ?

I don't think so. At least the current setup seems to work for Firefox, Chrome and surprisingly IE as well - for the case that failed on Travis, that is.

However...

Running tests manually in Firefox, it fails consistently for a different case. But it does so on master as well. So that's a different issue (#66).

Acceptance | ember-scrollbar: When element resized from no-overflow => overflow => no-overflow, no scrollbar is visible on mouseover (3, 4, 7)

> there is no overflow as 18 < 200px
  Expected: 18
  Result: 19

> there is overflow as 494 > 200px@ 1247 ms
  Expected: 494
  Result: 509

> failed
  Expected: 18
  Result: 19

Running tests in IE is a different nightmare: #67

[buschtoens]

After applying #68, there are no new failing test cases in IE compared to master, or #68 for that matter.

[buschtoens]

Rebased on the current master. All tests pass now (on Travis).

Before that I accidentally rebased onto my own origin/master instead of upstream/master and the tests still passed on Travis. So it was a random fail / race condition after all.

Personally I would have preferred it, if my PR was to blame, because this is probably impossible to track down.

[alexander-alvarez]

*sigh... The struggles of web dev.

[offirgolan]

@buschtoens do we still need the includePolyfill option? I thought the last merged PR solved that issue.

[buschtoens]

@offirgolan of course you're​ correct. I forgot to remove the commit. 🙈

I'm currently commuting to work. I'll remove it, when I'm there. 😉

[buschtoens]

Commit removed and tests still pass. ✅ 😁

[offirgolan]

@buschtoens awesome job! Thanks for seeing this through! 😄