openui / open-ui

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

[invokers] Should invokers on dialog have a cancel action? #938

Open keithamus opened 10 months ago

keithamus commented 10 months ago

Somewhat related to https://github.com/openui/open-ui/issues/937.

In the explainer today there exists a cancel action for dialog, which cancels the dialog (in other words it fires the cancel and close events, not just the close event).

<dialog id=d>
  <button invokertarget=d invokeaction=cancel>This will cancel the dialog</button>
</dialog>

Should we keep this? The cancel event is fired when a CloseWatcher closes a dialog, it currently doesn't fire for buttons, so having these buttons cancel the dialog would be somewhat new behaviour.

My vote is that we should remove this, and cancel can be kept as an observation of the CloseWatcher behaviour, but I'd like to hear others' thoughts.

/cc @lukewarlow @mfreed7 @scottaohara @domenic

lukewarlow commented 10 months ago

For me it would make sense for cancel buttons to cancel the dialog. If the dialog is designed to make a decision or enter data and you don't then it's a "cancel" to me and the button should be able to signify this.

So I reckon we should keep it.

As a consumer of the dialog element I don't really care about close watchers Vs light dismiss (if that gets added) Vs my own cancel button. I just want an easy signal for "this didn't do the thing I was hoping it would"

css-meeting-bot commented 10 months ago

The Open UI Community Group just discussed [invokers] Should invokers on dialog have a cancel action?, and agreed to the following:

The full IRC log of that discussion <jarhar> keithamus: currently, dialogs can have two events. one is close and also closewatcher - confusing but the closewatcher wil close the dialog and will emit a close and cancel event
<jarhar> keithamus: closewatcher right now is escape key and back gesture on mobile
<jarhar> keithamus: with invokers, do we want to have a cancel action so that you can create a button that will also dispatch the cancel event and the close event
<masonf> q?
<bkardell_> no
<jarhar> keithamus: you can have a close action which dispatches close, so should we have one that does cancel and close
<jarhar> masonf: two choices are - just an action that is only close and another action that is close and cancel? or making the close action fire both?
<jarhar> brecht_dr: wondering about use case? why would you need both of them? i just dont understand
<jarhar> keithamus: close action ... theres a difference between canceling and escaping from the contents of the dialog
<jarhar> keithamus: and closing the dialog
<masonf> q+
<jarhar> keithamus: theres not a compelling use case, one of the questions that came up was: well we have close, which calls close() and that dispatches the close event
<jarhar> keithamus: and then there is the closewatcher which dispatches the cancel event
<jarhar> keithamus: you can determine whether it came from a closewatcher by listening to cancel
<jarhar> keithamus: you could argue that close button should fire cancel vs no button or cancel
<jarhar> q+
<bkardell_> q+
<jarhar> keithamus: theres a button in the footer, and you might want those things to be different
<jarhar> keithamus: when we have a dialog we put a close button in there
<jarhar> keithamus: ways to figure out if its the x button or another one of the buttons so i dont have a compelling use case
<jarhar> masonf: it feels like we shouldnt
<jarhar> masonf: were talking about invokers and modal dialogs
<jarhar> masonf: in that case, the invoker has to be within the modal dialog
<jarhar> masonf: which means its just another button in the dialog
<jarhar> masonf: theres nothing that tells you it has to have an x on it
<jarhar> masonf: feels like theres a nice separation that closewatcher does cancel, and button should just do close
<jarhar> masonf: that feels clean and easy
<scotto_> q+
<jarhar> masonf: trying to add somehow two kinds of actions is ... difference between cancel and close my heads gonna explode
<Luke> q+
<masonf> ack mason
<jarhar> keithamus: i think that the cancel event is that the user agent has made this gesture
<jarhar> keithamus: close button should only be for animations and cleanup, dialog could close because its accepted
<jarhar> keithamus: close is a generic this thing is going away, whereas cancel is like the user has dismissed this
<jarhar> keithamus: subsequent close event is now its gone away
<jarhar> keithamus: that same cancel behavior could be provided by js in a button that calls close
<masonf> jarhar: I helped implement the closewatcher integrations on Chromium, and it's complicated. I don't remember what cancel's point was.
<masonf> jarhar: lots of machinery around anti-abuse. We should just keep it close and leave cancel to the closewatcher.
<masonf> ack jarhar
<masonf> ack kardell
<jarhar> bkardell_: also wanted to say it should just be close
<masonf> ack bkarde
<jarhar> bkardell_: we have these nice invokers now and we want to map those to the ui
<brecht_dr> +1 (on just being close)
<jarhar> bkardell_: they cant be arbitrarily mapped to all things
<jarhar> bkardell_: a lot of things when you think of dialogs do have buttons in them that youre not going to be able to use an invoker for the same way
<jarhar> bkardell_: if the only action is close, then you can use an invoker
<jarhar> bkardell_: in many cases, you launch this and then clicking ok means you agree and it does cause the thing to close but that invoker also needs to do something else
<jarhar> keithamus: ideally clicking ok is a form submission where the method is dialog
<keithamus> Proposed resolution: Invokers will only provide a close action, and the cancel operation (and event) will be reserved for CloseWatcher or other UA gestures.
<jarhar> keithamus: does animations and event listeners
<Luke> q-
<masonf> ack scott
<jarhar> scotto_: reset button - what is that? is that a close or a cancel event?
<jarhar> scotto_: if one were thinking about closing a dialog submitting the dialog method on a form, thats a close event. is a reset a cancel or a close?
<jarhar> keithamus: i think reset does nothing and theres a conversation i want to ahve where it should close the dialog
<bkardell_> s/but that invoker also needs to do something else/but that isn't an invoker, it needs to do something else which also happens to, at the end, close the dialog somehow
<jarhar> scotto_: i thought it actually did close it
<jarhar> keithamus: i could be wrong
<jarhar> masonf: i dont think it closes it
<Luke> +1
<keithamus> RESOLVED: Invokers will only provide a close action, and the cancel operation (and event) will be reserved for CloseWatcher or other UA gestures.
lukewarlow commented 10 months ago

Screenshot_20231116-090439.png

Interestingly Android has a similar cancel Vs close situation but they do allow a cancel function for cancel buttons

lukewarlow commented 10 months ago

https://github.com/openui/open-ui/discussions/950#discussioncomment-7593669 - this discussion suggests that our assumption of a UA only cancel event isn't necessarily correct.

lukewarlow commented 6 months ago

Semi related to this I've raised https://github.com/whatwg/html/issues/10164 - to discuss adding a new requestClose() (name tbd) function to dialogs, if this is added we should update the 'cancel' name to be requestClose to match.

github-actions[bot] commented 3 weeks ago

There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label.