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

[Dialog] scrolling UX is bad because dialog is not scrolling when hovered overlay #1313

Closed
liesislukas opened this issue Jul 1, 2017 · 5 comments

Comments

@liesislukas
Copy link

Hi, i'm making usability testing and already see a lot feedback about bad UX with dialogs because when user is trying to scroll dialog scrolling does work only when mouse cursors is over the dialog contents if mouse is over the overlay scroll scrolls background page but not dialog, which is primary active content at that moment.

At least there should be a property if you really want to let user scroll background page when dialog is active.

Bug report

  • Package version(s): 1.21.0
  • Browser and OS versions: Version 59.0.3071.115 (Build officiel) (64 bits)

Steps to reproduce

  1. create random site with some html/text
  2. use dialog component Dialog
  3. scroll while hovered over dialog and when hovered overlay

Actual behavior

scroll is working not like users expect it to work. Bad ux.

Expected behavior

Dialog is scrolling always no matter where the mouse is, while dialog is primary active content.

image

@giladgray
Copy link
Contributor

@liesislukas huh interesting, dialogs on the docs site do not exhibit this behavior, and i can't figure out from inspecting your CSS why your page would scroll. any suggestions?
thanks for the detailed report!

@liesislukas
Copy link
Author

liesislukas commented Jul 2, 2017

@giladgray In fact on docs site dialog sometimes works one way, sometimes another. Here's a short video:

https://youtu.be/5grSqlJvDWM

you can check my dialog here:

https://www.geekystats.com/company/1/seoRank/monitor?viewMore=1&viewMoreTab=keyword

Use login: demo@gmail.com and psw: demo@gmail.com

p.s. if you find geekystats.com interesting just register with your email and i will send you notification when it's ready 👍

@llorca llorca self-assigned this Jul 13, 2017
@llorca llorca removed their assignment Jul 24, 2017
@giladgray
Copy link
Contributor

giladgray commented Aug 10, 2017

@liesislukas we've narrowed this issue down to something entirely out of our control: retina/HiDPI screens seem to support scrolling from the backdrop, but normal monitors do not. this can be repro'ed readily by plugging an external monitor into a Retina MBP and dragging a chrome window back and forth between monitors.

#1373 attempts to fix some of the scrolling woes in CSS but there seems to be little we can do about this particular issue.

@giladgray
Copy link
Contributor

@liesislukas actually ignore my comment above, #1373 totally fixes this now!

giladgray pushed a commit that referenced this issue Aug 10, 2017
…r scrolling

requires re-implementing canOutsideClickClose but it's just dead simple
fixes #1313
giladgray pushed a commit that referenced this issue Aug 10, 2017
#1373)

* Experimental support for both absolute and flex positioning

* Tweak positioning for scroll vs static

* fix ContextMenu close events by using rAF to delay hide() only

revert changes to Overlay, fix test, use rAF for popover focus logic

* ⭐ rewrite Overlay CSS positioning logic for maximum effect!

* update other component styles for latest Overlay
mostly reducing duplication!

* Disable text selection on backdrop

* Even out toast margin / toast container padding

* restore support for old container-less Dialog position

* revert padding/margin changes for legacy compat

* replace pointer-events with user-select on dialog to support container scrolling
requires re-implementing canOutsideClickClose but it's just dead simple
fixes #1313

* add .pt-dialog-container to markup example, remove custom docs section style

* fix isClickOutsideDialog logic
@liesislukas
Copy link
Author

liesislukas commented Aug 30, 2017

just updated to 1.26.0, I confirm that update fixes this issue on my app. Thank you :) 👍

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

No branches or pull requests

3 participants