diff --git a/src/components/NoteList.keyboard.test.tsx b/src/components/NoteList.keyboard.test.tsx index dcb3405d..ee3c27c0 100644 --- a/src/components/NoteList.keyboard.test.tsx +++ b/src/components/NoteList.keyboard.test.tsx @@ -1,12 +1,13 @@ import { useState } from 'react' import { fireEvent, render, screen, waitFor } from '@testing-library/react' -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { NoteList } from './NoteList' import { allSelection, mockEntries, } from '../test-utils/noteListTestUtils' import type { SidebarSelection, VaultEntry } from '../types' +import * as tabManagement from '../hooks/useTabManagement' function NoteListKeyboardHarness({ onOpen, @@ -40,6 +41,10 @@ function NoteListKeyboardHarness({ } describe('NoteList keyboard activation', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + it('focuses the list on click and continues arrow navigation from the clicked note', async () => { const onOpen = vi.fn() render() @@ -56,8 +61,10 @@ describe('NoteList keyboard activation', () => { fireEvent.keyDown(container, { key: 'ArrowDown' }) - expect(onOpen).toHaveBeenNthCalledWith(1, mockEntries[1]) - expect(onOpen).toHaveBeenNthCalledWith(2, mockEntries[2]) + await waitFor(() => { + expect(onOpen).toHaveBeenNthCalledWith(1, mockEntries[1]) + expect(onOpen).toHaveBeenNthCalledWith(2, mockEntries[2]) + }) }) it('navigates from global arrow keys when the editor is not focused', async () => { @@ -98,4 +105,16 @@ describe('NoteList keyboard activation', () => { expect(onEnterNeighborhood).toHaveBeenCalledWith(mockEntries[4]) }) }) + + it('prefetches note content on hover so click opens can use the warm path', () => { + const prefetchSpy = vi.spyOn(tabManagement, 'prefetchNoteContent').mockImplementation(() => {}) + render() + + const noteRow = screen.getByText('Facebook Ads Strategy').closest('[data-note-path]') + expect(noteRow).not.toBeNull() + + fireEvent.mouseEnter(noteRow!) + + expect(prefetchSpy).toHaveBeenCalledWith(mockEntries[1].path) + }) }) diff --git a/src/components/note-list/useNoteListModel.tsx b/src/components/note-list/useNoteListModel.tsx index 9795c041..8709c920 100644 --- a/src/components/note-list/useNoteListModel.tsx +++ b/src/components/note-list/useNoteListModel.tsx @@ -11,8 +11,9 @@ import type { import type { NoteListFilter } from '../../utils/noteListHelpers' import { countByFilter, countAllByFilter, countAllNotesByFilter } from '../../utils/noteListHelpers' import { NoteItem } from '../NoteItem' +import { prefetchNoteContent } from '../../hooks/useTabManagement' import type { MultiSelectState } from '../../hooks/useMultiSelect' -import { resolveHeaderTitle, type DeletedNoteEntry } from './noteListUtils' +import { isDeletedNoteEntry, resolveHeaderTitle, type DeletedNoteEntry } from './noteListUtils' import { filterEntriesByNoteListQuery, filterGroupsByNoteListQuery } from './noteListSearch' import { useChangeStatusResolver, @@ -341,6 +342,7 @@ function useRenderItem({ allEntries={entries} displayPropsOverride={displayPropsOverride} onClickNote={handleClickNote} + onPrefetch={isDeletedNoteEntry(entry) ? undefined : prefetchNoteContent} onContextMenu={contextMenuHandler} /> ), [ diff --git a/src/hooks/useNoteListKeyboard.test.ts b/src/hooks/useNoteListKeyboard.test.ts index fb9b3a38..1232ebf2 100644 --- a/src/hooks/useNoteListKeyboard.test.ts +++ b/src/hooks/useNoteListKeyboard.test.ts @@ -1,5 +1,5 @@ import { renderHook, act } from '@testing-library/react' -import { describe, it, expect, vi } from 'vitest' +import { beforeEach, afterEach, describe, it, expect, vi } from 'vitest' import { useNoteListKeyboard } from './useNoteListKeyboard' import type { VaultEntry } from '../types' @@ -30,9 +30,41 @@ function keyEvent(key: string, opts: Partial = {}): React.K return { key, preventDefault: vi.fn(), metaKey: false, ctrlKey: false, altKey: false, ...opts } as unknown as React.KeyboardEvent } +function installAnimationFrameStub() { + let nextId = 1 + const callbacks = new Map() + + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + const id = nextId++ + callbacks.set(id, callback) + return id + }) + vi.stubGlobal('cancelAnimationFrame', (id: number) => { + callbacks.delete(id) + }) + + return { + flushAnimationFrame: () => { + const pending = [...callbacks.entries()] + callbacks.clear() + for (const [, callback] of pending) callback(0) + }, + } +} + describe('useNoteListKeyboard', () => { const items = [makeEntry('/a.md', 'A'), makeEntry('/b.md', 'B'), makeEntry('/c.md', 'C')] const onOpen = vi.fn() + let flushAnimationFrame: () => void + + beforeEach(() => { + vi.clearAllMocks() + ;({ flushAnimationFrame } = installAnimationFrameStub()) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) it('initializes with no highlight', () => { const { result } = renderHook(() => @@ -48,27 +80,38 @@ describe('useNoteListKeyboard', () => { ) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) expect(result.current.highlightedPath).toBe('/a.md') + expect(open).not.toHaveBeenCalled() + act(() => flushAnimationFrame()) expect(open).toHaveBeenCalledWith(items[0]) }) - it('ArrowDown advances highlight', () => { + it('ArrowDown advances highlight and opens the latest highlighted note on the next frame', () => { + const open = vi.fn() const { result } = renderHook(() => - useNoteListKeyboard({ items, selectedNotePath: null, onOpen, enabled: true }), + useNoteListKeyboard({ items, selectedNotePath: null, onOpen: open, enabled: true }), ) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) expect(result.current.highlightedPath).toBe('/b.md') + expect(open).not.toHaveBeenCalled() + act(() => flushAnimationFrame()) + expect(open).toHaveBeenCalledTimes(1) + expect(open).toHaveBeenCalledWith(items[1]) }) it('ArrowDown clamps at end of list', () => { + const open = vi.fn() const { result } = renderHook(() => - useNoteListKeyboard({ items, selectedNotePath: null, onOpen, enabled: true }), + useNoteListKeyboard({ items, selectedNotePath: null, onOpen: open, enabled: true }), ) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) expect(result.current.highlightedPath).toBe('/c.md') + act(() => flushAnimationFrame()) + expect(open).toHaveBeenCalledTimes(1) + expect(open).toHaveBeenCalledWith(items[2]) }) it('ArrowUp highlights last item from no selection', () => { @@ -78,6 +121,8 @@ describe('useNoteListKeyboard', () => { ) act(() => result.current.handleKeyDown(keyEvent('ArrowUp'))) expect(result.current.highlightedPath).toBe('/c.md') + expect(open).not.toHaveBeenCalled() + act(() => flushAnimationFrame()) expect(open).toHaveBeenCalledWith(items[2]) }) @@ -110,7 +155,10 @@ describe('useNoteListKeyboard', () => { ) act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) act(() => result.current.handleKeyDown(keyEvent('Enter'))) + expect(open).toHaveBeenCalledTimes(1) expect(open).toHaveBeenCalledWith(items[0]) + act(() => flushAnimationFrame()) + expect(open).toHaveBeenCalledTimes(1) }) it('Enter does nothing when no item highlighted', () => { @@ -185,6 +233,7 @@ describe('useNoteListKeyboard', () => { }) expect(result.current.highlightedPath).toBe('/a.md') + act(() => flushAnimationFrame()) expect(open).toHaveBeenCalledWith(items[0]) }) @@ -208,4 +257,23 @@ describe('useNoteListKeyboard', () => { editor.remove() }) + + it('coalesces rapid arrow navigation into a single open for the latest highlighted note', () => { + const open = vi.fn() + const { result } = renderHook(() => + useNoteListKeyboard({ items, selectedNotePath: null, onOpen: open, enabled: true }), + ) + + act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) + act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) + act(() => result.current.handleKeyDown(keyEvent('ArrowDown'))) + + expect(result.current.highlightedPath).toBe('/c.md') + expect(open).not.toHaveBeenCalled() + + act(() => flushAnimationFrame()) + + expect(open).toHaveBeenCalledTimes(1) + expect(open).toHaveBeenCalledWith(items[2]) + }) }) diff --git a/src/hooks/useNoteListKeyboard.ts b/src/hooks/useNoteListKeyboard.ts index 6521c1ab..b4dfedd5 100644 --- a/src/hooks/useNoteListKeyboard.ts +++ b/src/hooks/useNoteListKeyboard.ts @@ -127,22 +127,87 @@ function useSelectionSync( }, [itemsRef, selectedNotePathRef, syncHighlightedPath]) } +interface ScheduledOpenState { + entry: VaultEntry | null + frameId: number | null +} + +function cancelScheduledOpen(stateRef: React.RefObject): void { + const frameId = stateRef.current.frameId + if (frameId !== null) cancelAnimationFrame(frameId) + stateRef.current.entry = null + stateRef.current.frameId = null +} + +function flushScheduledOpen( + stateRef: React.RefObject, + onOpen: (entry: VaultEntry) => void, + entry?: VaultEntry, +): void { + if (entry) stateRef.current.entry = entry + const nextEntry = stateRef.current.entry + if (!nextEntry) return + + if (stateRef.current.frameId !== null) cancelAnimationFrame(stateRef.current.frameId) + stateRef.current.entry = null + stateRef.current.frameId = null + onOpen(nextEntry) +} + +function scheduleOpenForNextFrame( + stateRef: React.RefObject, + onOpen: (entry: VaultEntry) => void, + entry: VaultEntry, +): void { + stateRef.current.entry = entry + if (stateRef.current.frameId !== null) return + + stateRef.current.frameId = requestAnimationFrame(() => { + flushScheduledOpen(stateRef, onOpen) + }) +} + +function useScheduledOpen(onOpen: (entry: VaultEntry) => void, enabled: boolean) { + const stateRef = useRef({ entry: null, frameId: null }) + + const scheduleOpen = useCallback((entry: VaultEntry) => { + scheduleOpenForNextFrame(stateRef, onOpen, entry) + }, [onOpen]) + + const flushOpen = useCallback((entry?: VaultEntry) => { + flushScheduledOpen(stateRef, onOpen, entry) + }, [onOpen]) + + const cancelOpen = useCallback(() => { + cancelScheduledOpen(stateRef) + }, []) + + useEffect(() => { + if (enabled) return + cancelOpen() + }, [cancelOpen, enabled]) + + useEffect(() => cancelOpen, [cancelOpen]) + + return { cancelOpen, flushOpen, scheduleOpen } +} + function useMoveHighlight({ items, selectedNotePath, highlightedPathRef, syncHighlightedPath, virtuosoRef, - onOpen, onPrefetch, + scheduleOpen, }: { items: VaultEntry[] selectedNotePath: string | null highlightedPathRef: React.RefObject syncHighlightedPath: (nextPath: string | null) => void virtuosoRef: React.RefObject - onOpen: (entry: VaultEntry) => void onPrefetch?: (entry: VaultEntry) => void + scheduleOpen: (entry: VaultEntry) => void }) { return useCallback((direction: 1 | -1) => { const currentIndex = resolveCurrentIndex(items, highlightedPathRef.current, selectedNotePath) @@ -153,9 +218,80 @@ function useMoveHighlight({ syncHighlightedPath(nextItem.path) virtuosoRef.current?.scrollIntoView({ index: nextIndex, behavior: 'auto' }) - onOpen(nextItem) + scheduleOpen(nextItem) onPrefetch?.(nextItem) - }, [highlightedPathRef, items, onOpen, onPrefetch, selectedNotePath, syncHighlightedPath, virtuosoRef]) + }, [highlightedPathRef, items, onPrefetch, scheduleOpen, selectedNotePath, syncHighlightedPath, virtuosoRef]) +} + +function resolveEntryForActivation( + items: VaultEntry[], + highlightedPathRef: React.RefObject, +): VaultEntry | undefined { + return resolveHighlightedEntry(items, highlightedPathRef.current) +} + +function handleNeighborhoodActivation(options: { + event: Pick + items: VaultEntry[] + highlightedPathRef: React.RefObject + cancelOpen: () => void + onEnterNeighborhood?: (entry: VaultEntry) => void | Promise +}): boolean { + const { + event, + items, + highlightedPathRef, + cancelOpen, + onEnterNeighborhood, + } = options + + const highlightedItem = resolveEntryForActivation(items, highlightedPathRef) + if (!highlightedItem) return false + + event.preventDefault() + cancelOpen() + void onEnterNeighborhood?.(highlightedItem) + return true +} + +function handleArrowNavigation( + event: Pick, + moveHighlight: (direction: 1 | -1) => void, +): boolean { + if (event.key === 'ArrowDown') { + event.preventDefault() + moveHighlight(1) + return true + } + + if (event.key === 'ArrowUp') { + event.preventDefault() + moveHighlight(-1) + return true + } + + return false +} + +function handleHighlightedOpen(options: { + event: Pick + items: VaultEntry[] + highlightedPathRef: React.RefObject + flushOpen: (entry?: VaultEntry) => void +}): boolean { + const { + event, + items, + highlightedPathRef, + flushOpen, + } = options + + const highlightedItem = resolveEntryForActivation(items, highlightedPathRef) + if (!highlightedItem) return false + + event.preventDefault() + flushOpen(highlightedItem) + return true } function useProcessKeyDown({ @@ -163,48 +299,45 @@ function useProcessKeyDown({ items, highlightedPathRef, moveHighlight, - onOpen, + flushOpen, + cancelOpen, onEnterNeighborhood, }: { enabled: boolean items: VaultEntry[] highlightedPathRef: React.RefObject moveHighlight: (direction: 1 | -1) => void - onOpen: (entry: VaultEntry) => void + flushOpen: (entry?: VaultEntry) => void + cancelOpen: () => void onEnterNeighborhood?: (entry: VaultEntry) => void | Promise }) { return useCallback((event: Pick) => { if (!enabled || items.length === 0) return if (isNeighborhoodKey(event)) { - const highlightedItem = resolveHighlightedEntry(items, highlightedPathRef.current) - if (!highlightedItem) return - event.preventDefault() - void onEnterNeighborhood?.(highlightedItem) + handleNeighborhoodActivation({ + event, + items, + highlightedPathRef, + cancelOpen, + onEnterNeighborhood, + }) return } if (usesCommandModifier(event) || event.altKey) return - if (event.key === 'ArrowDown') { - event.preventDefault() - moveHighlight(1) - return - } - - if (event.key === 'ArrowUp') { - event.preventDefault() - moveHighlight(-1) - return - } + if (handleArrowNavigation(event, moveHighlight)) return if (event.key !== 'Enter') return - const highlightedItem = resolveHighlightedEntry(items, highlightedPathRef.current) - if (!highlightedItem) return - event.preventDefault() - onOpen(highlightedItem) - }, [enabled, highlightedPathRef, items, moveHighlight, onEnterNeighborhood, onOpen]) + handleHighlightedOpen({ + event, + items, + highlightedPathRef, + flushOpen, + }) + }, [cancelOpen, enabled, flushOpen, highlightedPathRef, items, moveHighlight, onEnterNeighborhood]) } function useFocusHandlers({ @@ -274,21 +407,23 @@ export function useNoteListKeyboard({ const { itemsRef, selectedNotePathRef } = useKeyboardItemRefs(items, selectedNotePath) const { highlightedPathRef, highlightedPathState, syncHighlightedPath } = useHighlightedPath() const syncToCurrentSelection = useSelectionSync(itemsRef, selectedNotePathRef, syncHighlightedPath) + const { cancelOpen, flushOpen, scheduleOpen } = useScheduledOpen(onOpen, enabled) const moveHighlight = useMoveHighlight({ items, selectedNotePath, highlightedPathRef, syncHighlightedPath, virtuosoRef, - onOpen, onPrefetch, + scheduleOpen, }) const processKeyDown = useProcessKeyDown({ enabled, items, highlightedPathRef, moveHighlight, - onOpen, + flushOpen, + cancelOpen, onEnterNeighborhood, }) const handleKeyDown = useCallback((event: React.KeyboardEvent) => { @@ -301,6 +436,9 @@ export function useNoteListKeyboard({ syncHighlightedPath, }) useGlobalKeyboardHandling({ enabled, containerRef, processKeyDown }) + useEffect(() => { + cancelOpen() + }, [cancelOpen, selectedNotePath]) const highlightedPath = items.some((entry) => entry.path === highlightedPathState) ? highlightedPathState