From df2452dcaff531af9c43d08b41100db96f925add Mon Sep 17 00:00:00 2001 From: Luca Rossi Date: Mon, 2 Mar 2026 11:00:41 +0100 Subject: [PATCH] fix: back/forward nav arrows reopen replaced tabs instead of skipping them (#172) handleReplaceActiveTab removes the old tab from the tabs array, but the old path remained in the navigation history. goBack(isTabOpen) then skipped it because the tab was no longer open, returning null and making the buttons appear broken. Fix: filter by vault entry existence (not tab-open state) and reopen closed tabs via handleSelectNote when navigating back/forward. Co-authored-by: Test Co-authored-by: Claude Opus 4.6 --- src/App.tsx | 24 ++++++++++----- src/components/TabBar.test.tsx | 27 +++++++++++++++++ src/hooks/useNavigationHistory.test.ts | 42 ++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index e0fad570..1eb4a8e6 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -155,23 +155,33 @@ function App() { navFromHistoryRef.current = false }, [notes.activeTabPath]) // eslint-disable-line react-hooks/exhaustive-deps -- navHistory.push is stable - const isTabOpen = useCallback((path: string) => notes.tabs.some(t => t.entry.path === path), [notes.tabs]) + const isEntryExists = useCallback((path: string) => vault.entries.some(e => e.path === path), [vault.entries]) const handleGoBack = useCallback(() => { - const target = navHistory.goBack(isTabOpen) + const target = navHistory.goBack(isEntryExists) if (target) { navFromHistoryRef.current = true - notes.handleSwitchTab(target) + 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, isTabOpen, notes]) + }, [navHistory, isEntryExists, vault.entries, notes]) const handleGoForward = useCallback(() => { - const target = navHistory.goForward(isTabOpen) + const target = navHistory.goForward(isEntryExists) if (target) { navFromHistoryRef.current = true - notes.handleSwitchTab(target) + 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, isTabOpen, notes]) + }, [navHistory, isEntryExists, vault.entries, notes]) // Mouse button 3/4 (back/forward) and macOS trackpad two-finger swipe useEffect(() => { diff --git a/src/components/TabBar.test.tsx b/src/components/TabBar.test.tsx index 3d638c09..d9b13c13 100644 --- a/src/components/TabBar.test.tsx +++ b/src/components/TabBar.test.tsx @@ -215,6 +215,33 @@ describe('TabBar', () => { expect(screen.queryByTestId('tab-new-indicator')).not.toBeInTheDocument() }) + it('renders nav back/forward buttons', () => { + const tabs = makeTabs(['Alpha']) + render() + expect(screen.getByTestId('nav-back')).toBeInTheDocument() + expect(screen.getByTestId('nav-forward')).toBeInTheDocument() + }) + + it('disables nav buttons when canGoBack/canGoForward are false', () => { + const tabs = makeTabs(['Alpha']) + render() + expect(screen.getByTestId('nav-back')).toBeDisabled() + expect(screen.getByTestId('nav-forward')).toBeDisabled() + }) + + it('enables nav buttons and fires handlers on click', () => { + const onGoBack = vi.fn() + const onGoForward = vi.fn() + const tabs = makeTabs(['Alpha']) + render() + + fireEvent.click(screen.getByTestId('nav-back')) + expect(onGoBack).toHaveBeenCalledTimes(1) + + fireEvent.click(screen.getByTestId('nav-forward')) + expect(onGoForward).toHaveBeenCalledTimes(1) + }) + it('switches tab on click', () => { const onSwitchTab = vi.fn() const tabs = makeTabs(['Alpha', 'Beta']) diff --git a/src/hooks/useNavigationHistory.test.ts b/src/hooks/useNavigationHistory.test.ts index 6b987fd3..60876c39 100644 --- a/src/hooks/useNavigationHistory.test.ts +++ b/src/hooks/useNavigationHistory.test.ts @@ -115,6 +115,48 @@ describe('useNavigationHistory', () => { expect(result.current.canGoForward).toBe(false) }) + it('goBack without predicate returns closed-tab paths (replace scenario)', () => { + const { result } = renderHook(() => useNavigationHistory()) + // Simulate: open A, then B replaces A, then C replaces B + act(() => { result.current.push('/a'); result.current.push('/b'); result.current.push('/c') }) + + // Without a predicate, goBack returns /b even though its tab was replaced + let target: string | null = null + act(() => { target = result.current.goBack() }) + expect(target).toBe('/b') + + act(() => { target = result.current.goBack() }) + expect(target).toBe('/a') + }) + + it('goBack with entry-exists predicate skips deleted notes but returns replaced tabs', () => { + const { result } = renderHook(() => useNavigationHistory()) + act(() => { result.current.push('/a'); result.current.push('/deleted'); result.current.push('/c') }) + + // Simulate: /deleted was removed from vault, but /a still exists + const vaultPaths = new Set(['/a', '/c']) + const isEntryExists = (p: string) => vaultPaths.has(p) + + let target: string | null = null + act(() => { target = result.current.goBack(isEntryExists) }) + // Should skip /deleted and return /a + expect(target).toBe('/a') + }) + + it('goForward with entry-exists predicate skips deleted notes', () => { + const { result } = renderHook(() => useNavigationHistory()) + act(() => { result.current.push('/a'); result.current.push('/deleted'); result.current.push('/c') }) + act(() => { result.current.goBack(); result.current.goBack() }) + + const vaultPaths = new Set(['/a', '/c']) + const isEntryExists = (p: string) => vaultPaths.has(p) + + let target: string | null = null + act(() => { target = result.current.goForward(isEntryExists) }) + // Should skip /deleted and return /c + expect(target).toBe('/c') + }) + it('handles long navigation chain', () => { const { result } = renderHook(() => useNavigationHistory()) act(() => {