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 <test@test.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
24
src/App.tsx
24
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(() => {
|
||||
|
||||
@@ -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(<TabBar tabs={tabs} activeTabPath={tabs[0].entry.path} {...defaultProps} />)
|
||||
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(<TabBar tabs={tabs} activeTabPath={tabs[0].entry.path} {...defaultProps} canGoBack={false} canGoForward={false} />)
|
||||
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(<TabBar tabs={tabs} activeTabPath={tabs[0].entry.path} {...defaultProps} canGoBack canGoForward onGoBack={onGoBack} onGoForward={onGoForward} />)
|
||||
|
||||
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'])
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
Reference in New Issue
Block a user