Datagrid design review: Sorting

[elycheea]

Design review

• Component epic #1607 #2499
• Links: PAL guidance / Storybook v2 / Storybook v1
  • Sorting (Carbon)

---

Standards

• [x] All pattern updates/changes/iterations have been discussed with the component developer
• [x] Patterns have been approved by either DSAG or another approving entity

Pattern and behavior

• [x] The component behaves according to the guidelines set by the pattern maintainer
• [x] The component’s UI matches the pattern specifications set by the pattern maintainer
• [x] The component’s motion matches the specifications set by the pattern maintainer(s)
• [x] The UI produced is responsive, cross-browser, and responds to the currently set Carbon theme.
• [x] Colors, themes, sizes and additional props are true and correct, aligning with Carbon set tokens (unless otherwise specified by the pattern)
• [x] Paddings/margins/spacings are true and correct, in both desktop and mobile views

Storybook

• [x] A functioning component renders in Storybook
• [x] The Storybook displays multiple scenarios that show how the component can be used
• [x] Changing props in the Storybook updates the component

Edited to reflect @Ashley-Bock’s review!

### Tasks
- [ ] https://github.com/carbon-design-system/ibm-products/issues/3227
- [ ] https://github.com/carbon-design-system/ibm-products/issues/3247

[elycheea]

@Ashley-Bock to help with this review 🙌

[elycheea]

@kristastarr Feel free to remove any checks that don’t apply.

---

Accessibility review

Keyboard

Level 1

• [x] Properly code the navigation order
  • [x] Ensure interactive elements are reachable with the keyboard
  • [x] Where possible, achieve the desired tab order by adjusting the DOM, rather than overriding the tabindex
  • [x] When overriding the default focus indicator, confirm focus is highly visible
• [x] Implement established keyboard conventions
  • [x] Use standard HTML elements where possible, using CSS to alter appearance not behavior
  • [x] Be familiar with established keyboard conventions
  • [x] Implement keyboard operation for custom elements

Level 2

• [ ] Ensure keyboard access to complex interactions - see comments about row splitters

Text and non-text

Level 1

• [ ] Implement correct structure and metadata - see comments about table description and aria-sort
  • [ ] Code headings according to specifications

Level 2

• [ ] Add additional structure where appropriate
  • [x] Add additional context to generic link names
  • [ ] Consider adding regions and other ARIA roles to distinguish text groupings

Level 3

• [x] Order content appropriately in the DOM

User input

N/A

[Ashley-Bock]

Standards

• [x] All pattern updates/changes/iterations have been discussed with the component developer No changes

• [x] Patterns have been approved by either DSAG or another approving entity

Pattern and behavior

• [x] The component behaves according to the guidelines set by the pattern maintainer

• [x] The component’s UI matches the pattern specifications set by the pattern maintainer

• [x] The component’s motion matches the specifications set by the pattern maintainer(s)

• [x] The UI produced is responsive, cross-browser, and responds to the currently set Carbon theme.

• [x] Colors, themes, sizes and additional props are true and correct, aligning with Carbon set tokens (unless otherwise specified by the pattern)

• [x] Paddings/margins/spacings are true and correct, in both desktop and mobile views

Storybook

• [x] A functioning component renders in Storybook

  • [x] The Storybook displays multiple scenarios that show how the component can be used

• [x] Changing props in the Storybook updates the component No probs to configure

[Ashley-Bock]

No issues between guidance and storybook. Only thing to consider, do designers have to always signify which column the table is sorted by default? In storybook, it doesn't appear that it shows which column has the default sorting and the guidance doesn't mention it either.

[elycheea]

@Ashley-Bock I believe the default sort is actually unsorted. It might be the same as the “row index” in Storybook. @matthewgallo Don’t see any mention in our docs if we support default sort? (Linked v1 since v2 docs are undergoing the Storybook upgrade.)

[kristastarr]

The keyboard interactions look good, with the exception of column resizing - see #4. There are a few screenreader issues when testing with voiceover setting on Mac.

1. Table description

   • Currently voiceover provides only a vague description of the table. Need to add aria-label, aria-describedbyor caption to provide accessible description of the table. Although this is something that the developer implementing this component will need to supply, so probably something we should mention in guidelines or possible add a prop for? See: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/table_role

2. Aria sort

   • Need to check markup of the table - should be using aria-sort so that the screenreader announces to the user that the table is sortable

Current:

Current:

Correct example from https://developer.mozilla.org/en-US/docs/Learn/HTML/Tables/Advanced:

3. Reading first row after clicking sort button

   • When clicking CNTRL+Option+space to click the sort button when using screenreader, the screenreader reads out the first table row, without any context. It shouldn't be reading any table content.

4. Row splitters:

   • Row splitters are not interactable via keyboard focus, but are interactable via screen reader when navigating using CNTRL+Option+ L/R/Up/Down arrows
   • When interacting via screenreader use, the row splitters aren't working properly - Was this worked on in a different PR?

Current: Current:

[kristastarr]

5. Table contents when navigating horizontally
   • When using CNTRL+Option+L/R or Up/Down arrows to navigate horizontally through the table with the screen reader, it reads the the contents of a cell as a group, saying the same content three times. To minimize the amount of times you need to arrow L and R, the content inside the cell shouldn't be inside a group

Current:

[kristastarr]

Text in the gray box indicates what the screenreader is reading out. Lmk if you want to talk through any of these issues if they are not clear by the descriptions 😄

[elycheea]

@kristastarr @matthewgallo The bit on row splitters — I think this is coming from the resizable columns feature. I think we should be able to re-add the resizing behavior so might be worth having you take a review at the behavior once we have that working again. Don’t think that one needs to be a part of the Sorting followup though.

Not sure if we have a good solution of the groups at the moment either ... I’m guessing it’s also part of a number of other extensions. Might need some ideas on how we approach this. How critical do you think this one is for our initial release? :thinking:

[kristastarr]

For the row splitters, yes I think it makes sense to fix it on the resizable columns feature since that impacts multiple other components.

For the groups - I don't think I delved into the code for this one, but can take a look - did you guys see what's causing them to be "grouped"?

[elycheea]

Opened a new issue for investigating why cells are being interpreted as groups so we can close this one!