Skip to content

Comments

Merge ReorderController into ListController#553

Closed
mjauvin wants to merge 15 commits intodevelopfrom
wip/list-controller-reorder-mode
Closed

Merge ReorderController into ListController#553
mjauvin wants to merge 15 commits intodevelopfrom
wip/list-controller-reorder-mode

Conversation

@mjauvin
Copy link
Member

@mjauvin mjauvin commented May 12, 2022

Currently working for SimpleTree and Sortable models. Won't work with NestedTree.

@mjauvin mjauvin self-assigned this May 12, 2022
@mjauvin mjauvin marked this pull request as draft May 12, 2022 01:26
Base automatically changed from wip/1.2 to develop July 15, 2022 07:00
@mjauvin
Copy link
Member Author

mjauvin commented Jul 24, 2022

@LukeTowers @bennothommo @jaxwilko this work for SimpleTree and Sortable models. Won't work with NestedTree.

Any idea how we could implement this for NestedTree (since the List widget is way different than the current ReorderController)

Do we even WANT to make this work for NestedTree in the list widget?

@bennothommo
Copy link
Member

Having a think about this abstractly, I'm now a bit 50/50 on reordering as it stands.

The problems we're going to face are:

  • People are going to try reordering lists with hundreds or even thousands of records, how are we going to handle these scenarios?
  • Reordering in nested trees has to span multiple directions - not only do items have to be reordered in the same level, but potentially reordered up the hierarchy within other "branches" of the tree, or within parent categories, grandparent categories and so on.
  • People are going to try reordering lists that have another sort order used on the list, such as ordering by name or date. I think in terms of usability, we want people to be able to still use sorting normally through the table headers, but this will mean that any manual ordering will be overridden. A simple solution would be a "reorder mode" which automatically sorts the list by the sortable column(s) and enables the reordering UI.
  • Reordering through drag/drag UI implies a single item being reordered one at a time. This is going to be cumbersome and unwieldly if someone wants to reorder multiple records.
  • Realistically, we want the same user experience for reordering, regardless of the type of storage of the data, whether it be sortable, simple tree or nested tree, otherwise it could be frustrating to people who don't know the difference (ie. clients).

I don't have a potential solution right now. :(

@mjauvin
Copy link
Member Author

mjauvin commented Jul 27, 2022

@bennothommo

People are going to try reordering lists with hundreds or even thousands of records, how are we going to handle these scenarios?

I understand your concern, but what's the difference with the current ReorderController?

Reordering in nested trees has to span multiple directions - not only do items have to be reordered in the same level, but potentially reordered up the hierarchy within other "branches" of the tree, or within parent categories, grandparent categories and so on.

The current ReorderController supports this, we could add a button that opens a popup to reorder nested trees with a different list markup maybe?

People are going to try reordering lists that have another sort order used on the list, such as ordering by name or date. I think in terms of usability, we want people to be able to still use sorting normally through the table headers, but this will mean that any manual ordering will be overridden. A simple solution would be a "reorder mode" which automatically sorts the list by the sortable column(s) and enables the reordering UI.

Yeah, that's a good idea.

Reordering through drag/drag UI implies a single item being reordered one at a time. This is going to be cumbersome and unwieldly if someone wants to reorder multiple records.

Again, I agree but this is still the case with the current ReorderController.

Realistically, we want the same user experience for reordering, regardless of the type of storage of the data, whether it be sortable, simple tree or nested tree, otherwise it could be frustrating to people who don't know the difference (ie. clients).

Maybe the "reorder mode" you suggested above could trigger a change in the list markup to use what the ReorderController currently uses? Once the list is reordered, switching the "reorder mode" off would bring back the standard list markup.

@bennothommo
Copy link
Member

bennothommo commented Jul 27, 2022

@mjauvin I meant the above more as a brain dump of problems that I foresee with reordering in general, as I imagine this PR along with the Sortable Relations trait is all towards implementing sortable relations within the list widget?

The reason I've added it here is because if we decide to pivot our implementation based on what we discuss here, it might be worth trying to catch it early and perhaps set it up within this PR.

@LukeTowers
Copy link
Member

@bennothommo with regards to

People are going to try reordering lists with hundreds or even thousands of records, how are we going to handle these scenarios?

I believe I've mentioned it in one of these issues but I'm not really concerned with that use case. There's some good articles out there that make the very valid point that if you are dealing with a large number of records you almost certainly do not want to be supporting people manually ordering them. Instead, more care should be taken with available options for how they can be sorted when displayed and perhaps considering a sort of enhanced prioritization sorting method like "pinned" or "flagged" records that get booted to the top.

@mjauvin mjauvin mentioned this pull request Aug 25, 2022
@LukeTowers
Copy link
Member

@mjauvin I'm fine with your responses, what else do you need from @bennothommo or myself?

@mjauvin
Copy link
Member Author

mjauvin commented Aug 27, 2022

@LukeTowers I'll keep working on it then.

@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in this pull request for the last 6 months.
If you intend to continue working on this pull request, please respond within 3 days.
If this pull request is critical for your business, please reach out to us at wintercms@luketowers.ca.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Feb 26, 2023
@LukeTowers LukeTowers added enhancement PRs that implement a new feature or substantial change and removed stale Issues/PRs that have had no activity and may be archived labels Feb 26, 2023
@mjauvin mjauvin removed their assignment May 22, 2023
@mjauvin mjauvin added wontfix stale Issues/PRs that have had no activity and may be archived and removed enhancement PRs that implement a new feature or substantial change Status: In Progress labels May 22, 2023
@mjauvin
Copy link
Member Author

mjauvin commented May 22, 2023

I won't be pursuing this, at least not this way.

@LukeTowers
Copy link
Member

What way do you want to accomplish this @mjauvin?

@mjauvin
Copy link
Member Author

mjauvin commented May 22, 2023

I don't know yet.

@LukeTowers
Copy link
Member

@mjauvin I'm open to working on this, have a client project that will need it very soon.

What did you end up doing for your projects for this functionality?

@mjauvin
Copy link
Member Author

mjauvin commented May 3, 2025

@LukeTowers I built a custom behavior that I use in my controllers. I can share the code if you're interested.

@LukeTowers
Copy link
Member

@mjauvin sure, that would be great!

@mjauvin
Copy link
Member Author

mjauvin commented May 4, 2025

@mjauvin sure, that would be great!

There you go.

files.zip

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

Labels

stale Issues/PRs that have had no activity and may be archived

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants