ghosh / Micromodal

⭕ Tiny javascript library for creating accessible modal dialogs
https://micromodal.now.sh/
MIT License
3.53k stars 228 forks source link

Ignore the focused element state after closing the modal #163

Open WORMSS opened 5 years ago

WORMSS commented 5 years ago

Hello, we have a strange situation where the modal is returning focus to the button that opened it, but the button was on a menu that has hidden itself away (correctly), so when it closes, it is forcing that button back into view but destroying the layout of the rest of the menu in the process by changing the scrollTop of an element that should never be moved.

When I open the modal, is there anyway I can say (don't return to the opener when you close) ?

gautamz07 commented 5 years ago

@WORMSS can you create a isolated example or a jsfiddle ?

WORMSS commented 5 years ago

I can indeed, but I wont be able to make it for while, I am so far behind in my current project it's unreal.

My work around currently is to

element.blur();
openModel();

but due to the abstraction we have, the openModel() code may be nested, or the event is a vue 'emit' and may not have direct (easy) access to the element that triggered the code, so it's just a little messy. Fair easier if we could just tell the model to not store the current selected element for that particular opening.

showModal () {
  this.activeElement = document.activeElement
  ...
}

becomes

showModal () {
  this.activeElement = (this.config.returnFocusOnClose && document.activeElement) || undefined
  ...
}

Obviously returnFocusOnClose can be named something better, I am bad at naming API variables.

ghosh commented 5 years ago

@WORMSS When you mention "the button is hidden correctly", what exactly do you mean?

I'm not too keen on adding an additional API to disable focus restoration. As a library we have taken a stance on what a accessible modal should be and this API can be easily abused.

However, this brings up an interesting case. We could conditionally disable focus restoration on elements which have aria-hiddenproperty on them. This should theoretically solve this issue in a graceful manner.

WORMSS commented 5 years ago

We are animating the button off screen (because everything has to look fancy nowadays), and this button has been animated behind a bunch of other elements and possibly off the bottom of the screen. (Eg, hidden away).

The elements are still within the dom, but from a UI/UX point of view, they are not seen by the user. When we close the modal, it tries to do this.activeElement.focus() which causes undesirable scrolling (using scrollTop) by the browser, and we want to basically say 'we know the button will be hidden when this modal closes, do not try to return to it.'

I don't believe we currently apply any aria-hidden within our codebase.