Skip to content

fix(Select): keep the user's scroll position when the scroll down button remounts - #2109

Open
xyrolle wants to merge 1 commit into
huntabyte:mainfrom
xyrolle:fix/select-scroll-button-remount-realign
Open

fix(Select): keep the user's scroll position when the scroll down button remounts#2109
xyrolle wants to merge 1 commit into
huntabyte:mainfrom
xyrolle:fix/select-scroll-button-remount-realign

Conversation

@xyrolle

@xyrolle xyrolle commented Aug 19, 2026

Copy link
Copy Markdown

The problem

Scrolling a Select (or Combobox) list back up from the bottom snaps the viewport onto the highlighted item.

SelectScrollDownButtonState realigns the viewport onto the highlighted item on every mounted transition of the scroll down button:

watch(
	() => this.scrollButtonState.mounted,
	() => {
		if (!this.scrollButtonState.mounted) return;
		if (this.scrollIntoViewTimer) clearTimeout(this.scrollIntoViewTimer);
		this.scrollIntoViewTimer = afterSleep(5, () => {
			const activeItem = this.root.highlightedNode;
			if (!activeItem) return;
			this.root.scrollHighlightedNodeIntoView(activeItem);
		});
	}
);

That button renders under {#if canScrollDown}, and canScrollDown goes false at the bottom of the list and true again as soon as the viewport leaves the bottom by more than its paddingTop. So the button unmounts at the bottom and remounts on the user's very first scroll back up — and the realign fires straight into their gesture.

The highlighted item is wherever it was put when the list opened, which for a pointer-driven scroll (no keyboard, cursor never over an item) is the first item. So "align onto the highlight" means "jump to the top".

Repro

  1. Render a Select with a Select.ScrollDownButton and enough items to scroll — the demo on the docs site reproduces it.
  2. Open it with a click. Do not hover an item or use the keyboard, so the highlight stays on the first item.
  3. Scroll to the bottom of the list.
  4. Scroll up by a few pixels.

The viewport jumps back to the top instead of moving a few pixels.

The test added here measures it: from scrollTop 2195 of a 2200 maxScroll, a 5px scroll up leaves the viewport at 0. Where I first hit this — a 25-option list in a 262px viewport, maxScroll 878 — a 5px wheel up from the bottom landed at 124 instead of 873, and a 30px wheel up also landed at 124. With this change they land at 873 and 848.

Relationship to existing fixes

This looks like the same family as two recent ones:

  • fix(Select): scroll jumping #2005 (fix(Select): scroll jumping) introduced contentIsPositioned and routed this exact afterSleep through the new scrollHighlightedNodeIntoView guard. But that guard only checks viewportNode && contentIsPositioned, and both stay true the whole time the content is open — so the remount realign survived it. This is the path that fix did not close.
  • fix: cancel DismissibleLayer afterSleep timer on destroy (#2080) #2087 (fix(DismissibleLayer)) was an afterSleep whose callback ran without a condition that was still true when it fired. Same shape here: by the time the 5ms timer runs, "the button just mounted" no longer implies "the content is still settling".

The fix

SelectContentState gains a userHasScrolled latch. It is set when the user scrolls the viewport by hand — wheel or touchmove on the viewport, or holding a scroll button — and reset when the content closes. The remount realign returns early once it is set.

The realign therefore stays free to converge while the content is settling, and stops the moment the user takes the scroll position over.

Why not isUserScrolling? That flag already exists on SelectScrollButtonImplState and reusing it looked like the obvious one-line fix. It does kill this bug — but it also breaks the settle it is guarding, and the second test here demonstrates that in this repo.

isUserScrolling is set from the scroll listener, and the realign scrolls the viewport itself, so the content's own programmatic scrolls set the flag too. When the scroll buttons sit in the flex flow (as they do in the docs demo), mounting and unmounting them resizes the viewport under the alignment, so the open-time settle needs more than one pass — and with isUserScrolling as the guard the later passes are suppressed by the earlier one's own scroll. Opening a select whose selected item sits far down the list then leaves that item short of fully visible: swapping the guard makes should still scroll the selected item into view when opening fail with expected 244 to be less than or equal to 204, the item sitting exactly one item-height below the fold, while the remount test still passes.

wheel and touchmove are the signals only a person can produce, which is why the latch listens for those rather than for scroll.

What this deliberately does not change

  • Keyboard navigation still scrolls the highlighted item into view — that goes through setHighlightedNode, untouched.
  • The open-time settle is untouched. Nothing latches until the user actually scrolls, so a select that opens scrolled to its selection still does.
  • The scroll buttons' auto-scroll still works; holding one just also marks the position as the user's, so releasing it does not snap back.
  • SelectScrollUpButtonState needed no change — it has no realign-on-mount watch.
  • Dragging a scrollbar is not treated as a user scroll, because the select viewport hides its scrollbar (scrollbar-width: none), so there is none to drag. If that ever changes, scrollend detection would be the thing to add.

One extra line: the watch in SelectScrollButtonImplState ended with a bare if (this.isUserScrolling) return; as its final statement, which does nothing. Removed, since it reads like the guard this bug needed and is not one.

Tests

Two tests in select.browser.test.ts, sharing a new select-scroll-buttons-test.svelte harness:

  • should keep the user's scroll position when the scroll down button remounts — the regression. Against the unpatched build it fails expected 0 to be greater than or equal to 2180, deterministically across all retries.
  • should still scroll the selected item into view when opening — pins the settle this fix must not break, and is what rules out the isUserScrolling variant above.

The existing scroll coverage (select-scroll-jump-test.svelte, from #2005) measures page scroll on open and does not mount the scroll buttons at all — no select harness in the repo does, which is probably why this path stayed uncovered.

The harness takes an overlayScrollButtons flag. The remount test overlays the buttons so their flapping does not change the viewport's scroll geometry, which keeps that test deterministic; in the flow they resize the viewport as they flap and the list settles ~24px short of the bottom. The settle test leaves them in the flow, which is the docs-demo layout and the one where the extra alignment pass matters.

pnpm -F tests test:browser --run src/tests/select passes 74/74 on both chromium and webkit. The rest of the browser suite has some pre-existing failures on my machine (webkit focus-management timeouts, mostly), unchanged by this branch and in components this does not touch.

Happy to rename anything or move the latch if you would rather it live on a different state.

@changeset-bot

changeset-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1eb939d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
bits-ui Patch

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

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor
built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
bits-ui ✅ Ready (View Log) Visit Preview 1b83558

…ton remounts

The scroll down button unmounts at the bottom of the list and remounts as
soon as the viewport leaves it, and its mount effect realigned the viewport
onto the highlighted item. A small scroll up from the bottom therefore
jumped back to the highlighted item, which for a pointer-driven scroll is
the first item at the top of the list.

SelectContentState now carries a userHasScrolled latch, set by wheel and
touchmove on the viewport and by the scroll buttons' own auto-scroll, and
reset when the content closes. The realign is skipped once it is set, so it
still converges while the content is settling and stops once the user owns
the scroll position.
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.

1 participant