Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Announce Snackbar appearance to screen reader #437

Merged
merged 6 commits into from
Aug 12, 2024

Conversation

TimRoe
Copy link
Contributor

@TimRoe TimRoe commented Aug 9, 2024

Description of Change

Recognized the issue with the screen reader not reading the announceForAccessibility was only a problem in iOS and it appeared to be an issue due to the synchronous nature of the storybook test case so iOS was immediately refocusing the button at near enough the same time to not do the announceForAccessibility.

Resolved by creating a useEffect on the Snackbar display that:

  1. Has an empty dependency array so the useEffect is only run once on initial render since otherwise the announceForAccessibility was being triggered multiple times (on appearance and disappearance)
  2. Has a 50ms delay so that if whatever is bringing up the Snackbar is synchronous (not generally how it's used, but could be) it still announces correctly

Testing Packages

Screenshots/Video

iOS:

RPReplay_Final1723481022.MP4

Android:

Screen_Recording_20240812-125152_Expo.Go.mp4

Flagship iOS (somewhat janky from app-level focus that they'll need to address during implementation):

RPReplay_Final1723483670.MP4

Flagship Android (also somewhat janky in a different way from app-level focus that they'll need to address during implementation):

Screen_Recording_20240812-133427_VA.mp4

Testing

Tested it performs as expected in Storybook. Also tested flagship which was a bit janky, but due to app-level setting focus.

  • Tested on iOS
  • Tested on Android
  • Tested on Web
    • N/A

PR Checklist

Code reviewer validation:

  • General
    • PR is linked to ticket(s)
    • PR has changelog label applied if it's to be included in the changelog
    • Acceptance criteria:
      • All satisfied or
      • Documented reason for not being performed or
      • Split to separate ticket and ticket is linked by relevant AC(s)
    • Above PR sections adequately filled out
    • If any breaking changes, in accordance with the pre-1.0.0 versioning guidelines: a CU ticket has been created for the VA Mobile App detailing necessary adjustments with the package version that will be published by this ticket
  • Code
    • Tests are included if appropriate (or split to separate ticket)
    • New functions have proper TSDoc annotations

Publish

While changes do warrant a new version per the versioning guidelines, postponed for later following functional changes to Snackbar.

Base automatically changed from feature/415-narin-snackbar-hook-offset to main August 12, 2024 17:01
@TimRoe TimRoe marked this pull request as ready for review August 12, 2024 17:39
@TimRoe TimRoe requested a review from a team as a code owner August 12, 2024 17:39
Copy link
Contributor

@narin narin left a comment

Choose a reason for hiding this comment

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

Looks good. Did you identify how the app would fix the jankiness? If so we should make note of it in the implementation ticket.

@TimRoe
Copy link
Contributor Author

TimRoe commented Aug 12, 2024

Did you identify how the app would fix the jankiness?

I did not.

It may involve changes on our end. There is a slightly different function, announceForAccessibilityWithOptions, which can make iOS queue up the announcement behind other screen reader behavior similar to how Android behaves. There's also settings for Android to be assertive and interrupt, but not sure how easily we can apply that to announceForAccessibility. There's enough ambiguity that I think we can leave it to flagship implementing and work with them on how best to sort it out then.

I have added a comment to the handoff ticket so they are on the lookout for it at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants