Copy by Value vs by Reference on Drop

[drubarth]

I've added the angular-dragdrop into my code and it works quite well, except for one issue I'm having. When I drag an array element that contains objects and drop it onto a new array, the objects inside my array element appear to be getting passed by value rather than by reference.

I believe it's the line in your code that does the angular copy (dropItem = angular.copy(dropItem);) which is breaking the inheritance chain. I would be great to have an option to maintain the connection with the original objects.

Thoughts on this? I'm still learning JS and Angular, so please let me know if I'm missing something here.

[codef0rmer]

Earlier I used to copy by reference which has been changed because of #85

I think, I'll have to support both to be configurable.

[drubarth]

Having an option would be great. After reading the other issue, it seems that the deep copy should be the option and that maintaining the prototypal inheritance should be the default behavior since that is how Javascript works by default when assigning objects.

I have a work around, but it's kind of ugly, so just curious if you have a time frame when this option might be available. If I felt more confident with my Javascript skills I'd attempt it myself. Thanks.

[drubarth]

This looks easier than I thought. I'm happy to make the change, but I want to make sure you agree with my thinking. (1) remove the angular.copy from around the dragItem on line 79 -- my thinking is that dragging should never copy, it should be a question when dropping. (2) add an option to the documentation, perhaps called "deepCopy". (3) add an if statement around the dropItem with the angular.copy which checks if the option deepCopy is true. If true, then use the angular.copy. If it's not true, then the normal prototypal chain will remain in effect. Another reason for making the deepCopy option default to false is that jQuery's drag-and-drop copies by reference, so it would be staying in sync with how it works.

Thoughts? I found that simply removing the angular.copy fixed my issue, so I was able to dump the ugly work around. Would like to get this issue fixed though so that I can stay in sync with your code.

Thanks for creating this module. It's awesome!

[codef0rmer]

@drubarth : I think we should keep deep copy by default using angular.copy and allow shallow copy using angular.extend. Configuring it this way makes sense to me. What do you say?

[drubarth]

If you're not doing the deep copy, there's nothing else to do. To be a more specific, here are the code changes I would make.

I would completely remove the following line. I don't think there is a reason to copy on drag, only on drop.

dragItem = angular.copy(angular.isArray(dragModelValue) ? dragModelValue[jqyoui_pos] : dragModelValue);

And then I would do the following with the dropItem.

if (dropSettings.deepCopy ) {
dropItem = angular.copy(dropItem);
}

And it all works. I don't see a reason to use the angular.extend in this situation. I'm sure you're a much stronger Javascript and Angular developer than I am, so if I'm missing something, please let me know.

I personally think it's important to follow the jQuery assumptions, one of which is that by default it copies by reference.

thanks,

Darryl

----- Original Message -----

| From: "codef0rmer" notifications@github.com
| To: "codef0rmer/angular-dragdrop"
| angular-dragdrop@noreply.github.com
| Cc: "drubarth" dr@exyst.com
| Sent: Tuesday, December 30, 2014 10:36:49 AM
| Subject: Re: [angular-dragdrop] Copy by Value vs by Reference on Drop
| (#143)

| @drubarth : I think we should keep deep copy by default using
| angular.copy and allow to shallow copy using angular.extend .
| Configuring it this way makes sense to me. What do you say?
| —
| Reply to this email directly or view it on GitHub .