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

Modal should be contained in a landmark #61

Closed
mjbp opened this issue Jan 11, 2023 · 5 comments
Closed

Modal should be contained in a landmark #61

mjbp opened this issue Jan 11, 2023 · 5 comments
Assignees
Labels
accessibility An accessibility based enhancement or bug

Comments

@mjbp
Copy link
Collaborator

mjbp commented Jan 11, 2023

The modal component used for modal confirmation and modal search patterns moves the dialog markup in the source (this is necessary to prevent tab/swiping out of the modal), this means that the dialogs are no longer contained in a landmark.

I suggest we

  • add an appropriate landmark to the element containing the dialog (search for search modal, region for confirmation)
  • add line to ACs requiring a containing landmark
@mjbp mjbp added the accessibility An accessibility based enhancement or bug label Jan 11, 2023
@sarah-storm sarah-storm self-assigned this Jan 12, 2023
@mjbp mjbp assigned susannah-rogers and unassigned sarah-storm Jan 13, 2023
@mjbp
Copy link
Collaborator Author

mjbp commented Jan 13, 2023

@susannah-rogers This is now on Netlify ready for testing.

@susannah-rogers
Copy link

@mjbp I can see the landmark has been added, but NVDA isn't picking it up...
nvda-landmarks

@mjbp
Copy link
Collaborator Author

mjbp commented Jan 26, 2023

@susannah-rogers @sarah-storm updated this one to add an aria-labelledby to the landmark which helps NVDA to identify it.

Sarah noted:
"Added an 'aria-labelledby' to the region landmarks so that NVDA will pick them up. I've re-used the label from the dialog, I'm not sure if the repetition of this is OK here - or maybe we need to think of other instructional wording. It would be good to test if it's read twice and see what Susannah thinks. It seemed to be for me that it was only read once as the regions are only dealt with one at a time by NVDA, but I'm aware I'm really not a typical user!"

On Netlify for you to test at your convenience.

@mjbp mjbp assigned susannah-rogers and unassigned sarah-storm Jan 26, 2023
@susannah-rogers
Copy link

@sarah-storm The landmark on https://storm-ui-patterns.netlify.app/example/modal-confirmation/ is now showing as 'Are you sure?' in NVDA rather than 'region'. This means that it reads 'are you sure' twice - full text below:
'Are you sure. Dialog: This will permanently remove this item. Delete button. Are you sure? Button delete, button cancel.'

@sarah-storm
Copy link
Contributor

Closing this ticket as complete. Looking at the W3c example of role="dialog" they are also re-using a header as the aria-labelledby marker, therefore I think this is acceptable to do.

https://www.w3.org/WAI/GL/wiki/Using_the_region_role_to_identify_a_region_of_the_page

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility An accessibility based enhancement or bug
Projects
None yet
Development

No branches or pull requests

3 participants