Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Popup] Should light dismiss be cancellable on popup? #321

Closed
melanierichards opened this issue Apr 9, 2021 · 7 comments
Closed

[Popup] Should light dismiss be cancellable on popup? #321

melanierichards opened this issue Apr 9, 2021 · 7 comments
Labels
popover The Popover API

Comments

@melanierichards
Copy link
Collaborator

On MS Edge Explainers #457, @mfreed7 asked:

During implementation, the question has come up about whether the Escape key light dismiss behavior (or really, any of the light dismiss behavior such as clicking outside) should be author-cancellable. I.e. user hits Escape, but author has added 'keydown' event handler on document.body, and does preventDefault(). Should the currently-top popup be hidden?

@domenic said:

First, I think the platform needs to have both author-cancelable and non-cancelable close signals. Notably, exiting fullscreen cannot be author-cancelable. But, exiting a <dialog> is (and we have some strong feedback that it should continue to be).

Second, I think that the behavior should be uniform. So, you can prevent dialog closing by preventDefault()ing Esc on document. It also has an explicit cancel event which can be preventDefault()ed. (But, its close cannot be preventDefault()ed; that happens after the fact.) This means that, IMO, anything with cancelable close behavior should behave the same way, both in terms of keydown event listeners (on desktop) and also having a cancel event where applicable.

