diff --git a/src/chat-prefix-suggestions.ts b/src/chat-prefix-suggestions.ts index 983af790..215230dc 100644 --- a/src/chat-prefix-suggestions.ts +++ b/src/chat-prefix-suggestions.ts @@ -28,3 +28,37 @@ export function isPrefixPopoverUsable( ): boolean { return showPopover && suggestions.length > 0; } + +/** + * Keep the keyboard-selected suggestion visible. + * + * The popover scrolls when it has more suggestions than fit, but arrow keys + * only move the selection, so it would walk out of view. `block: 'nearest'` + * scrolls the list by the smallest amount and leaves it alone when the row is + * already visible. + */ +export function scrollSelectedSuggestionIntoView( + popover: HTMLElement | null, + index: number +): void { + const item = popover?.children[index] as HTMLElement | undefined; + item?.scrollIntoView?.({ block: 'nearest' }); +} + +/** + * The suggestion the keyboard currently points at. + * + * The selected index is kept in state and outlives the list it was chosen + * from: typing narrows the suggestions, so an index picked on row 12 of 30 can + * point past the end of a list of 3. Read as-is it selects nothing, and Enter + * then tries to apply a suggestion that does not exist. Fall back to the first + * row instead, so the selection is always a real row while the list has any. + */ +export function activeSuggestionIndex( + selectedIndex: number, + suggestionCount: number +): number { + return selectedIndex >= 0 && selectedIndex < suggestionCount + ? selectedIndex + : 0; +} diff --git a/src/chat-sidebar.tsx b/src/chat-sidebar.tsx index c0f46e79..24152795 100644 --- a/src/chat-sidebar.tsx +++ b/src/chat-sidebar.tsx @@ -103,8 +103,10 @@ import { recordStoppedTurn } from './chat-stopped-turn'; import { upsertMessageById } from './chat-transcript'; import { IClaudeSessionInfo } from './api'; import { + activeSuggestionIndex, isPrefixPopoverUsable, - prefixesMatching + prefixesMatching, + scrollSelectedSuggestionIntoView } from './chat-prefix-suggestions'; import { NOTEBOOK_GENERATION_PROGRESS_EVENT, @@ -2910,6 +2912,18 @@ function SidebarComponent(props: any) { }; const popoverUsable = isPrefixPopoverUsable(showPopover, prefixSuggestions); + const activePrefixSuggestionIndex = activeSuggestionIndex( + selectedPrefixSuggestionIndex, + prefixSuggestions.length + ); + useEffect(() => { + if (popoverUsable) { + scrollSelectedSuggestionIntoView( + autocompleteRef.current, + activePrefixSuggestionIndex + ); + } + }, [popoverUsable, activePrefixSuggestionIndex]); const applyPrefixSuggestion = async (prefix: string) => { let mcpArguments = ''; @@ -3397,6 +3411,9 @@ function SidebarComponent(props: any) { const filterPrefixSuggestions = (prmpt: string) => { setPrefixSuggestions(prefixesMatching(originalPrefixes, prmpt)); + // The narrowed list is a different list, so start from its first row + // rather than keep an index chosen from the previous one. + setSelectedPrefixSuggestionIndex(0); }; const resetPrefixSuggestions = () => { @@ -3464,7 +3481,7 @@ function SidebarComponent(props: any) { event.stopPropagation(); event.preventDefault(); if (popoverUsable) { - applyPrefixSuggestion(prefixSuggestions[selectedPrefixSuggestionIndex]); + applyPrefixSuggestion(prefixSuggestions[activePrefixSuggestionIndex]); return; } @@ -3475,7 +3492,7 @@ function SidebarComponent(props: any) { if (popoverUsable) { event.stopPropagation(); event.preventDefault(); - applyPrefixSuggestion(prefixSuggestions[selectedPrefixSuggestionIndex]); + applyPrefixSuggestion(prefixSuggestions[activePrefixSuggestionIndex]); return; } } else if (event.key === 'Escape') { @@ -3491,7 +3508,7 @@ function SidebarComponent(props: any) { if (popoverUsable) { setSelectedPrefixSuggestionIndex( - (selectedPrefixSuggestionIndex - 1 + prefixSuggestions.length) % + (activePrefixSuggestionIndex - 1 + prefixSuggestions.length) % prefixSuggestions.length ); return; @@ -3522,7 +3539,7 @@ function SidebarComponent(props: any) { if (popoverUsable) { setSelectedPrefixSuggestionIndex( - (selectedPrefixSuggestionIndex + 1 + prefixSuggestions.length) % + (activePrefixSuggestionIndex + 1 + prefixSuggestions.length) % prefixSuggestions.length ); return; @@ -4619,7 +4636,7 @@ function SidebarComponent(props: any) { {prefixSuggestions.map((prefix, index) => (
prefixSuggestionSelected(event)} > diff --git a/tests/ts/chat-prefix-suggestions.test.ts b/tests/ts/chat-prefix-suggestions.test.ts index 688e29d1..b9d69de1 100644 --- a/tests/ts/chat-prefix-suggestions.test.ts +++ b/tests/ts/chat-prefix-suggestions.test.ts @@ -4,8 +4,10 @@ // popover used to keep claiming Enter after the prompt was typed past every // match, so such a message could not be sent from the keyboard at all. import { + activeSuggestionIndex, isPrefixPopoverUsable, - prefixesMatching + prefixesMatching, + scrollSelectedSuggestionIntoView } from '../../src/chat-prefix-suggestions'; const PREFIXES = ['@mcp', '/clear', '/newNotebook', '/newPythonFile']; @@ -53,3 +55,86 @@ describe('isPrefixPopoverUsable', () => { expect(isPrefixPopoverUsable(false, ['/clear'])).toBe(false); }); }); + +describe('scrollSelectedSuggestionIntoView', () => { + // jsdom does not implement scrollIntoView, so each row gets a spy. + function popoverWithRows(count: number) { + const popover = document.createElement('div'); + const spies: jest.Mock[] = []; + for (let i = 0; i < count; i++) { + const row = document.createElement('div'); + const spy = jest.fn(); + row.scrollIntoView = spy; + spies.push(spy); + popover.appendChild(row); + } + return { popover, spies }; + } + + it('scrolls only the selected row, by the smallest amount', () => { + const { popover, spies } = popoverWithRows(30); + scrollSelectedSuggestionIntoView(popover, 17); + expect(spies[17]).toHaveBeenCalledWith({ block: 'nearest' }); + spies.forEach((spy, i) => { + if (i !== 17) { + expect(spy).not.toHaveBeenCalled(); + } + }); + }); + + it('follows the selection when it wraps from the first row to the last', () => { + const { popover, spies } = popoverWithRows(30); + scrollSelectedSuggestionIntoView(popover, 29); + expect(spies[29]).toHaveBeenCalledTimes(1); + }); + + it('ignores a missing popover, an out-of-range index, or no scrollIntoView', () => { + const { popover } = popoverWithRows(2); + expect(() => scrollSelectedSuggestionIntoView(null, 0)).not.toThrow(); + expect(() => scrollSelectedSuggestionIntoView(popover, 5)).not.toThrow(); + (popover.children[0] as any).scrollIntoView = undefined; + expect(() => scrollSelectedSuggestionIntoView(popover, 0)).not.toThrow(); + }); +}); + +describe('activeSuggestionIndex', () => { + const COMMANDS = Array.from({ length: 30 }, (_, i) => `/command${i}`); + + it('keeps an index that still points at a row', () => { + expect(activeSuggestionIndex(0, 30)).toBe(0); + expect(activeSuggestionIndex(12, 30)).toBe(12); + expect(activeSuggestionIndex(29, 30)).toBe(29); + }); + + it('falls back to the first row when typing narrowed the list past the index', () => { + // Arrow down to row 12 of 30, then type a character that leaves 3 matches. + const narrowed = prefixesMatching(COMMANDS, '/command1'); + expect(narrowed.length).toBeLessThan(13); + // Read as-is the stale index selects nothing, and Enter would then apply + // `undefined`. + expect(narrowed[12]).toBeUndefined(); + const index = activeSuggestionIndex(12, narrowed.length); + expect(narrowed[index]).toBeDefined(); + expect(index).toBe(0); + }); + + it('treats a negative or out-of-range index as the first row', () => { + expect(activeSuggestionIndex(-1, 5)).toBe(0); + expect(activeSuggestionIndex(5, 5)).toBe(0); + }); + + it('returns 0 for an empty list, where the popover claims no keys anyway', () => { + expect(activeSuggestionIndex(3, 0)).toBe(0); + expect(isPrefixPopoverUsable(true, [])).toBe(false); + }); + + it('lets ArrowDown/ArrowUp wrap from a valid row after the list shrinks', () => { + const narrowed = ['/command1', '/command10', '/command11']; + const active = activeSuggestionIndex(12, narrowed.length); + // Same arithmetic as the key handlers in chat-sidebar.tsx. + const down = (active + 1 + narrowed.length) % narrowed.length; + const up = (active - 1 + narrowed.length) % narrowed.length; + expect(narrowed[down]).toBeDefined(); + expect(narrowed[up]).toBe('/command11'); + }); +});