Skip to content

fix(combobox): scroll highlighted option into view (comboboxes-5)#3377

Merged
cixzhang merged 1 commit into
mainfrom
navi/fix/combobox-scroll-into-view
Jul 2, 2026
Merged

fix(combobox): scroll highlighted option into view (comboboxes-5)#3377
cixzhang merged 1 commit into
mainfrom
navi/fix/combobox-scroll-into-view

Conversation

@cixzhang

@cixzhang cixzhang commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Part of the accessibility & keyboard-management program (#3343). Fixes comboboxes-5.

Problem

In Selector, MultiSelector, and BaseTypeahead, the option list is a fixed-height (300px max-height) scroll container. When navigating options with the arrow keys, the highlighted option was never scrolled into view. Once keyboard navigation moved past the visible window (~8 items), the virtual cursor walked off-screen — the user could no longer see which option was highlighted, even though aria-activedescendant correctly pointed at it.

CommandPalette already solves this in CommandPaletteItem via a useEffect that calls el.scrollIntoView({block: 'nearest'}) whenever the item becomes highlighted. The three combobox surfaces were missing the equivalent behavior.

Fix

Replicate the proven CommandPalette pattern. Each component already tracks a highlightedIndex and produces stable option element ids (getItemId(index)), so the smallest, least-invasive change is a useEffect keyed on highlightedIndex that scrolls the highlighted option into view while the popup is open:

useEffect(() => {
  if (!popover.isOpen || highlightedIndex < 0) {
    return;
  }
  document
    .getElementById(getItemId(highlightedIndex))
    ?.scrollIntoView?.({block: 'nearest'});
}, [popover.isOpen, highlightedIndex, getItemId]);
  • Only runs when the listbox is actually open and an option is highlighted.
  • Uses {block: 'nearest'} to avoid jumpy scrolling (matches CommandPalette).
  • Client-only (effect), so no SSR concern.

Applied to:

  • packages/core/src/Selector/Selector.tsx
  • packages/core/src/MultiSelector/MultiSelector.tsx
  • packages/core/src/Typeahead/BaseTypeahead.tsx

Tests

Added one keyboard-navigation test per component. Each spies on Element.prototype.scrollIntoView, opens the dropdown, presses ArrowDown a few times, and asserts scrollIntoView was called with {block: 'nearest'} — mirroring the mocking approach used in CommandPaletteItem.test.tsx (jsdom does not implement scrollIntoView).

  • Selector.test.tsx
  • MultiSelector.test.tsx
  • Typeahead.test.tsx (covers BaseTypeahead)

Verification

  • npx tsc --project packages/core/tsconfig.build.json → exit 0
  • npx eslint <changed files> → clean
  • npx vitest run packages/core/src/{Selector,MultiSelector,Typeahead,CommandPalette} → 9 files, 140 tests passing

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 2, 2026
@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 2, 2026 4:36am

Request Review

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@cixzhang cixzhang merged commit 5a5fdfb into main Jul 2, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants