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

The watcher stack design, and preventing creation of new CloseWatchers, is problematic #19

Closed domenic closed 2 years ago

domenic commented 2 years ago

Consider the following setup:

function createCustomDialog(...) {
  let watcher;
  try {
    watcher = new CloseWatcher(...);
    watcher.onclose = ...;
  } catch {
    // oh well, we won't get close signal support for this dialog
  }

  // ... etc. ...
}

button.onclick = () => {
  createCustomDialog(1);
  createCustomDialog(2);
  createCustomDialog(3);
};

The intention of this proposal in such a scenario, to avoid abuse, is that two of the dialogs are closable via close signals (such as the Android back button): one because we got user activation from clicking the button, and one as the "free close watcher". The third dialog is not closeable with close signals.

However, the code above is bad because it causes the dialog which fails to get close signals to be dialog 3, the topmost dialog.

This ends up with a very strange experience: three dialogs open, 3 on top of 2 on top of 1. You press the Android back button, and 2 closes. Then you press it again, and 1 closes. And you press it again, and you go back through the session history.

The user would expect the topmost dialog, dialog 3, to close first. But this is not what they got.

There is no easy way to do this in the current proposal! We need to change the API or behavior in some way. I think we need to ensure that most-recently-created CloseWatchers get priority. But this means the whole design of the constructor throwing is probably not right! Maybe we can go back to some sort of event-based approach?

bathos commented 2 years ago

Reading this I initially pictured a pair of events like e.g. "closehandleable" and "closeunhandleable". After all, we might also close 3 programmatically in which case 1 would seemingly be handleable again. But to do useful stuff with that knowledge (like e.g. closing 1 and 2 together if 1 is not otherwise handleable) does seem to suggest CloseWatcher itself might be misaligned with real patterns - the "free stack" means rather little if the close stack is fixed at max 2 and while the "dialog stack" itself is unbounded.

(Have not dug into the history here much, so apologies if this is redundant w/ prior design discussions.)

domenic commented 2 years ago

Having thought about this more, I think going all the way to an event-based approach isn't workable. Doing so loses the coordination benefits of close watchers. We'd have to rely on every different widget to coordinate so that only one of them responded to the global close event. This is basically impossible when you have multiple libraries, plus platform features like <dialog> or popup="".

So we need something with a stack. We just need to be more aware that the stack is limited in size. Note that the size limit is not fixed at 2; rather it's at roughly 1 + # of user activations.

An initial idea is that we need to explicitly kick things out of the bottom of the stack. So, new CloseWatcher() never throws. It just sometimes will cause the oldest existing close watcher to become inactive (i.e., leave the stack). This design makes me a bit uneasy because of the action at a distance, but I think that might just be unavoidable.

jakearchibald commented 2 years ago

Another idea (although maybe this is what @bathos is thinking):

createCustomDialog(1);

This can add to the stack, due to the initial 'free' watcher. Stack is now: [[dialog1Watcher]].

createCustomDialog(2);

This can add to the stack, due to the interaction. Stack is now: [[dialog1Watcher], [dialog2Watcher]].

createCustomDialog(3);

This cannot add to the stack, but instead it joins the stack at the same position as dialog2Watcher. Stack is now: [[dialog1Watcher], [dialog2Watcher, dialog3Watcher]].

So now, signalling close will signal to both dialog2Watcher and dialog3Watcher.

bathos commented 2 years ago

@jakearchibald Yep, that was where my mind was going: what you described is the exact behavior I was imagining I’d want to achieve in this scenario, but because I wasn’t really sure if it was also the behavior other folks would typically want, I instead started thinking about whether there could be finer grained affordances somewhere that would permit userland “bookkeeping”.

Given we were both thinking along these lines, though, maybe it really is what most folks would want to have happen? If so, I’d be very happy to do no bookkeeping at all :)

Disclosure tho: while I suspect our app could benefit significantly from this API, I haven’t spent much time looking at it yet, so you’ll wanna weigh my opinion accordingly; it’s way more gut than reason.

domenic commented 2 years ago

I think I like the idea of over-closing dialogs/etc. more than under-closing them. Basically if you open two dialogs with a single user activation, you should expect a single close signal to close them both as well.

I'll see if I can spec/implement that behavior.

smaug---- commented 2 years ago

Isn't overclosing very surprising behavior, at least if the user of API isn't informed about it when creating new CloseWatchers.

(Not that I understand why there is that 'free' CloseWatcher. That feels like an odd inconsistency in the API)

domenic commented 2 years ago

the user of API isn't informed about it when creating new CloseWatchers.

I'm not sure exactly what you mean by informing the user of the API. Via documentation? That would certainly be part of the plan. Or via some readable property of the CloseWatcher? That also seems reasonable although hard to figure out the exact API shape.

(Not that I understand why there is that 'free' CloseWatcher. That feels like an odd inconsistency in the API)

See https://github.com/WICG/close-watcher#user-activation-gating :

The allowance for a single non-activation-triggered CloseWatcher is to allow use cases like "session inactivity timeout" modals, or high-priority interrupt popups. The page can create a single one of these at a given time, which we believe strikes a good balance between meeting realistic use cases and preventing abuse.

smaug---- commented 2 years ago

The API should be such that the user of it can know the expected behavior.