readium / readium-js-viewer

👁 ReadiumJS viewer: default web app for Readium.js library
BSD 3-Clause "New" or "Revised" License
553 stars 186 forks source link

Focus not trapped in several dialogs #533

Closed becka11y closed 8 years ago

becka11y commented 8 years ago

This issue is a Bug

Focus is still not trapped in some of the Readium dialogs: About Share Reader Bookmark

Related issue(s) and/or pull request(s)

146

Expected Behavior

When the user presses tab or shift-tab to change the item with focus in the dialog, focus should not leave the dialog. The focus should cycle through only items in the dialog while the dialog is open.

Observed behavior

With the About dialog open, pressing tab to move focus causes the focus to leave the dialog even though it is still open.

Steps to reproduce

Open the About dialog Press Tab to cycle through the items in the dialog. Once you reach the last sha-link, focus will move outside of the dialog. Tested on Safari Version 9.1.1 (11601.6.17) and focus goes up the to the bookmark list.

Test file(s)

Product

Have only tested in Safari to far but expect the same problem in all browsers.

danielweck commented 8 years ago

To fix, simply replicate this code:

https://github.com/readium/readium-js-viewer/commit/1a777550a295589436c6fcfafa14ccc16dff1340 ("add EPUB" dialog)

https://github.com/readium/readium-js-viewer/commit/1fae05c70d02e3c2d51bc43072da99a2a1dfb829 ("EPUB details" dialog)

becka11y commented 8 years ago

The About dialog doesn't have any buttons to catch the tab press. I can add an id to the last link on the dialog but the first link is in a message. I can add an id into all of the translations of that message - although that may be a bit fragile.
The other option I see is to find the first and last focusable items programmatically and add the tab /shift tab handlers. Do you have a preference?

It also isn't a simple fix for the share reader bookmark "dialog" because this is created programmatically via the dialog.js code. I'm thinking I will generate the id's for the dialog using the title and button type (close, dismiss, etc).

becka11y commented 8 years ago

I submitted a pull request (https://github.com/paypal/bootstrap-accessibility-plugin/pull/103) to fix this in the bootstrap accessibility plugin. We will see what happens....

danielweck commented 8 years ago

Well done Becky! Fixed by https://github.com/readium/readium-js-viewer/pull/553