brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

Inconsistent behavior #330

Closed DamirSvrtan closed 5 years ago

DamirSvrtan commented 5 years ago

Hi! 👋 First of all, thank you for creating this gem!

It seems to me there is some inconsistent behavior in the gem between creating and updating items in a list.

If we have three items that currently live in the list:

id position
1 1
2 2
3 3

a) if we create an item and we insert it at position 3, we would push the item number 3 to position number 4.

id position
1 1
2 2
3 4
4 3

b) but if we want to just manipulate the first item (not creating a new one), and set that it's position is on number 3, we will not push the item with ID 3 to position number 4, we will actually push it to position number 2.

id position
1 3
2 1
3 2

To recap:

Does this seem to you a bit like inconsistent behavior? On create I am using the AR create method, and for reordering i get the same behavior no matter if i use insert_at or update(position: ..).

Is there a more consistent API within the gem or am I looking at this wrong?

brendon commented 5 years ago

Hi @DamirSvrtan, thank you for your kind words :)

I'll explain what happens on an update. It happens in two steps, first, 1 is removed from the list which decrements the positions of 2 and 3 (so you can see that 2 and 3 assume the position values of 1 and 2). Next 1 is re-added to the list at position 3. If you have more than 3 items you'd see those items increment their position, so the item at position 3 already would become 4 and the rest with higher positions than that would be incremented by 1.

I hope that makes sense :) Let me know if that doesn't answer your question though.

DamirSvrtan commented 5 years ago

Hi @brendon! Thanks for the answer, now I understand what happens under the hood!

However, this does seem somewhat inconsistent in a sense that the consumers of the api that are implementing a drag and drop feature have to remember whether the item is coming from the top or the bottom of the list and depending on that implement logic where to insert the item. Or am I looking at it wrong?

brendon commented 5 years ago

Drag and drop is totally tricky! Yes you either have to send through extra information about where the list item has been dropped (i.e. before or after another item id) or just rewrite the whole list:

module Sortable
  def sort(array_of_ids)
    array_of_ids.each.with_index(1) do |id, index|
      where(:id => id).update_all(:position => index)
    end
  end
end

It'd be good if we could just give a list position (i.e. a 0 based position of where we want the item in the overall list of items and have acts_as_list sort things out from there.)

DamirSvrtan commented 5 years ago

Hey @brendon! Yeah, I expected that acts_as_list already has methods such as insert_after or insert_before but that they might not be documented well. Thanks for the explanations! Closing this, but just interested at wether there were some initiatives to do such work or was it considered too error prone due to scopes?

brendon commented 5 years ago

Thanks @DamirSvrtan. It's not on our list of things to do, but we're happy to accept pull requests if you wanted to have a go at implementing the functionality :)