desandro / draggabilly

:point_down: Make that shiz draggable
https://draggabilly.desandro.com
MIT License
3.86k stars 386 forks source link

Error under Safari / OS X 10.11 #158

Closed hyperknot closed 6 years ago

hyperknot commented 7 years ago

I'm having a problem with using Draggabilly (master) under Safari / OS X 10.11:

TypeError: null is not an object (evaluating 'styleSide.indexOf')
_getPositionCoord — draggabilly.js:197
_getPosition — draggabilly.js:187
_create — draggabilly.js:130
Draggabilly — draggabilly.js:102

The flow looks like this:

  this.options = extend( {}, this.constructor.defaults );
  this.option( options );

-->  this._create();
proto._create = function() {

  // properties
  this.position = {};
-->  this._getPosition();
// get x/y position from style
proto._getPosition = function() {
  var style = getComputedStyle( this.element );
-->  var x = this._getPositionCoord( style.left, 'width' );
  var y = this._getPositionCoord( style.top, 'height' );
proto._getPositionCoord = function( styleSide, measure ) {
-->  if ( styleSide.indexOf('%') != -1 ) {
    // convert percent into pixel for Safari, #75
    var parentSize = getSize( this.element.parentNode );
desandro commented 7 years ago

Thanks for reporting this issue. I'm sorry to see you're having trouble with Draggabilly. Could you provide a reduced test case? See Submitting Issues in the contributing guidelines.

hyperknot commented 7 years ago

@desandro I still cannot make a test case, but the reason why it's broken is that Safari can return null for getComputedStyle '.left and '.top. So we need to include a check not to run the indexOf function on null, but just return NaN for example.

The later lines will convert all NaN and "auto" values to 0 afterwards.

I've submitted a PR which fixes it.

desandro commented 7 years ago

Thanks for this. I'll evaluate adding this fix in the next release.

desandro commented 6 years ago

No one else has chimed in on this issue. So I'm going to close as wontfix. Please follow up if this issue needs another look at.