Skip to content
New issue

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

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

Already on GitHub? # to your account

Bug scrollTo #190

Open
usb248 opened this issue Mar 7, 2017 · 5 comments
Open

Bug scrollTo #190

usb248 opened this issue Mar 7, 2017 · 5 comments

Comments

@usb248
Copy link

usb248 commented Mar 7, 2017

On the demo page, this one is broken "Scoll to top when finished".
Any scroll is trigger .... I'm on last FF Windows 10...

@EragonJ
Copy link
Owner

EragonJ commented Mar 10, 2017

@usb248 what you mean it's broken ? it works well here, any screenshot or console log from inspector ?

@usb248
Copy link
Author

usb248 commented Mar 11, 2017

Yes, obviously

cp

I mean, when the tooltip disappear, any scroll isn't triggered...

@EragonJ
Copy link
Owner

EragonJ commented Mar 15, 2017

@usb248 ok it's kinda weird. But I don't have such environment (windows 10 + FF) to test it. But the error may be related to jquery based on your screenshot. Can you try to download Trip.js and use different version of jquery to give it a try ? thanks.

@parasolx
Copy link

I just review the code and found at line 615:

if (this.settings.backToTopWhenEnded) {
  this.$root.animate({ scrollTop: 0 }, 'slow');
}

The selector return for animate is the root of document which result with "body" element. I can summarized like below:
$('html,body') works in chrome, firefox, safari.
$('body') works in chrome and safari.
$('html') works in firefox.

To make sure it works in most of the browser, the line should directly change like below:

if (this.settings.backToTopWhenEnded) {
   $('html,body').animate({ scrollTop: 0 }, 'slow');
}

Same goes for line 1190:
this.$root.animate({ scrollTop: tripBlockTop - OFFSET }, 'slow');
should change to
$('html, body').animate({ scrollTop: tripBlockTop - OFFSET }, 'slow');

phroggar added a commit to phroggar/Trip.js that referenced this issue Jan 3, 2019
By changing $root from body to html the scrolling is fixed ( see https://stackoverflow.com/questions/46071588/chrome-61-jquery-scrolling-not-working-anymore ). Affects EragonJ#190  and EragonJ#195 

If the sel option is used an leads to a hidden element, the positioning will not work. As a quickfix you can make sure that only visible elements are selected by adding :visible to the CSS selector. Affects EragonJ#168 

Please note that these changes have not been tested on various browsers and the ':visible' change is a bit dirty. But the changes worked for me so far and i wanted them documented for everyone else.
@kojot1234
Copy link
Contributor

kojot1234 commented Feb 19, 2019

@EragonJ Would really appreciate if you could merge the above change and make a new release. Thanks.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants