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

Maintain sort property order when changing sort direction #1281

Closed
cmrgKoradir opened this issue Sep 24, 2020 · 6 comments · Fixed by #4089
Closed

Maintain sort property order when changing sort direction #1281

cmrgKoradir opened this issue Sep 24, 2020 · 6 comments · Fixed by #4089
Labels

Comments

@cmrgKoradir
Copy link

SETUP
Take any GridPro and make it multi-sort.

Experienced under:
Vaadin 14.2.3
Chrome 84.0.4147.89 (Official Build) (64-bit)
Ubuntu 20.04.1 LTS

IS
Whatever sort property you activate last is considered the one that should be sorted by first.
(Weird behaviour, imo, but apparently a feature since vaadin/vaadin-grid#1624 was not labelled a bug.)
If you have multiple sort properties selected, and you change the sort direction of one, that will count as being "activated last" and will therefore become the new first sort property.
animated gif of unintended behaviour

SHOULD
Changing sort direction on an already active sort property should maintain the existing sort order.

@tomivirkki
Copy link
Member

This also is intended behavior and not a bug. I took a look at some of the other data grids out there and seems that some follow the same behavior (the last sorted column gets the first priority in sorting) and some do the opposite. Changing this now in vaadin-grid would be a breaking change, UX-wise. What we could do is to offer some more control over what happens on column sort so it would be easier to apply individual preferences for the behavior. But that would also be an enhancement, not a bugfix.

Some adjustments can be made with the current API already:

  • Use dataProvider instead of items. It will receive the sort order on every data request. Using this information, the application logic can handle the effective sort order in any fashion.
  • If the sorter arrow numbering doesn't follow the desired behavior, the numbers can be hidden with a style module or vaadin-grid can be extended (or monkey-patched) to override the numbering logic in vaadin-grid-sort-mixin (the latter option would of course mean accessing private API)

@cmrgKoradir
Copy link
Author

I will admit, I am VERY surprised this is "intended behaviour" because it's confusing to just about everybody I show this to.

I took a look at some of the other data grids out there and seems that some follow the same behavior

Of course they do, they quite simply have no other choice if they want to use "plain Vaadin".

Use dataProvider instead of items. It will receive the sort order on every data request. Using this information, the application logic can handle the effective sort order in any fashion.

Yes, but maintaining the sort property order when you change the sort direction requires either overriding the sort property selection logic in the vaadin component or slapping additional bookkeeping on top of it to correct vaadin's selection before we pass it on. Because we can no longer make assumptions about the property order in the backend.

@tomivirkki
Copy link
Member

tomivirkki commented Sep 25, 2020

Of course they do, they quite simply have no other choice if they want to use "plain Vaadin".

I meant data grids from vendors other than Vaadin

Yes, but maintaining the sort property order when you change the sort direction requires either overriding the sort property selection logic in the vaadin component or slapping additional bookkeeping on top of it to correct vaadin's selection before we pass it on. Because we can no longer make assumptions about the property order in the backend.

Yes, additional bookkeeping would most likely be required. A dedicated API for the purpose would be ideal.

@mvysny
Copy link
Member

mvysny commented Oct 13, 2020

@tomivirkki could you please post links to videos or pages from those vendors other than Vaadin, so that we can compare?

@tomivirkki
Copy link
Member

@mvysny Here's one https://fancygrid.com/samples/sorting/multi

@vaadin-bot vaadin-bot transferred this issue from vaadin/vaadin-grid May 19, 2021
@vaadin-bot vaadin-bot added enhancement New feature or request vaadin-grid labels May 19, 2021
@web-padawan
Copy link
Member

To me it looks like the proper way of fixing this would be adding new property e.g. reverseSortOrder to modify this block:

if (sorter.direction) {
this._sorters.unshift(sorter);
}

  if (this.reverseSortOrder) {
    this._sorters.push(sorter);
  } else {
    this._sorters.unshift(sorter);
  }

Then we would also need to update sorters when reverseSortOrder is set to true / false while grid is already sorted.

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

Successfully merging a pull request may close this issue.

6 participants