Consider stopping propagation on capturing the ESC keydown

[AleksandrSl]

Use case: I have a multiple selection of entities on my page and then I do a butch actions on them which opens a modal. I want to be able to clean the selection by pressing Esc, but preserve the selection if the modal is opened and I cancel it by pressing Esc.

Question/Suggestion: I can implement this by checking whether any modal is opened at the moment I press Esc, so I won't clear the selection in this case. Would it make sense to stop propagation of Esc keydown event that closes the modal, so it won't reach the rest of the page

Possible workaround: I can get the event that cause hide modal event and make this myself in a wrapper.

This code logs event as undefined, I'm not sure yet why

instance.on('hide', (node, event) => {
        console.log('hide', event, node)
})

Sorry if that was already discussed, but I didn't find the issue on GH

[KittyGiraudel]

Hello Aleksandr,

Thanks for reporting, that’s a fair point. Could you please tell me which version of a11y-dialog you use? I‘ll try to give you a workaround (if there is one).

[AleksandrSl]

We use 7.5.2, thank you

[AleksandrSl]

Two points:

1. About logging

instance.on('hide', (node, event) => {
        console.log('hide', event, node)
})

That's just me being stupid or something console quirks. I see the event being there in the debugger 2. The problem is that I add my own listener to the document, so it fires before the one in the modal.

Adding my own listener to Esc on the modal dialog node works, but I think that's just some fortune in the order of the event listeners

Btw, I see that in the current implementation the listener is added via this.$el.addEventListener('keydown', this.bindKeypress, true) (proabably unreleased version) and in the file I see in my node_modules it's added on document document.addEventListener('keydown', this._bindKeypress);. Both can trigger after the listeners on the document itself. But if we do this.$el.addEventListener('keydown', this.bindKeypress) it should be possible to cancel the event manually inside the on('hide') if I want this. Why is the capture necessary?

[KittyGiraudel]

The “current” implementation, as in the code on main is the upcoming version 8. It’s currently available as a pre-release 8.0.0-rc.1, and most likely will be released as is since it’s ready and working. We just haven‘t gotten around to it yet.

I am not entirely sure why we went with capturing the events I must say. Maybe @mxmason remembers?

In any case, you could give it a shot to see if that’s better for you. The document is available on next, and there is a migration guide.

[KittyGiraudel]

Since we don’t have more information at this point, I’m going to release 8.0.0 without this change. However if we realize that this is a problem, I‘ll issue a patch where we remove the capturing.

[AleksandrSl]

Sorry for not having much time to invest in this, thanks for the comment!

[KittyGiraudel]

I am going to close this one since it didn’t come up again for a year. Please do reopen if you feel like we should rediscuss it.