Elgato_dark/readest
1
0
Fork 0
mirror of https://github.com/readest/readest synced 2026-04-21 13:37:44 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix(library): fixed the All button in groups breadcrumbs navigation bar, closes #3782 (#3832)

Browse source
This commit is contained in:
Huang Xin 2026-04-12 01:55:08 +08:00 committed by GitHub
parent 030a7c0823
commit 2a49e93cf7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 50 additions and 209 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
.gitignore vendored
View file

@ -49,6 +49,7 @@ fastlane/metadata/android/en-US/changelogs
result*
.playwright-mcp/
.gstack
.claude/worktrees
.claude/settings.local.json

147
apps/readest-app/src/__tests__/app/library/use-library-navigation.test.ts
View file

@ -1,147 +0,0 @@
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]!,
);
});
});

51
apps/readest-app/src/app/library/hooks/useLibraryNavigation.ts
View file

@ -1,51 +0,0 @@
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],
);
}

60
apps/readest-app/src/app/library/page.tsx
View file

@ -62,7 +62,6 @@ 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,
@ -152,16 +151,6 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP
}
}, []);
// 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();
@ -206,6 +195,55 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP
sessionStorage.setItem('lastLibraryParams', searchParams?.toString() || '');
}, [searchParams]);
// Strip the empty `group=` param that `handleLibraryNavigation` sets as a
// workaround for a Next.js 16.2 static-export regression (see the NOTE
// above `handleLibraryNavigation` for full context). This effect runs
// after the router.replace() has committed, so React has already
// re-rendered with the new (empty) group state; we're only rewriting the
// URL cosmetically via window.history.replaceState — Next.js' patched
// replaceState will pick up the new canonical URL without triggering
// another navigation.
useEffect(() => {
if (typeof window === 'undefined') return;
if (searchParams?.get('group') !== '') return;
const url = new URL(window.location.href);
url.searchParams.delete('group');
const cleanHref = `${url.pathname}${url.search}${url.hash}`;
window.history.replaceState(null, '', cleanHref);
}, [searchParams]);
// Unified navigation function that handles scroll position and direction.
// Workaround for a Next.js 16.2 static-export regression: navigating to a
// same-pathname URL with an empty search string causes `router.replace()`
// to silently no-op (e.g. `/library?group=foo` -> `/library`), which broke
// the breadcrumb "All" button. By always calling `params.set('group',
// targetGroup)` — including when `targetGroup` is an empty string — the
// resulting URL becomes `/library?group=` instead of `/library`, which
// Next.js does commit. The trailing empty `group=` is stripped via a
// cleanup effect below (purely cosmetic URL rewrite). See
// https://github.com/readest/readest/issues/3782.
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 — always `set` so the search string is non-empty
// even when targetGroup is '' (the Next.js 16.2 workaround).
const params = new URLSearchParams(searchParams?.toString());
params.set('group', targetGroup);
navigateToLibrary(router, `${params.toString()}`);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[searchParams, router],
);
useEffect(() => {
const doCheckAppUpdates = async () => {
if (appService?.hasUpdater && settings.autoCheckUpdates) {