fix: scroll the selected item into view by value, not stale aria-selected (#389)

cmdk/src/index.tsx

@@ -470,7 +470,13 @@ const Command = React.forwardRef<HTMLDivElement, CommandProps>((props, forwarded
   }
 
   function scrollSelectedIntoView() {
-    const item = getSelectedItem()
+    // Resolve the selected item from `state.current.value` (the source of truth)
+    // rather than `getSelectedItem()`, which reads `[aria-selected="true"]`. On a
+    // search change the newly-selected item is already in the DOM, but its
+    // `aria-selected` attribute only flips on the next React commit, so scrolling
+    // by that attribute lands on the previously-selected item (#389). The item's
+    // `data-value` already matches the new selection, so resolve by that instead.
+    const item = getSelectedItemByValue()
 
     if (item) {
       if (item.parentElement?.firstChild === item) {
@@ -489,6 +495,20 @@ const Command = React.forwardRef<HTMLDivElement, CommandProps>((props, forwarded
     return listInnerRef.current?.querySelector(`${ITEM_SELECTOR}[aria-selected="true"]`)
   }
 
+  /**
+   * Resolve the selected item element from `state.current.value` instead of the
+   * `aria-selected` attribute, which lags one React commit behind a value change
+   * (see `scrollSelectedIntoView`). Comparing `data-value` in JS rather than via a
+   * built selector also avoids escaping arbitrary values into `querySelector` (#387).
+   */
+  function getSelectedItemByValue() {
+    const value = state.current.value
+    if (!value) return undefined
+    return Array.from(listInnerRef.current?.querySelectorAll(ITEM_SELECTOR) || []).find(
+      (item) => item.getAttribute(VALUE_ATTR) === value,
+    )
+  }
+
   function getValidItems() {
     return Array.from(listInnerRef.current?.querySelectorAll(VALID_ITEM_SELECTOR) || [])
   }

test/pages/scroll.tsx

@@ -0,0 +1,23 @@
+import { Command } from 'cmdk'
+import * as React from 'react'
+
+// 50 fixed-height items in a short, scrollable list. Zero-padded, distinct values
+// so a search can narrow to a single item far down the list, then broaden back.
+const items = Array.from({ length: 50 }, (_, i) => `item-${String(i).padStart(2, '0')}`)
+
+const Page = () => {
+  return (
+    <Command>
+      <Command.Input placeholder="Search…" />
+      <Command.List style={{ height: 120, overflow: 'auto' }}>
+        {items.map((value) => (
+          <Command.Item key={value} value={value} style={{ height: 30 }}>
+            {value}
+          </Command.Item>
+        ))}
+      </Command.List>
+    </Command>
+  )
+}
+
+export default Page

test/scroll.test.ts

@@ -0,0 +1,28 @@
+import { expect, test } from '@playwright/test'
+
+test.describe('scroll selected item into view', async () => {
+  test.beforeEach(async ({ page }) => {
+    await page.goto('/scroll')
+  })
+
+  // Regression for #389. When a search change selects a new first item, the list
+  // must scroll to that item, not the one that was selected before the change.
+  // The bug: scrollSelectedIntoView read `[aria-selected="true"]`, which lags one
+  // React commit behind the value change, so it scrolled to the stale selection.
+  test('scrolls to the new first item after broadening the search (#389)', async ({ page }) => {
+    const input = page.locator('[cmdk-input]')
+    const list = page.locator('[cmdk-list]')
+
+    // Narrow to a single item far down the list; it becomes the selection.
+    await input.fill('item-49')
+    await expect(page.locator('[cmdk-item][aria-selected="true"]')).toHaveAttribute('data-value', 'item-49')
+
+    // Broaden: every item returns and "item-00" becomes the selected first item.
+    await input.fill('')
+    await expect(page.locator('[cmdk-item][aria-selected="true"]')).toHaveAttribute('data-value', 'item-00')
+
+    // The list must be scrolled back to the top (the new selection), not left
+    // parked at the stale "item-49" position near the bottom.
+    await expect.poll(() => list.evaluate((el) => el.scrollTop)).toBeLessThan(30)
+  })
+})