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

Jump to bottom

Improve scrolling of dialogs #16783

Merged
merged 5 commits into from Dec 15, 2020
Merged

Improve scrolling of dialogs #16783

merged 5 commits into from Dec 15, 2020

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 24, 2020

Description:

Page will be frozen at the current scroll position and only the dialog will scroll (if too high for window)

Currently only tested in Chrome & Firefox on Windows. We need to do some more cross browser testing before merging.

fixes #14968

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@sgiehl
Improve scrolling of dialogs
e009380 
Page will be frozen at the current scroll position and only the dialog will scroll (if too heigh for window)
@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 24, 2020
@sgiehl sgiehl added this to the 4.1.0 milestone Nov 24, 2020
@diosmosis
Copy link
Member

cross browser testing via browserstack/locally:

  • on windows, IE 11, scrolling in the dialog works, but outside the modal doesn't seem to scroll (not sure if it's related to this or just broken)

image

  • works in windows, edge
  • works in mac/chrome
  • works in mac/firefox
  • works in mac/safari
  • works in ios/safari
  • doesn't quite work in android/chrome, when I open the dialog it scrolls to near the top for some reason (the original scroll position changes)

@diosmosis
Copy link
Member

diosmosis commented Dec 11, 2020 •
edited

code looks good 👍, there are some screenshot test failures

@sgiehl
Copy link
Member Author

sgiehl commented Dec 14, 2020

I've changed that to use $(window).scrollTop() instead of window.scrollY, which should fix the behavior for IE11

@diosmosis diosmosis merged commit 646043b into 4.x-dev Dec 15, 2020
@diosmosis diosmosis deleted the dialogscrolling branch December 15, 2020 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.1.0
Development

Successfully merging this pull request may close these issues.

Screen to smoothly scroll back up to initial position after closing a popover
2 participants
@sgiehl @diosmosis