Modals are focused on opening, for improved acessibility #690
Conversation
|
You can test this, checkout how I'm doing so with the Dropdown work I'm doing on the dropdown-revisited branch tests. |
|
This is a great start for accessibility of the modal! Have you considered ensuring that keyboard navigation via tabbing does not allow you to tab to the components under the modal? This is another concern to deal with for Modal accessibility. I understand if that's more than you'd like to tackle in this PR, but if you're up to it we can wait till that's also available. (Note: http://www.w3.org/WAI/PF/aria-practices/#modal_dialog) |
|
Thanks @mtscout6 for pointing out the tests, I'll start by adding the test case. And I'm happy to add the extra for not allowing to focus on the components under de modal, which makes sense. I'll try to tackle these today. |
|
You rock, thanks! |
|
added tests. Will still develop the feature to only focus on the components under the modal, today I hope |
|
The test looks great! Can you squash the commits and use the commit message standard outlined in the Contributing Guide? That way this change will be documented in the automated changelog when we push a release. |
|
sure, would this be considered a fix ([fixed])? On Sat, May 23, 2015 at 3:07 PM, Matt Smith notifications@github.com
|
Adds test case for focusing on the modal when opening it
|
Perfect! Everything here LGTM, you can do the additional work to keep focus within the modal on another PR. I opened issue #723 which defines that separate problem. Need another review by one of the @react-bootstrap/collaborators |
|
👍 |
Modals are focused on opening, for improved acessibility
| }, | ||
|
|
||
| focusModalContent () { | ||
| this.lastFocus = document.activeElement; |
There was a problem hiding this comment.
Is it connected with iframes ?
I'm just not aware of use cases for this.
(have no such expirience)
There was a problem hiding this comment.
As far as I can tell yes it is in relation to iFrames. I wasn't even really aware of that property and why to use it till I read up on it this weekend. So thanks @taion for steering us in a good direction.
There was a problem hiding this comment.
Thank you.
It is important for me to know about it.
Also I'll be able to do more comprehensive code reviews with regard to iFrames.
There was a problem hiding this comment.
iframes aren't that high on my list of concerns, I was just following the other code.
Modals are now focused when opening them, as requested by #612 . I've also stored the element that was previously focused, so its focused can be restored when the Modal is closed (it is the same behaviour Bootstrap does).
I was unable to do tests for this, as it has a dependency on document.activeElement (and the tests I've looked into only used TestUtilities.renderIntoDocument, which renders to a detached DOM node, so I'm guessing we can't test this without attaching it to the DOM).