stimulus-components / stimulus-sortable

A Stimulus controller to reorder lists with drag-and-drop.
https://stimulus-sortable.stimulus-components.com
MIT License
86 stars 19 forks source link

Nesting Sortable Controllers #16

Open AlexKeyCodes opened 2 years ago

AlexKeyCodes commented 2 years ago

Nesting sortable lists is causing the parent list to no longer be sortable, by this I mean the parent items are no longer able to drag and drop, and no data is being sent to the server. The child list works as expected though.

Nesting works so long as the child list is not inside of a sortable item. So in the example below, if the nested sortable div is moved outside of the parent div (a sortable item) then sorting works for both parent and child. Hopefully that makes sense.

If this is intentional, is there a work around to allow this html structure and have both parent and child lists be sortable?

<div data-controller="sortable" data-sortable-animation-value="150" data-sortable-resource-name-value="delivery_menu" data-sortable-param-name-value="sort">
  <% menus.each_with_index do |menu, i| %>
    <div data-sortable-update-url="<%= reorder_restaurant_settings_online_orders_menu_path(menu) %>">
      <h2>
        <%= menu.name %>
      </h2>
      <div data-controller="sortable" data-sortable-animation-value="150" data-sortable-resource-name-value="delivery_menu_item" data-sortable-param-name-value="sort">
        <% menu.menu_items.order(:sort).each do |menu_item| %>
          <%= link_to menu_item.name,
                      edit_restaurant_settings_online_orders_menu_meal_path(menu_item, menu_id: menu.id),
                      remote: true,
                      data: { sortable_update_url: reorder_restaurant_settings_online_orders_menu_meal_path(menu, menu_item) } %>
        <% end %>
      </div>
    </div>
  <% end %>
</div>

EDIT: Here is the same scenario but easier to read and copy/pastable.

<div data-controller="sortable">
  <div>
    <h1>
      Parent 1
    </h1>
    <ul data-controller="sortable">
      <li>Child 1</li>
      <li>Child 2</li>
      <li>Child 3</li>
    </ul>
  </div>
  <div>
    <h1>
      Parent 2
    </h1>
    <ul data-controller="sortable">
      <li>Child 1</li>
      <li>Child 2</li>
      <li>Child 3</li>
    </ul>
  </div>
  <div>
    <h1>
      Parent 3
    </h1>
    <ul data-controller="sortable">
      <li>Child 1</li>
      <li>Child 2</li>
      <li>Child 3</li>
    </ul>
  </div>
</div>
henrydjacob commented 1 year ago

Any solution for this problem

AlexKeyCodes commented 1 year ago

Any solution for this problem

Unfortunately I was unable to find a solution.

stefsava commented 1 year ago

It seem works.

https://playcode.io/1028519

Maybe you have an old version?

AlexKeyCodes commented 1 year ago

It seem works.

https://playcode.io/1028519

Maybe you have an old version?

@stefsava It only allows you to drag the parent 1 position. In your example you can't move Parent 1 to the third position without first moving it to position two. Also when you add data-sortable-update-url to the parent, its not sending any data to the specified url.

valikos commented 1 year ago

I faced with the same issue. I tried add to the sortable instance custom group name. Unfortunately problem still the same.

elalemanyo commented 1 year ago

The problem is the use of native HTML5 drag and drop API by SortableJS. Having nested list inside lists and reordering them causes disconnect and connect. Here is a small example I did to check the issue.

https://user-images.githubusercontent.com/3856862/223846575-65e4ab05-2a7f-45c2-b557-0e0c9d5cd34c.mp4

A possible solution would be to use targets for each list that need to be sortable and nested. Here is a small example.

https://user-images.githubusercontent.com/3856862/223847346-0a3ac9b9-8a65-4e19-a703-58b6b2c908d7.mp4

jdmcleod commented 11 months ago

@elalemanyo Thank you for that solution! Manually creating the sortable elements fixed the issue for me.

serge-effetmonstre commented 4 months ago

I tried @elalemanyo's solution but the only reason it works is because it does not destroy() any of the nested targets sortable instances. As soon as you do, it breaks just like the other solution. So the current "fix" seems to be about never destroying the sortables. I do this by overriding the disconnect() method with an empty function and never calling super.disconnect() or sortable.destroy(). A proper fix would be much nicer...

elalemanyo commented 4 months ago

@serge-effetmonstre can you explain better the error? Maybe we can look for a better solution

serge-effetmonstre commented 4 months ago

@elalemanyo the error is that if you call sortable.destroy(), then we get the broken behaviour you see in your first video.

Here is the solution you've used but modified to call destroy(), so you can see the problem, and how using stimulus targets does not actually fix the underlying issue:

application.register('sortable', class extends Controller {
  static targets = ['sortable']

  initialize() {
    this.sortables = new Map();
  }

  sortableTargetConnected(el) {
    this.sortables.set(el, 
      Sortable.create(el, {
        animation: 150,
        ghostClass: 'bg-light',
        dragClass: 'bg-white',
        onEnd: function() {
          console.log('sort ended') 
        }
      })
    );
  }

  sortableTargetDisconnected(el) {
    let s = this.sortables.get(el);
    s.destroy();
    this.sortables.delete(el);
  }
})
james-em commented 4 months ago

I have put some effort investigating as well and here is what I discovered.

I used @elalemanyo example here https://codepen.io/elalemanyo/pen/JjargEP?editors=0011

When dragging an item, the node moves around in the DOM causing stimulus target to disconnect and reconnect. Doing so, we then go ahead and create a new Sortable instance on that new element. This is wrong.

SortableJS moves element around in the DOM in a way that it doesn't disconnect event listeners and doesn't require a new Sortable initialization. Creating a Sortable instance on the targetConnected event actually creates duplicate instance on that element when dragging items around.

For some reason, as soon as we call .destroy() on a Sortable instance while dragging, even if it's a child list, the dragging stops.

Truthfully, we should not destroy a Sortable instance unless it really is removed from the DOM.

Sortable documentation provides these static method

Sortable.active:Sortable - The active Sortable instance.
Sortable.dragged:HTMLElement - The element being dragged.
Sortable.ghost:HTMLElement - The ghost element.
Sortable.clone:HTMLElement - The clone element.
Sortable.get(element:HTMLElement):Sortable - Get the Sortable instance on an element.

Using those we should be able to figure out when connecting/disconnecting a target if we should take action or not i.e looking if our current disconnected/connected item is a child node of Sortable.dragged or maybe checking if Sortable.active is defined is enough? When that is the case, we shouldn't destroy the Sortable instance nor we should create a new one. However, we would need to fetch back the Sortable instance as it would be undefined in the new controller instance https://github.com/stimulus-components/stimulus-sortable/blob/master/src/index.ts#L32 using Sortable.get(element:HTMLElement):Sortable

james-em commented 4 months ago

Here is a fixed exemple of @elalemanyo

https://codepen.io/James-St-Pierre/pen/ZEZZqew

PR: https://github.com/stimulus-components/stimulus-sortable/pull/28

MiltonRen commented 3 months ago

@james-em Thank you so much for the fix! Saved me so many hours 😃