WICG / close-watcher

A web API proposal for watching for close requests (e.g. Esc, Android back button, ...)
https://html.spec.whatwg.org/multipage/interaction.html#close-requests-and-close-watchers
71 stars 5 forks source link

close() should not trigger cancel, at least by default #28

Closed domenic closed 1 year ago

domenic commented 1 year ago

@josepharhar noticed that for <dialog>, dialogEl.close() does not trigger the cancel event. Only a proper user-driven close signal will trigger a cancel event.

I tend to think the CloseWatcher design is more useful for developers: if you have code like

dialog.oncancel = e => {
  if (userHasUnsavedData) {
    e.preventDefault();
  }
};

dialog.querySelector(".close-button").onclick = () => dialog.close();

you probably would be surprised that clicking .close-button skips your unsaved-data check in the cancel event, and instead goes straight to the close event. You're forced to duplicate the unsaved-data check inside the onclick handler, which is silly.

However, symmetry between <dialog> and CloseWatcher is very important, to avoid web developer confusion. That's why we're using the close/cancel event names in the first place.

My proposal is that we provide both behaviors:

Naming for the cancel + close variant remains the tricky problem. I like signalClose() (meaning "send the close signal as if the user did"), but per #13 @annevk and @smaug---- did not. Some options:

The first two options seem nice and straightforward, but they have the minor drawback of making the cancel-and-close version more verbose, and demoting it to feeling second-class. (Whereas, per my above reasoning, I think it's actually the more likely behavior.) The latter two options are trickier but maybe worthwhile?

We can also consider porting whatever we come up with here to <dialog>.

josepharhar commented 1 year ago

The first two options seem nice and straightforward, but they have the minor drawback of making the cancel-and-close version more verbose, and demoting it to feeling second-class

What about just calling it "cancel()"? Just a thought, I don't have strong opinions on this

domenic commented 1 year ago

That's... pretty great! I'm promoting that to front-runner, pending any further opinions.

tbondwilkinson commented 1 year ago

cancel does really feel like a winner :)

Otherwise yeah I think focusing on the distinction between a UI close and a close action would be useful. requestClose would be my suggestion, but shrug

domenic commented 1 year ago

I'm deciding on requestClose(), after writing some example code and realizing that stuff like

myPicker.querySelector(".close-button").onclick = () => watcher.requestClose();

seems more readable than

myPicker.querySelector(".close-button").onclick = () => watcher.cancel();
domenic commented 1 year ago

Fixed this in the spec PR and in this repo's explainer. I also did a general update from "close signals" to "close requests".

Still TODO, updating Chromium.