From 09f7498e86d74deaca89571fc4ef819bced77250 Mon Sep 17 00:00:00 2001 From: Test Date: Mon, 6 Apr 2026 10:29:27 +0200 Subject: [PATCH] fix: Cmd+Option+arrow navigation now follows the visible note list order The keyboard navigation was computing its own note list with hardcoded sortByModified, which didn't match the actual UI order (Inbox sorts by createdAt, custom types use configurable sorts, etc.). Now the NoteList component writes its sorted list to a shared ref that the navigation hook reads directly, ensuring navigation always matches visual order. Also fixes navigation not wrapping at list boundaries per spec. Syncs CodeScene thresholds with current remote scores. Co-Authored-By: Claude Opus 4.6 (1M context) --- .codescene-thresholds | 4 +- src/App.tsx | 6 +- src/components/NoteList.tsx | 9 +- src/hooks/useAppCommands.ts | 4 +- src/hooks/useKeyboardNavigation.test.ts | 120 +++++++++++++++++------- src/hooks/useKeyboardNavigation.ts | 32 ++----- 6 files changed, 109 insertions(+), 66 deletions(-) diff --git a/.codescene-thresholds b/.codescene-thresholds index 8a1c5229..51bd2ac6 100644 --- a/.codescene-thresholds +++ b/.codescene-thresholds @@ -1,2 +1,2 @@ -HOTSPOT_THRESHOLD=9.41 -AVERAGE_THRESHOLD=9.30 +HOTSPOT_THRESHOLD=9.33 +AVERAGE_THRESHOLD=9.27 diff --git a/src/App.tsx b/src/App.tsx index 23938861..9bc2ab05 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -51,7 +51,7 @@ import { ConfirmDeleteDialog } from './components/ConfirmDeleteDialog' import { UpdateBanner } from './components/UpdateBanner' import { invoke } from '@tauri-apps/api/core' import { isTauri, mockInvoke } from './mock-tauri' -import type { SidebarSelection, InboxPeriod } from './types' +import type { SidebarSelection, InboxPeriod, VaultEntry } from './types' import type { NoteListItem } from './utils/ai-context' import { filterEntries, filterInboxEntries, type NoteListFilter } from './utils/noteListHelpers' import { openNoteInNewWindow } from './utils/openNoteWindow' @@ -83,6 +83,7 @@ function App() { setNoteListFilter('open') }, []) const layout = useLayoutPanels(noteWindowParams ? { initialInspectorCollapsed: true } : undefined) + const visibleNotesRef = useRef([]) const [toastMessage, setToastMessage] = useState(null) const dialogs = useDialogs() @@ -455,6 +456,7 @@ function App() { const commands = useAppCommands({ activeTabPath: notes.activeTabPath, activeTabPathRef: notes.activeTabPathRef, entries: vault.entries, + visibleNotesRef, modifiedCount: vault.modifiedFiles.length, activeNoteModified: vault.modifiedFiles.some(f => f.path === notes.activeTabPath), selection, @@ -587,7 +589,7 @@ function App() { {selection.kind === 'filter' && selection.filter === 'pulse' ? ( setViewMode('all')} /> ) : ( - diffToggleRef.current()} views={vault.views} /> + diffToggleRef.current()} views={vault.views} visibleNotesRef={visibleNotesRef} /> )} diff --git a/src/components/NoteList.tsx b/src/components/NoteList.tsx index 4118fff3..9c54ad08 100644 --- a/src/components/NoteList.tsx +++ b/src/components/NoteList.tsx @@ -140,9 +140,10 @@ interface NoteListProps { onDiscardFile?: (relativePath: string) => Promise onAutoTriggerDiff?: () => void views?: ViewFile[] + visibleNotesRef?: React.MutableRefObject } -function NoteListInner({ entries, selection, selectedNote, noteListFilter, onNoteListFilterChange, inboxPeriod = 'all', modifiedFiles, modifiedFilesError, getNoteStatus, sidebarCollapsed, onSelectNote, onReplaceActiveTab, onCreateNote, onBulkArchive, onBulkTrash, onBulkRestore, onBulkDeletePermanently, onEmptyTrash, onUpdateTypeSort, updateEntry, onOpenInNewWindow, onDiscardFile, onAutoTriggerDiff, views }: NoteListProps) { +function NoteListInner({ entries, selection, selectedNote, noteListFilter, onNoteListFilterChange, inboxPeriod = 'all', modifiedFiles, modifiedFilesError, getNoteStatus, sidebarCollapsed, onSelectNote, onReplaceActiveTab, onCreateNote, onBulkArchive, onBulkTrash, onBulkRestore, onBulkDeletePermanently, onEmptyTrash, onUpdateTypeSort, updateEntry, onOpenInNewWindow, onDiscardFile, onAutoTriggerDiff, views, visibleNotesRef }: NoteListProps) { const { modifiedPathSet, modifiedSuffixes, resolvedGetNoteStatus } = useModifiedFilesState(modifiedFiles, getNoteStatus) const { isSectionGroup, isFolderView, isInboxView, isAllNotesView, isChangesView, showFilterPills } = useViewFlags(selection) @@ -169,6 +170,12 @@ function NoteListInner({ entries, selection, selectedNote, noteListFilter, onNot return map }, [isChangesView, modifiedFiles]) const { isEntityView, isTrashView, isArchivedView, searched, searchedGroups, expiredTrashCount } = useNoteListData({ entries, selection, query, listSort, listDirection, modifiedPathSet, modifiedSuffixes, subFilter, inboxPeriod: isInboxView ? inboxPeriod : undefined, views }) + // Keep the visible notes ref in sync for keyboard navigation (Cmd+Option+Arrow) + if (visibleNotesRef) { + visibleNotesRef.current = isEntityView + ? searchedGroups.flatMap((g) => g.entries) + : searched + } const deletedCount = useMemo( () => isChangesView ? (modifiedFiles ?? []).filter((f) => f.status === 'deleted').length : 0, [isChangesView, modifiedFiles], diff --git a/src/hooks/useAppCommands.ts b/src/hooks/useAppCommands.ts index 0aa0426a..1b19c417 100644 --- a/src/hooks/useAppCommands.ts +++ b/src/hooks/useAppCommands.ts @@ -12,6 +12,7 @@ interface AppCommandsConfig { activeTabPath: string | null activeTabPathRef: React.MutableRefObject entries: VaultEntry[] + visibleNotesRef: React.RefObject modifiedCount: number selection: SidebarSelection onQuickOpen: () => void @@ -219,8 +220,7 @@ export function useAppCommands(config: AppCommandsConfig): CommandAction[] { useKeyboardNavigation({ activeTabPath: config.activeTabPath, - entries: config.entries, - selection: config.selection, + visibleNotesRef: config.visibleNotesRef, onReplaceActiveTab: config.onReplaceActiveTab, onSelectNote: config.onSelectNote, }) diff --git a/src/hooks/useKeyboardNavigation.test.ts b/src/hooks/useKeyboardNavigation.test.ts index ff9473ce..7e93930a 100644 --- a/src/hooks/useKeyboardNavigation.test.ts +++ b/src/hooks/useKeyboardNavigation.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { renderHook, act } from '@testing-library/react' -import type { VaultEntry, SidebarSelection } from '../types' +import type { VaultEntry } from '../types' import { useKeyboardNavigation } from './useKeyboardNavigation' vi.mock('../mock-tauri', () => ({ @@ -44,13 +44,11 @@ describe('useKeyboardNavigation', () => { makeEntry({ path: '/vault/c.md', title: 'C', modifiedAt: 1700000001 }), ] - const selection: SidebarSelection = { kind: 'filter', filter: 'all' } let addedListeners: { type: string; handler: EventListenerOrEventListenerObject }[] = [] beforeEach(() => { vi.clearAllMocks() addedListeners = [] - // Track added listeners for cleanup verification const origAdd = window.addEventListener const origRemove = window.removeEventListener vi.spyOn(window, 'addEventListener').mockImplementation((type: string, handler: EventListenerOrEventListenerObject, opts?: boolean | AddEventListenerOptions) => { @@ -66,24 +64,25 @@ describe('useKeyboardNavigation', () => { vi.restoreAllMocks() }) - it('registers keydown listener on mount', () => { - renderHook(() => + function renderNav(activeTabPath: string | null, noteList: VaultEntry[] = entries) { + const visibleNotesRef = { current: noteList } + return renderHook(() => useKeyboardNavigation({ - activeTabPath: '/vault/a.md', entries, selection, - onReplaceActiveTab, onSelectNote, + activeTabPath, + visibleNotesRef, + onReplaceActiveTab, + onSelectNote, }) ) + } + it('registers keydown listener on mount', () => { + renderNav('/vault/a.md') expect(addedListeners.some(l => l.type === 'keydown')).toBe(true) }) it('navigates to next note on Cmd+Alt+ArrowDown', () => { - renderHook(() => - useKeyboardNavigation({ - activeTabPath: '/vault/a.md', entries, selection, - onReplaceActiveTab, onSelectNote, - }) - ) + renderNav('/vault/a.md') act(() => { window.dispatchEvent(new KeyboardEvent('keydown', { @@ -91,16 +90,11 @@ describe('useKeyboardNavigation', () => { })) }) - expect(onReplaceActiveTab).toHaveBeenCalled() + expect(onReplaceActiveTab).toHaveBeenCalledWith(entries[1]) }) it('navigates to previous note on Cmd+Alt+ArrowUp', () => { - renderHook(() => - useKeyboardNavigation({ - activeTabPath: '/vault/b.md', entries, selection, - onReplaceActiveTab, onSelectNote, - }) - ) + renderNav('/vault/b.md') act(() => { window.dispatchEvent(new KeyboardEvent('keydown', { @@ -108,16 +102,11 @@ describe('useKeyboardNavigation', () => { })) }) - expect(onReplaceActiveTab).toHaveBeenCalled() + expect(onReplaceActiveTab).toHaveBeenCalledWith(entries[0]) }) - it('selects first note when no active tab', () => { - renderHook(() => - useKeyboardNavigation({ - activeTabPath: null, entries, selection, - onReplaceActiveTab, onSelectNote, - }) - ) + it('does not wrap when at the last note and pressing Down', () => { + renderNav('/vault/c.md') act(() => { window.dispatchEvent(new KeyboardEvent('keydown', { @@ -125,16 +114,49 @@ describe('useKeyboardNavigation', () => { })) }) - expect(onSelectNote).toHaveBeenCalled() + expect(onReplaceActiveTab).not.toHaveBeenCalled() + expect(onSelectNote).not.toHaveBeenCalled() + }) + + it('does not wrap when at the first note and pressing Up', () => { + renderNav('/vault/a.md') + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { + key: 'ArrowUp', metaKey: true, altKey: true, bubbles: true, + })) + }) + + expect(onReplaceActiveTab).not.toHaveBeenCalled() + expect(onSelectNote).not.toHaveBeenCalled() + }) + + it('selects first note when no active tab and pressing Down', () => { + renderNav(null) + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { + key: 'ArrowDown', metaKey: true, altKey: true, bubbles: true, + })) + }) + + expect(onSelectNote).toHaveBeenCalledWith(entries[0]) + }) + + it('selects last note when no active tab and pressing Up', () => { + renderNav(null) + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { + key: 'ArrowUp', metaKey: true, altKey: true, bubbles: true, + })) + }) + + expect(onSelectNote).toHaveBeenCalledWith(entries[2]) }) it('does nothing without modifier keys', () => { - renderHook(() => - useKeyboardNavigation({ - activeTabPath: '/vault/a.md', entries, selection, - onReplaceActiveTab, onSelectNote, - }) - ) + renderNav('/vault/a.md') act(() => { window.dispatchEvent(new KeyboardEvent('keydown', { @@ -145,4 +167,32 @@ describe('useKeyboardNavigation', () => { expect(onReplaceActiveTab).not.toHaveBeenCalled() expect(onSelectNote).not.toHaveBeenCalled() }) + + it('navigates in the order provided by visibleNotesRef (not by modifiedAt)', () => { + // Provide notes in reverse-alpha order (C, B, A) regardless of modifiedAt + const customOrder = [entries[2], entries[1], entries[0]] + renderNav('/vault/c.md', customOrder) + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { + key: 'ArrowDown', metaKey: true, altKey: true, bubbles: true, + })) + }) + + // Should navigate to B (next in custom order), not based on modifiedAt + expect(onReplaceActiveTab).toHaveBeenCalledWith(entries[1]) + }) + + it('does nothing when note list is empty', () => { + renderNav('/vault/a.md', []) + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { + key: 'ArrowDown', metaKey: true, altKey: true, bubbles: true, + })) + }) + + expect(onReplaceActiveTab).not.toHaveBeenCalled() + expect(onSelectNote).not.toHaveBeenCalled() + }) }) diff --git a/src/hooks/useKeyboardNavigation.ts b/src/hooks/useKeyboardNavigation.ts index 3fdb3a6b..5fff2795 100644 --- a/src/hooks/useKeyboardNavigation.ts +++ b/src/hooks/useKeyboardNavigation.ts @@ -1,26 +1,13 @@ -import { useEffect, useMemo, useRef } from 'react' -import { filterEntries, sortByModified, buildRelationshipGroups } from '../utils/noteListHelpers' -import type { VaultEntry, SidebarSelection } from '../types' +import { useEffect, useRef } from 'react' +import type { VaultEntry } from '../types' interface KeyboardNavigationOptions { activeTabPath: string | null - entries: VaultEntry[] - selection: SidebarSelection + visibleNotesRef: React.RefObject onReplaceActiveTab: (entry: VaultEntry) => void onSelectNote: (entry: VaultEntry) => void } -function computeVisibleNotes( - entries: VaultEntry[], - selection: SidebarSelection, -): VaultEntry[] { - if (selection.kind === 'entity') { - return buildRelationshipGroups(selection.entry, entries) - .flatMap((g) => g.entries) - } - return [...filterEntries(entries, selection)].sort(sortByModified) -} - function navigateNote( visibleNotesRef: React.RefObject, activeTabPathRef: React.RefObject, @@ -36,7 +23,10 @@ function navigateNote( const nextIndex = currentIndex === -1 ? (direction === 1 ? 0 : notes.length - 1) - : (currentIndex + direction + notes.length) % notes.length + : currentIndex + direction + + // Clamp to list bounds — don't wrap around + if (nextIndex < 0 || nextIndex >= notes.length) return const nextNote = notes[nextIndex] if (currentPath) { @@ -53,16 +43,10 @@ function useLatestRef(value: T): React.RefObject { } export function useKeyboardNavigation({ - activeTabPath, entries, selection, + activeTabPath, visibleNotesRef, onReplaceActiveTab, onSelectNote, }: KeyboardNavigationOptions) { - const visibleNotes = useMemo( - () => computeVisibleNotes(entries, selection), - [entries, selection], - ) - const activeTabPathRef = useLatestRef(activeTabPath) - const visibleNotesRef = useLatestRef(visibleNotes) const onReplaceRef = useLatestRef(onReplaceActiveTab) const onSelectNoteRef = useLatestRef(onSelectNote)