SortableJS / Sortable

Reorderable drag-and-drop lists for modern browsers and touch devices. No jQuery or framework required.
https://sortablejs.github.io/Sortable/
MIT License
29.59k stars 3.7k forks source link

MultiDrag: Simultaneous multiselection across different lists #2173

Open tomasmenezes opened 2 years ago

tomasmenezes commented 2 years ago

Is there any way to simultaneously select multiple items across different lists sharing the same group and drag them (to another one or any of the original lists)?

It seems that, at the moment, multidrag selection is confined to one list at a time, with selected items in one list being deselected if an item in another list is selected.

Edit 27/06 - react-sortablejs v6.1.4 issue: Additionally, when multiple items are dragged to a different list, the selection seemingly disappears on drag end. However, by selecting a few items of the target list immediately after, the selection size includes the previous items which then cause a crash on drag end (still inside the target list).

jjeff commented 2 years ago

I just discovered this issue myself. Don't know if this is a "bug" or an "enhancement". But it really should be a part of MultiDrag. If anyone wants to implement this, I'd be willing to (co)sponsor development.

jjeff commented 2 years ago

I've created a Code Pen to illustrate this issue: https://codepen.io/jjeff/pen/rNLEZGL

The MultiDrag plugin does not allow you to select items from multiple lists simultaneously. Selecting an item from a 2nd list will unselect previous selections. So items from multiple lists cannot be dragged together.

SortableMultiDragNoMultipleLists2

jjeff commented 2 years ago

Additionally, when multiple items are dragged to a different list, the selection seemingly disappears on drag end. However, by selecting a few items of the target list immediately after, the selection size includes the previous items which then cause a crash on drag end (still inside the target list).

I'm not seeing this behavior in my example.

jjeff commented 2 years ago

I've funded a Bountysource listing for a PR on the MultiDrag plugin to add simultaneous multiselection across different lists. https://app.bountysource.com/issues/110133588-multidrag-simultaneous-multiselection-across-different-lists

tomasmenezes commented 2 years ago

Additionally, when multiple items are dragged to a different list, the selection seemingly disappears on drag end. However, by selecting a few items of the target list immediately after, the selection size includes the previous items which then cause a crash on drag end (still inside the target list).

I'm not seeing this behavior in my example.

Thank you for all the input. The described behavior seems to be a wrapper bug limited to the react-sortablejs implementation - I edited the initial post to reflect that.

jjeff commented 2 years ago

FWIW, I think this is going to require some restructuring of MultiDrag's newIndicies and oldIndicies array elements. Currently, each one is structured as follows:

interface MultiDragIndex { // I'm just making up a name for this type
  index: number
  multiDragElement: HTMLElement
}

The problem is that if we allow dragging from multiple Sortable lists at once, then the index doesn't mean anything. Index of what? So it will probably need to look something like this:

interface MultiDragIndex {
  index: number // now within the parentElement
  multiDragElement: HTMLElement // same as before
  parentElement: HTMLElement // or maybe sortableElement? indexParent?
}

In order to maintain backward compatibility, we'll also probably need a new switch to enable dragging between lists… something like mutiDragBetweenLists: true. Otherwise old implementations of MultiDrag may receive unexpected index values. But maybe @owen-m1, @RubaXa, or another maintainer has an opinion here.

maxbol commented 2 years ago

This looks like fun, I'll take a gander!

maxbol commented 2 years ago

In order to maintain backward compatibility, we'll also probably need a new switch to enable dragging between lists… something like mutiDragBetweenLists: true. Otherwise old implementations of MultiDrag may receive unexpected index values. But maybe @owen-m1, @RubaXa, or another maintainer has an opinion here.

Couldn't this be accomplished by simply checking wether the multidrag component has a group option set? Since this behavior isn't actually supported atm, any code "relying" on this configuration would already be considered broken, no?

jjeff commented 2 years ago

Couldn't this be accomplished by simply checking wether the multidrag component has a group option set? Since this behavior isn't actually supported atm, any code "relying" on this configuration would already be considered broken, no?

It would certainly be cleaner than adding another setting option. I also can't think of a reason why someone would want to maintain the old behavior. If an item CAN be dragged between lists, it should be able to be SELECTED to drag between those lists. 👍

