Amphtml: [amp-lightbox] focus outside should close the lightbox

[catherinehoule]

Describe the new feature or change to an existing feature you'd like to see

When I use a Screen reader, since the focus is not trapped in the lightbox I can tab and continue my reading in the page but the lightbox stays open. We known it's possible to close with esc but will the user know/do it ? I think a focus outside should be considered as a click outside... so closing it.

Additional irritant

We had to add a close event that focus on the trigger that opened the lightbox so if we close the lightbox later in the read, then the focus go back to this button and I loose where I am at.

When 2 buttons open the same lightbox, I didn't find a way to go back to the good trigger so I had to duplicate the lightbox and that shouldn't be required

[caroqliu]

Thanks for filing this issue! This is definitely a worthwhile feature for amp-lightbox. I'm tagging it as a good first issue for now and happy to work with whoever wants to take it on.

[catherinehoule]

@nainar I filled this 2 months ago. As you wrote this article yesterday, would be nice to continue the work on accessibility here ;)

[catherinehoule]

@caroqliu I'd like to help on that. My work could free me some time to focus on helping accessibility in amp. Let me know if we can collaborate!

[caroqliu]

Sounds great, thanks for volunteering your time to contribute! For this change I'd recommend:

1. Add an event listener for blur, similar to the keydown listener added in the open_ method. The callback for this event listener should call close_ with trust level of ActionTrust.HIGH. Optionally specify the once option on the listener since
2. Test manually with a sample in /examples/amp-lightbox.amp.html. We also want to confirm that once a user "blurs" away from the amp-lightbox they can't "focus" back on the element without first triggering an "open" action on it.
3. Add unit tests for:
   
   
   
   • If a user "blurs" away from the amp-lightbox subtree, it should close.
   
   
   • If a user "blurs" by focusing on an element that is in the amp-lightbox subtree, it should stay open.
   
   
4. Send for review. :)

[caroqliu]

Discussed further offline with the following new finding:

It is not so simple to take away focus from the lightbox because the focus may not be in the subtree upon open in the first place. Some publishers will use [autofocus] to focus on a lightbox sub element when the open event occurs, as discussed in #13332. In all other cases focus stays on the element that triggered the lightbox to open, even after activation. To that effect we may want to detect and place focus with the following pattern wrt accessibility best practices:

1. If a publisher uses autofocus or the AMP focus action on lightbox open, respect the focus that was manually placed.
2. If a publisher does not specify an item to be focused on lightbox open, but there is a focusable element in the lightbox subtree, try to detect an existing close button, similar to amp-sidebar's implementation. If one is found, focus this button.
3. If a publisher does not specify an item to be focused on ligihtbox open, and there is no focusable content in the lightbox subtree, create a close button explicitly for the SR and focus that, similar to amp-sidebar's implementation.

We also want the following focus management behavior for closing the lightbox modal: If a user explicitly closes the lightbox by activating an element with the close AMP action, focus is restored to where it was before the lightbox opened, likely on the original element which activated it to open. It's not clear yet if the lightbox does it currently, so further manual testing is required to assess if there is more work to be done here. Again, amp-sidebar sets precedence for this as well.

As a result we may split this up into two PRs to address the following:

• Detecting and/or placing focus on lightbox open
• Closing the lightbox on blur