stimulus-sortable icon indicating copy to clipboard operation
stimulus-sortable copied to clipboard

Stimulus-sortable as a nested sortable not working

Open ducho opened this issue 4 years ago • 3 comments

Hi @michaelbaudino ! Can you tell me if this component works as a nested? If so, how to use it as a nested?

ducho avatar Oct 12 '21 11:10 ducho

Hi @ducho, I'm not sure why you mentioned me since I'm not a maintainer of this project, but I'd recommend asking @guillaumebriday :smirk:

michaelbaudino avatar Oct 21 '21 09:10 michaelbaudino

The issue seems to be that the nested sortable controller is receiving a disconnect message from Stimulus.

To fix this locally I modified my code to have the parent sortable apply a class to the body when the drag starts and remove it when the drag completes. I actually need that for a different aspect of my user interface. I was able to use this "state" in the disconnect logic of the nested sort.

  disconnect() {
    // If we are dragging any item around, ignore the disconnect message
    // and leave the state of this sortable along. However, if no drag
    // is happening, we will process the disconnect request
    if (!document.body.classList.contains('active-dragging')) {
      super.disconnect()
    }
  }

I suspect there might be another state that could be tested to prevent this. I also suspect that it has something to do with an interaction between SortableJS cloning/moving logic and a MutationObserver in Stimulus that is attempting to do some automatic clean up.

gstark avatar May 30 '22 13:05 gstark

So for me this was a huge pain, especially in a nested Sortable.

Thanks @gstark for pointing me in the right direction! I ended up with a way more convoluted logic, especially since you can't just use the onStart and onEnd SortableJS methods as onStart is called too late.

I ended up extending my SortableJS Stimulus controller like this:

export default class extends Sortable {
	bodyClass = 'sortablejs-sorting'
	awaitingConnect = false
	awaitingDisconnect = false

	listenerOpts = {
		capture: true,
		passive: true,
	}
	downListener = (e) => {
		document.body.classList.add(this.bodyClass)
	}
	upListener = (e) => {
		document.body.classList.remove(this.bodyClass)
		this.handleWaiting()
	}

	handleWaiting = () => {
		if (this.awaitingDisconnect) {
			this.awaitingDisconnect = false
			this.disconnect()
		}
		if (this.awaitingConnect) {
			this.awaitingConnect = false
			this.connect()
		}
	}
	connect() {
		// Stimulus listens to DOM change events and calls connect() and disconnect() automatically
		// this unfortunately breaks SortableJS, so we need to (attempt to) prevent this behavior.
		// This behavior also needs to be global to apply to other sortables that might be in the same group,
		// so we use a body class to share the state.

		if (this.options.supportPointer){
			this.element.addEventListener('pointerdown', this.downListener, this.listenerOpts)
			this.element.addEventListener('pointerup', this.upListener, this.listenerOpts)
		} else {
			this.element.addEventListener('mousedown', this.downListener, this.listenerOpts)
			this.element.addEventListener('mouseup', this.upListener, this.listenerOpts)
			this.element.addEventListener('touchstart', this.downListener, this.listenerOpts)
			this.element.addEventListener('touchend', this.upListener, this.listenerOpts)
		}

		if (!document.body.classList.contains(this.bodyClass)) {
			super.connect()
		} else {
			this.awaitingConnect = true
		}
	}

	disconnect() {
		if (this.options.supportPointer){
			this.element.removeEventListener('pointerdown', this.downListener, this.listenerOpts)
			this.element.removeEventListener('pointerup', this.upListener, this.listenerOpts)
		} else {
			this.element.removeEventListener('mousedown', this.downListener, this.listenerOpts)
			this.element.removeEventListener('mouseup', this.upListener, this.listenerOpts)
			this.element.removeEventListener('touchstart', this.downListener, this.listenerOpts)
			this.element.removeEventListener('touchend', this.upListener, this.listenerOpts)
		}

		if (!document.body.classList.contains(this.bodyClass)) {
			super.disconnect()
		} else {
			this.awaitingDisconnect = true
		}
	}

	get defaultOptions() {
		return {
			onEnd: this.upListener.bind(this),
		}
	}

	// .....
}

This seems to be a fully-working workaround, but I'd suggest that anyone who considers using Stimulus-Sortable and wants to use groups for moving items between lists reconsiders. It's a pain.

And I don't really fault the authors either; I'm not even sure what the proper fix is. The way Stimulus works seems to be kinda incompatible with SortableJS architecture (which, on its own, also makes sense).

Amunak avatar Mar 03 '23 15:03 Amunak