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

feat(router): configuration of scroll behavior for Link and useRouter #1231

Merged
merged 7 commits into from
Feb 17, 2025

Conversation

tylersayshi
Copy link
Contributor

follow up for: #1098 (comment)

cc @wesbos thanks for this call out

Copy link

vercel bot commented Feb 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
waku ⬜️ Ignored (Inspect) Visit Preview Feb 17, 2025 7:07am

Copy link

codesandbox-ci bot commented Feb 15, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@tylersayshi
Copy link
Contributor Author

@dai-shi all very true. I didn't give the api enough though. How about something like this:

/**
 * Indicates that the link should not scroll when clicked. By default, waku will 
 * scroll when the path changes. This is a way to explicitly not change the scroll 
 * position on navigation.
 */
 dontScroll?: boolean;

alternate names: disableScroll or noScroll

Let me know what you think and I'll change the code accordingly

@dai-shi
Copy link
Owner

dai-shi commented Feb 16, 2025

@tylersayshi I'm afraid my comment wasn't very clear.

I'm actually fine with scroll naming, but the options are:

  • undefined (default): scroll based on newPath check (= current Link behavior)
  • false: no scroll
  • true: always scroll

Copy link
Owner

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

LGTM

@dai-shi dai-shi merged commit 51961d0 into dai-shi:main Feb 17, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants