Skip to content

Page scrolls down on load #4065

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

Closed
ezfe opened this issue Feb 23, 2022 · 8 comments · Fixed by #4846
Closed

Page scrolls down on load #4065

ezfe opened this issue Feb 23, 2022 · 8 comments · Fixed by #4846
Labels
bug Something isn't working help wanted PRs welcomed. The implementation details are unlikely to cause debate p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Milestone

Comments

@ezfe
Copy link

ezfe commented Feb 23, 2022

Describe the bug

When the page loads, the window scrolls down to the first element. This means that the margin-top applied above the first element is skipped over.

Reproduction

https://github.com/ezfe/scroll-issue-reproduction

Use npm run dev and open the page. The text will appear at the top of the screen, with the margin-top scrolled up above the top of the screen.

Appears to only happen in Safari.

Logs

[Log] [vite] connecting... (client, line 184)
[Log] [vite] connected. (client, line 211)

System Info

z
  System:
    OS: macOS 12.3
    CPU: (8) arm64 Apple M1
    Memory: 111.86 MB / 8.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 17.4.0 - /opt/homebrew/bin/node
    npm: 8.3.2 - /opt/homebrew/bin/npm
  Browsers:
    Edge: 98.0.1108.56
    Firefox: 98.0
    Safari: 15.4
  npmPackages:
    @sveltejs/adapter-static: next => 1.0.0-next.28 
    @sveltejs/kit: next => 1.0.0-next.278 
    svelte: ^3.44.0 => 3.46.4

Severity

annoyance

Additional Information

No response

@Rich-Harris Rich-Harris added the bug Something isn't working label Mar 2, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone Mar 5, 2022
@JSaretin
Copy link

JSaretin commented Mar 9, 2022

<script>
import { browser } from '$app/env';
if (browser) {
	document.body.scrollIntoView(0);
}
</script>
<slot/>

to get rid of this issue, create a __layout.svelte and add the above code.

@benmccann benmccann added the p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. label Mar 17, 2022
@benmccann benmccann added help wanted PRs welcomed. The implementation details are unlikely to cause debate and removed help wanted labels Apr 4, 2022
@ezfe
Copy link
Author

ezfe commented Apr 26, 2022

This code doesn't work unless I add a small delay. I also had to either scroll to the html element or add scroll-margin-top to the body.

import { browser } from '$app/env';
if (browser) {
	setTimeout(() => {
		document.scrollingElement.scrollIntoView();
	}, 100);
}

I haven't been able to get this to work properly with just CSS.

@mrienstra
Copy link

The description should mention that this issue is dependent on using handle to set ssr to false (as your reproduction does).

disableScrollHandling might be part of a work-around.

This looks like a promising place to start messing around:
https://github.com/sveltejs/kit/blob/master/packages/kit/src/runtime/client/client.js#L291-L327

I'm new to SvelteKit so I can't immediately dive in to this, I'll try to circle back to it.

@ezfe
Copy link
Author

ezfe commented May 3, 2022

Ah, that might explain why I had some trouble reproducing it at first

@timothycohen
Copy link
Contributor

This regressed in b622888. As far as I can tell, it's only reproducible on Safari SPA reload (or tab reopen), not on internal navigation. I'm thinking perhaps L302 could change to root.focus({ preventScroll: true });.

But actually, as far as I understand, this code resets focus on page navigation and then restores the original tabIndex. Forgive the ignorance (also new to SvelteKit), but does this code even need to run on a SPA refresh?

@mrienstra
Copy link

mrienstra commented May 5, 2022

@timothycohen, I tested:

L302 could change to root.focus({ preventScroll: true });

... And it resolved the issue! 🎉

I ran tests before and after, alas I wasn't able to run all tests before, but promisingly, no new failures after.

FYI, Playwright tests are running in Chromium only, so this bug wouldn't be captured by tests, since it only occurs in WebKit
Source: https://discord.com/channels/457912077277855764/750401468569354431/971828447863242842
[edit: putting aside whether or not there is an existing test which would otherwise capture this bug]

@timothycohen
Copy link
Contributor

Ahh, yes, that was me in the discord. I was able to create a fail/pass test only by adding webkit to the Playwright config, and doing so exposed 13 other failing tests. Not sure how the core team wants to handle that, but I'm going to take a look at the other failing webkit enabled tests.

Rich-Harris pushed a commit that referenced this issue May 13, 2022
* prevent scroll when resetting page focus

* changeset
@ZerdoX-x
Copy link

I have the same issue when loading sumsub's widget in iframe.. Not sure how to reproduce it :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted PRs welcomed. The implementation details are unlikely to cause debate p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants