Skip to content

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

Merged
huntabyte merged 1 commit into
huntabyte:mainfrom
xyrolle:fix/select-scroll-button-remount-realign
Sep 8, 2026
Merged

huntabyte merged 1 commit into
huntabyte:mainfrom
xyrolle:fix/select-scroll-button-remount-realign

Conversation

@xyrolle

@xyrolle xyrolle commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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.

@huntabyte huntabyte left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM. Confirmed the scroll remount regression against the base and verified the fix in Chromium and WebKit. Select and Combobox tests pass, initial alignment and close/reopen behavior are preserved, and the patch changeset is present.

@huntabyte
huntabyte merged commit 157af64 into huntabyte:main Sep 8, 2026
6 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 8, 2026
dadezzz pushed a commit to dadezzz/events-cash-register that referenced this pull request Sep 12, 2026
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [bits-ui](https://github.com/huntabyte/bits-ui) | [`2.19.0` → `2.19.1`](https://renovatebot.com/diffs/npm/bits-ui/2.19.0/2.19.1) | ![age](https://developer.mend.io/api/mc/badges/age/npm/bits-ui/2.19.1?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/bits-ui/2.19.0/2.19.1?slim=true) |

---

### Release Notes

<details>
<summary>huntabyte/bits-ui (bits-ui)</summary>

### [`v2.19.1`](https://github.com/huntabyte/bits-ui/releases/tag/bits-ui%402.19.1)

[Compare Source](https://github.com/huntabyte/bits-ui/compare/bits-ui@2.19.0...bits-ui@2.19.1)

##### Patch Changes

- fix(Avatar): detach image handlers on destroy and actually apply the load cleanup - [#&#8203;2050](huntabyte/bits-ui#2050) ([#&#8203;2128](huntabyte/bits-ui#2128))

- fix(Menu): `Menu.GroupHeading` no longer sets `role="group"`, `Menu.Separator` now sets `role="separator"` ([#&#8203;2091](huntabyte/bits-ui#2091))

- fix(DismissibleLayer): guard the deferred focus handler against teardown - [#&#8203;2080](huntabyte/bits-ui#2080) ([#&#8203;2126](huntabyte/bits-ui#2126))

- fix(DateField): apply date segments in year/month/day order so a valid day isn't clamped by the placeholder's month in day-first locales ([#&#8203;2123](huntabyte/bits-ui#2123))

- fix(Select): keep the user's scroll position when the scroll down button remounts. The 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, so a small scroll up from the bottom jumped back to the highlighted item. ([#&#8203;2109](huntabyte/bits-ui#2109))

- fix(DismissibleLayer): outside clicks shortly after a layer opens no longer fail to dismiss it ([#&#8203;2111](huntabyte/bits-ui#2111))

  `DismissibleLayerState` reset its per-interaction state through a 20ms debounce. Because the
  layer's `watch` runs its cleanup once on every open, each layer scheduled a reset 20ms into its
  own lifetime. An outside `pointerdown` landing 10-20ms after that cleanup had its
  "responsible layer" flag cleared by the stale reset before the debounced interact-outside
  handler ran, so the handler bailed and the layer stayed open. The reset is now synchronous.

- fix(ScrollArea): prevent resize work from reading destroyed state after unmount ([#&#8203;2122](huntabyte/bits-ui#2122))

- fix(Combobox): highlight the first matching item after custom filtering updates the rendered items. ([#&#8203;2129](huntabyte/bits-ui#2129))

- fix(Accordion): cancel deferred content work on destroy to avoid `derived_inert` ([#&#8203;2127](huntabyte/bits-ui#2127))

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC43OS42IiwidXBkYXRlZEluVmVyIjoiNDQuNzkuNiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->
@xyrolle

xyrolle commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Just noticed that you've merged this one, great! I'll keep the fixes coming then, I have lots and lots of local patches 🙂

xyrolle added a commit to xyrolle/bits-ui that referenced this pull request Sep 16, 2026
…ontent settles

Positioning is not the end of settling: chrome that mounts once the content is placed (scroll buttons in the flex flow, a header, a footer) shrinks the viewport under the open-time alignment and left a far-down selection below the fold. The content now watches the viewport with a ResizeObserver and realigns on every resize until the user scrolls, which retires the scroll down button's mount-time realign — the code huntabyte#2109 had to guard — and moves the wheel/touchmove latch onto the content so a select without scroll buttons has it too.

This branch was successfully deployed

1 active deployment
Preview — 1b835588 Deployed Aug 19, 2026 by github-actions[bot]
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