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

[terra-dropdown]Added aria-haspoup and aria-controls attribute #3936

Merged
merged 32 commits into from
Oct 31, 2023

Conversation

AH106586Harika
Copy link
Contributor

@AH106586Harika AH106586Harika commented Oct 13, 2023

Summary

fixed- Screenreader not communicating menu

What was changed:
Added aria-haspoup and aria-controls attribute.

Why it was changed:
The screen reader is not announcing that it has a menu when the drop down button is navigation

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

This PR resolves:

UXPLATFORM-9694


Thank you for contributing to Terra.
@cerner/terra

@AH106586Harika AH106586Harika requested a review from a team as a code owner October 13, 2023 12:09
@sdadn
Copy link
Contributor

sdadn commented Oct 13, 2023

Is this PR ready for review?

@AH106586Harika
Copy link
Contributor Author

Is this PR ready for review?

Yes.

Copy link
Contributor

@supreethmr supreethmr left a comment

Choose a reason for hiding this comment

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

Please get approval from A11y Team before PR merge

Comment on lines +220 to +221
aria-haspopup="true"
aria-controls={dropDownMenuListId}
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to add aria-controls for split button as well :

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented.

Comment on lines +13 to +18
beforeAll(() => {
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue('00000000-0000-0000-0000-000000000000');
});

afterAll(() => {
mockSpyUuid.mockRestore();
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add similar spy for splitbutton test as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@rahulkumar8cs
Copy link

+1

@supreethmr supreethmr added ⭐ Accessibility Reviewed Accessibility has been reviewed and approved. and removed Accessibility Review Required labels Oct 30, 2023
@github-actions github-actions bot temporarily deployed to preview-pr-3936 October 31, 2023 05:05 Destroyed
@sugan2416 sugan2416 merged commit 376977c into main Oct 31, 2023
22 checks passed
@sugan2416 sugan2416 deleted the dropdown-aria-property branch October 31, 2023 07:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📦 terra-dropdown-button ⭐ Accessibility Reviewed Accessibility has been reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants