w3c / uievents

UI Events
https://w3c.github.io/uievents/
Other
147 stars 52 forks source link

Introduce contextmenuclose or similar event. #309

Open flackr opened 3 years ago

flackr commented 3 years ago

There is a contextmenu event fired when the user right clicks or long presses (with touch) to show a context menu (Note #279 proposes adding this event to UI Events). There is however no notification about the closing of that menu. Developers typically call preventDefault() on the contextmenu event to create a custom menu, however they have to implement custom logic to try to detect under what circumstances it should close. There are many scenarios under which a contextmenu should be closed, that would be easier to handle given a single event to listen for:

I propose adding a contextmenuclose (or contextmenucancel or contextmenudismiss) event which will be dispatched under the conditions when the platform would close a native contextmenu.

hober commented 3 years ago

I wonder if it might make more sense to introduce a generic dismiss event that we could also use for e.g. <dialog> or custom modals (see domenic/close-watcher or similar efforts, such as Indie UI from back in the day).

(Mentioning domenic/close-watcher#1 so that this gets linked over there)

mustaqahmed commented 3 years ago

Yes, a generic event seems logical here. Thanks.

While we don't expect modals inside modals in general, we can have context-menu from inside an active <dialog>! Wondering what's the best way to handle this. Maybe the order of dismiss events will implicitly determine what is being dismissed? Like the first event will be for the "topmost" modal and so on? Alternately, the dismiss event can carry some "dismiss target" information.

Thoughts?

flackr commented 2 years ago

I like the idea of trying to make this a generic event, and this works well for most of the ways that context menus can be dismissed, though it gets particularly tricky for dismissing a contextmenu on a drag, since I think dragging should be allowed within a dialog without dismissing it. I suppose one way to explain this particular quirk would to say if a dragstart targets an element that is presumably outside of the dialog element (i.e. the element the contextmenu is anchored on), it should implicitly dismiss the dialog.

flackr commented 2 years ago

@hober I think I may have not correctly grok'd your idea initially. To restate, we would add a dismiss event which would be fired when dialog or custom modals are closed, and could also be fired in these cases where a contextmenu should be dismissed? The firing of this dismiss event would not cause anything to close unless the developer handled that event by doing so.

Given this, a developer implementing a custom contextmenu would listen to the dismiss event and close their custom contextmenu UI in response. They would also need to make sure that pressing escape dismisses their contextmenu UI and not another dialog or popup element showing on the page. They would also need to make sure that they close their contextmenu UI if another element is focused or if focus of the window is lost.

My main concern is where the developer would see the dismiss event dispatched from. Presumably it would be the element which had the contextmenu event. However, if so, if that element was also a dialog element how would they listen for a dismiss of the contextmenu and not the dialog, or vice versa? My presumption is that contextmenu dismissal would have a different target but I'm not sure what that would be if you right clicked on the root of a dialog.

e.g.

<dialog> <!-- right click in here --> </dialog>

@mustaqahmed Another awkwardness (probably unavoidable) is that the browser wouldn't know if a custom contextmenu has been dismissed within the application so the developer may see a spurious dismiss / contextmenuclose event sometime later long after their custom contextmenu was dismissed internally within the application.

Would it make sense to start a PR for the contextmenuclose event and discuss / comment there with issues / ideas for how to deal with these issues.

mustaqahmed commented 2 years ago

I just sent out a draft PR (#322) that makes the new event non-cancelable and always follow a contextmenu event regardless of whether the latter was cancelled or not.

Please add your suggestions/comments there.

EDIT: Updated the PR link to point to the latest change.

mustaqahmed commented 2 years ago

Yikes, the PR got merged prematurely as I was typing above!

mustaqahmed commented 2 years ago

Please add your suggestions/comments in the new PR: #322.

flackr commented 2 years ago

@mfreed7 I wanted to point your attention to this issue in the context of popup dismissal https://github.com/openui/open-ui/issues/426. In particular, what would a dismiss event look like? Would it be dispatched whenever a popup was dismissed?

It sounds like if dismiss is just a way for developers to observe that a popup / dialog was dismissed, then having such an event doesn't need to block https://github.com/openui/open-ui/issues/426, though it would mean that developers could only implement context menus properly by using popup elements as they wouldn't have the events if their application only consisted of e.g. a canvas.

mfreed7 commented 2 years ago

Thanks for the ping. For the popup API, there are two events proposed, show and hide. The hide event is fired whenever the popup is hidden for any reason. It is not cancelable.

For this issue, I think you'd want a new/fresh event like contextmenudismiss, because it needs to be fired on the element that was the originator of the context menu, not the context menu itself.

Let me know if I understood your question correctly. If so, https://github.com/openui/open-ui/issues/426 would be resolved by making the new contextmenudismiss event be a light dismiss trigger for popups. I think that part makes total sense.

mustaqahmed commented 1 year ago

We seem to have concluded that a separate contextmenudismiss event is logical here which would be orthogonal to popup events, right? If so, please take a look at our proposed PR https://github.com/w3c/uievents/pull/322.

@mfreed7 how can we mark the new event as a light dismiss trigger as per your comment above?

mfreed7 commented 1 year ago

We seem to have concluded that a separate contextmenudismiss event is logical here which would be orthogonal to popup events, right? If so, please take a look at our proposed PR #322.

@mfreed7 how can we mark the new event as a light dismiss trigger as per your comment above?

So the Popover spec has landed, and I think you'd just need to modify the "light dismiss" algorithm here:

https://html.spec.whatwg.org/multipage/popover.html#light-dismiss-open-popovers

by watching for the appropriate new event. Note that since the comments above, one change was that pointer-up is the light dismiss trigger, and there is logic to handle the case where the user is trying to highlight text starting within the popover. (That's the "popover pointerdown target" stuff.) I'm not sure how that'll interact with the contextmenudismiss event - perhaps it won't.