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 Window isn't fully accessible - WCAG 2.0 - 2.1.2 No Keyboard Trap #8855

Closed
knoobie opened this issue Mar 16, 2017 · 8 comments
Closed
Assignees
Labels
Milestone

Comments

@knoobie
Copy link
Contributor

knoobie commented Mar 16, 2017

The current implementation of all modal windows act as keyboard trap and don't comply to the WCAG 2.0 - 2.1.2 No Keyboard Trap

Vaadin Version:
All

Example:
http://demo.vaadin.com/sampler/#ui/structure/window

Steps to reproduce:

  • Open the Example URL
  • Press "Open a new Window"
  • Select the Property "Modal"
  • Window gets a darker background and pushed to center of the screen
  • Press Tab until you hit the bottom

Expected behaviour:

A Web application brings up a dialog box. At the bottom of the dialog are two buttons, Cancel and OK. When the dialog has been opened, focus is trapped within the dialog; tabbing from the last control in the dialog takes focus to the first control in the dialog. The dialog is dismissed by activating the Cancel button or the OK button.

Source: WCAG 2.0 - 2.1.2 No Keyboard Trap

Current behaviour:

  • The Keyboard Focus is trapped at the bottom and can only return with SHIFT+TAB in reverse order to the top side of the modal window.

Additional problem:

Focused min/max/close Icons don't get a focus indicator when using the keyboard to navigate. Mouse:hover works fine.

Moved additional problem to #8918

@pleku pleku added the bug label Mar 23, 2017
@torok-peter torok-peter self-assigned this Oct 20, 2017
@torok-peter
Copy link
Contributor

Teemu remembered that the focus trap causing this problem was implemented in Fw7 as a bugfix, to stop the focus from leaving the modal dialog. Issue imported to Fw repo:
#3960
Original PR review: https://dev.vaadin.com/review/#/c/1564/

@knoobie
Copy link
Contributor Author

knoobie commented Oct 23, 2017

The user should not be allowed to leave the modal windows, that's correct. Just the way how it was implemented is kinda broken.

@torok-peter
Copy link
Contributor

torok-peter commented Oct 31, 2017

The current fix is not perfect, but "good enough", in that it does circulate the focus to the top of the dialog when Tab was pressed on the last component of the dialog, and similarly for Shift+Tab. Known issues are:

  • The selector used by the fix may not find all valid focusable elements in the dialog. It works with most of the typical elements though, except links (because links don't have a tabindex). So if the last element in the dialog is a link (or potentially some other uncommon component), Shift+Tabbing from the top of the dialog will eventually get the user to the last but one element in the dialog instead. If this turns out to be an issue to a concrete user in real life, we will of course improve the selector.
  • As it turned out, the current Framework implementation includes some non-editable elements such as radio button groups and (probably) layouts when the user is moving around with Tab or Shift+Tab. This means that when Tab is pressed on the last component of the dialog, the focus gets onto the topmost editable element after 4 Tab presses instead of 3 as one would expect (first Tab moves the focus to the Resize/Restore button, 2nd to the Close button - although these are not highlighted properly as per Window Controls Icons don't get color indication when focused with keyboard #8918 -, 3rd would be the topmost editable element, but there is one in between, which is probably the enclosing layout div). Filed as Non-focusable elements focused during keyboard navigation #10279 .

@knoobie
Copy link
Contributor Author

knoobie commented Oct 31, 2017

Hey Peter, thank you for your work! I can investigate the framework behaviour after your pr gets approved and merged to see how it works for our sightless user. I think that's okay because we kinda knew the limitation of your second point.

Additional I had the following idea:

  • instead of holding a list of all focusable we could reuse the current behaviour of the framework
  • currently the framework traps you at the top and bottom - why not redirect the next tab from the bottom to the 'top trap' and the shift + tab from top to the 'bottom trap'

Example:

<window>
<div  id="window-gwt-xy-top" tabindex="0" aria-label="Top of the Window 'Caption'">

</content>

<div  id="window-gwt-xy-bottom" tabindex="0" aria-label="Bottom of the Window 'Caption'">
</window>

@tsuoanttila
Copy link
Contributor

Hi @knoobie, reliably catching the focus moving away from the trap is relatively difficult thing to catch. If you have an idea how to catch it and move the focus from one trap to the other, we'd love to see it in a PR and test it. Especially if it's less complex and easy to understand!

@Wnt
Copy link
Contributor

Wnt commented Nov 1, 2017

In our project we created a focusable element to the end of the Window which only purpose is to move the focus to the Window's first element when it's focus listener is called. To prevent the focus leaving the other way around (Shift + Tab) there is also another focusable element in beginning of the Window that moves the focus to the last component of the Window.

There are still special cases where this won't work, e.g. manually set tab indices to same number in the underlying UI and within the Window

@tsuoanttila
Copy link
Contributor

That's pretty much what @torok-peter did in his patch for the whole Window. When focusing the trap it moves to focus to the first/last element.

@knoobie
Copy link
Contributor Author

knoobie commented Nov 1, 2017

@Wnt @tsuoanttila - I had the same idea with my example above. I wanted to reused the current elements that traps the user/focus in the window - instead of blocking tabbing there, the tab should navigate the user to the other side of the window.

This of course adds two invisible focusable fields to the UI that are kinda annoying as well - but only at the start and end of the window (with aria-label of course) so it's okay I guess.

Focus == bottom && Key == TAB =>
document.getElementById('window-gwt-xy-top').focus();

Focus == top && Key == TAB && SHIFT =>
document.getElementById('window-gwt-xy-bottom').focus();

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

No branches or pull requests

5 participants