Improve modal accessibility #1198
Conversation
**Why**: Using the tab key, focus could be moved outside of the modal into the background. This PR traps the focus in the modal and automatically focuses the first interactive element.
|
One more note, this fix applies to all modals using the |
|
|
||
| body_element = page.find('body') | ||
| body_element.send_keys [:shift, :tab] | ||
| expect(page.evaluate_script('document.activeElement.innerText')).to eq( |
There was a problem hiding this comment.
would it make sense to also test that closing the modal does not "trap" the focus? I think that is what you are setting up when you have this.trap[showing ? 'activate' : 'deactivate'](); so would be good to have test coverage for that, too!
There was a problem hiding this comment.
Ah, good point, will add.
There was a problem hiding this comment.
@jessieay added additional test to make sure focus is returned to the original element which is part of the deactivate function.
There was a problem hiding this comment.
Looks good. Once you add the test that @jessieay suggested, feel free to merge.
**Why**: To ensure that focus is returned to the original element and is no longer inside the modal
Why: Using the tab key, focus could be moved outside of the modal into the background. This PR traps the focus inside the modal and automatically focuses the first interactive element.