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.

Sign up for GitHub

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

Jump to bottom

Accessibility issues with modal dialog #229

Open
roelvangils opened this issue Nov 22, 2020 · 1 comment
Open

Accessibility issues with modal dialog #229

roelvangils opened this issue Nov 22, 2020 · 1 comment
Labels
accessibility Accessibility can be improved in treatment question Further information is requested

Comments

@roelvangils
Copy link

roelvangils commented Nov 22, 2020 •
edited
Loading

The Modal is not built with accessibility in mind. Some refactoring is required.

Erik and Roel reported back these issues (plus some recommendations).

SC 1.1.1 Non-text Content

  • Issue: The close button of the modal is an icon font. See Fontawesome.

SC 1.3.2 Meaningful Sequence

  • Issue: The heading is visually at the same height as the close button but the button gets focused first.
    Fix: Is the heading always the name of the dialog? Either place the heading before the button visually and in code, or lower the heading a bit so the visual order is correct.

SC 3.1.2 Language of Parts

  • Issue: The language of each bit of text should be determinable.
    Fix: Can users set the language of (parts of) a modal?

Recommendations

  • The modal dialog does not follow the ARIA Authoring Practices example. This causes multiple issues. The dialog is not truly Modal. The content in the background can still be navigated with keyboard, the content in the background can still be selected and the content in the background can still be accessed with a screen reader (including things like heading lists). The page can be scrolled. Following the Authoring Practices should fix some of these issues. Putting all modals as direct children of, and at the end of, the <body> can be a good start. This makes it easy to put something like aria-hidden on the rest of the DOM.
  • The keyboard focus is not placed in the modal. My advice would be to put the focus on the entire modal. This makes sure the title is announced by a screen reader and all attributes are found right away. The content of the modal can be placed in aria-describedby.
  • The modal is in its own context, yet the heading is an <h4>. Under what heading does this one fall in the hierarchy? I would suggest using an <h1>.
  • Role="dialog" can set a screen reader in so-called application mode. Put a role="document" on a wrapper of the content to prevent this.
  • In the demo, all buttons do the same thing.
  • Escape does not close the modal. This can be expected by the user.
  • There's a lot of discussions (always) about buttons and if they should get cursor: pointer in CSS. The theory is that buttons should have enough affordance (we know we can click them) of their own, and pointers are for links. If you do put them on buttons, start thinking about (the design of) other components as well. Labels are also clickable, but don't always a get pointer.
@MichaelCastiau
Copy link
Contributor

@TriangleJuice As mentioned before, the Modal component codebase itself combined with the issues listed above would entice a complete rewrite of the modal component, (possibly) introducing breaking changes. However, rewriting this component should be rather easy, since functionality is very limited.

@MichaelCastiau MichaelCastiau added accessibility Accessibility can be improved in treatment question Further information is requested labels Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accessibility Accessibility can be improved in treatment question Further information is requested
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@roelvangils @MichaelCastiau