Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.parentSize is undefined error after updating to v2 #131

Closed
Densaugeo opened this issue May 3, 2016 · 6 comments · May be fixed by atlsecsrv-com/docs#42 or atlsecsrv-com/docs#43
Closed

.parentSize is undefined error after updating to v2 #131

Densaugeo opened this issue May 3, 2016 · 6 comments · May be fixed by atlsecsrv-com/docs#42 or atlsecsrv-com/docs#43
Labels

Comments

@Densaugeo
Copy link

I tried updating (from 1.2.4 to 2.1.0) and started getting an error 'TypeError: parentSize is undefined'. After looking through the code, it seems the new version expects a draggable element to already be in the DOM tree before creating the Draggabilly instance.

I have some classes for UI elements that are normally instantiated first and then added to the tree. How hard would it be to handle unattached elements? Since they can't be dragged before they're attached, it shouldn't be too hard. I'm going to play around with this a bit and see if I can fix it.

@Densaugeo
Copy link
Author

Densaugeo commented May 3, 2016

Having a little trouble with the testing. My understanding is that I need to run both npm install and bower install, then open a web server at the project root and navigate to /test/index.html. Test 9 is failing on my setup (using the master branch, without making any changes to it).

However, making Draggabilly.prototype._getPositionCoord return zero when the parent is null seems to fix #131.

Am I missing something on the testing? I'll send a PR when I know if I'm running these tests properly.

Update: Test 9 passes in Chrome, but not in Firefox. (This is still for the unmodified master branch, so maybe it is another bug).

@Densaugeo Densaugeo mentioned this issue May 3, 2016
@desandro
Copy link
Owner

desandro commented May 3, 2016

Thanks for reporting this issue. Before we try to fix it, can you put together a reduced test case that demonstrates the bug? See Submitting issues guidelines

I appreciate you taking a swing at resolving this with a PR, but I'd like to understand the issue first.

@Densaugeo
Copy link
Author

How do I set codepen to use the same JS engine as Firefox? This error only happens in Firefox, so it doesn't show up in codepen.

@desandro
Copy link
Owner

desandro commented May 4, 2016

I'm looking for a CodePen that triggers the bug in Firefox.

@Densaugeo
Copy link
Author

Densaugeo commented May 4, 2016

Okay, I have made one that reproduces the bug. If you open it in Chrome you will get a draggable element, but in Firefox Draggabilly throws an error.

This bug only happens in Firefox, only when the position has a % in it, and only when the Draggabilly instance is created before the element is put in the DOM tree.

Edit: it's at http://codepen.io/Densaugeo/pen/WwLpGW

@desandro
Copy link
Owner

desandro commented May 4, 2016

Thank you for that! Yes, given that scenario — with an attached not-in-DOM element, set with percent style position, it will cause an error. We can fix it by checking for parentNode. A fix will be in the next version of Draggabilly. Until that release is out, you can add this code:

Draggabilly.prototype._getPositionCoord = function( styleSide, measure ) {
  if ( styleSide.indexOf('%') != -1 ) {
    // convert percent into pixel for Safari, #75
    var parentSize = getSize( this.element.parentNode );
    // prevent not-in-DOM element throwing bug, #131
    return !parentSize ? 0 :
      ( parseFloat( styleSide ) / 100 ) * parentSize[ measure ];
  }
  return parseInt( styleSide, 10 );
};

See demo http://codepen.io/desandro/pen/ac56da88215168e0aa9d7bc9069eaf04/

Thank you so much for reporting this bug! 🐞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment