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

Unable to disableScrollHandling from sveltekit:navigation-start event #3220

Closed
nonphoto opened this issue Jan 6, 2022 · 9 comments · Fixed by #4948
Closed

Unable to disableScrollHandling from sveltekit:navigation-start event #3220

nonphoto opened this issue Jan 6, 2022 · 9 comments · Fixed by #4948
Labels
feature request New feature or request router
Milestone

Comments

@nonphoto
Copy link

nonphoto commented Jan 6, 2022

Describe the bug

I'm not sure whether to classify this as a bug or a feature request. disableScrollHandling is very restrictive as to when it can be called. It seems like only actions and onMount work. However, I would like to disable scroll handling for every page on which a specific component is mounted, so I can't use onMount because the component might be mounted across multiple pages. The sveletekit:navigation-start event seems perfect for this, but it is not possible to disable scroll handling from there.

It's worth noting that I tried $: { if ($navigating) { disableScrollHandling() } } and beforeUpdate(disableScrollHandling) and those aren't allowed either

Reproduction

https://stackblitz.com/edit/sveltekit-yzgemn?file=src/routes/index.svelte

Clicking on "About" link causes Error: Can only disable scroll handling during navigation.

Logs

start.js:516 Uncaught (in promise) Error: Can only disable scroll handling during navigation
    at Renderer.disable_scroll_handling (start.js:516)
    at disableScrollHandling_ (navigation.js:27)
    at Router._navigate (start.js:272)
    at start.js:176


### System Info

```shell
System:
    OS: Linux undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: Unknown - /bin/jsh
  Binaries:
    Node: 14.16.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /bin/yarn
    npm: 7.17.0 - /bin/npm
  npmPackages:
    @sveltejs/kit: ^1.0.0-next.216 => 1.0.0-next.216 
    svelte: ^3.42.6 => 3.44.3 


### Severity

serious, but I can work around it

### Additional Information

_No response_
@bluwy
Copy link
Member

bluwy commented Jan 6, 2022

However, I would like to disable scroll handling for every page on which a specific component is mounted, so I can't use onMount because the component might be mounted across multiple pages.

I don't understand this. Why not call disableScrollHandling in the component onMount instead?

This sounds like a feature request, but I'm not sure if we need it as we had documented that it should be called on component mount. For context, the reason calling from sveltekit:navigation-start doesn't work is because the event is emitted at the very start of the navigation, which the new route had not been rendered yet (nor mounted).

@bluwy bluwy added feature request New feature or request router labels Jan 6, 2022
@nonphoto
Copy link
Author

nonphoto commented Jan 6, 2022

As far as I know there's no way to write a component that mounts on every navigation. For example if I want to disable scroll handling from a component at the layout level, that component will only mount when navigating to each page for the first time.

This is maybe just documentation clarity though. It should probably clarify that navigation-start happens before navigation, not at the start of navigation. I would definitely expect $: { if ($navigating) { disableScrollHandling() } } to work though based on the wording in the docs right now.

@bluwy
Copy link
Member

bluwy commented Jan 8, 2022

I think I understand your point now. It would likely be tedious to manually conditionally call disableScrollHandling on a per-page basis (as a workaround), but on one hand it's intended to be that way because we discourage the use of the function.

As far as I know there's no way to write a component that mounts on every navigation. For example if I want to disable scroll handling from a component at the layout level, that component will only mount when navigating to each page for the first time.

This is maybe just documentation clarity though. It should probably clarify that navigation-start happens before navigation, not at the start of navigation. I would definitely expect $: { if ($navigating) { disableScrollHandling() } } to work though based on the wording in the docs right now.

I think start and before are both still ambiguous and interchangeable for the navigation lifecycle, and I think the docs for disableScrollHandling is clear though as it mentions to work for onMount and actions only. When we start a navigation, the new page is not guaranteed to be mounted yet.


Back to the feature request of this issue, I'm trying to think some ways of implementing it, but I feel it would open a can of worms. So maybe the better path forward is to document this, but it's not clear to me what else should we elaborate on.

If you have some ideas, feel free to open a pull request though.

@nonphoto
Copy link
Author

For some more context I'm prototyping a library for transitioning between pages that would need to control the scroll position itself, hence why I would need to disable it on every page. I agree that normally it would make sense to have this be difficult but as a library author I wouldn't want the user to have to add a component to every page file. Ideally just the layout would suffice.

Either way I'll make a pull request for the docs when I get a chance. I think it could just be more explicit that only onMount and actions work.

@ghost
Copy link

ghost commented Jan 26, 2022

I just noticed that server side redirects from an endpoint to the same page also triggers a scroll back to the top. I think given that in such a case Svelte Kit doesn't actually reload the page, but only reruns the load function of the current page component there should at least be a way to turn this behavior off and I can't get this to work. Imagine an app with a simple no js form for a todo app like this:

<form method="post" action="/api/todo">
  <input type="text" name="text" />
  <button>Add Item</button>
</form>

And after saving it to the DB you redirect back to the same page which will rerun load and update the page:

return { status: 302, headers: { location: '/' } };

But it will also scroll back to the top. Couldn't get help on the Discord.

@nonphoto
Copy link
Author

nonphoto commented Feb 6, 2022

@HibiscusCoffee I think that's because form submissions are not handled client side, so you are getting the default browser behavior when the page reloads and starts at the top again. Maybe submit as a separate feature request?

@Rich-Harris
Copy link
Member

As of #4948 you can call disableScrollHandling() inside afterNavigate, which is the solution to this problem. The navigation-start event shouldn't be relied on (we'll very likely get rid of it in an upcoming version).

The form thing is a separate issue, but very much expected since a form submission reloads the page. To prevent that you would need to handle the submit event and preventDefault

@Rich-Harris Rich-Harris added this to the 1.0 milestone May 16, 2022
@nonphoto
Copy link
Author

Thanks! I think that takes care of my issue.

Rich-Harris added a commit that referenced this issue May 23, 2022
Rich-Harris added a commit that referenced this issue May 23, 2022
* allow disableScrollHandling to be called in afterNavigate - fixes #3220

* fix tests

* remove .only

* format
@kvetoslavnovak
Copy link
Contributor

I just noticed that server side redirects from an endpoint to the same page also triggers a scroll back to the top. I think given that in such a case Svelte Kit doesn't actually reload the page, but only reruns the load function of the current page component there should at least be a way to turn this behavior off and I can't get this to work. Imagine an app with a simple no js form for a todo app like this:

<form method="post" action="/api/todo">
  <input type="text" name="text" />
  <button>Add Item</button>
</form>

And after saving it to the DB you redirect back to the same page which will rerun load and update the page:

return { status: 302, headers: { location: '/' } };

But it will also scroll back to the top. Couldn't get help on the Discord.

Having exactly the same problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request router
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants