fix: keep palette keyboard selection stable
This commit is contained in:
@@ -278,6 +278,28 @@ describe('CommandPalette', () => {
|
|||||||
expect(onClose).toHaveBeenCalled()
|
expect(onClose).toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('keeps keyboard selection stable when the mouse is already over a row', () => {
|
||||||
|
render(<CommandPalette open={true} commands={commands} onClose={onClose} />)
|
||||||
|
|
||||||
|
fireEvent.mouseEnter(screen.getByText('Commit & Push'))
|
||||||
|
fireEvent.keyDown(window, { key: 'ArrowDown' })
|
||||||
|
fireEvent.keyDown(window, { key: 'Enter' })
|
||||||
|
|
||||||
|
expect(commands[1].execute).toHaveBeenCalledOnce()
|
||||||
|
expect(commands[2].execute).not.toHaveBeenCalled()
|
||||||
|
expect(onClose).toHaveBeenCalledOnce()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('lets real mouse movement take over command selection', () => {
|
||||||
|
render(<CommandPalette open={true} commands={commands} onClose={onClose} />)
|
||||||
|
|
||||||
|
fireEvent.mouseMove(screen.getByText('Commit & Push'))
|
||||||
|
fireEvent.keyDown(window, { key: 'Enter' })
|
||||||
|
|
||||||
|
expect(commands[2].execute).toHaveBeenCalledOnce()
|
||||||
|
expect(onClose).toHaveBeenCalledOnce()
|
||||||
|
})
|
||||||
|
|
||||||
it('keeps a short query keyboard-selectable after ArrowDown and Enter', () => {
|
it('keeps a short query keyboard-selectable after ArrowDown and Enter', () => {
|
||||||
const changeNoteType = makeCommand({
|
const changeNoteType = makeCommand({
|
||||||
id: 'change-note-type',
|
id: 'change-note-type',
|
||||||
|
|||||||
@@ -484,7 +484,7 @@ function CommandRow({ command, selected, onHover, onSelect }: CommandRowProps) {
|
|||||||
selected ? 'bg-accent' : 'hover:bg-secondary',
|
selected ? 'bg-accent' : 'hover:bg-secondary',
|
||||||
)}
|
)}
|
||||||
onClick={onSelect}
|
onClick={onSelect}
|
||||||
onMouseEnter={onHover}
|
onMouseMove={onHover}
|
||||||
>
|
>
|
||||||
<span className="text-sm text-foreground">{command.label}</span>
|
<span className="text-sm text-foreground">{command.label}</span>
|
||||||
{command.shortcut && (
|
{command.shortcut && (
|
||||||
|
|||||||
@@ -33,6 +33,34 @@ describe('NoteSearchList', () => {
|
|||||||
const onItemClick = vi.fn()
|
const onItemClick = vi.fn()
|
||||||
const onItemHover = vi.fn()
|
const onItemHover = vi.fn()
|
||||||
|
|
||||||
|
const renderList = ({
|
||||||
|
listItems = items,
|
||||||
|
selectedIndex = 0,
|
||||||
|
getItemKey = (item: TestItem) => item.id,
|
||||||
|
itemClick = onItemClick,
|
||||||
|
itemHover,
|
||||||
|
activateOnMouseDown,
|
||||||
|
emptyMessage,
|
||||||
|
}: {
|
||||||
|
listItems?: TestItem[]
|
||||||
|
selectedIndex?: number
|
||||||
|
getItemKey?: (item: TestItem, index: number) => string
|
||||||
|
itemClick?: (item: TestItem, index: number) => void
|
||||||
|
itemHover?: (index: number) => void
|
||||||
|
activateOnMouseDown?: boolean
|
||||||
|
emptyMessage?: string
|
||||||
|
} = {}) => render(
|
||||||
|
<NoteSearchList
|
||||||
|
items={listItems}
|
||||||
|
selectedIndex={selectedIndex}
|
||||||
|
getItemKey={getItemKey}
|
||||||
|
onItemClick={itemClick}
|
||||||
|
onItemHover={itemHover}
|
||||||
|
activateOnMouseDown={activateOnMouseDown}
|
||||||
|
emptyMessage={emptyMessage}
|
||||||
|
/>,
|
||||||
|
)
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks()
|
vi.clearAllMocks()
|
||||||
})
|
})
|
||||||
@@ -112,39 +140,17 @@ describe('NoteSearchList', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
it('shows empty message when no items', () => {
|
it('shows empty message when no items', () => {
|
||||||
render(
|
renderList({ listItems: [], getItemKey: () => '', emptyMessage: 'No matching notes' })
|
||||||
<NoteSearchList
|
|
||||||
items={[]}
|
|
||||||
selectedIndex={0}
|
|
||||||
getItemKey={() => ''}
|
|
||||||
onItemClick={onItemClick}
|
|
||||||
emptyMessage="No matching notes"
|
|
||||||
/>,
|
|
||||||
)
|
|
||||||
expect(screen.getByText('No matching notes')).toBeInTheDocument()
|
expect(screen.getByText('No matching notes')).toBeInTheDocument()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('shows default empty message', () => {
|
it('shows default empty message', () => {
|
||||||
render(
|
renderList({ listItems: [], getItemKey: () => '' })
|
||||||
<NoteSearchList
|
|
||||||
items={[]}
|
|
||||||
selectedIndex={0}
|
|
||||||
getItemKey={() => ''}
|
|
||||||
onItemClick={onItemClick}
|
|
||||||
/>,
|
|
||||||
)
|
|
||||||
expect(screen.getByText('No results')).toBeInTheDocument()
|
expect(screen.getByText('No results')).toBeInTheDocument()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('calls onItemClick when an item is clicked', () => {
|
it('calls onItemClick when an item is clicked', () => {
|
||||||
render(
|
renderList()
|
||||||
<NoteSearchList
|
|
||||||
items={items}
|
|
||||||
selectedIndex={0}
|
|
||||||
getItemKey={(item) => item.id}
|
|
||||||
onItemClick={onItemClick}
|
|
||||||
/>,
|
|
||||||
)
|
|
||||||
fireEvent.click(screen.getByText('Beta Notes'))
|
fireEvent.click(screen.getByText('Beta Notes'))
|
||||||
expect(onItemClick).toHaveBeenCalledWith(items[1], 1)
|
expect(onItemClick).toHaveBeenCalledWith(items[1], 1)
|
||||||
})
|
})
|
||||||
@@ -184,17 +190,15 @@ describe('NoteSearchList', () => {
|
|||||||
expect(preventDefault).toHaveBeenCalledOnce()
|
expect(preventDefault).toHaveBeenCalledOnce()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('calls onItemHover when mouse enters an item', () => {
|
it('does not call onItemHover for mouse enter alone', () => {
|
||||||
render(
|
renderList({ itemHover: onItemHover })
|
||||||
<NoteSearchList
|
|
||||||
items={items}
|
|
||||||
selectedIndex={0}
|
|
||||||
getItemKey={(item) => item.id}
|
|
||||||
onItemClick={onItemClick}
|
|
||||||
onItemHover={onItemHover}
|
|
||||||
/>,
|
|
||||||
)
|
|
||||||
fireEvent.mouseEnter(screen.getByText('Gamma Experiment'))
|
fireEvent.mouseEnter(screen.getByText('Gamma Experiment'))
|
||||||
|
expect(onItemHover).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('calls onItemHover when the mouse actually moves over an item', () => {
|
||||||
|
renderList({ itemHover: onItemHover })
|
||||||
|
fireEvent.mouseMove(screen.getByText('Gamma Experiment'))
|
||||||
expect(onItemHover).toHaveBeenCalledWith(2)
|
expect(onItemHover).toHaveBeenCalledWith(2)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -36,6 +36,47 @@ interface NoteSearchListItemProps<T extends NoteSearchResultItem> {
|
|||||||
activateOnMouseDown?: boolean
|
activateOnMouseDown?: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type SearchItemPressEvent = MouseEvent<HTMLButtonElement> | PointerEvent<HTMLButtonElement>
|
||||||
|
|
||||||
|
function useSearchItemActivation<T extends NoteSearchResultItem>({
|
||||||
|
item,
|
||||||
|
index,
|
||||||
|
onItemClick,
|
||||||
|
activateOnMouseDown,
|
||||||
|
}: Pick<NoteSearchListItemProps<T>, 'item' | 'index' | 'onItemClick' | 'activateOnMouseDown'>) {
|
||||||
|
const pressActivatedRef = useRef(false)
|
||||||
|
|
||||||
|
const activateItem = () => onItemClick(item, index)
|
||||||
|
|
||||||
|
const clearPressActivation = () => {
|
||||||
|
pressActivatedRef.current = false
|
||||||
|
}
|
||||||
|
|
||||||
|
const activateFromPress = (event: SearchItemPressEvent) => {
|
||||||
|
event.preventDefault()
|
||||||
|
if (!activateOnMouseDown) return
|
||||||
|
|
||||||
|
event.stopPropagation()
|
||||||
|
if (pressActivatedRef.current) return
|
||||||
|
|
||||||
|
pressActivatedRef.current = true
|
||||||
|
window.setTimeout(clearPressActivation, 0)
|
||||||
|
activateItem()
|
||||||
|
}
|
||||||
|
|
||||||
|
const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
|
||||||
|
if (!activateOnMouseDown) {
|
||||||
|
activateItem()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
event.preventDefault()
|
||||||
|
event.stopPropagation()
|
||||||
|
}
|
||||||
|
|
||||||
|
return { activateFromPress, handleClick }
|
||||||
|
}
|
||||||
|
|
||||||
function NoteSearchListItem<T extends NoteSearchResultItem>({
|
function NoteSearchListItem<T extends NoteSearchResultItem>({
|
||||||
item,
|
item,
|
||||||
index,
|
index,
|
||||||
@@ -44,31 +85,12 @@ function NoteSearchListItem<T extends NoteSearchResultItem>({
|
|||||||
onItemHover,
|
onItemHover,
|
||||||
activateOnMouseDown,
|
activateOnMouseDown,
|
||||||
}: NoteSearchListItemProps<T>) {
|
}: NoteSearchListItemProps<T>) {
|
||||||
const pressActivatedRef = useRef(false)
|
const { activateFromPress, handleClick } = useSearchItemActivation({
|
||||||
|
item,
|
||||||
const activateFromPress = (event: MouseEvent<HTMLButtonElement> | PointerEvent<HTMLButtonElement>) => {
|
index,
|
||||||
event.preventDefault()
|
onItemClick,
|
||||||
if (!activateOnMouseDown) return
|
activateOnMouseDown,
|
||||||
|
})
|
||||||
event.stopPropagation()
|
|
||||||
if (pressActivatedRef.current) return
|
|
||||||
|
|
||||||
pressActivatedRef.current = true
|
|
||||||
window.setTimeout(() => {
|
|
||||||
pressActivatedRef.current = false
|
|
||||||
}, 0)
|
|
||||||
onItemClick(item, index)
|
|
||||||
}
|
|
||||||
|
|
||||||
const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
|
|
||||||
if (activateOnMouseDown) {
|
|
||||||
event.preventDefault()
|
|
||||||
event.stopPropagation()
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
onItemClick(item, index)
|
|
||||||
}
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
@@ -83,7 +105,7 @@ function NoteSearchListItem<T extends NoteSearchResultItem>({
|
|||||||
onPointerDownCapture={activateFromPress}
|
onPointerDownCapture={activateFromPress}
|
||||||
onMouseDownCapture={activateFromPress}
|
onMouseDownCapture={activateFromPress}
|
||||||
onClick={handleClick}
|
onClick={handleClick}
|
||||||
onMouseEnter={() => onItemHover?.(index)}
|
onMouseMove={() => onItemHover?.(index)}
|
||||||
>
|
>
|
||||||
<span className="flex min-w-0 flex-1 items-center gap-1.5 truncate text-sm text-foreground">
|
<span className="flex min-w-0 flex-1 items-center gap-1.5 truncate text-sm text-foreground">
|
||||||
{item.TypeIcon && (
|
{item.TypeIcon && (
|
||||||
|
|||||||
@@ -161,6 +161,17 @@ describe('QuickOpenPalette', () => {
|
|||||||
expect(onClose).toHaveBeenCalled()
|
expect(onClose).toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('keeps keyboard selection stable when the mouse is already over a result', () => {
|
||||||
|
render(<QuickOpenPalette open={true} entries={entries} onSelect={onSelect} onClose={onClose} />)
|
||||||
|
|
||||||
|
fireEvent.mouseEnter(screen.getByText('Gamma Experiment'))
|
||||||
|
fireEvent.keyDown(window, { key: 'ArrowDown' })
|
||||||
|
fireEvent.keyDown(window, { key: 'Enter' })
|
||||||
|
|
||||||
|
expect(onSelect).toHaveBeenCalledWith(entries[1])
|
||||||
|
expect(onClose).toHaveBeenCalledOnce()
|
||||||
|
})
|
||||||
|
|
||||||
it('does not go below the last item with ArrowDown', () => {
|
it('does not go below the last item with ArrowDown', () => {
|
||||||
render(<QuickOpenPalette open={true} entries={entries} onSelect={onSelect} onClose={onClose} />)
|
render(<QuickOpenPalette open={true} entries={entries} onSelect={onSelect} onClose={onClose} />)
|
||||||
|
|
||||||
|
|||||||
@@ -75,6 +75,28 @@ async function dispatchAppCommand(page: Page, id: string): Promise<void> {
|
|||||||
}, id)
|
}, id)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function openQuickOpenFromKeyboard(page: Page): Promise<void> {
|
||||||
|
await dispatchShortcutEvent(page, {
|
||||||
|
key: 'p',
|
||||||
|
code: 'KeyP',
|
||||||
|
ctrlKey: false,
|
||||||
|
metaKey: true,
|
||||||
|
shiftKey: false,
|
||||||
|
altKey: false,
|
||||||
|
bubbles: true,
|
||||||
|
cancelable: true,
|
||||||
|
})
|
||||||
|
await expect(page.getByTestId('quick-open-palette')).toBeVisible({ timeout: 5_000 })
|
||||||
|
}
|
||||||
|
|
||||||
|
function selectedQuickOpenTitle(page: Page) {
|
||||||
|
return page.getByTestId('quick-open-palette').locator('div.bg-accent > button:has([data-testid="note-search-item-icon"])').first()
|
||||||
|
}
|
||||||
|
|
||||||
|
function quickOpenTitleAt(page: Page, index: number) {
|
||||||
|
return page.getByTestId('quick-open-palette').locator('button:has([data-testid="note-search-item-icon"])').nth(index)
|
||||||
|
}
|
||||||
|
|
||||||
async function installGlobalSearchResultsHarness(page: Page): Promise<void> {
|
async function installGlobalSearchResultsHarness(page: Page): Promise<void> {
|
||||||
await page.waitForFunction(() => Boolean(window.__mockHandlers?.search_vault))
|
await page.waitForFunction(() => Boolean(window.__mockHandlers?.search_vault))
|
||||||
await page.evaluate((results) => {
|
await page.evaluate((results) => {
|
||||||
@@ -134,17 +156,7 @@ test.describe('keyboard command routing', () => {
|
|||||||
test('desktop shortcut bridge opens quick open through both Cmd+P and Cmd+O @smoke', async ({ page }) => {
|
test('desktop shortcut bridge opens quick open through both Cmd+P and Cmd+O @smoke', async ({ page }) => {
|
||||||
await openFixtureVaultDesktopHarness(page, tempVaultDir)
|
await openFixtureVaultDesktopHarness(page, tempVaultDir)
|
||||||
|
|
||||||
await dispatchShortcutEvent(page, {
|
await openQuickOpenFromKeyboard(page)
|
||||||
key: 'p',
|
|
||||||
code: 'KeyP',
|
|
||||||
ctrlKey: false,
|
|
||||||
metaKey: true,
|
|
||||||
shiftKey: false,
|
|
||||||
altKey: false,
|
|
||||||
bubbles: true,
|
|
||||||
cancelable: true,
|
|
||||||
})
|
|
||||||
await expect(page.getByTestId('quick-open-palette')).toBeVisible({ timeout: 5_000 })
|
|
||||||
await expect(page.locator('input[placeholder="Search notes..."]')).toBeFocused()
|
await expect(page.locator('input[placeholder="Search notes..."]')).toBeFocused()
|
||||||
|
|
||||||
await page.keyboard.press('Escape')
|
await page.keyboard.press('Escape')
|
||||||
@@ -164,6 +176,31 @@ test.describe('keyboard command routing', () => {
|
|||||||
await expect(page.locator('input[placeholder="Search notes..."]')).toBeFocused()
|
await expect(page.locator('input[placeholder="Search notes..."]')).toBeFocused()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('quick open ignores a stationary pointer until it moves @smoke', async ({ page }) => {
|
||||||
|
await openFixtureVaultDesktopHarness(page, tempVaultDir)
|
||||||
|
await openQuickOpenFromKeyboard(page)
|
||||||
|
|
||||||
|
const initialTitle = await selectedQuickOpenTitle(page).textContent()
|
||||||
|
const secondTitle = quickOpenTitleAt(page, 1)
|
||||||
|
const targetTitle = await secondTitle.textContent()
|
||||||
|
const secondTitleBox = await secondTitle.boundingBox()
|
||||||
|
if (!initialTitle) throw new Error('Quick open did not select an initial note')
|
||||||
|
if (!targetTitle) throw new Error('Quick open did not render a second note title')
|
||||||
|
if (!secondTitleBox) throw new Error('Quick open did not render a second note box')
|
||||||
|
|
||||||
|
await page.keyboard.press('Escape')
|
||||||
|
await expect(page.getByTestId('quick-open-palette')).not.toBeVisible({ timeout: 5_000 })
|
||||||
|
await page.mouse.move(
|
||||||
|
secondTitleBox.x + secondTitleBox.width / 2,
|
||||||
|
secondTitleBox.y + secondTitleBox.height / 2,
|
||||||
|
)
|
||||||
|
|
||||||
|
await openQuickOpenFromKeyboard(page)
|
||||||
|
await expect(selectedQuickOpenTitle(page)).toHaveText(initialTitle)
|
||||||
|
await page.keyboard.press('ArrowDown')
|
||||||
|
await expect(selectedQuickOpenTitle(page)).toHaveText(targetTitle)
|
||||||
|
})
|
||||||
|
|
||||||
test('global search arrow keys move one result at a time @smoke', async ({ page }) => {
|
test('global search arrow keys move one result at a time @smoke', async ({ page }) => {
|
||||||
await openFixtureVaultDesktopHarness(page, tempVaultDir)
|
await openFixtureVaultDesktopHarness(page, tempVaultDir)
|
||||||
await installGlobalSearchResultsHarness(page)
|
await installGlobalSearchResultsHarness(page)
|
||||||
|
|||||||
Reference in New Issue
Block a user