-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Why does hide popover not have a cancelable event? #8973
Comments
I'm not sure what the reason for this was, but I can confirm that chrome's implementation only has cancelable on showPopover not hidePopover. Maybe it would have been a lot of extra complexity to handle some popovers not closing when other code calls HideAllPopoversUntil? Mason probably knows |
I thought a bit about this myself, I think potentially the complexity was for hiding popovers when the popover state changed to none, where you don't want to allow cancelling the event. |
So we discussed this several times in OpenUI (here and here, primarily) and the main reason is that it's a huge footgun to cancel the hide event. Much of the logic of popover, as you know, is around handling the interactions between nested and non-nested popovers, and correctly handling light dismiss behaviors. Much of that unravels if one of the popovers cancels it's hide event. When that happens, it's kind of viral, and you end up having to re-implement all of the one-at-a-time and light dismiss behaviors in JS, likely incorrectly. Also, nobody came forward with a concrete use case that needed it. Since the |
Okay, I guess we'll just leave this as-is then. |
A use case I ran into is a datepicker (in a popover) that can also be changed via an input (not in the popover). I want all of the behavior of popover="auto" (esc to close, click outside to close, autoclosing other popovers) except when clicking on the input that controls the popover, in which case I don't want to close the popover. I guess I need to set popover="manual" and handle all of the light-dismiss behavior myself. |
Yeah, with that use case I think you'll need to use |
Ran into this when showing a popover on The initial Seems like allowing |
It seems a bit weird that only one
beforetoggle
event is cancelable. So you can only prevent a popover from being shown, not from being hidden.cc @mfreed7 @josepharhar @nt1m
The text was updated successfully, but these errors were encountered: