Sortable icon indicating copy to clipboard operation
Sortable copied to clipboard

DnD between groups with a non-drop target in between results in a drop into the last valid location

Open hms opened this issue 4 years ago • 6 comments

  • What result are you trying to achieve and why?

I have several groupings of sort area's (see the code example linked below). Easiest way to think about the problem is: 5 sortable areas, each with a small number of items.

Sortable areas 1, 2, 4, 5 are valid drop zones via put: true Sortable area 3 is an invalid drop zone via put: false

As you drag an item from sortable area 1 through sortable area 2, the item being dragged will display each possible drop location as you drag. This is Good(™). However, when we cross into sortable area 3, which is not accepting drops (via put: false), the drag item will stay in sortable area 2 as the last valid drop zone.

The problem is if my user drops in sortable area 3, the drag item will drop in the last valid sortable location which in this case is in sortable area 2. This is counter intuitive as we just dropped in an invalid, yet we're getting a result that suggests everything worked as expected (from the application perspective).

In order of preference, I would like:

  • a drop in an invalid sortable area to be treated as a non-valid DnD operation and put things back to where they started from.
  • some kind of indication we dropped in an invalid sort area, so I can write the JS to put things back to where they stared from / at least inform my user on what's going on.

Killer feature request: Support animating reversing the DnD operation (on a function or even better, onEnd with a "undo" return result), since in theory, we know the starting and ending locations of all elements involved and you clearly know JS way better than I do as there is no way in the world I could have crafted this fantastic library.

  • Where are you getting stuck at?

Figuring out how to detect the drop request was in an invalid sortable drop zone. I believe I have enough data in the onEnd event to glue things back to the starting state. It won't be pretty since it's just going to "pop" back to that state, but at least things will be correct form my applications perspective and I'll have the opportunity to message my user so they know what's going on and why.

Reproduction

https://codesandbox.io/s/fervent-flower-9zsd0?file=/src/index.js

Hopefully, the above codesandbox is accessible to everyone.

To replicate the issue, grab element "three" and drag and drop it between elements "thirteen" and "fourteen". Everything works great.

Reset the codesandbox

Grab element "three", but this time, drop between elements "nine" and "ten" and you will see that element "three" finishes after element "seven". This makes it appear as if everything was successful, when from the applications (vs. sortablejs) perspective, we just had a failure.

Version

package version
sortablejs. 1.12.5
@types/sortablejs 1.10.6.

hms avatar Sep 23 '20 23:09 hms

Update:

I've taken a new direction that gets me most the way.

using the onStart(), I can annotate the sort region with a visual clue for the user that this is not the drop zone they are looking for.

using a pull function, I can switch between clone and true based on if the user is dragging over the bad drop zone. We will clone if they drop in the bad drop zone because it leaves the source item right where it was and that's perfect for a DnD operation that I'm going to effectively ignore.

In onEnd(), if the drop was in the bad drop zone (despite the visual clues not to do that), we simply event.item.remove() and prevent the normal onEnd() processing that's responsible for updating the database that tracks the position in the list. Otherwise, there is just a little house keeping and we process as usual.

This is 90% of the required functionality. The only thing missing now is how to tell the bad drop zone target sortable that I don't want it to sort. What makes this harder is for a variety of reason, I may not have access to the sortable object variable (I'm still exploring various options).

Question: Is there anyway to get access to the sortable instance from either event object, to, from objects? I've tried several stategies without any luck.

hms avatar Sep 24 '20 02:09 hms

@hms, great work on the demo! I had to re-add the missing items for it to work, but it clearly demonstrates the issue. We're currently implementing something similar, would you mind sharing or updating the demo with your onStart() and onEnd() implementation? It would be highly appreciated!

aried3r avatar Oct 19 '20 11:10 aried3r

Sorry for the delay in responding -- deadlines....

I'll look into posting something in the next week or so

hms avatar Oct 30 '20 21:10 hms

@hms Your problem description & partial solution were very helpful, thank you!

canadaduane avatar Jan 04 '21 00:01 canadaduane

For reference this is also affecting my implementation. It seems like an odd feature not to have. The rest of the library is very good. Pitty @hms didn't get around to posting his solution...

garrm avatar Jan 16 '24 02:01 garrm

Hello all,

I'm sorry for not following up on my original post. I ended up going a completely different direction in my UI that allowed for a much simpler solution in the end. Also, I was learning JS on the fly while writing this application and this sort function, so I'm sure there is plenty that could be improved and a couple of things that are just plain wrong. One significant note: I was building this in Rails and this class is based on Stimulus.js

  • connect() is called when the HTML element this code is attached to is rendered.
  • My UI (at the time) has multiple sections that can never be sorted / ordered. They are identified by an id=category-uncategorized || category-discarded || staged-libraries
  • My UI grouped the sorted regions in ULs and each sortable item was an LI
  • Each LI element had a drag handle
  • FetchRequest is another Rails package that made my life easier with the async messaging to the server to persist the results.

connect() { this._sortable = new Sortable(this.element, { group: { name: "shared", put: this.canPut.bind(this), }, // handle: '.library-element', handle: ".drag-handle", filter: ".ignore", sort: this.canSort(this), animation: 150, onEnd: this.end.bind(this), onChoose: this.onChoose.bind(this), onUnchoose: this.onUnchoose.bind(this), }); }

canSort(that) { return !( that.element.id === "category-uncategorized" || that.element.id === "category-discarded" || that.element.id === "staged-libraries" ); }

canPut(to) { return !to.el.classList.contains("no-drop"); }

// we compare each LI element for a duplicate with the event.item. // If we find a dup (by name), we set the .opacity-25 on the UL element // to make the entire drop region look like it won't accept a drop onChoose(event) { let sourceUL = event.target; let allULs = document.getElementsByTagName("UL");

let name = event.item.dataset.name;

for (let ul of allULs) {
  if (ul === sourceUL) {
    continue; // don't look for a dup name in our source category
  }

  if (ul.id === "category-discarded" || ul.id === "staged-libraries") {
    ul.classList.add("opacity-25", "no-drop");
    continue;
  }

  let liElements = ul.getElementsByTagName("LI");
  for (let li of liElements) {
    if (li.dataset.name === name) {
      ul.classList.add("opacity-25", "no-drop");
    }
  }
}

}

// Function to remove the .opacity-25, where ever it might be // removeOpacity() { onUnchoose() { let allULs = document.getElementsByTagName("UL");

for (let ul of allULs) {
  ul.classList.remove("opacity-25", "no-drop");
}

}

// updates the dragHandle color // Means we can choose to not redraw the library_element if we so choose (we will choose) updateDragHandle(event) { const el = event.item.getElementsByClassName("drag-handle")[0]; const remove = event.from.dataset.sortableLibraryBgCssValue; const add = event.to.dataset.sortableLibraryBgCssValue; el.classList.remove(remove); el.classList.add(add); }

// On drop, // * make sure we didn't drop into a 'no-drop' location -- if so, don't do anything // and let sortable put things back for us. // * Otherwise, figure out our from and to data, and tell Rails about the new // location (Category and Position) async end(event) { if ( event.to.classList.contains("no-drop") || event.to.id === "staged-libraries" || event.to.id === "category-discarded" ) { return; }

event.preventDefault();

this.updateDragHandle(event);

let id = event.item.dataset.id;
let url = this.urlValue.replace(":id", id);
let data = new FormData();
data.append("position", event.newIndex + 1); // the + 1 used to be required, now not so much?
data.append("category_id", event.to.dataset.categoryIdValue);

const request = new FetchRequest("patch", url, {
  responseKind: "turbo-stream",
  body: data,
});
const response = await request.perform();
if (response.ok) {
  const _text = await response.text;
  if (this.successFunc !== undefined) {
    await this.successFunc();
  }
}

}

hms avatar Jan 16 '24 04:01 hms