Can the CloseWatcher API be utilized in a11y-dialog?

[mayank99]

The CloseWatcher API provides a browser-native mechanism for listening to "close requests" from the operating system. This includes Esc keypresses on desktop, the "Back" gesture on mobile, as well as some other close requests, such as those from assistive technology (notably VoiceOver on iOS).

Can a11y-dialog make use of this API to offer a better, more inclusive user experience?

I'd imagine it could be added in the same place where it is currently listening for Esc keypresses.

https://github.com/KittyGiraudel/a11y-dialog/blob/a5fe088dac8567368b53899d5da39304f64c2cd1/src/a11y-dialog.ts#L106

Since CloseWatcher is quite new, feature detection would need to be utilized to conditionally call the API only in supported browsers, while still continuing to call the existing bindKeypress code in older browsers.

[KittyGiraudel]

Hey Mayank! Thanks for opening an issue. I wasn’t aware of this API, so this is pretty cool to explore it. I opened a prototype in #717 which is half-decent, but I’m not convinced by the API itself tbh.

[mayank99]

Thanks for looking into this! I responded to some of the comments in your PR, and I mostly agree with your criticism of the API shape.

However I do want to emphasize that this API provides new, important functionality that is either impossible or very difficult to achieve without using it. For example, there is no other way to respond to the "Back" interaction in Android or VoiceOver+iOS.

[KittyGiraudel]

Thanks for your review! Don't get me wrong, I'm all in favor of implementing it, I just don't know how to solve the lack of event details problem. Right now, a lot of implementations rely on having access to the event when closing the dialog, and that would essentially no longer work since the close watcher event is basically useless.

[KittyGiraudel]

I revisited the approach a little. Instead of moving all the internal calls to .hide(..) through the CloseWatcher instance — which causes the aforementioned issue with the event details being lost — I stuck to just instantiating a CloseWatcher which calls .hide(..) on close.

I tested it on Chrome Android and it closes the dialog when using the “Back” gesture instead of navigate back one page, which is awesome. I haven’t tried VoiceOver, but I assume it works similarly. That’s a really low effort and worthy addition, so I’d be keen on releasing it in v8.2.0.

The only remaining problem is that a specific Cypress test related to the [popover] attribute now consistently fails. I don’t understand why, as I cannot reproduce the problem either in Chrome or Firefox, where things behave as expected. I don’t even know if the problem is in Cypress, cypress-real-events or something else. It’s a bit wild.

[mayank99]

Awesome! I quite like this small snippet:

new CloseWatcher().onclose = this.hide

Just a thought: should this replace the bindKeypress handling? Having both seems redundant and could maybe cause unexpected issues.

if (typeof CloseWatcher !== 'undefined') {
    new CloseWatcher().onclose = this.hide
} else {
    this.$el.addEventListener('keydown', this.bindKeypress, true)
}

As for the Cypress tests, I wonder if the Escape keypress fired by cypress-real-events doesn't actually trigger a close request in the browser. But that wouldn't explain why it only happens when used together with popover.

[KittyGiraudel]

Just a thought: should this replace the bindKeypress handling? Having both seems redundant and could maybe cause unexpected issues.

Mmh, no, we cannot do that. For two reasons:

1. The bindKeypress method handles more than just ESC, it also handles TAB. We could work around that problem by splitting that method into 2 separate ones: one for ESC and one for TAB.
2. As mentioned in my previous comments, right now listening to hide events on the dialog instance (or the dialog container DOM element) allows accessing the event object itself (see docs). This is typically used to access which element was clicked (like which button, or whatnot). If we decide to rely exclusively on the CloseWatcher API, we completely lose that entire thing since all we end up getting are close events from the CloseWatcher.

So, to solve that situation we either:

• Have to keep both, like in the current state of the PR. As you said, it may causes issues, but I played with it in the playground, and it seems to behave totally fine. Nested dialogs still work as well. And besides that Cypress oddity, all tests pass.
• Provide a way to switch to the CloseWatcher API exclusively, maybe with an instantiation option. This way, the implementor would explicitly opt-in to use that API (and all its quirks that we could document properly). The drawback is that we don’t benefit from the advantage of this API unless we opt-in to use it.

[mayank99]

It might be worth testing the order of events. If close gets called first, then it will remove the bindKeypress event handler (via hide), which could again be a breaking change (since the event.detail will be undefined).

https://github.com/KittyGiraudel/a11y-dialog/blob/a5fe088dac8567368b53899d5da39304f64c2cd1/src/a11y-dialog.ts#L137

[KittyGiraudel]

Well, I think this is fine because we actually never call .close(..) or .requestClose(..) on the close watcher ourselves in the code. It’s only for the back button of Android or the command of VoiceOver (or whatever other technology hooking onto it).

So whenever we click a dialog closer (like the close button or the backdrop, or even the ESC key), we actually don’t go through the watcher whatsoever. We internally just call .hide(..). So there is no risk of double event or anything.

At least, that’s how I understand it.