Git Product home page Git Product logo

close-watcher's People

Contributors

cwilso avatar domenic avatar josepharhar avatar lukewarlow avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

close-watcher's Issues

This problem is not solved by `keydown`

On desktop platforms, this problem is currently solved by listening for the `keydown` event, and closing the component when the <kbd>Esc</kbd> key is pressed. Built-in platform APIs, such as [`<dialog>`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/showModal), [fullscreen](https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API), or [`<input type="date">`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/date), will do this automatically. Note that this can be easy to get wrong, e.g. by accidentally listening for `keyup` or `keypress`.

I thought you might want to add another reason for this API. Make this dialog

<dialog>name: <input type="text"></dialog>

Write this code

const dialog = document.querySelector('dialog');
dialog.show();

window.addEventListener('keydown', e => {
  if (e.key === 'Escape') {
    dialog.close();
  }
});

Switch to Japanese input, click in the input box, type some Japanese, See the Input Method Editor appear, press Esc to close the Input Method Editor. See the Dialog box get closed even though that was not the user's intent.

This might also inform the API's design.

Rename signalClose()

Feedback from @annevk and @smaug---- is that signalClose() as a method name is too confusing with the existing AbortSignal. Especially after #13.

Probably the best alternate name is just close().

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

@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:

  • closeWatcher.close() behaves like dialog.close(), and goes straight to the close event.

  • closeWatcher.bikeshed() behaves as if a close signal has been sent, doing both cancel + close.

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:

  • cancelAndClose()
  • close({ withCancel: true })
  • Rename the whole "close signals" concept to something like "close command" or "close triggers", and then go with closeCommand() or triggerClose() or something like that.
  • Come up with some verb that is meant to indicate "the whole process of closing, which might possibly be canceled": e.g. exit(), shutdown().

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>.

Integrate with AbortSignal

Feedback from @annevk and @smaug---- is that you should be able to do something like this:

const controller = new AbortController();
const watcher = new CloseWatcher({ signal: controller.signal });

controller.abort();

which will work the same as

const watcher = new CloseWatcher();
watcher.destroy();

The advantage, of course, is for cases where you have a single AbortSignal tied to many different things, one of which might be a CloseWatcher.

Which document should be targeted?

Some close signals don't target a specific document, such as Android's 'back' action. In a page containing iframes, which document gets the signal?

Option 1: Always the top level.
Option 1a: …unless an element in an iframe is full-screen, in which case it goes to the iframe.
Option 2: Whichever document last had focus.
Option 3: The stack is moved to the traversable. If the close action targets a particular document (such as esc key), then it takes the last item on the stack for this document. Otherwise, it's the last item on the stack.

Option 3 could get complicated due to all the thread hopping.

Should the Esc key count as user activation?

When coding up the current spec/explainer draft, I realized that the Esc key counts as user activation. This means that on desktop, all the fancy abuse prevention mechanisms for the cancel event do nothing: since pressing Esc means there is always recent user activation, the cancel event will always fire (and can be preventDefault()ed).

On the one hand: that seems fine! On desktop we don't have to worry about back trapping, and we can already preventDefault() the Esc keydown event. No issues; this makes the API easier to use on desktop as you don't have to worry so much about user activation!

On the other hand: this is a weird divergence between desktop and Android (or desktop and other platforms where the close signal is not a user activation), and makes code harder to test. So maybe we should spec it so that an Esc keypress (or other user action) that sends a close signal, never counts as user activation, regardless of the platform.

Why not just an event?

Why does this API involve instantiating an object and assigning callback functions to it, instead of an event dispatched to window and/or to the currently keyboard focused element? I'm not saying it has to be that, perhaps there is a good reason, but there's no "alternatives considered" section to explain it.

Support out-of-bounds clicks as a close signal

When thinking about the new Popup API proposal, the close watcher currently supports a number of the close signals for a popup including esc and back button on certain UAs.

One of the new close signals however is clicking or tapping outside of the bounds of the popup.

Clicking or tapping outside the bounds of the pop-up. Any mousedown event will trigger all open pop-ups to be hidden, starting from the top of the pop-up stack, and ending with the nearest open ancestral pop-up of the mousedown event's target Node. This means clicking on a pop-up or its trigger or anchor elements will not hide that pop-up.

I was thinking that it would be good if any new behavior provided automatically by the browser could be encapsulated in APIs that would allow component developers who could not use the popup API for one reason or another to still take advantage of the dismiss behavior.

In the explainer there is the example of the sidebar click, which adds an event listener and then calls into the close watcher directly.

I wonder whether we can have an automatic option instead. So that example would become:

const hamburgerMenuButton = document.querySelector('#hamburger-menu-button');
const sidebar = document.querySelector('#sidebar');

hamburgerMenuButton.addEventListener('click', () => {
  const watcher = new CloseWatcher({element: sidebar, outOfBoundsClick: true});

  sidebar.animate([{ transform: 'translateX(-200px)' }, { transform: 'translateX(0)' }]);

  watcher.onclose = () => {
    sidebar.animate([{ transform: 'translateX(0)' }, { transform: 'translateX(-200px)' }]);
  };

Support blur as a close signal

When thinking about the new Popup API proposal, the close watcher currently supports a number of the close signals for a popup including esc and back button on certain UAs.

One of the new close signals however is focus leaving the popup.

Using keyboard or other navigation sources to move the focus off of the pop-up. When the focus changes, all pop-ups up to the "nearest open ancestral pop-up" for the newly-focused element will be hidden. Note that this includes subframes and the user changing windows (a window blur) - both will cause all open pop-ups to be closed.

I was thinking that it would be good if any new behavior provided automatically by the browser could be encapsulated in APIs that would allow component developers who could not use the popup API for one reason or another to still take advantage of the dismiss behavior.

Perhaps the API could be something like:

const hamburgerMenuButton = document.querySelector('#hamburger-menu-button');
const sidebar = document.querySelector('#sidebar');

hamburgerMenuButton.addEventListener('click', () => {
  const watcher = new CloseWatcher({element: sidebar, blur: true});

  sidebar.animate([{ transform: 'translateX(-200px)' }, { transform: 'translateX(0)' }]);

  watcher.onclose = () => {
    sidebar.animate([{ transform: 'translateX(0)' }, { transform: 'translateX(-200px)' }]);
  };

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

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?

Scoping close events

This looks very interesting! As someone who works on component libraries, this is something I've often wanted - especially the ability to tie into hardware back button and screen reader close signals. Apologies if the proposal isn't ready for feedback yet - feel free to disregard if so.

One question I had is if it makes sense for close events to be scoped. At the moment, my understanding is that the proposal is that they are global. If multiple CloseWatcher instances are created, then the most recently created instance is triggered. This feels like it could be potentially problematic in cases where you might have multiple open "panels" or some such UI. Did you consider whether it might make sense to scope close events to a DOM element and trigger the event by bubbling from the activeElement? Another potential advantage to that approach would be the ability to include pointer events such as clicking outside the element as a close signal.

Another question is whether JavaScript can potentially cancel a close signal. For example, if you had a custom input within a dialog that cleared when you pressed the escape key, you wouldn't want the dialog to close. Would preventDefault on the keyboard event prevent the close signal from firing (i.e. does the event need to bubble all the way back to the document - or whatever element the handler is scoped to if the above option is used)?

PlayStation button

In Europe the circle button is the one that goes back. (It used to be triangle in the early models.)

<dialog> integration compat issue

The current spec attempts to integrate <dialog> with close watchers very directly. However, due to the anti-abuse restrictions, this breaks compat on desktop OSes in some cases.

As background, our anti-abuse restrictions are that you basically get to intercept one close signal per user activation, to avoid back trapping with the Android back button. If you try to intercept more, you will either not be able to create a CloseWatcher, or you will not get a cancel event.

However, we apply those restrictions uniformly across platforms, including on desktop platforms where the signal is the non-abusable Esc key. The idea is to avoid platform-specific differences and make it easier to test-once, run-on-any-user's-device.

This seems fine for the new CloseWatcher class. But for <dialog> this does have compat impacts:

  • If you showModal() many <dialog>s without user activation, only the first (bottommost) will be closeable with Esc. Others will ignore close signals. (This avoids creating tons of <dialog>s on Android which consume all the back button presses.)
  • If no user activation has happened since the dialog was shown, its cancel event will not fire. (Since the cancel event can be preventDefault()ed, which consumes a close signal, which we don't want to let happen an unlimited number of times.)

I think these behaviors are good for integrating the Android back button with <dialog>s. (Note that no browser has such integration today, so it's a question about whether the proposed integration, subject to the above restrictions, is better than nothing.)

And I think in the real world these restrictions should be survivable for <dialog>s even on desktop. People should generally not open more than one <dialog> without user activation. But, it is a concern.

Ideas:

  • Leave it as is, and take the compat hit. IMO it will be manageable, or at least worth trying, and the benefit of getting uniform behavior across mobile and desktop is worth it.

  • <dialog> gets special-casing for Esc key handling to preserve compat. Basically if the close signal is an Esc key, we bypass the usual checks and use a separate path for reacting to that, instead of using the close signal infrastructure.

    • This version does not really give <dialog> special powers compared to what a web developer could write. It's just messy. See below for more.
  • <dialog> still fires cancel events, but they are not cancelable if there haven't been enough user activation. This is a quick fix that might help people who happen to be using <dialog>'s cancel as a synonym for close instead of using it as a method of preventing the dialog from closing.

  • We change the close watcher infrastructure in general to only apply anti-abuse measures on close signals where it is necessary (e.g. Android). This means that if you are working on desktop, you can create unlimited CloseWatcher instances and get unlimited cancel events, since consuming Esc key presses is not dangerous. Similarly on iOS devices (although there close signals are rare). But if you are working on Android the new CloseWatcher() constructor might throw, or the resulting CloseWatcher might not fire a cancel event.

    • We can have a variant of this where instead of new CloseWatcher() throwing, it returns an already-deactivated CloseWatcher that will never fire events, which might be a bit less destructive for those who only test on desktop.
    • This might be especially desirable if we expect web developers to be hacking together this sort of solution on their own. I.e., maybe web developers will prefer non-uniform behavior if it gets them more reliable desktop close signals, and will start doing hacks like using CloseWatcher only on Android, or listening for Esc in addition to using the CloseWatcher.

/cc @smaug---- @annevk @josepharhar

iPhones don't have a consistent system close/cancel/exit gesture

iPhones don't have a consistent system close/cancel/exit gesture. Sometimes it's swipe down, sometimes it's swipe up, sometimes swipe sideways, sometimes it's an onscreen button. There's generally some kind of visual affordance that's specific to the particular scene being shown.

There's the home button or home affordance to return to the Home screen, but apps don't have the ability to override that.

That's not to say this API isn't useful but I'm not sure what the right thing to do would be on iOS. Don't expose the API at all? Expose, but close event never fires? Make up a close gesture for the web only (but then how would users know about it)?

(On iPad, Cmd+period is an equivalent to Escape that works even on the magic keyboard, but iPads can be used with no physical keyboard, and this key combination is somewhat obscure and mostly only known to old school Mac users).

Potential editorial improvement: give close watchers cancel and close actions

In https://chromium-review.googlesource.com/c/chromium/src/+/3502624 @josepharhar stumbled across a mismatch between implementation and spec techniques that made me think we could improve the spec.

In particular, right now conceptual close watchers have close actions.

The close action for the CloseWatcher class's close watcher is to signal close. The close action for the <dialog> element's close watcher is to cancel the dialog.

These two close actions consist of very similar steps. Roughly:

  • If allowed to do so,
    • Fire a cancel event on the object
    • Consume the user-activation-for-close-watchers
    • Bail out if the cancel event was canceled
  • Do actual closing stuff

We can probably move this infrastructure into the conceptual close watcher, by giving them "cancel actions" and "close actions", and then wrapping up that logic. This better ensures that all close watcher users actually do the correct thing with regard to checking whether cancel steps are allowed and consuming the user activation. And it makes it easier to write any future specs that use close watchers.

Potential issue with late-arriving initial popover

I'm thinking specifically of cookie banners, as these seems to load particularly late.

Let's say a site contains a cookie banner that uses CloseWatcher:

  1. User navigates to site.
  2. Time passes, because the cookie banner loads slowly.
  3. Cookie banner appears.
  4. User presses esc and the cookie banner closes.

Great, but…

  1. User navigates to site.
  2. Time passes, because the cookie banner loads slowly.
  3. User opens a sidebar, which also uses CloseWatcher.
  4. Cookie banner appears over the top.
  5. User presses esc expecting the cookie banner to disappear, but the sidebar closes.

Right now the spec only allows a CloseWatcher to be created without user activation if the close watcher stack is empty (roughly). Maybe it should change so that the stack may only contain one close watcher that was created without user activation?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.