feat(sort): allowing sorting by multiple columns

[relair]

Extended the Sort interface for active to allow string array and SortDirection to allow dictionary object. Adding matMultiColumn property to MatSort that will allow multi column sorting. Updating MatTableDataSource to handle multi column sorting. Adding a counter for sort-header that shows sorting precedence.

Breaking change: Because of extended interface of MatSort, properties active and direction now need to be cast to string for existing usages. Fixes #7226.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

[googlebot]

CLAs look good, thanks!

[ngbot]

Hi @relair! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

[GO3LIN]

@jelbourn @andrewseguin Have you got the time to review this PR, I saw the the issue is labeled P3, hence it's an important PR. I think that the community is in a real need of this feature.

[joshorvis]

@jelbourn @andrewseguin Have you got the time to review this PR, I saw the the issue is labeled P3, hence it's an important PR. I think that the community is in a real need of this feature.

<--- Member of the community in real need of this feature... Please with cherries on top?

[jelbourn]

I put off reviewing this while we were working on getting v7 out, and now I'm currently doing a bit of travel. I should be able to take a look sometime next week.

[jelbourn]

@relair we appreciate time time spent working on this. I took an initial look and have some high-level comments:

• Rather than adding multi-column sorting to the existing MatSort directive, we'd prefer to introduce a new MatMultiSort directive. The new directive would share relevant code with MatSort, but would let people only pull in the specific behavior they want in their app.
• I'm not sure the number indicator next to the arrow indicators is a good fit for the component. I've reached out to some UX designers inside Google to see what they would recommend here. In the one app I've looked at that already supports multi-column sorting (GCP), they don't use a visual indicator at all for the non-primary columns.
• We have to make sure that screen-reader users are able to interact with the sorting effectively. As it stands now, the extra number in the header doesn't do enough to communicate the sort state. Unfortunately the W3C doesn't yet have any guidance on multi-column sorting, so there's no obvious solution. My best guess would be adding an aria-describedby to the sort button that describes the sort status (this would have to come from MatSortHeaderIntl). I'd recommend trying the component out in a screen-reader (either NVDA on Windows or VoiceOver on macOS) and seeing how it goes. If you're unfamiliar with using screen-readers, Rob Dodson has some great videos in his a11ycasts series.

[relair]

@jelbourn Thank you for taking a look:

• I will see if I can setup MatMultiSort. To be honest I didn't like all those if statements either, so it would be good to get rid of some of them.
• I think I will change indicator so developer could add it if they want it (as I think it would be nice to have and I would like to have it for my project), but by default it wouldn't be there.
• I will check those screen readers and try to make sense of it, we will see how it goes.

[relair]

For now I will make MatSortHeader work for both MatSort and MatMultiSort as I think it would be awkward for the user to need to define different directive name for sort headers when using MatMultiSort although I could change it too.
For now I will leave those if statements in _isSorted and _updateArrowDirection, although I could change it to something implementing strategy pattern (where strategy would be picked in the constructor based on which MatSort is available).

[relair]

I have finished the refactoring, I did create MatMultiSort that shares some code with MatSort trough base class. I also refactored MatSortHeader so it is using strategy pattern to chose the logic which needs to use (no more if statements everywhere), I have also fixed aria description for MatSortHeader so whenever it is sorted by several columns multiple headers would just have description that says it is sorted ascending or descending.

[flatstadt]

Hi! Is there any plans on merging this branch any time soon? I've testing the implementation and it's been working great for me. Good job and thanks @relair

[the-avid-engineer]

Pleeeeeeeaaase!

[Only1MrAnderson]

Been watching this for a while it will be sad to see if this PR becomes outdated and kicked to the side

[FirstVertex]

Here's how I got it running. Many thanks to @relair for this great work!

1. Used Github GUI to fork @angular/material2 into my Github account
2. Used Github GUI to create a PR from relair/material2/tree/multi-column-sorting
   label on Github
3. PR accepted and merged!
4. git clone'd my angular/material2 repo to my local harddrive from https://github.com/hughanderson4/material2
5. run yarn in root of local material2
6. Inside there I found I had to install grunt and run the command gulp material:build-release it is documented here
7. It puts in dist/releases/material my copy of material with multi-column-sorting
8. Delete node_modules/@angular/material folder of your site
9. In your site in package.json delete @angular/material line and run npm i ../material2/dist/releases/material it gonna make @angular/material reference in package.json start with file:../
10. You need to set "preserveSymlinks"=true angular.json build option for local package reference see my SO post here about it lol
11. Change matSort to matMultiSort in your markup
12. Change your .ts ViewChild to be MatMultiSort type and your Subject<Sort> to Subject<MultiSort>
13. Then your sortControl active is array instead of string and sortControl's direction is an object

[andrewseguin]

