openui / open-ui

Maintain an open standard for UI and promote its adherence and adoption.
https://open-ui.org
Other
3.59k stars 191 forks source link

Dismissing popovers when focus leaves #1060

Closed josepharhar closed 5 months ago

josepharhar commented 5 months ago

I just saw @SaraSoueidan talk at CSS day about how you must make popovers close with javascript when focus leaves them in order to make them accessible, which seems like a reasonable default for popover=auto. Was this behavior considered here before?

@mfreed7 @scottaohara

lukewarlow commented 5 months ago

Duplicate of https://github.com/openui/open-ui/issues/1047

scottaohara commented 5 months ago

yes - this has been discussed a few times and as Luke notes, the latest time was in https://github.com/openui/open-ui/issues/1047

i don't know exactly what was said, but that's an oversimplification if that was the takeaway message.

We should probably update the explainer or something to try and mitigate this (or at least have something to point to beyond previous github issues)

scottaohara commented 5 months ago

provided a much longer explanation over here - https://github.com/openui/open-ui/issues/1047#issuecomment-2154662021

SaraSoueidan commented 5 months ago

Hi! Sara here 🙏🏻

Just to clarify: I mentioned that the popover should close if you are using it to create more complex patterns like dialogs. The point I was making is that popover enables some behavior but you may need to add more behavior if you use it for modal dialogs or if the open "popover" obscures focusable elements on the page, like the navigation drawer.

My example was not meant to be a generalization. It was a reminder that popover does specific things and depending on your use case you may need to use JS to make your pattern accessible. 🙏🏻

scottaohara commented 5 months ago

i would argue that even for non-modal dialogs (because obviously focus leaving modal dialogs should not even be a thing), auto dismiss behavior could be rather frustrating for some users outside of very specific use cases. I say this because some users would very much prefer a non-modal dialog 'soft trapped' focus within it - which is understandably a major change from existing functionality, so not likely to become a new default - but also exactly why auto dismiss should not be a default either (especially for dialog[popover], talk about really screwing people over that literally want the opposite of that behavior)

appreciate that you meant it as a generalization sara, and this was a misunderstanding of that fact. so thanks for clearing that up.

mfreed7 commented 5 months ago

Thanks for the additional comments here. I just want to emphasize the last thing in Scott's post on #1047, which is that the current behavior (not dismissing when the focus leaves) was specifically decided based on accessibility. That behavior, when applied generally is more frustrating than helpful. So we wanted the default to be generally useful, while the special case behavior is still possible. But this is yet another reason why I'm fairly against making all of the light dismiss triggers optional - it's hard to reason about, and I don't want to throw developers to the wolves to figure it out each time. Let's do the right thing generally, and deal with the special cases as special cases.

One additional point: popovers always dismiss with one press of the ESC key, no matter where focus resides. So it's always easy for keyboard users to dismiss a popover. For that reason also, it's better not to auto-close when focus happens to jump out of the popover. If that was on purpose, the user can hit ESC to close the popover. If that wasn't on purpose (which is more likely) they can easily shift-Tab to get back to the content they were navigating.