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

Light dismiss on page scroll? #240

Closed
dandclark opened this issue Dec 10, 2020 · 13 comments
Closed

Light dismiss on page scroll? #240

dandclark opened this issue Dec 10, 2020 · 13 comments
Labels
popover The Popover API select These are issues that relate to the select component

Comments

@dandclark
Copy link
Collaborator

dandclark commented Dec 10, 2020

Discussed this yesterday with @BoCupp-Microsoft, @melanierichards, @ipopescu93

The definition of "light dismiss" at https://open-ui.org/components/select#light-dismiss states that <select> closes when the user hits escape, or when focus moves outside of the listbox due to user action or script.

Should "light dismiss" also occur when the page is scrolled? Our thinking is that yes, it should, because like a focus change this indicates user intent to switch their attention to other content on the page, so a popup like the <select> listbox should be closed.

At the same time we should consider panning (e.g. pinch zoom on a mobile device). We think this should not cause light dismiss, because unlike scrolling, the user is probably still interested in the current content. For example they may be zooming in to see it better, so it would be wrong to close it.

Note, the "light dismiss" definition currently lives inside <select> but I expect it to generalize to other control types that have a popup. See also #241.

Thoughts?

@gregwhitworth
Copy link
Member

@melanierichards did you do any light dismiss research to possibly inform the direction here based on common paradigms or any user studies?

@mrmckeb
Copy link
Collaborator

mrmckeb commented Dec 13, 2020

I've seen this technique many times, but from experience, it's often used as a way to avoid expensive recalculations of absolutely positioned popup containers.

You can see this in effect in this example:
https://developer.microsoft.com/en-us/fluentui#/controls/web/dropdown

If that is the primary reason people are implementing selects in this way, then it might be solved by providing a more flexible select, or a reusable popup component.

@gregwhitworth
Copy link
Member

Looking at a variety of browsers they seem to dismiss on scroll, as @mrmckeb this seems to be common in component libraries as well. Browsers, do not allow scrolling however on the page if hit-testing is on the popup however (this is consistent in all UAs).

Looking at a few other component libs:

  • Evergreen - Doesn't dismiss on scroll and allows scrolling while mouse is over the popup
  • Material - Doesn't dismiss on scroll and allows scrolling while mouse is over the popup
  • Lightning - Uses native select for the popup
  • AntDesign - Doesn't dismiss on scroll and allows scrolling while mouse is over the popup
  • Atlassian - Doesn't dismiss on scroll but doesn't scroll when mouse is over popup
  • Carbon - Uses native select for the popup

I prefer the scrolling to not occur when the mouse is over the popup, especially since some selects will have a scroller and keeping them consistent I think is good. I am a bit indifferent on whether it should dismiss or not if the mouse is outside of the popup. It's a pretty evenly split between component libs and UAs here.

@una
Copy link
Collaborator

una commented Dec 16, 2020

I think this could be an unintended behavior. For example, If one is filling out a form, opens the select, and then realizes the select overflows the page, they would need to scroll down to get it into view. If the select lightdissmiss-es, the user won't know how far down to scroll to re-center it, and it would require another click open. This feels like an annoying behavior, and is particularly relevant on mobile, as you mention above with zoom.

@mrmckeb
Copy link
Collaborator

mrmckeb commented Dec 17, 2020

I think it would be great to reach out to some people that have implemented this behaviour to understand why they did.

My theory is that in many cases, this is a workaround for managing the complexity of fixed position elements on scroll and/or locking scroll when selects are open.

I also prefer that scroll is locked when the select is open. I don't feel that dismissing/closing on scroll is the right path.

@Malvoz
Copy link
Contributor

Malvoz commented Dec 27, 2020

FYI there may be some overlap/similarities between this suggested "light dismiss" (on scroll) and the proposed "modal close signals" (WICG/proposals#18). I.e. the explainer talks about "a custom picker input (e.g. date picker)" but does not mention scrolling as a signal for closing it. Maybe that's a consideration to bring up there?

@una una added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jan 11, 2021
@ericwbailey
Copy link
Collaborator

I agree with @mrmckeb. I'd also caution about this kind of dismissal pattern from a motor disability perspective (hand tremors, Parkinson's, accidental jostling, etc.).

@melanierichards
Copy link
Collaborator

Adding data for native <select>:

  • Chromium-based browsers (Win, Mac): scrolling with mouse "over" select popup window does not dismiss the window. Scrolling elsewhere does dismiss. There seems to be a little bit of buggy behavior where I can get the popup window to stick around, but it behaves as if it is position: fixed, rather than scrolling along with the in-page portion of the control
  • Firefox (Win, Mac): scrolling with mouse "over" select popup window does not dismiss the window. Scrolling elsewhere does dismiss.
  • IE :) (Win): page scroll interactions are completely blocked when the popup is open.
  • Safari (Mac): page scroll interactions are completely blocked when the popup is open.

No one persists the popup and maintains its position relative to the in-page portion of the select control.

@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jan 26, 2021
@melanierichards melanierichards added popover The Popover API select These are issues that relate to the select component labels Apr 12, 2021
@melanierichards
Copy link
Collaborator

Added tags to clarify that this applies to <selectmenu> (customizable select) and popup. Not an exhaustive list for applicability.

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

@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 24, 2022

