Managing focus

[acnorrisuk]

Looking at the demos it looks like there is some focus management missing on the modals. Here's what needs to be added to fix it:

• When a modal is open, focus should be sent to the modal (either on the element itself via .focus() and tabindex="-1" or sent to the first interactive element.

• When a modal is open, focus should be trapped inside the modal (so you can't access elements behind it when tabbing with the keyboard).

• When a modal is open, pressing the escape key should close the modal.

• It would be nice if the modals had an explicit close button with an accessible name (e.g. <button aria-label="close">X</button>.

• When a modal is closed, focus should be send back to the element which opened it.

An example of an accessible modal:
http://edenspiekermann.github.io/a11y-dialog/example/

The WAI-ARIA authoring practices guide on modals:
https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html

[electerious]

Thanks for the suggestions! :) I will see what I can do while keeping the module simple.

[adam-jones-net]

Another vote for adding support for pressing the ESC key to close, that's kind of a convention with many lightbox/popups

[electerious]

@Arkid This should do the job for now:

document.body.addEventListener('keypress', (e) => {
  if (e.key === "Escape") instance.close()
})

[adam-jones-net]

@Arkid This should do the job for now:

document.body.addEventListener('keypress', (e) => {
  if (e.key === "Escape") instance.close()
})

Thanks for this yes I was going to implement that at some point too. Although not necessary, for the record, anyone wanting to do via jQuery could do:

$(document).keyup(function(e) {
    if (e.keyCode == 27 && instance) {
        instance.close();
    }
})

[yliharma]

I think there may also be a bug on focusing on input elements inside the lightbox: if in the onShow callback I add something like instance.element().querySelector("input#myinput").focus();
it doesn't focus on the element.

[JiveDig]

@yliharma I'm struggling to force focus on an element as well. Using very similar code. Did you get this figured out?

[JiveDig]

I got it to work via:

instance.show(() => {
	instance.element().querySelector( '.basicLightbox__placeholder > *:first-child' ).focus();
});

For whatever reason, I couldn't get it to work with the onShow callback.

[yliharma]

I got it to work via:

instance.show(() => {
	instance.element().querySelector( '.basicLightbox__placeholder > *:first-child' ).focus();
});

For whatever reason, I couldn't get it to work with the onShow callback.

It works!! We'll never know why, but thank you :)