refactor: extract useAppNavigation hook from App.tsx — reduce god component churn (#194)

Co-authored-by: Test <test@test.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Luca Rossi
2026-03-12 07:24:46 +01:00
committed by GitHub
parent 8137125569
commit a15f36ec6a
3 changed files with 234 additions and 52 deletions

View File

@@ -26,7 +26,6 @@ import { useDialogs } from './hooks/useDialogs'
import { useVaultSwitcher } from './hooks/useVaultSwitcher'
import { useGitHistory } from './hooks/useGitHistory'
import { useUpdater, restartApp } from './hooks/useUpdater'
import { useNavigationHistory } from './hooks/useNavigationHistory'
import { useAutoSync } from './hooks/useAutoSync'
import { useConflictResolver } from './hooks/useConflictResolver'
import { useIndexing } from './hooks/useIndexing'
@@ -36,7 +35,7 @@ import { useBuildNumber } from './hooks/useBuildNumber'
import { useOnboarding } from './hooks/useOnboarding'
import { useThemeManager } from './hooks/useThemeManager'
import { useEditorSaveWithLinks } from './hooks/useEditorSaveWithLinks'
import { useNavigationGestures } from './hooks/useNavigationGestures'
import { useAppNavigation } from './hooks/useAppNavigation'
import { useAiActivity } from './hooks/useAiActivity'
import { useBulkActions } from './hooks/useBulkActions'
import { useDeleteActions } from './hooks/useDeleteActions'
@@ -179,53 +178,13 @@ function App() {
})
}, [vault.entries]) // eslint-disable-line react-hooks/exhaustive-deps -- notes.setTabs is stable (useState setter)
const navHistory = useNavigationHistory()
// Push to navigation history whenever the active tab changes (user-initiated)
const navFromHistoryRef = useRef(false)
useEffect(() => {
if (notes.activeTabPath && !navFromHistoryRef.current) {
navHistory.push(notes.activeTabPath)
}
navFromHistoryRef.current = false
}, [notes.activeTabPath]) // eslint-disable-line react-hooks/exhaustive-deps -- navHistory.push is stable
const isEntryExists = useCallback((path: string) => vault.entries.some(e => e.path === path), [vault.entries])
const handleGoBack = useCallback(() => {
const target = navHistory.goBack(isEntryExists)
if (target) {
navFromHistoryRef.current = true
if (notes.tabs.some(t => t.entry.path === target)) {
notes.handleSwitchTab(target)
} else {
const entry = vault.entries.find(e => e.path === target)
if (entry) notes.handleSelectNote(entry)
}
}
}, [navHistory, isEntryExists, vault.entries, notes])
const handleGoForward = useCallback(() => {
const target = navHistory.goForward(isEntryExists)
if (target) {
navFromHistoryRef.current = true
if (notes.tabs.some(t => t.entry.path === target)) {
notes.handleSwitchTab(target)
} else {
const entry = vault.entries.find(e => e.path === target)
if (entry) notes.handleSelectNote(entry)
}
}
}, [navHistory, isEntryExists, vault.entries, notes])
useNavigationGestures({ onGoBack: handleGoBack, onGoForward: handleGoForward })
// O(1) path lookup map — rebuilt only when vault.entries changes
const entriesByPath = useMemo(() => {
const map = new Map<string, VaultEntry>()
for (const e of vault.entries) map.set(e.path, e)
return map
}, [vault.entries])
const { handleGoBack, handleGoForward, canGoBack, canGoForward, entriesByPath } = useAppNavigation({
entries: vault.entries,
tabs: notes.tabs,
activeTabPath: notes.activeTabPath,
onSelectNote: notes.handleSelectNote,
onSwitchTab: notes.handleSwitchTab,
})
// MCP UI bridge: react to AI-driven open/highlight/vault-change events
const openNoteByPath = useCallback((path: string) => {
@@ -473,7 +432,7 @@ function App() {
onSwitchTab: notes.handleSwitchTab, onReplaceActiveTab: notes.handleReplaceActiveTab,
onSelectNote: notes.handleSelectNote,
onGoBack: handleGoBack, onGoForward: handleGoForward,
canGoBack: navHistory.canGoBack, canGoForward: navHistory.canGoForward,
canGoBack: canGoBack, canGoForward: canGoForward,
themes: themeManager.themes, activeThemeId: themeManager.activeThemeId,
onSwitchTheme: themeManager.switchTheme,
onCreateTheme: async () => {
@@ -615,8 +574,8 @@ function App() {
onTitleSync={handleTitleSync}
rawToggleRef={rawToggleRef}
diffToggleRef={diffToggleRef}
canGoBack={navHistory.canGoBack}
canGoForward={navHistory.canGoForward}
canGoBack={canGoBack}
canGoForward={canGoForward}
onGoBack={handleGoBack}
onGoForward={handleGoForward}
leftPanelsCollapsed={!sidebarVisible && !noteListVisible}

View File

@@ -0,0 +1,134 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { renderHook, act } from '@testing-library/react'
import { useAppNavigation } from './useAppNavigation'
import type { VaultEntry } from '../types'
function makeEntry(path: string): VaultEntry {
return { path, filename: path.split('/').pop()!, title: path, isA: null, aliases: [] } as VaultEntry
}
function makeTab(entry: VaultEntry) {
return { entry, content: '' }
}
describe('useAppNavigation', () => {
let onSelectNote: ReturnType<typeof vi.fn>
let onSwitchTab: ReturnType<typeof vi.fn>
beforeEach(() => {
onSelectNote = vi.fn()
onSwitchTab = vi.fn()
})
function renderNav(overrides: {
entries?: VaultEntry[]
tabs?: Array<{ entry: VaultEntry; content: string }>
activeTabPath?: string | null
} = {}) {
const entries = overrides.entries ?? [makeEntry('/a.md'), makeEntry('/b.md'), makeEntry('/c.md')]
const tabs = overrides.tabs ?? []
const activeTabPath = overrides.activeTabPath ?? null
return renderHook(() =>
useAppNavigation({ entries, tabs, activeTabPath, onSelectNote, onSwitchTab }),
)
}
// --- entriesByPath ---
describe('entriesByPath', () => {
it('builds a Map from entries for O(1) lookup', () => {
const entries = [makeEntry('/a.md'), makeEntry('/b.md')]
const { result } = renderNav({ entries })
expect(result.current.entriesByPath.get('/a.md')).toBe(entries[0])
expect(result.current.entriesByPath.get('/b.md')).toBe(entries[1])
expect(result.current.entriesByPath.get('/missing.md')).toBeUndefined()
})
})
// --- canGoBack / canGoForward initial state ---
describe('initial state', () => {
it('starts with canGoBack=false and canGoForward=false', () => {
const { result } = renderNav()
expect(result.current.canGoBack).toBe(false)
expect(result.current.canGoForward).toBe(false)
})
})
// --- navigation history integration ---
describe('navigation via activeTabPath changes', () => {
it('pushes to history when activeTabPath changes, enabling goBack', () => {
const entries = [makeEntry('/a.md'), makeEntry('/b.md')]
const tabA = makeTab(entries[0])
const tabB = makeTab(entries[1])
const { result, rerender } = renderHook(
({ activeTabPath, tabs }) =>
useAppNavigation({ entries, tabs, activeTabPath, onSelectNote, onSwitchTab }),
{ initialProps: { activeTabPath: '/a.md' as string | null, tabs: [tabA] } },
)
// Navigate to /b.md
rerender({ activeTabPath: '/b.md', tabs: [tabA, tabB] })
expect(result.current.canGoBack).toBe(true)
expect(result.current.canGoForward).toBe(false)
})
it('handleGoBack switches to the tab if it is open', () => {
const entries = [makeEntry('/a.md'), makeEntry('/b.md')]
const tabA = makeTab(entries[0])
const tabB = makeTab(entries[1])
const { result, rerender } = renderHook(
({ activeTabPath, tabs }) =>
useAppNavigation({ entries, tabs, activeTabPath, onSelectNote, onSwitchTab }),
{ initialProps: { activeTabPath: '/a.md' as string | null, tabs: [tabA, tabB] } },
)
rerender({ activeTabPath: '/b.md', tabs: [tabA, tabB] })
act(() => { result.current.handleGoBack() })
expect(onSwitchTab).toHaveBeenCalledWith('/a.md')
})
it('handleGoBack opens entry via onSelectNote if not in tabs', () => {
const entries = [makeEntry('/a.md'), makeEntry('/b.md')]
const tabB = makeTab(entries[1])
const { result, rerender } = renderHook(
({ activeTabPath, tabs }) =>
useAppNavigation({ entries, tabs, activeTabPath, onSelectNote, onSwitchTab }),
{ initialProps: { activeTabPath: '/a.md' as string | null, tabs: [tabB] } },
)
rerender({ activeTabPath: '/b.md', tabs: [tabB] })
act(() => { result.current.handleGoBack() })
expect(onSelectNote).toHaveBeenCalledWith(entries[0])
})
it('handleGoForward works after going back', () => {
const entries = [makeEntry('/a.md'), makeEntry('/b.md')]
const tabA = makeTab(entries[0])
const tabB = makeTab(entries[1])
const { result, rerender } = renderHook(
({ activeTabPath, tabs }) =>
useAppNavigation({ entries, tabs, activeTabPath, onSelectNote, onSwitchTab }),
{ initialProps: { activeTabPath: '/a.md' as string | null, tabs: [tabA, tabB] } },
)
rerender({ activeTabPath: '/b.md', tabs: [tabA, tabB] })
act(() => { result.current.handleGoBack() })
expect(result.current.canGoForward).toBe(true)
act(() => { result.current.handleGoForward() })
expect(onSwitchTab).toHaveBeenCalledWith('/b.md')
})
})
})

View File

@@ -0,0 +1,89 @@
import { useCallback, useEffect, useMemo, useRef } from 'react'
import { useNavigationHistory } from './useNavigationHistory'
import { useNavigationGestures } from './useNavigationGestures'
import type { VaultEntry } from '../types'
interface TabLike {
entry: { path: string }
}
interface UseAppNavigationParams {
entries: VaultEntry[]
tabs: TabLike[]
activeTabPath: string | null
onSelectNote: (entry: VaultEntry) => void
onSwitchTab: (path: string) => void
}
/**
* Encapsulates browser-style back/forward navigation for the app:
* - Navigation history (push on tab change, back/forward traversal)
* - Mouse button & trackpad gesture bindings
* - O(1) path→entry lookup map
*/
export function useAppNavigation({
entries,
tabs,
activeTabPath,
onSelectNote,
onSwitchTab,
}: UseAppNavigationParams) {
const navHistory = useNavigationHistory()
// Push to navigation history whenever the active tab changes (user-initiated)
const navFromHistoryRef = useRef(false)
useEffect(() => {
if (activeTabPath && !navFromHistoryRef.current) {
navHistory.push(activeTabPath)
}
navFromHistoryRef.current = false
}, [activeTabPath]) // eslint-disable-line react-hooks/exhaustive-deps -- navHistory.push is stable
const isEntryExists = useCallback(
(path: string) => entries.some(e => e.path === path),
[entries],
)
const handleGoBack = useCallback(() => {
const target = navHistory.goBack(isEntryExists)
if (target) {
navFromHistoryRef.current = true
if (tabs.some(t => t.entry.path === target)) {
onSwitchTab(target)
} else {
const entry = entries.find(e => e.path === target)
if (entry) onSelectNote(entry)
}
}
}, [navHistory, isEntryExists, entries, tabs, onSelectNote, onSwitchTab])
const handleGoForward = useCallback(() => {
const target = navHistory.goForward(isEntryExists)
if (target) {
navFromHistoryRef.current = true
if (tabs.some(t => t.entry.path === target)) {
onSwitchTab(target)
} else {
const entry = entries.find(e => e.path === target)
if (entry) onSelectNote(entry)
}
}
}, [navHistory, isEntryExists, entries, tabs, onSelectNote, onSwitchTab])
useNavigationGestures({ onGoBack: handleGoBack, onGoForward: handleGoForward })
// O(1) path lookup map — rebuilt only when entries change
const entriesByPath = useMemo(() => {
const map = new Map<string, VaultEntry>()
for (const e of entries) map.set(e.path, e)
return map
}, [entries])
return {
handleGoBack,
handleGoForward,
canGoBack: navHistory.canGoBack,
canGoForward: navHistory.canGoForward,
entriesByPath,
}
}