microsoft / fast

The adaptive interface system for modern web experiences.
https://www.fast.design
Other
9.28k stars 594 forks source link

Use close watcher when supported in place of escape key handlers #6877

Closed lukewarlow closed 4 months ago

lukewarlow commented 10 months ago

Pull Request

📖 Description

This PR uses the CloseWatcher API [1][2] in place of escape key listeners where it's supported. This allows components such as dialog to be dismissed using extra close signals (e.g. Android back gesture)

[1] https://html.spec.whatwg.org/multipage/interaction.html#the-closewatcher-interface [2] https://developer.chrome.com/blog/new-in-chrome-120#close-watcher

🎫 Issues

https://github.com/microsoft/fast/issues/6878

👩‍💻 Reviewer Notes

Need to test all changed components to ensure they correctly dismiss when escape key is pressed both in Chrome and a non Chromium browser (e.g. Safari or Firefox).

Additionally can test these components on Android and see the back gesture now correctly dismisses rather than unexpected navigating.

📑 Test Plan

✅ Checklist

General

Component-specific

⏭ Next Steps

lukewarlow commented 10 months ago

@microsoft-github-policy-service agree

KingOfTac commented 10 months ago

Thanks for the PR and bringing this awesome new platform feature to my attention! I do have a couple of questions though.

  1. What would be the differences in behavior in supporting and non-supporting browsers, if any?
  2. Does this change fix any critical issues and if not, would we be better off waiting on this change until browser support has gotten better and we can avoid branching logic based on feature support?
lukewarlow commented 10 months ago
  1. Other than in some edge case scenarios where close watchers can end up stacking where the escape key listeners might not there should be no difference.

  2. It's arguable how important this is. An alternative to the branching logic would be a polyfill that internalised the escape key listening.

One benefit to supporting this early is that you get Android back gesture support (including Talkback), you may also get support for other assistive technologies (haven't tested others).

If this isn't supported then there will be a UX difference between the native popover and dialogs and the custom ones which may lead to user frustration.

janechu commented 4 months ago

Closing this, due to #6951 we are only putting in fixes or critical features.