Skip to content
This repository was archived by the owner on Mar 10, 2024. It is now read-only.

enable retention of prototypes when dragging #46

Closed
wants to merge 1 commit into from
Closed

enable retention of prototypes when dragging #46

wants to merge 1 commit into from

Conversation

boneskull
Copy link

  • currently, the hack to get around lack of IE support for custom dataTransfer types is debilitating
  • practically, when items are dropped, they lose their reference and any associated prototype, because they are serialized and deserialized to and from JSON
  • this PR works around that by implementing a simple cache that the directives use to store and retrieve data
  • transferred object is always passed into event handler(s) when possible
  • demo demo/advanced is now updated to use this functionality

Without functional tests, it's hard to say whether or not this strategy is susceptible to race conditions. We aren't targeting older browsers, but I have tested this on current versions of Chrome, Safari and Firefox, and it works in all three.

- currently, the hack to get around lack of IE support for custom `dataTransfer` types is debilitating
- practically, when items are dropped, they lose their reference and any associated prototype, because they are serialized and deserialized to and from JSON
- this PR works around that by implementing a simple cache that the directives use to store and retrieve data
- transferred object is always passed into event handler(s) when possible
- demo `demo/advanced` is now updated to use this functionality
@marceljuenemann
Copy link
Owner

Thanks a lot for your pull request!

The serialzation/deserialization is not due to IE support, it's how native drag & drop is supposed to work. By using a local ID you loose the ability to drag & drop from/to other browser tabs or applications (although this has to be enabled with dnd-external-sources due to IE not supporting custom mime types). Therefore, I won't merge this PR.

@boneskull
Copy link
Author

@marceljuenemann

That makes sense, if that's your use case. It isn't ours, however.

As a general-purpose, in-browser drag/drop directive, this is by far the cleanest code I've seen out of evaluating many. So, we'll probably go ahead and maintain a fork, perhaps renamed; I'm sure you'll understand.

@boneskull
Copy link
Author

(It's not my desire to maintain a fork; if you wish me to reimplement this behavior as an option, I can do that)

@marceljuenemann
Copy link
Owner

It's not really my use case, I'm no longer using this directive. It just ended up with a lot of users, so I feel obliged to maintain it from time to time...

Actually, your proposal to implement it as an option is worth a second thought. It's not really how the API is supposed to be used, but as long as the browsers don't implement it properly and we need workarounds anyways, we could consider it.

Here are some more thoughts:

  • How are you going to handle duplicate objects? ng-repeat does not support that. When moving objects everything might work out if they are deleted in the same digest cycle, but this directive also supports copying objects.
  • Your code can be massively simplified due to the fact that there can only be one element that is dragged at any time. You can just replace dndDragTypeWorkaround.isDragging with the reference to the current object, no need to transfer any IDs.
  • We could use this behavior whenever dnd-external-sources is not activated, or make it a new flag.

PS: It looks like you only need a subset of this directive's features, so if you want to fork it go ahead, it will definitely be easier for you ;)

@boneskull
Copy link
Author

Re copying...

ngRepeat will only freak out if the objects have the same internal $$hashKey afaik. I believe angular.copy() will not copy that. I can't recall if that's what you're doing--don't have it in front of me.

I'll adress your other questions and concerns tomorrow (maybe 18 hours from now or so). 😄

@marceljuenemann
Copy link
Owner

Well, currently the object is serialized, given to the native drag & drop handler of the OS, and then deserialized from the data that the OS gives us when dropped. Since it uses angular.toJSON the prototype is lost.

In your first post you wrote you want to retain the reference, so that won't work with angular.copy. Also note that until angular 1.2, angular.copy is doing a deep copy of the prototype, which changes the type of the object and might make this feature useless (see angular/angular.js#1427)

@boneskull
Copy link
Author

Copying objects is wrought with peril. We don't need it, so perhaps we will go with that fork idea.

@boneskull boneskull closed this Mar 30, 2015
@Stalinko
Copy link

Really needed this option...

I use dnd plugin for sorting an array of complex objects, so when it serializes and then deserializes them it spoils the objects totally and they become useless after dragging. It would be cool if the plugin would have really moved the objects in the array (list) without copying or serializing them.

@marceljuenemann
Copy link
Owner

I'm about to add dnd-callback which will allows sidestepping the directive's dataTransfer and moving elements without serialization: 446925a

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants