feat: add multi-column sort to Table#4713
Conversation
Add multi-column sorting to Table, activated by shift-clicking sortable column headers or via a per-column sort menu. - Per-column sort menu: set ascending/descending direction (shown as a checked state), add to / remove from the multi-column sort, with disabled-reason tooltips on unavailable actions. - Priority badges on sorted column headers and a Clear sort control. - A polite live region announcing the current sort order (and when sorting is cleared), reading localized header text from the DOM and joining columns with locale-appropriate separators. - Focus management so clearing the sort keeps keyboard focus on a sortable column header instead of dropping to the document body. - Styling fixes so the sort icon and priority badge do not overlap truncated header text in narrow columns. - i18n strings for all new labels and screen reader announcements, plus test-utils and unit tests covering the new behavior.
Adds translations for the multi-column sort strings across supported locales.
…electors snapshot
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4713 +/- ##
==========================================
+ Coverage 97.59% 97.61% +0.01%
==========================================
Files 950 955 +5
Lines 30642 30926 +284
Branches 11229 11368 +139
==========================================
+ Hits 29906 30189 +283
- Misses 689 730 +41
+ Partials 47 7 -40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ection + priority strings Renames i18nStrings.sortDropdown.* to i18nStrings.sortDropdownX and replaces the single i18nStrings.liveAnnouncementSortColumn ICU string with ariaLabels.sortAscending/ sortDescending + ariaLabels.sortPriority, composed by ariaLabels.liveAnnouncementSortOrder. Matches the restructured AWS-UI-Components-I18n source (merged in CR-289393273). Includes best-guess translations for 12 locales for the renamed/new keys; real Totoro translations to follow.
| >(); | ||
|
|
||
| // `multiSort` defaults to on; the rest default to off. | ||
| const multiSortEnabled = urlParams.multiSort !== 'false' && urlParams.multiSort !== false; |
There was a problem hiding this comment.
This chunk repeats the same type of conversion 6 times and we even already have it here:
components/pages/app/app-context.tsx
Lines 83 to 85 in 82568c5
Can we deduplicate this?
There was a problem hiding this comment.
removed, wasn't needed
| <PermutationsPage title="Table multi-column sort permutations" i18n={{}}> | ||
| <PermutationsView | ||
| permutations={permutations} | ||
| render={permutation => ( |
There was a problem hiding this comment.
Minor: render takes the array index as second argument, so this could be
render={(permutation, index) => (
<Table
{...permutation}
ariaLabels={{ ...ariaLabels, tableLabel: `Items ${index}` }}
/>
)}
Also, is this ARIA label very meaningful?
|
|
||
| // Removes the trigger's vertical padding and borders so an `icon`-variant trigger matches the | ||
| // compact `inline-icon` footprint. The class is doubled to override the button variant styles | ||
| // (`.button.variant-icon`), which button-dropdown CSS is loaded after. |
There was a problem hiding this comment.
Are we 100% sure that this stylesheet is always loaded after the button styles in all cases (what if for example components are loaded asynchronously), and that selector specificity is not going to break this?
What about delegating this class to the button component? There we can even do something like this to make sure we don't mix both styles:
.variant-icon:not(.compact), (...) { ... }
There was a problem hiding this comment.
good catch, moved to button styling
| const { container } = render( | ||
| <Table | ||
| items={items} | ||
| columnDefinitions={extraColumns ?? columnDefinitions} |
There was a problem hiding this comment.
Nit: "extra" suggests that they will be added rather than replace the default columnDefinitions. What about renaming extraColumns as columnDefinitions and columnDefinitions as defaultColumnDefinitions?
| margin-inline-start: auto; | ||
| } | ||
|
|
||
| &-clear-sort { |
There was a problem hiding this comment.
The slot only ever contains one element (the clear sort button), so I don't think these flex styling is needed
| } | ||
| return ( | ||
| <InternalLiveRegion hidden={true} tagName="span"> | ||
| <span>{announcement}</span> |
| } | ||
|
|
||
| // The multi-sort priority badge renders alongside the sort arrow inside the | ||
| // absolutely-positioned sort icon. Reserve extra inline-end space for it so it |
There was a problem hiding this comment.
I'm OK with not including this in this scope but it would be nice if the sort icon (and the newly added priority badge) is not rendered with absolute position
| .header-cell-main-with-menu { | ||
| display: flex; | ||
| align-items: center; | ||
| // stylelint-disable-next-line no-descending-specificity |
There was a problem hiding this comment.
Can we avoid overriding our own linting rules, e.g, by adding classnames to the inner element (.header-cell-content in this case) instead of relying on the DOM structure? I think this is what we do elsewhere
| } | ||
|
|
||
| .header-cell-main { | ||
| /* When the wrapper is present without a sort menu, behave as if it weren't there. */ |
There was a problem hiding this comment.
How does this selector alone that there is no sort menu? It is added unconditionally: https://github.com/cloudscape-design/components/pull/4713/changes#diff-f948a1d80d749dbe50dd7f1835c64a59e6e5c18ca7b8e380a2a559b65cbb4479R246
| display: contents; | ||
| } | ||
|
|
||
| .sort-priority-badge { |
There was a problem hiding this comment.
Flex-related styles are not needed if there is only one child. Same for the ruleset below
| ]; | ||
|
|
||
| return ( | ||
| <span className={styles['sort-menu']} onClick={e => e.stopPropagation()}> |
There was a problem hiding this comment.
Can the stopPropagation be handled by the button dropdown instead? Is this wrapper necessary?
|
Can we test the feature alongside |
2 participants
Description
Adds opt-in multi-column sorting for
TablethroughmultiColumnSort.Customers can sort by multiple columns using Shift+click or Shift+keyboard activation, manage each column through a sort-options menu, see numbered sort priorities, and clear the active sort. The change also adds screen-reader announcements, localized strings, public test-utils support, generated API documentation, and demo/permutation pages.
The public API is additive and opt-in. Existing single-column sorting behavior is unchanged.
Related links, issue #: n/a
How has this been tested?
Added new unit tests
Review checklist
Correctness
multiColumnSortis optional and single-column sorting is unchanged.Security
Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.