Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-table] Provide screenreader value for row selection column header #1892

Merged
merged 12 commits into from
Nov 17, 2023

Conversation

cm9361
Copy link
Contributor

@cm9361 cm9361 commented Nov 15, 2023

Summary

What was changed:
The Table component was updated such that a screenreader value was associated with the row selection column header. The component was also updated to properly read the header text and error indicator in the proper order. It now also properly reads the sort indicator and associated direction changes on all screenreaders.

Why it was changed:
The change was made to improve the accessibility of the Table component.

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

N/A

This PR resolves:

UXPLATFORM-9867
UXPLATFORM-9865
UXPLATFORM-9685


Thank you for contributing to Terra.
@cerner/terra

@cm9361 cm9361 marked this pull request as ready for review November 15, 2023 22:56
@mjpalazzo
Copy link
Contributor

@cm9361 I did the following validation with Edge/JAWS. I have a question for the row selection example:
Table Column States example - Pressed 'T' to navigate to table. JAWS announced, "Table with 6 columns and 3 rows, Table column 1 row 1, Patient, errors associated with column, and sorted ascending button." This is now working as required.
Row Selection example, Row selection mode enabled - Pressed 'T' to navigate to table. JAWS announced, "Table with 4 columns and 6 rows, Table with row selection, column 1 row 1, row selection button." JAWS now announces something for the checkbox column header, but do we want it to announce "button" or "row selection column header"?
Pressed Down Arrow to navigate the header cells until I reached the first check box cell. JAWS announced, "row 3 checkbox not checked." This is row 2. JAWS announced the wrong row number
Pressed Space to check the checkbox. JAWS announced Space, row 3, Fleck, Arthur, Fleck, Arthur selection, column 1 row 2 checkbox checked. To clear checkbox, press space bar. Row 2 selected. The problem with the incorrect row index is present. Is that being addressed in a different PR?

@cm9361
Copy link
Contributor Author

cm9361 commented Nov 17, 2023

@cm9361 I did the following validation with Edge/JAWS. I have a question for the row selection example: Table Column States example - Pressed 'T' to navigate to table. JAWS announced, "Table with 6 columns and 3 rows, Table column 1 row 1, Patient, errors associated with column, and sorted ascending button." This is now working as required. Row Selection example, Row selection mode enabled - Pressed 'T' to navigate to table. JAWS announced, "Table with 4 columns and 6 rows, Table with row selection, column 1 row 1, row selection button." JAWS now announces something for the checkbox column header, but do we want it to announce "button" or "row selection column header"? Pressed Down Arrow to navigate the header cells until I reached the first check box cell. JAWS announced, "row 3 checkbox not checked." This is row 2. JAWS announced the wrong row number Pressed Space to check the checkbox. JAWS announced Space, row 3, Fleck, Arthur, Fleck, Arthur selection, column 1 row 2 checkbox checked. To clear checkbox, press space bar. Row 2 selected. The problem with the incorrect row index is present. Is that being addressed in a different PR?

Thank you for pointing out these issues. There must have been a breaking change at some point. We would need to resolve the issue in another PR. Could you please log a JIRA?

Comment on lines 66 to 70
/**
* Boolean value indicating whether or not the header cell is focused.
*/
isDisplayVisible: PropTypes.bool,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the prop description correct? It seems like the value is used to show/hide the display text.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I made a mistake here. Ugh!

Copy link
Contributor

@sdadn sdadn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question about the prop description but code wise it looks good so approving it.

@github-actions github-actions bot temporarily deployed to preview-pr-1892 November 17, 2023 20:55 Destroyed
Copy link
Contributor

Fails
🚫 Please include a CHANGELOG entry for each changed package this PR. Looks like a CHANGELOG entry is missing for:
  • terra-table

Generated by 🚫 dangerJS against 6196bc9

@cm9361 cm9361 merged commit d5d0c1b into main Nov 17, 2023
21 checks passed
@cm9361 cm9361 deleted the UXPLATFORM-9867 branch November 17, 2023 21:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants