dragstart handler in initElements prevents elements from being draggable
Closed this issue · 15 comments
First of all -- great plugin. I really like it.
In initElements, there's a bit of code to "disable dragging images in IE" --
line 202 --
var self = this
, $this = $(this)
, xpos
, prevXPos = false
, ypos
, prevYPos = false
, mouseDown = false
, scrollLeft
, scrollTop
, throttleTimeout = 1000 / settings.throttleFPS
, lastMove
, elementFocused
;
// prevent drag and drop images in ie
$(document).bind('dragstart', function(e) {
return !!elementFocused;
});
If I run kinetic() on a container which has draggable elements, none of those elements are draggable anymore. dragstart is executed, but this always returns false since elementFocused is never initialized. This prevents the rest of the drag/drop stuff that jQuery does to stop after dragstart is done.
Removing this code completely eliminates this problem. Maybe this event handler needs to be smarter about stopping dragging.
There's another issue related to jquery draggable elements. Is this
the same as yours?
the-taylors.org
twitter.com/davetayls
skype: davetayls
On 12 Mar 2012, at 20:54, sidoh <reply@reply.github.com
wrote:
First of all -- great plugin. I really like it.
In initElements, there's a bit of code to "disable dragging images
in IE" --line 219 --
var self = this , $this = $(this) , xpos , prevXPos = false , ypos , prevYPos = false , mouseDown = false , scrollLeft , scrollTop , throttleTimeout = 1000 / settings.throttleFPS , lastMove , elementFocused ; // prevent drag and drop images in ie $(document).bind('dragstart', function(e) { return !!elementFocused; });If I run
kinetic()on a container which has draggable
elements, none of those elements are draggable anymore. dragstart is
executed, but this always returns false sinceelementFocused</
code> is never initialized. This prevents the rest of the drag/drop
stuff that jQuery does to stop after dragstart is done.Removing this code completely eliminates this problem. Maybe this
event handler needs to be smarter about stopping dragging.
Reply to this email directly or view it on GitHub:
#14
An issue with kinetic, or with jQuery itself? I don't think this is a jQuery problem, if that's what you mean. I'd expect this behavior given that the dragstart event handler that gets registered in the code above always returns false. It is, of course, equivalent to $(document).bind('dragstart', function() { return false; }), which will stop any otherwise draggable element from being draggable.
Sorry, no. I mean there is another jquery.kinetic issue logged which
is to do with using it alongside jquery ui draggable. Is this what you
are doing?
the-taylors.org
twitter.com/davetayls
skype: davetayls
On 12 Mar 2012, at 21:07, sidoh <reply@reply.github.com
wrote:
An issue with kinetic, or with jQuery itself? I don't think this is
a jQuery problem, if that's what you mean. I'd expect this behavior
given that the dragstart event handler that gets registered in the
code above always returns false. It is, of course, equivalent to
$(document).bind('dragstart', function() { return false; })</
code>, which will stop any otherwise draggable element from being
draggable.
Reply to this email directly or view it on GitHub:
#14 (comment)
On github? I didn't see it! Sorry if I missed it. I definitely looked pretty carefully...
Yes, what I'm trying to do is use draggable alongside kinetic. Removing the aforementioned event handler completely fixes this.
Sorry - I didn't look very carefully. Of course, you're using elementFocused in the closure, so it was stupid for me to assume it was always uninitialized.
Still, this is definitely causing trouble with draggable. If I remove it, draggable elements work once again, and kinetic works just fine.
Great that's really helpful. I'll take a look soon.
On 12 Mar 2012, at 21:27, sidoh <reply@reply.github.com
wrote:
Sorry - I didn't look very carefully. Of course, you're using
elementFocused in the closure, so it was stupid for me to assume it
was always uninitialized.Still, this is definitely causing trouble with draggable. If I
remove it, draggable elements work once again, and kinetic works
just fine.
Reply to this email directly or view it on GitHub:
#14 (comment)
Ah - I think I see the problem. On draggable elements, I'm doing this:
.bind('mousedown touchstart', function() {
$('#canvas').kinetic('detach');
})
to prevent scrolling on draggable elements.
This unbinds the event listener that sets elementFocused, but not the one that checks to see if its set. Probably just have to make detach unbind the document.dragstart listener along with all of the rest.
Yep, that fixed it. :)
@NilsHolmstrom i'm bringing you in to this thread as it seems like you are both trying to achieve the same thing. And I'd rather start a new issue than reopen an old one.
@sidoh: @NilsHolmstrom has created this
I've started a test page, but i've got to run to something. Will continue a little later.
Sorry for the spam on issue 12... I included it on a commit message meaning to say this one.
Here's a jsfiddle using my branch:
http://jsfiddle.net/JDQjv/
Notice that you have to detach kinetic whenever you start dragging something. If this something isn't inside the element that kinetic is acting on it, it doesn't seem like it should get messed with.
Maybe instead of using $(document).bind("dragstart", dragFilter), you should do something like $('*', $this).bind("dragstart", dragFilter). I didn't include that in my fork, but maybe it's worth thinking about?
Use this jsfiddle instead. The other one didn't fix anything for the newly created draggable elements:
hey @sidoh thanks, this is really useful as i'm really stacked for time at the moment.
could you check a couple of things?
- one of the main reasons that line is in there is to prevent unwanted image dragging in ie7/ie8, does your initial attempt attaching it to the current context ($this) work cross browser? as this seems like a better context aware solution.
- instead of declaring dragFilter as a private function and then attaching it, can you just move the exact same code down to the other settings.events functions. this will be neater i think
nice one, i'll run it through the usual checks when you're ready and merge it in
I don't have easy access to a Windows machine, so I'm not sure whether or not things work nicely on IE.
I pushed to my branch, making these two changes:
- Moved the
dragFilterfunction into an anonymous member of thesettings.eventshash, as you suggested. - Bind the now nameless
dragFilterto$this instead of $ (document)
The result is viewable in this jsfiddle, and seems to work perfectly fine.
It initially seemed a little strange to me that you'd have to do anything to get draggable elements to work. To prevent this, it seems like it wouldn't be too difficult to extend the jQuery draggable function (if it's detected) to automatically attach the appropriate event handlers:
if ($.fn.draggable) {
var kinetic_detach = function() {
$('#kinetic').kinetic('detach');
};
var kinetic_attach = function() {
$('#kinetic').kinetic();
};
var _baseDraggable = $.fn.draggable;
$.fn.draggable = function(arguments) {
return _baseDraggable.apply(this, arguments)
.on('dragstart', kinetic_detach)
.on('dragstop', kinetic_attach);
};
}That's what's going on in the jsfiddle, as you noticed. No need to do anything to make an element draggable except call draggable on it. Globally changing draggable seems a little creepy, though...
I have added a function to allow you to use alongside draggable called filterTarget. There is a new test file called ui-draggable.html which shows how to do it. This seemed like a more broad solution to the immediate problem.
filterTarget {function(target)}
Return false from this function to prevent capturing the scroll
New 1.5 version is ready in master. Thanks for your help
Nice solution!