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

Change $root to html and only use visible element #198

Closed
wants to merge 1 commit into from

Conversation

phroggar
Copy link

@phroggar phroggar commented 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 #190 and #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 #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.

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.
@EragonJ
Copy link
Owner

EragonJ commented Feb 21, 2019

Thanks for help, but it's weird to use :visible selector here because I think it's dev's main job to handle this edge case not by Trip.js. I already merged a patch to fix html > body issue, so close this one!

@EragonJ EragonJ closed this Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants