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

Switch alert show/hide to a prop #581

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Switch alert show/hide to a prop #581

merged 1 commit into from
Oct 30, 2024

Conversation

echappen
Copy link
Contributor

@echappen echappen commented Oct 29, 2024

Changes proposed in this pull request:

  • Switch Alert show-hiding to a prop (this ensures that the aria-live region in the Alert component is present on page load)

Related issues

Closes #531
Overrides #573

Submitter checklist

  • Added logging is not capturing sensitive data and is set to an appropriate level (DEBUG vs INFO etc)
  • Updated relevant documentation (README, ADRs, explainers, diagrams)

Security considerations

None, UI refactor only

@echappen echappen requested a review from a team as a code owner October 29, 2024 20:48
@echappen echappen changed the title Switch alert show-hiding to a prop Switch alert show/hide to a prop Oct 29, 2024
@beepdotgov
Copy link
Contributor

@echappen Just to check, should I be seeing messages/alerts in aria-live regions on the Manage Users page?

(If I should be testing somewhere else, just let me know.)

@echappen
Copy link
Contributor Author

echappen commented Oct 30, 2024

@beepdotgov Yep, Alerts are now wrapped in aria-live regions (which are now present on page-load). So whatever text is in the alert should be the text read by the SR. You can trigger success alerts by updating roles for any user on the Manage Users page.

This ensures that the aria-live region in the Alert component is present on page load.
@beepdotgov
Copy link
Contributor

Ach, sorry — must’ve had something cached. I see the role="status" container now!

Copy link
Contributor

@beepdotgov beepdotgov left a comment

Choose a reason for hiding this comment

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

Just want to say that I was able to update some permissions, and the success notification was successfully read out each time! Exciting to see this fixed! 🎉

@echappen
Copy link
Contributor Author

@beepdotgov Thank you for all your manual testing and collaboration on this!

@echappen echappen merged commit 380d4ff into main Oct 30, 2024
3 checks passed
@echappen echappen deleted the eoc/531-alerts branch October 30, 2024 18:00
@beepdotgov
Copy link
Contributor

beepdotgov commented Oct 30, 2024

Thank you for doing such thoughtful research on this, @echappen. I’ve learned a lot watching you work through this issue!

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

Successfully merging this pull request may close these issues.

When closing an overlay drawer, the success message isn’t announced [Safari/VoiceOver]
2 participants