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

[terra-folder-tree] Fix incorrect screenreader announcement of indexes #2106

Merged
merged 4 commits into from
Mar 28, 2024

Conversation

sycombs
Copy link
Contributor

@sycombs sycombs commented Mar 27, 2024

Summary

This PR fixes an issue where screenreaders would announce subfolder items with sibling parents as being part of the same group. This has been done by manually setting the values for aria-setsize and aria-posinset instead of relying on browser calculations.

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed
    Tested with Voiceover and JAWS.

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

This PR resolves:

UXPLATFORM-10283


Thank you for contributing to Terra.
@cerner/terra

@github-actions github-actions bot temporarily deployed to preview-pr-2104 March 27, 2024 17:56 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2104 March 27, 2024 18:12 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2106 March 27, 2024 18:20 Destroyed
@sycombs sycombs self-assigned this Mar 27, 2024
@sycombs sycombs requested a review from a team March 27, 2024 19:06
@sycombs sycombs marked this pull request as ready for review March 27, 2024 19:17
Copy link
Contributor

@adoroshk adoroshk left a comment

Choose a reason for hiding this comment

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

I verified that while walking up/down the folder tree, it announces the level and item index in subset correctly. Looks like expected behavior.

@mjpalazzo
Copy link
Contributor

mjpalazzo commented Mar 27, 2024

@sycombs - I did some testing with JAWS with the basic folder tree example and it sounds like the issue is fixed. The levels and item index are announced correctly. I will do more testing with other examples.

* @private
* The position of the item among its sibling items in the same group (subfolder).
*/
posInSet: PropTypes.number,
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean position in set? Can we rename this to positionInSet or itemPosition for clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was torn between naming it posInSet, ariaPosInSet, or positionInSet as you suggested since this is a direct passthrough to aria-posinset. I'm not sure which is most appropriate - maybe ariaPosInSet since it matches the aria attribute?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh if it's a direct passthrough, then yeah, ariaPosInSet sounds good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated both prop names: eccff30

@mjpalazzo
Copy link
Contributor

mjpalazzo commented Mar 27, 2024

@sycombs - here are some observations:
Folder Tree Selection example - The indexes are working correctly. Please regard my previous comment. I was in the wrong version.
Folder Tree Expand Collapse example - the level in the label and level JAWS announces do not match. The indexes start with 0 but the labels start with 1.
Folder Tree Expand Collapse All example - same comment.

@github-actions github-actions bot temporarily deployed to preview-pr-2106 March 28, 2024 17:31 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2106 March 28, 2024 18:33 Destroyed
@mjpalazzo
Copy link
Contributor

@sycombs - here are some observations: Folder Tree Selection example - The indexes are working correctly. Please regard my previous comment. I was in the wrong version. Folder Tree Expand Collapse example - the level in the label and level JAWS announces do not match. The indexes start with 0 but the labels start with 1. Folder Tree Expand Collapse All example - same comment.

Great work, @sycombs! Those examples are much better and the component is working perfectly! I approved.

@sycombs sycombs merged commit 6b56fb4 into main Mar 28, 2024
22 checks passed
@sycombs sycombs deleted the folder-tree_fix-index branch March 28, 2024 20:20
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