Resizing stalls if the mouse leaves the container

[davidje13]

Patch for #111

Thanks for giving me contributor access, but to begin with I'd like to stick to PRs so you get a chance to comment.

This series of commits fixes the linter issues I mentioned, moves event listeners from the parent node to the root node, and tidies up a few minor things (slightly better minification).

Tested in Chrome, FireFox and Safari

[davidje13]

Looks like the CI is complaining about a username:

Using config: /home/travis/build/nathancahill/Split.js/browserstack.json
Error: Configuration parameter username is required.
    at /home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/lib/config.js:59:13
    at Array.forEach (<anonymous>)
    at new exports.config (/home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/lib/config.js:57:48)
    at Object.exports.run (/home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/bin/cli.js:421:14)
    at Object.<anonymous> (/home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/bin/runner.js:40:8)
    at Module._compile (module.js:641:30)
    at Object.Module._extensions..js (module.js:652:10)
    at Module.load (module.js:560:32)
    at tryModuleLoad (module.js:503:12)
    at Function.Module._load (module.js:495:3)
Error: Configuration parameter username is required.
    at /home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/lib/config.js:59:13
    at Array.forEach (<anonymous>)
    at new exports.config (/home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/lib/config.js:57:48)
    at Object.exports.run (/home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/bin/cli.js:421:14)
    at Object.<anonymous> (/home/travis/.nvm/versions/node/v9.2.0/lib/node_modules/browserstack-runner/bin/runner.js:40:8)
    at Module._compile (module.js:641:30)
    at Object.Module._extensions..js (module.js:652:10)
    at Module.load (module.js:560:32)
    at tryModuleLoad (module.js:503:12)
    at Function.Module._load (module.js:495:3)
Invalid Command
The command "browserstack-runner" exited with 1.

Any ideas?

[nathancahill]

I think the CI is set up to work with my GH OAuth.. let me see if I can fix that.

[nathancahill]

Also, can you post the new minified build size in PRs to compare?

[davidje13]

That build is 1.8kB according to the npm build script.

[nathancahill]

This looks great. I'm ok with all of it except the change in the callback calls to getOption(options, 'onDrag', NOOP)(). I think it's more clear to do the check, and then call the function if it's set.

Other than that, looks stellar! ⭐️

[davidje13]

@nathancahill fair enough. I mainly made that change for consistency with the other options, though I think it did save a few bytes too.

How would you feel about another alternative; storing onDrag / onDragBegin / onDragEnd in constants (with NOOP defaults) like the other options then calling those constants? It's the same behaviour but means the code looks a bit clearer (onDrag() rather than getOption(options, 'onDrag', NOOP)()). I generally prefer NOOP callbacks over explicit checks because it makes it impossible to have silly mistakes like checking for onDragBegin then calling onDragEnd, for example. It also reduces the need for branching which helps with cyclomatic complexity.

But if you don't want that I'm fine to revert those to how it used to be. Let me know.

[nathancahill]

I like that second option better, that's good with me.

[nathancahill]

Didn't want to kill your enthusiasm on this minor detail, so I'm going to merge as is and we can revisit in the future. Thanks for the great PR!

[davidje13]

@nathancahill Apologies for going a bit silent on this; ran out of weekend then had (still have) other things to deal with. Will get back to this soon.