Skip to content

Conversation

@nateeo
Copy link
Contributor

@nateeo nateeo commented Sep 8, 2023

given that beforeNavigate and afterNavigate are fired even on links with data-sveltekit-reload, initially I found it a bit confusing that onNavigate didn't, so it might be good to clarify that in some of the docs.

@changeset-bot
Copy link

changeset-bot bot commented Sep 8, 2023

⚠️ No Changeset found

Latest commit: 6773f0b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@nateeo nateeo changed the title Update onNavigate doc about full-page navigation behaviour docs: Clarify onNavigate's full-page navigation behaviour Sep 8, 2023
@ghostdevv ghostdevv added the documentation Improvements or additions to documentation label Sep 8, 2023

/**
* A lifecycle function that runs the supplied `callback` immediately before we navigate to a new URL.
* A lifecycle function that runs the supplied `callback` immediately before we navigate to a new URL, except during full-page navigations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd to me that we'd mention it only on this one and not on the other lifecycle functions

Suggested change
* A lifecycle function that runs the supplied `callback` immediately before we navigate to a new URL, except during full-page navigations.
* A lifecycle function that runs the supplied `callback` immediately before we navigate to a new URL except during full-page navigations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before/afterNavigate mention that it occurs we navigate to a new URL, which I read as including full-page navigations. Happy to clarify it by mentioning that they run on full-page navigations?

@benmccann benmccann changed the title docs: Clarify onNavigate's full-page navigation behaviour docs: clarify onNavigate's full-page navigation behaviour Sep 9, 2023
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants