diff --git a/apps/readest-app/src/__tests__/app/library/use-library-navigation.test.ts b/apps/readest-app/src/__tests__/app/library/use-library-navigation.test.ts new file mode 100644 index 00000000..6c7cf1a5 --- /dev/null +++ b/apps/readest-app/src/__tests__/app/library/use-library-navigation.test.ts @@ -0,0 +1,147 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; + +vi.mock('next/navigation', () => ({ + useRouter: vi.fn(), +})); + +import { useRouter, ReadonlyURLSearchParams } from 'next/navigation'; +import { useLibraryNavigation } from '@/app/library/hooks/useLibraryNavigation'; + +function mockRouter() { + return { + push: vi.fn(), + replace: vi.fn(), + back: vi.fn(), + forward: vi.fn(), + refresh: vi.fn(), + prefetch: vi.fn(), + }; +} + +function makeSearchParams(query: string): ReadonlyURLSearchParams { + // URLSearchParams shares the read-only subset of methods we need. + return new URLSearchParams(query) as unknown as ReadonlyURLSearchParams; +} + +describe('useLibraryNavigation', () => { + beforeEach(() => { + vi.clearAllMocks(); + document.documentElement.removeAttribute('data-nav-direction'); + }); + + it('navigates to /library (no query) when going back to root from a group', () => { + // Regression: clicking the breadcrumb "All" button from /library?group=foo + // must call router.replace('/library'). Previously this navigation was + // wrapped through next-view-transitions' useTransitionRouter which is + // incompatible with Next.js 16.2 RSC navigation when the pathname stays + // the same and all query params are removed, causing the click to do + // nothing on the first attempt. See readest/readest#3782. + const router = mockRouter(); + vi.mocked(useRouter).mockReturnValue(router as never); + + const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('group=foo'))); + + act(() => { + result.current(''); + }); + + expect(router.replace).toHaveBeenCalledTimes(1); + expect(router.replace).toHaveBeenCalledWith('/library', undefined); + }); + + it('preserves other search params when going back to root', () => { + const router = mockRouter(); + vi.mocked(useRouter).mockReturnValue(router as never); + + const { result } = renderHook(() => + useLibraryNavigation(makeSearchParams('groupBy=author&group=cf6342e&sort=title')), + ); + + act(() => { + result.current(''); + }); + + expect(router.replace).toHaveBeenCalledTimes(1); + const calledHref = router.replace.mock.calls[0]![0] as string; + expect(calledHref.startsWith('/library?')).toBe(true); + const calledParams = new URLSearchParams(calledHref.slice('/library?'.length)); + expect(calledParams.get('group')).toBeNull(); + expect(calledParams.get('groupBy')).toBe('author'); + expect(calledParams.get('sort')).toBe('title'); + }); + + it('navigates forward to a group by setting the group param', () => { + const router = mockRouter(); + vi.mocked(useRouter).mockReturnValue(router as never); + + const { result } = renderHook(() => useLibraryNavigation(makeSearchParams(''))); + + act(() => { + result.current('group-id-abc'); + }); + + expect(router.replace).toHaveBeenCalledWith('/library?group=group-id-abc', undefined); + }); + + it('replaces the group param when navigating between sibling groups', () => { + const router = mockRouter(); + vi.mocked(useRouter).mockReturnValue(router as never); + + const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('group=group-a'))); + + act(() => { + result.current('group-b'); + }); + + expect(router.replace).toHaveBeenCalledWith('/library?group=group-b', undefined); + }); + + it('sets data-nav-direction to "back" when returning to root from a group', () => { + const router = mockRouter(); + vi.mocked(useRouter).mockReturnValue(router as never); + + const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('group=foo'))); + + act(() => { + result.current(''); + }); + + expect(document.documentElement.getAttribute('data-nav-direction')).toBe('back'); + }); + + it('sets data-nav-direction to "forward" when entering a group', () => { + const router = mockRouter(); + vi.mocked(useRouter).mockReturnValue(router as never); + + const { result } = renderHook(() => useLibraryNavigation(makeSearchParams(''))); + + act(() => { + result.current('group-id'); + }); + + expect(document.documentElement.getAttribute('data-nav-direction')).toBe('forward'); + }); + + it('invokes onBeforeNavigate with the current group before navigating', () => { + const router = mockRouter(); + vi.mocked(useRouter).mockReturnValue(router as never); + const onBeforeNavigate = vi.fn(); + + const { result } = renderHook(() => + useLibraryNavigation(makeSearchParams('group=foo'), onBeforeNavigate), + ); + + act(() => { + result.current(''); + }); + + expect(onBeforeNavigate).toHaveBeenCalledTimes(1); + expect(onBeforeNavigate).toHaveBeenCalledWith('foo'); + // Order matters: callback should run before router.replace so callers can + // capture state (e.g. scroll position) of the leaving view. + expect(onBeforeNavigate.mock.invocationCallOrder[0]).toBeLessThan( + router.replace.mock.invocationCallOrder[0]!, + ); + }); +}); diff --git a/apps/readest-app/src/app/library/hooks/useLibraryNavigation.ts b/apps/readest-app/src/app/library/hooks/useLibraryNavigation.ts new file mode 100644 index 00000000..c4f8ea18 --- /dev/null +++ b/apps/readest-app/src/app/library/hooks/useLibraryNavigation.ts @@ -0,0 +1,51 @@ +import { useCallback } from 'react'; +import { ReadonlyURLSearchParams, useRouter } from 'next/navigation'; + +import { navigateToLibrary } from '@/utils/nav'; + +/** + * Hook for navigating between library views (group/subgroup/root) while + * setting a `data-nav-direction` attribute used by the directional view + * transition CSS. + * + * NOTE: This hook intentionally uses the plain Next.js `useRouter` instead of + * `useAppRouter` (which wraps router calls in `next-view-transitions`'s + * `useTransitionRouter`). The wrapped router is incompatible with Next.js + * 16.2's RSC navigation when only the search params change for the same + * pathname (e.g. `/library?group=foo` -> `/library`), which previously caused + * the breadcrumb "All" button to do nothing on the first click after entering + * a group. See https://github.com/readest/readest/issues/3782 and + * https://github.com/shuding/next-view-transitions/issues/65. + */ +export function useLibraryNavigation( + searchParams: ReadonlyURLSearchParams | null, + onBeforeNavigate?: (currentGroup: string) => void, +) { + const router = useRouter(); + + return useCallback( + (targetGroup: string) => { + const currentGroup = searchParams?.get('group') || ''; + + onBeforeNavigate?.(currentGroup); + + // Detect and set navigation direction so the view transition CSS can + // animate forward (entering a group) vs back (returning to a parent or + // the root) using the appropriate slide direction. + const direction = currentGroup && !targetGroup ? 'back' : 'forward'; + if (typeof document !== 'undefined') { + document.documentElement.setAttribute('data-nav-direction', direction); + } + + const params = new URLSearchParams(searchParams?.toString()); + if (targetGroup) { + params.set('group', targetGroup); + } else { + params.delete('group'); + } + + navigateToLibrary(router, `${params.toString()}`); + }, + [searchParams, router, onBeforeNavigate], + ); +} diff --git a/apps/readest-app/src/app/library/page.tsx b/apps/readest-app/src/app/library/page.tsx index bdaf0212..7363eaa9 100644 --- a/apps/readest-app/src/app/library/page.tsx +++ b/apps/readest-app/src/app/library/page.tsx @@ -61,6 +61,7 @@ import { BackupWindow } from './components/BackupWindow'; import { useDragDropImport } from './hooks/useDragDropImport'; import { useTransferQueue } from '@/hooks/useTransferQueue'; import { useAppRouter } from '@/hooks/useAppRouter'; +import { useLibraryNavigation } from './hooks/useLibraryNavigation'; import { Toast } from '@/components/Toast'; import { createBookGroups, @@ -151,31 +152,15 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP } }, []); - // Unified navigation function that handles scroll position and direction - const handleLibraryNavigation = useCallback( - (targetGroup: string) => { - const currentGroup = searchParams?.get('group') || ''; - - // Save current scroll position BEFORE navigation - saveScrollPosition(currentGroup); - - // Detect and set navigation direction - const direction = currentGroup && !targetGroup ? 'back' : 'forward'; - document.documentElement.setAttribute('data-nav-direction', direction); - - // Build query params - const params = new URLSearchParams(searchParams?.toString()); - if (targetGroup) { - params.set('group', targetGroup); - } else { - params.delete('group'); - } - - navigateToLibrary(router, `${params.toString()}`); - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [searchParams, router], - ); + // Unified navigation function that handles scroll position and direction. + // Uses a plain Next.js router (via useLibraryNavigation) instead of the + // view-transition-wrapped useAppRouter, because next-view-transitions@0.3.5 + // is incompatible with Next.js 16.2 RSC navigation when only search params + // change for the same pathname (e.g. /library?group=foo -> /library), which + // previously broke the breadcrumb "All" button on the first click after + // entering a group. See https://github.com/readest/readest/issues/3782 and + // https://github.com/shuding/next-view-transitions/issues/65. + const handleLibraryNavigation = useLibraryNavigation(searchParams, saveScrollPosition); useTheme({ systemUIVisible: true, appThemeColor: 'base-200' }); useUICSS();