Skip to content

fix(Select): keep aligning the highlighted item while the content settles - #2157

Open
xyrolle wants to merge 1 commit into
huntabyte:mainfrom
xyrolle:fix/select-align-while-settling
Open

xyrolle wants to merge 1 commit into
huntabyte:mainfrom
xyrolle:fix/select-align-while-settling

Conversation

@xyrolle

@xyrolle xyrolle commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

The problem

When a Select or Combobox opens, SelectContentState scrolls the highlighted item into view once the content is positioned (and again whenever the highlight changes):

watch([() => this.isPositioned, () => this.root.highlightedNode], () => {
	if (!this.isPositioned || !this.root.highlightedNode) return;
	this.root.scrollHighlightedNodeIntoView(this.root.highlightedNode);
});

Positioning is not the end of settling. Chrome that mounts after the content is placed — ScrollDownButton in the flex flow (it renders under {#if canScrollDown}, which is only known once the viewport has been measured), a header, a footer, a "loading" row that resolves — takes its height from the viewport, and the alignment that just put the selection on screen is undone by the resize. Open a select whose selected item sits far down the list, with a footer that appears after placement, and the selection is left below the fold by the footer's height.

The scroll buttons paper over their own case with a second alignment in SelectScrollDownButtonState, run 5 ms after the button mounts. That is the code #2109 had to guard: the button remounts every time the viewport leaves the bottom, so the mount-time realign fired into the user's scroll gesture. The guard (userHasScrolled) fixed the symptom; the mount event was never the right trigger. What the alignment actually needs to follow is the viewport's size.

The fix

The content watches the viewport with a ResizeObserver from the moment it is positioned, and realigns the highlighted item on every resize until the user takes the scroll position over — the userHasScrolled latch from #2109, now set by the content itself from wheel / touchmove on the viewport (and, as before, by pressing or moving over a scroll button). Once latched, the observer stops calling the alignment; keyboard navigation's own scroll-into-view is untouched.

That makes the scroll down button's mount-time realign redundant, so it is deleted along with its timer; the button's scroll listener for canScrollDown stays. The wheel / touchmove latch listeners move from SelectScrollDownButtonState — where they existed only if the app rendered <Select.ScrollDownButton> at all (the state is constructed outside the button's {#if}) — to the content, so a select without scroll buttons now has the latch too.

One behavioural difference worth naming: a scroll button remount that does not change the viewport's size (overlaid buttons) no longer triggers an alignment. Nothing needed that alignment — the highlighted item was already aligned at positioning — and it is the case #2109's regression test covers.

Not latched, as before: programmatic scrollTop writes, and scrollbar drags (the viewport hides its scrollbar).

Tests

New select-settling-content-test.svelte — 60 items in a 200 px content, no scroll buttons, and a footer inside the content that starts at 0 px — with two tests in select.browser.test.ts:

  • should keep the selected item in view while chrome mounts after the content is positioned — opens on the last item, waits for it to be in view and for the observer's initial notification to be behind us, then grows the footer to 48 px and, after that settles, to 96 px, expecting the item back in view after each. Two separately settled resizes so that a single delayed alignment (or the observer's initial notification alone) cannot pass it. Against unpatched main it fails on the first resize: the item is left 48 px below the viewport.
  • should stop realigning once the user has scrolled — same setup, then a wheel on the viewport and a scroll to the top, then the footer grows; after the observer has delivered, the viewport must still be at 0. As a control, with only the userHasScrolled guard removed from the observer callback this test fails (the viewport is realigned onto the last item), so it does exercise the observer, not just the absence of one.

The #2109 tests still pass: should keep the user's scroll position when the scroll down button remounts and should still scroll the selected item into view when opening (in-flow buttons — this is the case the deleted mount-time realign existed for, and the resize observer now covers it).

pnpm -F tests test:browser --run src/tests/select src/tests/combobox passes on chromium and webkit (282 tests). Synthetic wheel and scrollTop writes stand in for the gesture; they test the latch wiring, not native scrolling.

…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.
@xyrolle

xyrolle commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Run evidence (macOS, chromium + webkit via @vitest/browser + playwright, retry: 3 as configured; each block lists the per-file result line, the totals, and every failing test name after retries):

New tests against unpatched main (chromium, -t "Settling content") — d-red.log

  • ❯ |browser (chromium)| src/tests/select/select.browser.test.ts (76 tests | 1 failed | 74 skipped) 4490ms
  • Tests 1 failed | 1 passed | 74 skipped (76)
  • failing tests: Settling content > should keep the selected item in view while chrome mounts after the content is positioned

Branch: select + combobox, chromium + webkit — d2.log

  • ✓ |browser (chromium)| src/tests/combobox/combobox.browser.test.ts (65 tests) 2303ms
  • ✓ |browser (webkit)| src/tests/combobox/combobox.browser.test.ts (65 tests) 2722ms
  • ✓ |browser (chromium)| src/tests/select/select.browser.test.ts (76 tests) 3936ms
  • ✓ |browser (webkit)| src/tests/select/select.browser.test.ts (76 tests) 4892ms
  • Tests 282 passed (282)

Control: branch with only the userHasScrolled guard removed from the observer callback (chromium, -t "stop realigning") — d2-latch-red.log

  • ❯ |browser (chromium)| src/tests/select/select.browser.test.ts (76 tests | 1 failed | 75 skipped) 424ms
  • Tests 1 failed | 75 skipped (76)
  • failing tests: Settling content > should stop realigning once the user has scrolled

svelte-check — d2-check.log

  • svelte-check found 0 errors and 0 warnings

@changeset-bot

changeset-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 82a6a33

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 Sep 16, 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 82a6a33

This branch was successfully deployed

1 active deployment
Preview — 82a6a339 Deployed Sep 16, 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.

1 participant