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

Problem when sorting items in a container that has other non draggable children #164

Closed
paripsky opened this issue Jul 16, 2019 · 0 comments · Fixed by #180
Closed

Problem when sorting items in a container that has other non draggable children #164

paripsky opened this issue Jul 16, 2019 · 0 comments · Fixed by #180

Comments

@paripsky
Copy link

There is an issue with sorting items inside a container/between lists where there is a non draggable element.

for example:

<div
  *ngFor="let column of tasks"
  [sortablejs]="column"
  [sortablejsOptions]="options"
>
  <span>
    this span causes the issue
  </span>
  <div *ngFor="let task of column" class="task">
    <div>
      {{ task }}
      <div></div>
    </div>
  </div>
</div>

and the options object is

options: SortablejsOptions = {
  draggable: '.task'
}

when dragging items in a column or between columns, weird side effects happen:

  • sometimes an undefined item will be added to the task list in a column.
  • when dragging 1 item, sometimes 2 items will move between the lists.

I've setup a demo on StackBlitz: https://stackblitz.com/edit/angular-9eqvue
try moving the first item in the left column to the last spot on the right column,
you'll see that 2 items moved from the left column to the right one, which is obviously not what we want to happen.

after looking through the source files, I found that the issue is in the sortablejs.directive.ts file, when sorting, elements are moved from and to indexes that are relative to the container, and if we have a span tag that takes the 0 index, moving items between the bound arrays will sometimes be broken.
the first item in the array will be index 1 relative to the parent container, which will break our logic and cause unexpected behavior as shown in the StackBlitz demo.

My suggested fix in sortablejs.directive.ts is to change each usage of old/newIndex to old/newDraggableIndex, for example:

        bindings.injectIntoEvery(event.newIndex, bindings.extractFromEvery(event.oldIndex));

to:

        bindings.injectIntoEvery(event.newDraggableIndex, bindings.extractFromEvery(event.oldDraggableIndex));

there are 3-4 lines that use old/newIndex,
I tried changing them locally and it fixed the issue for me.
I can make a PR you'd like.

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

Successfully merging a pull request may close this issue.

1 participant