feat(table): add multi-sort columns to table

[kevinsperrine]

Closes #1376

Summary

Adds the ability to multi-sort columns within a table. A new option.hasMultiSort prop is added. The view.table.sort prop now accepts both a single sort objects { columnId: string; direction: 'ASC' | 'DESC' | 'NONE' } or an array of sort objects. The order of this array determines the order the multi-sort is applied to the table. There are new callback actions for interacting with the multi-sort column data: onSaveMultiSortColumns, onCancelMultiSortColumns, onAddMultiSortColumn, and onRemoveMultiSortColumn.

Change List (commits, features, bugs, etc)

• added TableMultiSortModal component
• updated snapshots
• add documentation
• cleaned up Table stories by adding knobs and removing duplicates
• renamed stories for ListBuilder/MenuButton to add number prefix, so they appear in correct location

Acceptance Test (how to verify the PR)

• The StatefulTable with multi-sort story should allow selecting and sorting by different columns
• The Table with multi-sort story should show the multi-sort modal and callbacks fire in the action panel when clicking save/cancel or add/remove buttons.

Regression Test (how to make sure this PR doesn't break old functionality)

• Existing Table stories using single-column sorting should function as previously expected

[netlify]

✔️ Deploy Preview for ai-apps-pal-angular ready!

🔨 Explore the source changes: e383c54

🔍 Inspect the deploy log: https://app.netlify.com/sites/ai-apps-pal-angular/deploys/60ca03e0aa98ed0008d45666

😎 Browse the preview: https://deploy-preview-2336--ai-apps-pal-angular.netlify.app

[netlify]

✔️ Deploy Preview for carbon-addons-iot-react ready!

🔨 Explore the source changes: e383c54

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-addons-iot-react/deploys/60ca03e0c2b7290007ff12ee

😎 Browse the preview: https://deploy-preview-2336--carbon-addons-iot-react.netlify.app/iframe

[kevinsperrine]

I think it's worth discussing a breaking change in V3 here, so that the customSort function is just a comparator instead of a method that takes the data, column, and direction. It would be much easier (and more efficient) to handle sorting related issues without having to maintain this backwards compatibility. Unless this method was chosen for a specific reason that I'm unaware of.

[bjornalm]

Don't know, here is the old PR for that so you can have a look
#843

[davidicus]

I think it was a functions so you can sort things that are not comparable like an error icon.

[kevinsperrine]

🤔 Wouldn't that still be possible with a comparator? They can use the renderDataFunction to render the icon, and then sortFunction could still just be a comparator as used here. It seems like the only thing the full function is accomplishing to me is cloning to not mutate the original array.

[stuckless]

The choice to use an overflow that pops up a dialog seems like an odd behaviour (but, I see that is coming straight from PAL, so I guess that is good). At the very least, IF I have to select the triple dot menu and select multi-sort, then it should open the dialog and configure the entry such that it selects the column that opened the dialog, so that all I need to do is press +. If existing sorts exist, and I click the triple dot on a column that is not already sorted then it should add a new row and set that selected column in the dropdown.

I see a couple of behavioural issues that PAL design does not address.
1). It would seem that once I create a sort, then clicking a table column will remove ALL sorts. This does not feel right. I would expect that clicking a table column that is sorted would just invert the sort of that table column, and not remove all the sorts from the table. Perhaps if I clicked a table column that is NOT sorted it might remove all other sorts, although I expect that customers would likely expect that action to just add another sorted column automatically.

2). The modal should allow the sorted columns to be reordered.

3). The modal should have a clear/remove all option, no?

@lukefirth do you have any comments on the behaviours here?

waiting on comment

[davidicus]

@dianatran18 do you have any feedback for @stuckless comments above?

[stuckless]

Modal in RTL is not correct. X is in the wrong location and the input components are not aligned correctly.

If I add more than 4 items it blows out of the dialog.

On a small screen if I open the dropdown in the dialog I can't select all the items, and keyboard navigation appears to now work either. Not sure if it's a dropdown issue or a dialog issue.

[stuckless]

do we really need a new 28k library to just sort data?

[kevinsperrine]

It's only 28K unpacked. It's a 2K module, and yeah, it simplifies the experience significantly when sorting in various directions and orders.

[stuckless]

I would be very conservative about adding new libraries. I'm not saying, don't, but you should verify that this library is "blessed". ie, it in the IBM open source list of approved libraries (you can check the wicked system). When you add a new library, it has to get legal approval eventually. That is why I question importing a library to do a sorting, when that seems pretty trivial.

[davidicus]

our OM is looking into what is happening with our code scans we are supposed to have for teams to use. We will hopefully have an answer soon on when we can expect them.

[dianasanborn]

I can provide some feedback, but just a note that the pattern defined here was from the original EDS/IoT PAL, so I wasn't involved in the original pattern/nor have I thoroughly looked into the multi-sort pattern myself.

1). It would seem that once I create a sort, then clicking a table column will remove ALL sorts. This does not feel right. I would expect that clicking a table column that is sorted would just invert the sort of that table column, and not remove all the sorts from the table. Perhaps if I clicked a table column that is NOT sorted it might remove all other sorts, although I expect that customers would likely expect that action to just add another sorted column automatically.

I briefly spoke with the the PAL team on this, and I think it's fine that if you click to sort on an already sorted column, it would invert the sort, and not remove all sorts from the table. And agreed that if you clicked on a column that is not already sorted, then it would remove all other sorts.

2). The modal should allow the sorted columns to be reordered.

3). The modal should have a clear/remove all option, no?

Again I'm not deeply familiar with the pattern defined, but these statements make sense to me.

I'm fine moving forward with this so we can close this out, but I would ask that the teams that will be implementing this in their products to get feedback on their users on what the expected behavior is for everything listed above.

[kevinsperrine]

@stuckless @dianatran18

Modal in RTL is not correct. X is in the wrong location and the input components are not aligned correctly.

I've fixed the alignment, but the X we will tackle with another issue, as that's in the ComposedModal component not directly related to this feature. #2396

If I add more than 4 items it blows out of the dialog.

This has been addressed. We had to resort to using Select components instead of Dropdown because there's currently no good solution for handling the overflow of dropdown menus within a modal. There are multiple issues in Carbon. See carbon-design-system/carbon#6288 and carbon-design-system/carbon#7565 (comment).

On a small screen if I open the dropdown in the dialog I can't select all the items, and keyboard navigation appears to now work either. Not sure if it's a dropdown issue or a dialog issue.

Keyboard navigation was working, but the modal wasn't grabbing focus, so you had to tab through all the items on the table in the background to get to the modal. The save button now grabs focus when the modal opens, so keyboard navigation is easier.

I also added testIds to the select menus and to the modal itself. Clicking on the sort button (for a column that's selected for multi-sort) will now invert the sort for that column, but clicking sort on a new column not included in multi-sort will reset the sort and only use that single column. The modal window now also has a "Clear sorting" button to wipe it all out.

[davidicus]

Hey @stuckless. Since design is going to revisit this pattern and make some further recommendations we have tried to make as many of your suggested changes as possible and for the ones we did not address I have created an issue for design to look into and verify.

If existing sorts exist, and I click the triple dot on a column that is not already sorted then it should add a new row and set that selected column in the dropdown.

Added to issue for design discussion

1). It would seem that once I create a sort, then clicking a table column will remove ALL sorts. This does not feel right. I would expect that clicking a table column that is sorted would just invert the sort of that table column, and not remove all the sorts from the table.

Kevin has addressed in this PR by having it sort in place if part of multi sort. For now non members of multi-sort will clear sort and add the simple sort.

Perhaps if I clicked a table column that is NOT sorted it might remove all other sorts, although I expect that customers would likely expect that action to just add another sorted column automatically.

Added to issue for design discussion

2). The modal should allow the sorted columns to be reordered.

This seems like a no brainer but is not part of the initial spec so we have opened another issue to track this enhancement.

3). The modal should have a clear/remove all option, no?

This was added to this PR

I believe these actions and future work of issue mentioned will address most of your comments. Can you give this another look? Thanks and thanks for the feature comments.

[kevinsperrine]

@stuckless Friendly bump on the updates to this PR. Can you give it another look now with @davidicus's #2336 (comment) in mind?