-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[fix] properly scroll if body has margin #2761
Conversation
🦋 Changeset detectedLatest commit: de4aacf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@benmccann I made quite a big change since your review. The main takeaways are noted in the PR description, and in the code comments. I feel like the implementation is starting to get hacky, but unless we find a better way, we may have to stick with it for now. |
packages/kit/test/apps/basics/src/routes/anchor-with-manual-scroll/anchor.svelte
Show resolved
Hide resolved
Thank you @bluwy for this PR. I really appreciate your work on this project. Can I help this PR to land by testing or anything? Or are we just waiting on a review? |
Thanks @gavinr. Yeah I'm currently waiting for Ben's review as he has more experience with the router. If you can help test this branch, that would be nice as well to bring more confidence in the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this with the reproduction case in #2760 and it does resolve that issue. Thank you!
niiiice, amazing work |
game changer! @Wolfr check this. |
Fixes #2760
Changes
document.documentElement
instead ofdocument.body
when checking for max scroll yNote: We may need to find a robust way to check for scrolling before the page
onMount()
runs. ThepageYOffset
trick is starting to bite us.Before submitting the PR, please make sure you do the following
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpx changeset
and following the prompts. All changesets should bepatch
until SvelteKit 1.0