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

dnd-drop index bug? #173

Closed
pisethdanh opened this issue Nov 17, 2015 · 10 comments
Closed

dnd-drop index bug? #173

pisethdanh opened this issue Nov 17, 2015 · 10 comments

Comments

@pisethdanh
Copy link

In a vertical list, when dragging an item from the top and placing it below, the index parameter is 1 more than it should be.

For example, if I drag the first item and drop it in the first opened slot, the index is 1.
However, if I drag an item from the bottom of the list to the top first opened slot, the index is 0.

The same behaviour occurs throughout the entire list. When dropping an item in the second spot from top down, the index is 2, but when dropping an item in the second spot from bottom up, the index is 1.

Can someone please investigate this issue?

Thank you

@pisethdanh
Copy link
Author

jjoedouglas,

Is this the fork you're referring to?
jjoekoullas@9f21552

If so, is it safe for me to use? You mentioned that you might have broken other functionality with this fork. Can you note what that functionality is? Perhaps this change might be suitable for me as well if I don't need that broken functionality.

Thank you for your response.

@jjoekoullas
Copy link

I don't know what I may have broken. Like I said, caveat emptor, it's up
to you to determine if it's appropriate for your use. It works for what I
use it for and that's all I can say for it.

I was referring to this:
https://github.com/jjoedouglas/angular-drag-and-drop-lists, not a specific
commit. I'm sure if you dig through the commits you'll probably find it.

On Tue, Nov 17, 2015 at 2:35 PM sethiron notifications@github.com wrote:

jjoedouglas,

Is this the fork you're referring to?
jjoekoullas/angular-drag-and-drop-lists@9f21552
jjoekoullas@9f21552

If so, is it safe for me to use? You mentioned that you might have broken
other functionality with this fork. Can you note what that functionality
is? Perhaps this change might be suitable for me as well if I don't need
that broken functionality.

Thank you for your response.


Reply to this email directly or view it on GitHub
#173 (comment)
.

@pisethdanh
Copy link
Author

I've tried applying the following commit:
jjoekoullas@c30d265

Is this the commit you're referring to? The behaviour is still the same. The index parameter in the drop callback is still 1 more than it should be when dragging and dropping an item from higher on the list down to a lower spot.

When debugging, this piece of code doesn't even execute. it is returning return stopDragover(); on line 335

@pisethdanh
Copy link
Author

Perhaps I'm approaching this all wrong. I initially thought the index parameter was the index of the item in the databound list.

The index is actually the index of the UI element placeholder.

This complicates things since I am using a filter on the databound list. The UI elements indices don't exactly correspond to the list that its bound to.

@fcosrno
Copy link

fcosrno commented Nov 20, 2015

I hit this same issue today. Not sure if this is the right approach, but I was able to solve it.This is a continuation of issue #168

The key is identifying the index of the previous object, as you found out. I noticed there is a empty destination object before the splice. This is the new position! So I grabbed the key of the empty object before the slice, then looked for the similar object at another key after the splice. It works, but seems hacky? Would love to know if there is a simpler approach.

Also, I removed dnd-moved from the list item. Not sure if that is the right approach.

function dropCallback(event, index, item, external, type, allowedType) {
    return dataService.getTest().then(function(response){
        if(!response){
            console.log('XHR failed... undoing drag');
            vm.items.splice(index, 1);
            return false;
        }
        console.log('XHR success!');
        // Get the key of the destination position. This will hold the future object
        for(var key in vm.items) {
            // This will be an empty object, however, Angular adds one property $$scope
            if(Object.keys(angular.copy(vm.items[key])).length<=1){
                var target_index = key;
            }
        }
        // Do the splice
        vm.items.splice(index,1,item);
        // Find an object equal to the one in target_index and remove it from the array
        angular.copy(vm.items).forEach(function(value, key) {
            if(key!=target_index && angular.toJson(value)==angular.toJson(vm.items[target_index])){
                vm.items.splice(key, 1);
            }
        });
        return item;
    });
}

@pisethdanh
Copy link
Author

I solved it another way, and it also seems hacky or less desirable as well. I couldn't rely on the index parameter coming in since that index is actually the index value of the dnd-placeholder UI element.

My datasource for the list is displaying a subset of the actual datasource using filters so the indices of the datasource objects don't exactly correspond to the indices of the UI elements.

What I've done is I used the item parameter to locally cache the "old" item and its index value, and just have the callback return item;

If the async call fails, I would just iterate through my datasource and remove the current item, and add the "old" item back in.

$scope.dropCallback = function (index, item) {
    // since the index parameter coming in is actually the index of the dnd-placeholder
    // ui element, we want to find the actual index of the databound object
    // so we can reverse the item's position later if the async call fails
    var oldIndex = -1;
    for (var i = 0; i < $scope.myList.length; i++) {
        if ($scope.myList[i].Id === item.Id) {
            oldIndex = i;
            break;
        }
    }

    // async call
    $http.post(...) {
        // aysnc call failed
        // remove the item and add it back to its previous index
        if (oldIndex !== -1) {
            for (var i = 0; i < $scope.myList.length; i++) {
                if ($scope.myList[i].Id === item.Id) {
                    $scope.myList.splice(i, 1);
                    break;
                }
            }

            $scope.myList.splice(oldIndex, 0, item);
        }
    }

    return item;
});

@fcosrno
Copy link

fcosrno commented Nov 23, 2015

Even simpler. Make a copy of your list on drag start. If the async call fails, reset the list.

HTML Markup

<ul dnd-list="vm.items">
    <li ng-bind-html="item.label" 
        ng-class="{'selected': vm.selected === item}" 
        dnd-moved="vm.moveCallback($index,item,event)" 
        ng-repeat="item in vm.items" 
        dnd-draggable="item" dnd-effect-allowed="move" 
        dnd-selected="vm.selected = item" 
        dnd-dragstart="vm.dragStartCallback(event)">
    </li>
</ul>

Angular Controller

function mainController(dataService){
    vm = this;
    vm.error=null;
    vm.resetItems = {};
    vm.items={{id:1,label:'Label 1'},{id:2,label:'Label 2'}};
    vm.moveCallback = moveCallback;
    vm.dragStartCallback = dragStartCallback;

    function dragStartCallback(){
        vm.resetItems = angular.copy(vm.items);
    }

    function moveCallback(originalIndex,item,event){
        vm.items.splice(originalIndex, 1);

        return dataService.updateQueue(vm.items).then(function(response){
            if(!response){
                vm.items = vm.resetItems;
                vm.error = "There was a server problem. If the problem persists please notify the system administrator.";
            }
        });
    }
}

There's a few things to notice:

  1. I'm not using dropCallback anymore. dnd-moved works fine by itself
  2. Example uses vm instead of $scope (be wary of copy/pasting)
  3. Use angular.copy when copying the list before the move, otherwise Angular will apply changes to it and cause unexpected results

@kretep
Copy link

kretep commented Nov 26, 2015

Isn't this the same issue as described in #54?

It caused me some confusion also.

@madisona
Copy link

@sethiron From reading a few other older issues, this appears to be intended behavior as @kretep mentions referencing #54. The reason you're seeing this behavior is because the original item is technically still in the list, it has probably just been hidden from view #54 (comment)

It would be nice if there was somewhere that explained this more explicitly. It bit me too at first.

@marceljuenemann
Copy link
Owner

As @madisona mentioned, the old item is still in the list, so the index might not be what you would expect. I should add some info about this to the doucmentation.

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

No branches or pull requests

6 participants