Of course, from a user/browser perspective, there's only one "selection" at a given time. So if items from another group get selected, we should probably deselect the currently selected items. Otherwise, there could be a weird user experience where a user selects items from several groups, however, when they drag, only some of the selected items get moved. Seems like that would be confusing.

maxbol commented 2 years ago

How would we realistically handle more complex group configurations? Let's say you have three groups (A, B, C) with two sortables each. A1 and A2 can put to B, but only A2 can put to C. Should items from multiple sortables in A be selectable together?

jjeff commented 2 years ago

How would we realistically handle more complex group configurations? Let's say you have three groups (A, B, C) with two sortables each. A1 and A2 can put to B, but only A2 can put to C. Should items from multiple sortables in A be selectable together?

Whoa! It does get confusing, doesn't it? I didn't realize that SortableJS could get that granular. Individual items within a given Sortable list can be assigned different group pull/put values? Or is this implemented by assigning multiple Sortable instances to the same list? At a certain point, we just need to leave it to the developer not to implement a confusing configuration. But this may be another argument for having a multiDragSelectFromMultipleLists option (or something more succinct). 🙂

But it does seem like the group settings for a given collection of lists is going to be the key to determining whether items can be selected together. Perhaps this MultiDrag functionality only works with simple group: "name" settings? But it seems like if an item is allowed to be pulled from our lists (with a common group name), then we should allow it to be selected/dragged. Then perhaps onAdd or onMove we determine which items are (not) put-able and disallow/revert the filtered items? I'm not looking at the code as a type this, so I may have some terminology wrong here.

maxbol commented 2 years ago

Individual items within a given Sortable list can be assigned different group pull/put values?

More like multiple sortable lists can be assigned the same group, but with different rules. The following works as expected (without cross-list selection):

var multiDrag1 = document.getElementById("multiDrag1"),
    multiDrag2 = document.getElementById("multiDrag2"),
    multiDrag3 = document.getElementById("multiDrag3"),
    multiDrag4 = document.getElementById("multiDrag4");

new Sortable(multiDrag1, {
    multiDrag: true,
    selectedClass: "selected",
    group: { name: "a", pull: ["b"], put: false }, // Pulls to B, not allowed to put in
    animation: 150,
});

new Sortable(multiDrag2, {
    multiDrag: true,
    selectedClass: "selected",
    group: { name: "a", pull: ["b", "c"], put: false }, // Pulls to B and C, not allowed to put in
    animation: 150,
});

new Sortable(multiDrag3, {
    multiDrag: true,
    selectedClass: "selected",
    group: { name: "b", pull: false, put: true }, // Allowed to put in, but can't pull anywhere
    animation: 150,
});

new Sortable(multiDrag4, {
    multiDrag: true,
    selectedClass: "selected",
    group: { name: "c", pull: false, put: true }, // Allowed to put in, but can't pull anywhere
    animation: 150,
});

There is something taxonomically confusing to me about groups overall, in that any settings that apply to a group for one component (in my opinion) reasonably should apply to all components in that group. As it stands it becomes kind of difficult to reason about the properties of a group, or what membership of a group entails.

Speaking of groups, are there any established patterns for finding all Sortable instances in a specific group? Alternatively, to find the element with a Sortable instance that is closest to any arbitrary DOM element? Otherwise I'll try to implement it, just wanted to avoid reinventing the wheel.

jjeff commented 2 years ago

Speaking of groups, are there any established patterns for finding all Sortable instances in a specific group?

Hmm… I can't figure out how you'd find all instances of Sortable in any group. I'm still trying to get my head around the whole expando thing. It kind of looks like you'd need to iterate through every attribute of every element on the page and see if it startsWith('Sortable'). Or maybe there's a clue in _detectNearestEmptySortable(). It looks like there's a sortables array that you could filter.

Don't know if I'm being helpful. 🙂

maxbol commented 2 years ago

Tried to make as non-intrusive a solution as possible. Please take a look when you can :)

The way it works:

As long as your group configurations are fairly reasonable, this shouldn't produce any weird results. However, funnily enough, cross-selecting between lists now allow you to resort a list even if it has sort: false. :)

jjeff commented 2 years ago

Thanks @maxbol! This is great. I've updated the Code Pen with a patched version of Sortable from your PR. I found a few issues. But I've commented on the pull request.