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

scrollTo feature not working anymore, notifications not visible etc #12625

Closed
tsteur opened this issue Mar 17, 2018 · 6 comments · Fixed by #12642
Closed

scrollTo feature not working anymore, notifications not visible etc #12625

tsteur opened this issue Mar 17, 2018 · 6 comments · Fixed by #12642
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Usability For issues that let users achieve a defined goal more effectively or efficiently.
Milestone

Comments

@tsteur
Copy link
Member

tsteur commented Mar 17, 2018

I noticed when a notification appears, example from an API or any other request error using piwikApi, then the notification is never scrolled to. At first I thought it was because it doesn't wait for angular to render a notification see b801546

This was not the case so I thought $.scrollTo is getting an element instead of a position but that's all correct as well after checking out the docs for it. So I tried various things directly like $.scrollTo(100) or $.scrollTo(document.getElementById('...'), 250) etc. and nothing worked in latest Chrome 64 on Mac.

Seeing that the library is very old I wouldn't be surprised if it is broken. I didn't test any other browsers but this is pretty annoying since you press "create" or "update" somewhere, and nothing happens. Only later when you figure out to scroll around you may notice the error notification if it hasn't disappeared by now.

@tsteur tsteur added Bug For errors / faults / flaws / inconsistencies etc. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. labels Mar 17, 2018
@fdellwing
Copy link
Contributor

There is a native Javascript method for this: https://www.w3schools.com/Jsref/met_element_scrollintoview.asp

Could you try that? Maybe it is just a bug with Chrome 64?

@Findus23
Copy link
Member

I think this has maybe to do that the library is using an old method to scroll that seems to have been removed.
https://stackoverflow.com/questions/45061901/chrome-61-body-doesnt-scroll

But as scrolltoView() is supported by every browser, that seems to be a simple solution:
https://caniuse.com/#feat=scrollintoview

@tsteur
Copy link
Member Author

tsteur commented Mar 18, 2018

If we use scrollIntoView need to make sure it works on IE 10+ / Edge as well but should as we wouldn't set smooth. As $.scrollTo seems to be only used in 2 places and they are all based on a node, scrolltoView() should work 👍

@tsteur
Copy link
Member Author

tsteur commented Mar 18, 2018

Just tested it with scrollIntoView and it works. However, so far we actually fade it in "smooth" or with some animation lazyScrollTo(): time: Specifies the duration of the animation in ms. Need to see if / how we can do this.

@tsteur tsteur added this to the 3.3.1 milestone Mar 18, 2018
@fdellwing
Copy link
Contributor

There is no really easy way to detect if smooth scrolling is supported, but following code does it: https://codepen.io/Fyrd/pen/RrqLBE

But what to use if not supported? Use the old method and hope?

@diosmosis
Copy link
Member

Found the issue w/ jquery.scrollTo. Here: https://github.com/matomo-org/matomo/blob/3.x-dev/libs/bower_components/jquery.scrollTo/jquery.scrollTo.js#L43 jquery.scrollTo selects the body element as the element to change the scrollLeft/scrollTop, which doesn't work in latest chrome. Using documentElement, however does work.

Unfortunately, I can't find a way to monkey patch this. But there exists a polyfill for smooth scrolling: http://iamdustan.com/smoothscroll/. Would need to test across browsers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Usability For issues that let users achieve a defined goal more effectively or efficiently.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants