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

MultiDrag: Simultaneous multiselection across different lists #2173

Open
tomasmenezes opened this issue Jul 8, 2022 · 15 comments
Open

MultiDrag: Simultaneous multiselection across different lists #2173

tomasmenezes opened this issue Jul 8, 2022 · 15 comments

Comments

@tomasmenezes
Copy link

tomasmenezes commented Jul 8, 2022

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
Copy link

jjeff commented Jul 26, 2022

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
Copy link

jjeff commented Jul 27, 2022

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
Copy link

jjeff commented Jul 27, 2022

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
Copy link

jjeff commented Jul 27, 2022

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
Copy link
Author

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
Copy link

jjeff commented Jul 27, 2022

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
Copy link

maxbol commented Jul 27, 2022

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

@maxbol
Copy link

maxbol commented Jul 28, 2022

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
Copy link

jjeff commented Jul 28, 2022

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
Copy link

maxbol commented Jul 28, 2022

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
Copy link

jjeff commented Jul 28, 2022

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
Copy link

maxbol commented Jul 28, 2022

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
Copy link

jjeff commented Jul 28, 2022

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
Copy link

maxbol commented Jul 30, 2022

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

The way it works:

  • Selections can be made across sortables, as long as all selected items belong to sortables in the same group.
  • On dragOver, the plugin iterates over all multi drag elements and tests:
    • If the checkPull rule of the elements parent sortable group allows a move into the target sortable
    • If the checkPut rule of the target sortable group allows a move from the elements parent sortable
  • If any of these tests fail, the drag is completed without insertion and cancelled

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
Copy link

jjeff commented Jul 31, 2022

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.

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

No branches or pull requests

3 participants