Skip to content
This repository has been archived by the owner on Feb 11, 2023. It is now read-only.

Scroll start always from the top of the page rather than from the current position #25

Closed
crs1138 opened this issue May 15, 2018 · 5 comments
Closed

Scroll start always from the top of the page rather than from the current position #25

crs1138 opened this issue May 15, 2018 · 5 comments

Comments

@crs1138
Copy link

crs1138 commented May 15, 2018

I am using Isotope2 and Scroll-JS on a project. There's a number of grid items in a Masonry layout. When a user clicks on a grid item it expands to full width and I want this item to scroll to the top of the viewport. My problem is that instead of scrolling from the current position within the page, the scroll starts from the top of the page. I've made a simplified case study in Codepen.
@markcellus
Copy link
Owner

Hey! Thanks for using the package and for your contribution. The problem is that the package detects current scroll position by evaluating document.body.scrollTop. But doesn't consistently return the correct scroll distance and is discouraged in modern browsers (in strict mode anyway).

To quickly fix this for your use case, just pass document.documentElement to the Scroll constructor instead of document.body.

new Scroll(document.documentElement);

and that should do the trick! Let me know if that works.

markcellus pushed a commit that referenced this issue May 16, 2018
Fix inaccurate current scroll position in #25
bade606
@markcellus
Copy link
Owner

@crs1138 I've opened #27 to fix this issue. Do you mind pulling that down and verifying that it fixes your issue?

markcellus pushed a commit that referenced this issue May 16, 2018
Fix inaccurate current scroll position in #25
809214c
markcellus pushed a commit that referenced this issue May 16, 2018
Fix inaccurate current scroll position in #25
2ffb0a4
markcellus pushed a commit that referenced this issue May 16, 2018
Fix inaccurate current scroll position in #25
a342d6a
markcellus pushed a commit that referenced this issue May 16, 2018
Merge pull request #27 from mkay581/update_to_document_element
9ab7d65 
Fix inaccurate current scroll position in #25
@markcellus markcellus mentioned this issue May 16, 2018
Merged
@markcellus
Copy link
Owner

markcellus commented May 17, 2018 •
edited

@crs1138 there seems to be a problem with your example. all of the grid-items are absolutely positioned and the parent element (<body> in your example) has no overflow content. Per README, your parent element needs to always have overflow content.

In addition to that, I suspect there is something being done to the elements in the other JS files that is causing your page to redraw back to 0 in height.

Either way, i'm not able to reproduce on the scroll-to-element demo, which seems to work fine even when clicking an elements further down on the page.

Going to close this for now, but if you are able to remove the additional JS files and reproduce the issue using only this package, that will give me something concrete to help fix your issue.

@crs1138
Copy link
Author

crs1138 commented May 17, 2018

It is the combo of using it with Isotope2 that has been crucial to my project as I was looking for a way to bring focus to an expanded grid item. Without that, there's no need for the scrolling. Yes, Isotope positions absolutely all the items and I should have read your readme instruction more carefully. Thanks for the time you put in.

@mraerino
Copy link

I'm still experiencing this with 1.9.1 - therefore I opened #28
Unfortunately using document.documentElement will not work on Safari...
Feel free to close my duplicate and reopen this.

@mraerino mraerino mentioned this issue Jun 20, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@crs1138 @markcellus @mraerino