fix(reader): apply page margin changes live on all platforms (#4898) (#4900)

Adjusting the top, bottom, left, or right page margin had no visible
effect until an unrelated setting (e.g. Show Header) was toggled.

The BooksGrid perf refactor (#4562) memoized the derived view/content
insets on the ViewSettings object identity. saveViewSettings mutates
ViewSettings in place (same reference), so the memo never recomputed on
a margin edit and the new margin never reached the paginator. Left and
right margins were always stale; top and bottom only refreshed when the
header/footer visibility (an effect dependency) changed, which is why
toggling the header appeared to apply a pending change.

Extract the inset derivation into useContentInsets and memoize by the
resolved numeric values instead of the object reference: identical
numbers across a page turn keep a stable reference (no re-render storm),
while a changed margin yields a new one that propagates to the renderer.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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> = {}): 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);
+  });
+});

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<BookCellProps> = ({
   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.

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 };
+};