diff --git a/apps/readest-app/src/__tests__/app/reader/hooks/useContentInsets.test.ts b/apps/readest-app/src/__tests__/app/reader/hooks/useContentInsets.test.ts new file mode 100644 index 00000000..b697a0fc --- /dev/null +++ b/apps/readest-app/src/__tests__/app/reader/hooks/useContentInsets.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { useContentInsets } from '@/app/reader/hooks/useContentInsets'; +import { ViewSettings } from '@/types/book'; + +const ZERO: { top: number; right: number; bottom: number; left: number } = { + top: 0, + right: 0, + bottom: 0, + left: 0, +}; + +const makeViewSettings = (overrides: Partial = {}): ViewSettings => + ({ + showHeader: true, + showFooter: true, + vertical: false, + writingMode: 'horizontal-tb', + marginTopPx: 44, + marginBottomPx: 44, + marginLeftPx: 16, + marginRightPx: 16, + compactMarginTopPx: 16, + compactMarginBottomPx: 16, + compactMarginLeftPx: 16, + compactMarginRightPx: 16, + ...overrides, + }) as ViewSettings; + +describe('useContentInsets', () => { + // Regression for #4898: saveViewSettings mutates the ViewSettings object in + // place (same reference), so left/right margin edits never reached the + // paginator because the derived insets were memoized on the object identity. + it('reflects an in-place left/right margin edit on the same viewSettings object', () => { + const viewSettings = makeViewSettings(); + const { result, rerender } = renderHook(({ vs, gi }) => useContentInsets(vs, gi), { + initialProps: { vs: viewSettings, gi: ZERO }, + }); + + expect(result.current.contentInsets.left).toBe(16); + expect(result.current.contentInsets.right).toBe(16); + + viewSettings.compactMarginLeftPx = 40; + viewSettings.compactMarginRightPx = 32; + rerender({ vs: viewSettings, gi: ZERO }); + + expect(result.current.contentInsets.left).toBe(40); + expect(result.current.contentInsets.right).toBe(32); + }); + + it('reflects an in-place top/bottom margin edit when header/footer are shown', () => { + const viewSettings = makeViewSettings(); + const { result, rerender } = renderHook(({ vs, gi }) => useContentInsets(vs, gi), { + initialProps: { vs: viewSettings, gi: ZERO }, + }); + + expect(result.current.contentInsets.top).toBe(44); + expect(result.current.contentInsets.bottom).toBe(44); + + viewSettings.marginTopPx = 60; + viewSettings.marginBottomPx = 52; + rerender({ vs: viewSettings, gi: ZERO }); + + expect(result.current.contentInsets.top).toBe(60); + expect(result.current.contentInsets.bottom).toBe(52); + }); + + it('adds the grid insets to the page margins', () => { + const viewSettings = makeViewSettings(); + const { result } = renderHook(() => + useContentInsets(viewSettings, { top: 10, right: 4, bottom: 8, left: 6 }), + ); + + expect(result.current.contentInsets.top).toBe(54); + expect(result.current.contentInsets.left).toBe(22); + }); + + it('keeps a stable contentInsets reference across a page turn (unchanged margins)', () => { + const viewSettings = makeViewSettings(); + const { result, rerender } = renderHook(({ vs, gi }) => useContentInsets(vs, gi), { + initialProps: { vs: viewSettings, gi: ZERO }, + }); + + const first = result.current.contentInsets; + // A page turn re-renders the cell without touching any margin setting. + rerender({ vs: viewSettings, gi: ZERO }); + + expect(result.current.contentInsets).toBe(first); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/BooksGrid.tsx b/apps/readest-app/src/app/reader/components/BooksGrid.tsx index 8313e832..d2f40fa8 100644 --- a/apps/readest-app/src/app/reader/components/BooksGrid.tsx +++ b/apps/readest-app/src/app/reader/components/BooksGrid.tsx @@ -10,7 +10,7 @@ import { useSidebarStore } from '@/store/sidebarStore'; import { useBookDataStore } from '@/store/bookDataStore'; import { useTranslation } from '@/hooks/useTranslation'; import { getGridTemplate, getInsetEdges } from '@/utils/grid'; -import { getViewInsets } from '@/utils/insets'; +import { useContentInsets } from '../hooks/useContentInsets'; import SearchResultsNav from './sidebar/SearchResultsNav'; import BooknotesNav from './sidebar/BooknotesNav'; import FoliateViewer from './FoliateViewer'; @@ -118,33 +118,10 @@ const BookCellInner: React.FC = ({ const config = getConfig(bookKey); const { book, bookDoc } = bookData || {}; - // viewSettings drives both viewInsets and the inset-derived geometry. - // Memoize off its identity so contentInsets stays stable while the - // user is just turning pages — viewSettings only changes when the - // user toggles settings, not on every relocate. Same logic for - // gridInsets which is keyed off the resolved Insets numbers. - const viewInsets = useMemo( - () => (viewSettings ? getViewInsets(viewSettings) : { top: 0, right: 0, bottom: 0, left: 0 }), - [viewSettings], - ); - const contentInsets = useMemo( - () => ({ - top: gridInsets.top + viewInsets.top, - right: gridInsets.right + viewInsets.right, - bottom: gridInsets.bottom + viewInsets.bottom, - left: gridInsets.left + viewInsets.left, - }), - [ - gridInsets.top, - gridInsets.right, - gridInsets.bottom, - gridInsets.left, - viewInsets.top, - viewInsets.right, - viewInsets.bottom, - viewInsets.left, - ], - ); + // viewInsets/contentInsets stay stable while the user is just turning pages + // (margins are unchanged) but update when a margin setting changes — even + // though saveViewSettings mutates viewSettings in place (#4898). + const { viewInsets, contentInsets } = useContentInsets(viewSettings, gridInsets); // Stable callback so HeaderBar doesn't see a new prop reference per // BooksGrid render. diff --git a/apps/readest-app/src/app/reader/hooks/useContentInsets.ts b/apps/readest-app/src/app/reader/hooks/useContentInsets.ts new file mode 100644 index 00000000..8f0b2189 --- /dev/null +++ b/apps/readest-app/src/app/reader/hooks/useContentInsets.ts @@ -0,0 +1,48 @@ +import { useMemo } from 'react'; +import { Insets } from '@/types/misc'; +import { ViewSettings } from '@/types/book'; +import { getViewInsets } from '@/utils/insets'; + +const ZERO_INSETS: Insets = { top: 0, right: 0, bottom: 0, left: 0 }; + +/** + * Resolve the reader's view insets (page margins) and content insets + * (grid insets + page margins) for a book cell. + * + * `saveViewSettings` mutates the `ViewSettings` object in place (keeping the + * same reference), so memoizing on the object identity would freeze the page + * margins: a margin edit changes a field but not the reference, so the memo + * never recomputes and the new margin never reaches the paginator (#4898). + * + * `getViewInsets` is cheap, so resolve it every render and memoize by the + * resolved numeric values instead: identical numbers across a page turn keep + * the same object reference (children bail out of re-rendering), while a + * changed margin yields a new reference that propagates to the paginator. + */ +export const useContentInsets = (viewSettings: ViewSettings | null, gridInsets: Insets) => { + const resolved = viewSettings ? getViewInsets(viewSettings) : ZERO_INSETS; + const viewInsets = useMemo( + () => resolved, + // eslint-disable-next-line react-hooks/exhaustive-deps + [resolved.top, resolved.right, resolved.bottom, resolved.left], + ); + const contentInsets = useMemo( + () => ({ + top: gridInsets.top + viewInsets.top, + right: gridInsets.right + viewInsets.right, + bottom: gridInsets.bottom + viewInsets.bottom, + left: gridInsets.left + viewInsets.left, + }), + [ + gridInsets.top, + gridInsets.right, + gridInsets.bottom, + gridInsets.left, + viewInsets.top, + viewInsets.right, + viewInsets.bottom, + viewInsets.left, + ], + ); + return { viewInsets, contentInsets }; +};