Another bug was reported that requests that scrolling not trigger light dismiss. In particular, it has a particularly good demo showing a use case that would be broken. (Scroll the text down a bit, and then click on either of the Font or Size pulldowns, and hover a different option. There's a scroll due to changing text size, which light dismisses the selectmenu.)

Reading back through the discussion on this issue, it sounds like the general consensus is also not to light dismiss on scroll, from a user's point of view. The condition to light dismiss on scroll was primarily added (IMHO) as a performance consideration related to anchor positioning. The way the anchor positioning discussion is going, however, sounds like that won't be as important.

@mfreed7 mfreed7 added agenda+ Use this label if you'd like the topic to be added to the meeting agenda and removed stale labels Mar 24, 2022
@css-meeting-bot
Copy link

The Open UI Community Group just discussed LightDismiss on scroll, and agreed to the following:

  • RESOLVED: Proposed resolution: A popup *should not* light-dismiss upon page/container scroll.
The full IRC log of that discussion <gregwhitworth> Topic: LightDismiss on scroll
<gregwhitworth> github: https://github.com//issues/240
<andrico1234> bkardell_: whether a popup should be light dismissed if container scrolls
<andrico1234> wrong name! sorry
<bkardell_> s/bkardell/masonf
<andrico1234> i'm sorry i had trouble with IRC and lost the last 30 seconds!
<una> q+
<andrico1234> masonf: one example on the issue, someone built a text preview page, and pull down fonts, as hovering over fonts, there's a preview of what the font looks like. because fonts are different sizes
<masonf> Proposed resolution: A popup *should not* light-dismiss upon page/container scroll.
<andrico1234> masonf: this causes the element to jank and causes lightdismisses
<dandclark> q+
<gregwhitworth> ack una
<andrico1234> una: i don't want dismissing on scroll, and keep thjat perspective. like on dialog, it's too easily to lose attention and focus of the element if you scroll. it's too easy to scroll away
<andrico1234> una: there are times when to scroll to get more information and then back to initial element. there m ight be an experience where you might want to dismiss on scroll, but shouldn't be default
<gregwhitworth> ack dandclark
<gregwhitworth> +1 to what everyone has said
<davatron5000> +1 to una for it being an add-on
<bkardell_> s/i don't want dismissing on scroll, and keep thjat perspective/Previously I argued that I didn't want dismissing on scroll, and keep that perspective
<andrico1234> dandclark: i agree, i don't see a good way of making it work with the current behaviour, if i want to light missmiss on scroll, it's easy for me to add a scrollhandler and implement that behaviour
<scotto_> also +1 to it being an option for light dismiss, but not a default
<bkardell_> +1
<JonathanNeal> +1 to the proposed resolution
<andrico1234> dandclark: i'm in favour of not light dismissing on scroll
<battaglr> +1
<tantek> +1
<gregwhitworth> RESOLVED: Proposed resolution: A popup *should not* light-dismiss upon page/container scroll.
<tantek> (scrolling shouldn't be stressful about changing state of other things)
<andrico1234> gregwhitworth: consider renaming initallyopened for popup

@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 31, 2022

Based on the discussion generally, and the meeting/resolution just now, I'm going to close this issue. Thanks everyone!

@mfreed7 mfreed7 closed this as completed Mar 31, 2022
@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Mar 31, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
Per [1], we have resolved that scrolling should *not* light
dismiss popups. This CL implements that change.

[1] openui/open-ui#240 (comment)

Bug: 1307772
Change-Id: I8749574733892bc33c1b19af954badb367d24139
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 1, 2022
Per [1], we have resolved that scrolling should *not* light
dismiss popups. This CL implements that change.

[1] openui/open-ui#240 (comment)

Bug: 1307772
Change-Id: I8749574733892bc33c1b19af954badb367d24139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564406
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988054}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
Per [1], we have resolved that scrolling should *not* light
dismiss popups. This CL implements that change.

[1] openui/open-ui#240 (comment)

Bug: 1307772
Change-Id: I8749574733892bc33c1b19af954badb367d24139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564406
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988054}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 1, 2022
Per [1], we have resolved that scrolling should *not* light
dismiss popups. This CL implements that change.

[1] openui/open-ui#240 (comment)

Bug: 1307772
Change-Id: I8749574733892bc33c1b19af954badb367d24139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564406
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988054}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 11, 2022
…from popups, a=testonly

Automatic update from web-platform-tests
Remove scroll-to-light-dismiss behavior from popups

Per [1], we have resolved that scrolling should *not* light
dismiss popups. This CL implements that change.

[1] openui/open-ui#240 (comment)

Bug: 1307772
Change-Id: I8749574733892bc33c1b19af954badb367d24139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564406
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988054}

--

wpt-commits: 5569f87494f50e4d3c55e925f5bc5f8c27c9200d
wpt-pr: 33472
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Apr 14, 2022
…from popups, a=testonly

Automatic update from web-platform-tests
Remove scroll-to-light-dismiss behavior from popups

Per [1], we have resolved that scrolling should *not* light
dismiss popups. This CL implements that change.

[1] openui/open-ui#240 (comment)

Bug: 1307772
Change-Id: I8749574733892bc33c1b19af954badb367d24139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564406
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988054}

--

wpt-commits: 5569f87494f50e4d3c55e925f5bc5f8c27c9200d
wpt-pr: 33472
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per [1], we have resolved that scrolling should *not* light
dismiss popups. This CL implements that change.

[1] openui/open-ui#240 (comment)

Bug: 1307772
Change-Id: I8749574733892bc33c1b19af954badb367d24139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564406
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988054}
NOKEYCHECK=True
GitOrigin-RevId: 6c41e8bcf16bcb75f8840851897569b35b85bd1e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API select These are issues that relate to the select component
Projects
None yet
Development

No branches or pull requests

9 participants