Conversation
…igation
AppDefault creates a TanStack router on a memory history and passed no scroll
options. router-core runs setupScrollRestoration on every client router whatever
the history, and its onRendered handler calls window.scrollTo({ top: 0, left: 0 })
after each navigation unless that navigation passed resetScroll: false. The
widget's own navigations never pass it, so opening the From/To token list,
selecting a token, settings or route details scrolled the host page to the top.
On a phone, where the widget usually sits below the fold, every tap moved the
user away from the widget.
The router is now created with scrollRestoration: () => false. router-core calls
that function from onRendered and returns early when it is false, before any
scroll work, so nothing touches the window the widget does not own.
The test pins both halves: that the option is a function and that it returns
false, plus that the router still uses a memory history with preload by intent.
Removing the option makes it fail.
🦋 Changeset detectedLatest commit: aa94b5e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which Linear task is linked to this PR?
No Linear task linked. GitHub issue: fixes #885.
Why was it implemented this way?
AppDefaultbuilds the widget's router with no scroll options:router-core calls
setupScrollRestorationfor every client router, whatever the history. ItsonRenderedhandler ends by scrolling the window to the top unless the navigation passedresetScroll: false(router._scroll.next = next.resetScroll ?? true), and the widget's own navigations never pass it. So opening the From/To token list, selecting a token, settings or route details scrolled the host page back to the top. On a phone, where the widget usually sits below the fold, every tap moved the user away from the widget.scrollRestoration: () => falseis the documented option for this case. Inscroll-restoration.js:Returning
falseexits before any scroll work, so nothing touches a window the widget does not own. An embedded widget on a memory history should never scroll the host page.Alternatives considered:
resetScroll: falseon each internal navigation. That is a per-call-site fix in a dozen places, and any navigation added later silently reintroduces the bug. The router-level option covers all of them, present and future.No public API change, and no change to scrolling inside the widget: its own lists and pages keep their positions.
Visual showcase (Screenshots or Videos)
Not attached. The repro in #885 is
window.scrollYgoing 569 → 0 when the To token list opens in a host page with content above the widget (Chrome, 390×844). The scroll itself is performed by router-core, so the test added here pins the router option instead of replaying the scroll.Checklist before requesting a review
Verification
pnpm --filter @lifi/widget test— 30 files, 321 tests passedpnpm --filter @lifi/widget check:types— cleanbiome check— cleansrc/AppDefault.test.tsxpins the option and asserts it returnsfalse. RemovingscrollRestoration: () => falsemakes it fail on that assertion.