(Note: the modal close signals explainer was written before I realized this about dialog. Per slightlyoff/history_api#13, we're probably going to update the event name for ModalCloseWatcher in the way the above paragraph discusses, i.e. change beforeclose to cancel, to match <dialog>. Maybe we'll even rename it to ModalCancelWatcher.)

I don't have a strong opinion on which category <popup> should go in. @melanierichards mentioned in #455 that the current plan is for it to be in the uncancelable category. If so, then I think it should behave like fullscreen and keydown listeners shouldn't be able to stop it, and it shouldn't have a cancel event, etc.

Now, there's an interesting wrinkle for author-cancelable close signals regarding the Android back button, which is that canceling them can be used to trap the user by disabling the back button. Our modal close signals explainer has some mitigations for this, and for platform predictability, it applies those mitigations on all platforms.

@melanierichards melanierichards added the popover The Popover API label Apr 9, 2021
@melanierichards
Copy link
Collaborator Author

My personal opinion remains: I recognize authors will want to cancel dismissal, but I think this could open up a vector for abuse. I would rather we try and explore solutions to the legitimate use cases for cancelling popup dismissal, e.g. an event or pseudo selector authors can hook into in order to style popups / animate them out as they are dismissed.

@flackr
Copy link

flackr commented Apr 15, 2021

FYI I started #335 to brainstorm / discuss how we could animate popups appearance / dismissal.

@github-actions
Copy link

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.

@github-actions github-actions bot added the stale label Mar 19, 2022
@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 30, 2022

We need to re-think the requirements for this issue, in the context of the new Popup API. In the new explainer, hiding the popup can be overridden via developer CSS (e.g. set display:block to override the UA's display:none rule), with the exception of top layer status. That last bit might make animations tough, though, since the element will immediately be removed from the top layer, and that alone might make it obscured by another element, so that the animation can't be seen.

@github-actions github-actions bot removed the stale label Mar 31, 2022
@mfreed7
Copy link
Collaborator

mfreed7 commented Apr 8, 2022

Increasingly, I think this is a key point of difference between popups and <dialog>. A dialog is a more "permanent" piece of UI that is around until explicitly dismissed. In contrast, a popup is more "transient", e.g. because it light dismisses when you leave it. As such, it feels like a dialog should be able to cancel its "hide" event (and it can). However, a popup should not be cancelable. In particular, there are some actions that would close a popup, such showing a modal dialog or a fullscreen element, and those would need to not be cancelable. So really only the light dismiss or programmatic (hidePopup()) closing might be cancelable. I wonder what the (good) use cases are.

As I mentioned in #335 (comment), I think this issue of cancelability can be separated from the ability to animate show/hide. So the only question (IMHO) is whether there's a use case for cancelling the hide of popups.

@mfreed7
Copy link
Collaborator

mfreed7 commented May 13, 2022

Increasingly, it seems to me like an anti-pattern to allow canceling the hide event for popups. Much of the machinery (and machinations on OpenUI issues) for popup is really geared toward doing the right thing for each type of popup. For example, only allowing one popup=hint at a time, or hiding popup=hints when popup=popups are shown, or just generally handling light dismiss correctly. Allowing developers to override all of that behavior sounds like mostly a footgun for what is likely to be corner-case use-cases. Perhaps, as pointed out in #495, for those use-cases, <popup=async> is the right solution?

@mfreed7 mfreed7 added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label May 13, 2022
@css-meeting-bot
Copy link

The Open UI Community Group just discussed [Popup] Should light dismiss be cancellable on popup?, and agreed to the following:

  • RESOLVED: the `show` and `hide` events should not be cancelable.
The full IRC log of that discussion <gregwhitworth> Topic: [Popup] Should light dismiss be cancellable on popup?
<gregwhitworth> github: https://github.com//issues/321
<gregwhitworth> masonf: the question is there are shown and hide events
<gregwhitworth> masonf: should they be canceleable
<una> q+
<gregwhitworth> masonf: currently they are not - my opinion is that they should not be canceleable
<gregwhitworth> masonf: we've spent a ton of time trying to get all of the behaviors to get right and allowing an easy developer override will end up being a footgun
<gregwhitworth> masonf: they may result in a11y issues
<gregwhitworth> masonf: there is an out, if you want to more easily control that then popup=async give you the control you may be looking for
<gregwhitworth> masonf: there are other more complicated cases, such as a full screen element is shown we need to force close the popups we would have to make the events not canceleable
<gregwhitworth> ack una
<masonf> q+
<gregwhitworth> una: it seems like it doesn't bring up any usecases, I'm inclined to not rock the boat
<gregwhitworth> una: I'm not seeing anything and we can re-visit this
<JonathanNeal> +1 to making it not-cancel-able. As popups contrast with `<dialog>`, popups feel more “temporary”, and I don’t see a use-case to make popups more like dialogs.
<gregwhitworth> ack masonf
<gregwhitworth> masonf: one more thing, there is a potential for a usecase for animations
<gregwhitworth> masonf: we have an issue opened to solve animations being better
<una> gregwhitworth: when i was doing early mockups on web components, i found being able to cancel the events valuable so I could hijack the behaviors and do something different
<JonathanNeal> +1 to what una says. I would prefer animations be handled in a different way than by allowing programatic cancellation.
<una> gregwhitworth: the capability of doing slotting in a lot of ways and the async approach is much more elegant
<JonathanNeal> As the issue mentions, authors could do “bad” things by using CSS to force the display of the popup, or potentially in the future by creating a never ending animation that _stalls_ the popup from closing.
<una> gregwhitworth: i didnt like it because i was undoing alot of the logic and had to re-author it, so i would +1 to the earlier a11y point because they're so intertwined, you end up having to cancel a lot of the events
<una> gregwhitworth: and I'm wanting to not cancel them all so +1 to not making them cancelable
<una> gregwhitworth: to provide the use case, I wanted to pivot from just down to showing hte scrollwheel touch event on mobile (inertia on scrolling)
<una> gregwhitworth: definitely required different logic, but again doesnt have to do with these exact events
<gregwhitworth> masonf: just to throw it out there, there may be usecases like that and I think popup=async without actually breaking the more defined usecases
<gregwhitworth> q?
<masonf> Proposed resolution: the `show` and `hide` events should not be cancelled.
<JonathanNeal> +1 to the proposed resolution
<masonf> Proposed resolution: the `show` and `hide` events should not be cancelable.
<JonathanNeal> +1 to the fixed proposed resolution.
<dandclark> +1
<masonf> RESOLVED: the `show` and `hide` events should not be cancelable.
<masonf> thanks tantek
<tantek> is here for your W3C obscura
<gregwhitworth> Zakim, end meeting
<Zakim> As of this point the attendees have been scotto_, miriam, JonathanNeal, masonf, bkardell_, stephstimac, una, tantek, flackr, dandclark
<Zakim> RRSAgent, please draft minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2022/05/26-openui-minutes.html Zakim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API
Projects
None yet
Development

No branches or pull requests

4 participants