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

Nesting Sortable Controllers #16

Open
AlexKeyCodes opened this issue Sep 2, 2022 · 13 comments
Open

Nesting Sortable Controllers #16

AlexKeyCodes opened this issue Sep 2, 2022 · 13 comments

Comments

@AlexKeyCodes
Copy link

AlexKeyCodes commented Sep 2, 2022

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

Any solution for this problem

@AlexKeyCodes
Copy link
Author

Any solution for this problem

Unfortunately I was unable to find a solution.

@stefsava
Copy link

stefsava commented Dec 6, 2022

It seem works.

https://playcode.io/1028519

Maybe you have an old version?

@AlexKeyCodes
Copy link
Author

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

valikos commented Feb 6, 2023

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

@elalemanyo
Copy link

elalemanyo commented Mar 8, 2023

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.

shot2023-03-08.at.21.49.06.mp4

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

shot2023-03-08.at.21.52.57.mp4

@jdmcleod
Copy link

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

@serge-effetmonstre
Copy link

serge-effetmonstre commented Apr 25, 2024

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

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

@serge-effetmonstre
Copy link

serge-effetmonstre commented Apr 25, 2024

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

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

james-em commented Apr 29, 2024

Here is a fixed exemple of @elalemanyo

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

PR: #28

@MiltonRen
Copy link

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

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

9 participants