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

Reorder positions #233

Closed
MarcReniu opened this issue Sep 23, 2016 · 3 comments
Closed

Reorder positions #233

MarcReniu opened this issue Sep 23, 2016 · 3 comments

Comments

@MarcReniu
Copy link

Imagine you have the next elements with missing higher positions:

  • ElementA in position 4
  • ElementB in position 5
  • ElementC in position 6

If I want to move position of ElementA to top of list, I can use move_to_top method, but in case I execute next move_higher over ElementB, it becomes position 4, and ElementA, position 5.

The final goal is to reorder all elements, considering top_of_list config. So, there is some method to perform it, or I have to do it manually?

@brendon
Copy link
Owner

brendon commented Sep 26, 2016

Hi @MarcReniu, there isn't currently a method to clean up the list orderings. move_higher simply swaps the position values of the two concerned records as far as I'm aware.

We're always keen to improve the library. If you have any ideas (and PR's) that can help, we'll do our best to see them merged. One feature I'd like is an auto-repair process that fixes up duplicate position values in a list.

@MarcReniu
Copy link
Author

I finally made a module to check and perform the reorder. It is ugly as f***, but it only makes the minimum possible queries, and no executes validations or callbacks.

You could use it as an starting point, or throw it to the garbage, as you wish.

module Ordenable
  extend ActiveSupport::Concern

  def valid_ordering? relation
    records = self.send(relation)
    records_class = records.class_name.constantize

    top_of_list_config = records_class.acts_as_list_top
    position_column = records_class.quoted_position_column.delete("`")
    current_order = records.collect(&:"#{position_column}")
    correct_order = (top_of_list_config...(records.size + top_of_list_config)).to_a

    return current_order == correct_order
  end

  def correct_ordering relation
    return if self.valid_ordering?(relation)

    records = self.send(relation)
    records_class = records.class_name.constantize

    top_of_list_config = records_class.acts_as_list_top
    position_column = records_class.quoted_position_column.delete("`")
    new_positions = (top_of_list_config...(records.size + top_of_list_config)).to_a

    records.each_with_index do |record, index|
      record.update_column(position_column, new_positions[index])
    end
  end
end

@brendon
Copy link
Owner

brendon commented Oct 3, 2016

Thanks @MarcReniu, the important part is this:

records.each_with_index do |record, index|
  record.update_column(position_column, new_positions[index])
end

which is what a lot of people do when processing a reorder request from the browser anyway (many drag and drop plugins send through just an array of id's. I prefer to set the position explicitly these days. That, coupled with our scope change detection allows for some pretty powerful reordering (i.e. automatic removal from the old scope, insertion at the specified position in the new scope.)

We recently merged a PR that dealt with items having the same position integer.

The fact that you have a gap between the top_of_list value and the first position is of no great consequence because what you really care about is the relative positioning. The user don't get to see the position value normally.

I recently had to refactor some code where I had the id of the item being moved, and an array of ids that included the id of the item being moved:

  # Expects an array of sibling_ids (strings), and a parent component_instance
  # This method relies on scope_changed? in acts_as_list to do the magic of positioning
  def move(parent, sibling_ids = [], scope = parent.children)
    self.parent = parent

    if sibling_ids.present?
      raise ArgumentError.new('self.id must exist in sibling_ids') unless sibling_ids.include?(id.to_s)

      if sibling_ids.size == 1
        self.position = 1
      elsif sibling_ids.size > 1
        position_among_siblings = sibling_ids.index(id.to_s)
        reference = scope.offset(position_among_siblings).first

        self.position = reference.position if reference
      end
    end

    save
  end

Sorry, that's a bit rambling but it might give you some ideas :)

I'll close this for now but do get in touch if you have any more issues :)

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

2 participants