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

Drag to reorder table rows #2370

Open
giant-robot opened this issue Sep 16, 2022 · 4 comments · May be fixed by #2371
Open

Drag to reorder table rows #2370

giant-robot opened this issue Sep 16, 2022 · 4 comments · May be fixed by #2371
Assignees

Comments

@giant-robot
Copy link
Contributor

It is not an uncommon need to be able to reorder rows in a table layout. Since SortableJS is already a project dependency, it shouldn't be too hard to implement

I have put together a proof of concept that enables the feature with the minimum amount of changes in the platform.

It introduces a new action that can be added to a dedicated column. This is an example applied to the RoleListLayout:

public function columns(): array
{
    return [
        TD::make()
            ->cantHide()
            ->render(function (Role $role) {
                return ReorderHandle::make()
                    ->method('reorder')
                    ->key($role->getKey());
            }),

        ...
    ];
}

Not sure this is the best way to go about it, though. Thoughts?

@giant-robot giant-robot linked a pull request Sep 16, 2022 that will close this issue
@tabuna
Copy link
Member

tabuna commented Sep 16, 2022

Hi, @giant-robot Can you help me understand the idea better? Because I'm looking at the above example and I don't understand. For example, I swapped positions 3 and 7. What value will come to the reorder method?

@giant-robot
Copy link
Contributor Author

Hey @tabuna, the current implementation will not pass arguments to the reorder method. Instead, each change will result in a POST XHR request with a payload than contains a key and an offset parameter.

key is anything you specify in the action using the key method for the item that is being dragged.
offset is the relative change in position for the same item.

You can access those directly from the request object inside the reorder method (it would be great if Request could be injected as an argument to the method when it is called).

So, for the code given above:

  • If you drag the Role with id 101 2 positions down, key would be 101 and offset would be 2.
  • If you drag the Role with id 101 4 positions up, key would be 101 and offset would be -4.

I hope that makes it clearer.

@tabuna
Copy link
Member

tabuna commented Sep 18, 2022

I like the idea of having this in a package. But I think we need to decide a few things before we release it:

  • It seems to me that we won't be able to use this effectively as an action. Maybe we should better use a class inherited from a Сell.
public function columns(): array
{
    return [
        Sortable::make('method', 'string or closure'),

        ...
    ];
}
  • It might be worth running this in much the same way as we do with async methods. Then we can expect pre-declared arguments in the specified method:
public function reorder(Role $role, int $offset): array
{
   //...
}
  • And lastly, I think we are very much missing an example that would show how to shift not only the current model, but all the others (they also have to change their sorting index).

I will try to work with you to improve these points in the near future.

@giant-robot
Copy link
Contributor Author

I've converted the sorting handle from an action to a Cell, as you suggest. It actually needs to be an extension of TD because of the contraints of the Table::columns method, but it seems to work fine.

Extending TD also has the benefit of being able to guess the sort key from the column name or use the render method for more complex cases. You can use it like so:

public function columns(): array
{
    return [
        ReorderHandle::make('id')
            ->method('reorder'),
        ...
    ];
}

Running the reorder method as an async method will be great. Let me know how we can achieve this.

Regarding the actual change of the sort key of the reordered models, I see your point but it feels to me like an implementation detail. There are more than one ways to do it and it really depends on the model itself.

Cheers,
George

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants