components icon indicating copy to clipboard operation
components copied to clipboard

feat(MatSort): Enable multi-sorting capability on current Interface

Open imerljak opened this issue 2 years ago • 3 comments

Feature Description

Currently MatSort has a simplistic design that enables sorting only on one column at a time.

Most of enterprise level applications should need multi-sorting capabilities on its datasets. And having it included would be really nice.

In simple terms, my proposal would be to expose a boolean input matMultiSort flag and refactor MatSort and MatSortHeader so that MatSort will hold the MatSortable's state, and expose isActive(id: string) => boolean and getNextDirection(id: string) => SortDirection that will be used instead of directly accessing active and direction.

MatSort will hold a sortState: Map<string, Sort> that can be used to query its state whenever a sort event occurs.

This change should avoid most (if not all) compatibility issues. But it would be best if we can deprecate the current sortChanged: EventEmitter<Sort> for one that emits Sort[] instead, this way we wont need to expose sortState (although most of the usage examples for this API expect one to access its internal state anyway).

Known issues

This change of sorting behavior will require a review of the included sorting method for MatTableDataSource, that was designed around a single column sort. Having multiple possible sortable columns will require a better strategy on sorting (weighting by index position?). Or, whenever the MatTableDataSource default implementation is used, multi-sorting could be disabled?

Use Case

We want to be able to enable sorting based on multiple columns at a time.

This should be done with minimal changes.

Proposed usage:

<!-- Adding matSortMultiple will enable multi-sorting -->
<table mat-table matSort matSortMultiple matSortActive="position" matSortDirection="asc" [dataSource]="dataSource">
    <ng-container matColumnDef="name">
    <th mat-header-cell *matHeaderCellDef mat-sort-header sortActionDescription="Sort by name">
      Name
    </th>
    <td mat-cell *matCellDef="let element"> {{element.name}} </td>
  </ng-container>

<ng-container matColumnDef="symbol">
    <th mat-header-cell *matHeaderCellDef mat-sort-header sortActionDescription="Sort by symbol">
      Symbol
    </th>
    <td mat-cell *matCellDef="let element"> {{element.symbol}} </td>
  </ng-container>
</table>
@ViewChild(MatSort) matSort: MatSort;

ngAfterViewInit() {
    matSort.sortChanged.pipe(map(() => Array.from(matSort.sortstate.values()))
    .subscribe((sorts: Sort[])=> {
        // do something with the Sort[] 
    });
}

imerljak avatar Dec 14 '21 16:12 imerljak

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

angular-robot[bot] avatar Mar 14 '22 15:03 angular-robot[bot]

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

angular-robot[bot] avatar Apr 03 '22 15:04 angular-robot[bot]

Is it possible to start the voting process again, this feature would be very helpful.

dzaunerOci avatar Dec 21 '23 10:12 dzaunerOci

Hello @imerljak,

Would you like to send us a PR for this? This feature request is open source, and anyone is welcome to work on it.

We have a requirement to be backwards compatible, so any PR would need to add multi-sorting without breaking the existing "single-sorting". If the PR changes the sort behavior when not multi-sorting, then we might need an opt-out or opt-in.

Best Regards,

Zach

zarend avatar Jan 02 '24 18:01 zarend

Hello @imerljak,

Would you like to send us a PR for this? This feature request is open source, and anyone is welcome to work on it.

We have a requirement to be backwards compatible, so any PR would need to add multi-sorting without breaking the existing "single-sorting". If the PR changes the sort behavior when not multi-sorting, then we might need an opt-out or opt-in.

Best Regards,

Zach

I have the required changes at my cloned repo. But will need to checkout if it is still working as expected since it has been some time already.

Will find some time to rebase it and run the tests again to see if it still works and open a PR.

If I recall correctly it is indeed backwards compatible. Or very close to.

Can't commit to a date though but will try to look into it as soon as possible.

imerljak avatar Jan 02 '24 19:01 imerljak

No problem, let us know if you have questions.

zarend avatar Jan 02 '24 19:01 zarend

@zarend Opened the MR with the changes for adding multi-sort.

Reviewed my previous implementation in order to avoid any kind of conflicts with current sorting behaviour. So, unless we want to keep track of the current internal sorting state, just toggling the multi-sort flag is enough to make it work.

From what I was able to test on the sorting algorithm it is working as expected, and the behaviour is exactly the same as we have in this other lib which I used as a comparison. For both sorting on multiple columns, as with the order in which we toggle the sorting and cycle through it.

No fancy shift metakey though.. didn't think it was needed atm.

Waiting for feedback over there.. https://github.com/angular/components/pull/28458

imerljak avatar Jan 21 '24 03:01 imerljak