Hey @relair, thanks for submitting this pull request. I apologize that it has taken so long for us to dive into it and get it merged into the library. You did a great job with this and I think we can work with this to get the functionality in.

I looked over the code and I recognized that we have an existing issue, which is likely what you found as well. The MatSortable determines if it is active by retrieving information from the container and then performing its own logic. You resolved this by creating strategies which I think was a valid solution, but I wonder if we can go another step forward and contain this logic within the MatSort and MatMultiSort directly.

I'm thinking we can have the MatSortable request its state from the container, which will tell it whether it is active, its direction, etc. That way, the MatSortable only reflects the state it is given rather than trying to determine it itself.

Another bit of contention the UX that I think we can solve if we allow the user to provide a custom template to be used to the right of the arrow.

I'm going to try and come up with a PR that inverts that state management a bit and we can go from there. Once again I'm sorry that we drop the ball on reviewing this sooner. We got a bit caught up in some other tasks like helping the core team land Ivy, but I think its important that we make sure we focus on contributors that want to help us and our users.

[andrewseguin]

I'm going to split this pull request up into three stages.

1. Refactor the sort header so that its sort container determines and provides its state (e.g. it is up to the sort container to determine if the sort header is sorted, not the sort header determining this based on sort container properties).
2. Add a new feature that allows the user to provide their own ng-template to proceed the sort arrow, which can be used for things like a multi-sort level/count indicator.
3. Add a new MatMultiSort that is a new sort container.

[relair]

Hey @andrewseguin, thank you for looking into getting this feature done. I did notice that I did that pull request at a bit unfortunate time as the new angular version was just being released, so I was waiting patiently until someone will find some time.
For my work I was cautious about doing big changes, so for example I didn't move the logic out of sort-header, even after wrapping it in strategies, so it will at least be still in the same file. I agree however that just getting the strategy directly from MatSort or MatMultiSort or some generic interface for them would be better than those if statements for selecting strategy.
Hopefully you will be able to change the interfaces so it will be easier to swap the sorting implementation. Probably you will end up with more breaking changes because of it, but it will probably be worth it in the end. I guess the feature will need to go to the next big version, because of breaking changes?

[andrewseguin]

Fortunately it looks like we'll avoid any breaking changes (except to the constructor of MatSort which will only affect users who extend it). However, we are already in the mode of targeting the next major so I expect this will land there.

Thanks again for starting us off on this feature, we are now talking about moving a lot of the core logic of the sort manager to the CDK and make it easier for users to extend and swap things around.

Feel free to take a look at the first step on this initial PR: #15171

Next steps after that will be to allow a custom template to be provided to follow the sort arrow (for the multi sort counter), and then a pull request to include your multi-sort using the new refactored setup

[raghufsmk]

Hi.. Could you please tell me when this feature will be merged to master?

Thanks in advance

[andrewseguin]

@raghufsmk I probably will not have the time to personally message you that this is ready, but you are welcome to keep an eye on issue to see when it is closed, which means it is merged in master

[Noppey]

How does it take months to get a review done? We are also waiting for this feature.

[relair]

Hmm, why is CLA complaining again?

[relair]

@andrewseguin Hi Andew, what is the progress on pulling this request? Do I need to do something more with that pull request that could help?

[andrewseguin]

@relair Hey sorry for the delay, we've had to shift a lot of our work into Ivy-related issues.

Right now we're waiting on getting this PR in, which is based off your work and lets us decouple the MatSort and MatSortHeader

[Noppey]

There is no activity in the PR that is apparently a blocker for this one? Except some emoticon labelling...
If it's gonna be 2021 before this feature is released, that's obviously up to you and totally fine, but please let us know so we can take a different path for these kind of functionalities.

[andrewseguin]

@Noppey This is at a lower priority than getting Ivy out the door, but we still very much value this use case. We're currently considering a bigger change where we bring some of this logic into the CDK, which add a bit more of a delay since we need to make sure we design it right.

[FirstVertex]

there's no good reason for this to be stalled out.
been waiting nearly a year.
we were told to wait until next major release. that has come and gone.
please it is terrible community experience to wait this long for good functionality to be merged.

[BRM013]

Can we get some examples on how to use the 'matSortActive' and 'matSortDirection' inputs on the MatMultiSort directive please. I'm trying to define a default sort in a template and I can't seem to define the structure correctly and get errors from .isSortDirectionValid().

[himanshugarg574]

Hey guys, Great work with IVY. Now that it is released, can we get an ETA on this ? Thanks.

[andrewseguin]

This PR helped to kick off design work towards this feature. I'll close it now in favor of those new PRs to come. It's on our backlog but not a priority unfortunately. Thanks again for the work on this, it really helped us identify how we can decouple the sort and header so that they can be reusable. We really value the work and I'm sorry that we could not put more resources into it right now

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}