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

[MCR-3625] MCCRS Record Number page a11y fixes #2053

Merged
merged 4 commits into from
Nov 16, 2023

Conversation

pearl-truss
Copy link
Contributor

Summary

This PR resolves 2 A11y issues in the MCCRS record number flow

Bug 1

  • Located on the submission summary page, after a MCCRS record number has been added for a submission
  • If you click on the MCCRS record number (that is a link to the mccrs system) you should now hear a clear label for this link instead of silence
  • If you click on the "Edit MCCRS record number" link you should now hear a clear label for this link instead of silence

Bug 2

  • Located on the Add MCCRS record number page
  • Tabbing through the breadcrumbs, the middle breadcrumb for the submission page should no longer be read out as current page
  • Added in this PR is the ability to tab to the third breadcrumb and hear that the mccrs record number page is the current page

Related issues

https://qmacbis.atlassian.net/browse/MCR-3625

Screenshots

Bug 1

Screen.Recording.2023-11-13.at.2.36.46.PM.mov

Bug 2

Screen.Recording.2023-11-14.at.10.43.26.AM.mov

@@ -15,7 +15,7 @@ const Crumb = (crumb: BreadcrumbItem) => {
if (link) {
return (
<Breadcrumb>
<Link asCustom={NavLink} to={link}>
<Link asCustom={NavLink} to={link} end>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This resolves an issue with react router dom where the submission page was being read out as the current page due to the url on that page being contained in the submissions/:submissionID/mccrs-record-number url which resulted in "current page" being read out for both breadcrumbs

{ text: 'MC-CRS record number' },
{
text: 'MC-CRS record number',
link: `/submissions/${id}/mccrs-record-number`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ticket called out that "I need the breadcrumb title to clearly call out the page I am on, for example “MC-CRS record number - Current Page”, so I easily understand where I am on the page.", so in order for this to be tab-able and read out by JAWS I turned it into a link. If there's a better solution please let me know

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. Let's go ahead and make link required then on the Breadcrumbs props itself. Sounds like there is an a11y impact when we don't ensure every item as a link. That way this doesn't reoccur

@@ -179,6 +180,11 @@ export const SubmissionSummary = (): React.ReactElement => {
className={
pkg.mccrsID ? styles.editLink : ''
}
aria-label={
pkg.mccrsID
Copy link
Contributor

@haworku haworku Nov 14, 2023

Choose a reason for hiding this comment

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

nit: I try to use a shared variable when I'm trying to align display text with aria-label text so that these cannot accidentally diverge

@pearl-truss pearl-truss merged commit ca8438a into main Nov 16, 2023
27 checks passed
@pearl-truss pearl-truss deleted the mcr-3625-mccrs-a11y-fixes branch November 16, 2023 14:24
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.

2 participants