Skip to content

Commit a00112f

Browse files
Extract useLatestRef/useRecenterSnap hooks, fix verse-0 echo nav
1 parent 48cf46c commit a00112f

8 files changed

Lines changed: 404 additions & 224 deletions

src/__tests__/components/InterlinearNavContext.test.tsx

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,38 @@ describe('InterlinearNavContext', () => {
105105
expect(result.current.rawScrRef).toEqual(ref);
106106
});
107107

108+
it('keeps the current verse when the host echoes a verse-0 reference for the chapter already shown', () => {
109+
// After a verse navigation the host re-broadcasts the chapter as a separate verse-0 reference.
110+
// Normalizing that to verse 1 would read as a fresh move off the verse the user is on, so a
111+
// verse-0 echo for the same book+chapter must stay sticky on the current verse.
112+
const { result, setRef, rerender } = renderNavMutable({
113+
book: 'GEN',
114+
chapterNum: 3,
115+
verseNum: 7,
116+
});
117+
expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 7 });
118+
119+
act(() => setRef({ book: 'GEN', chapterNum: 3, verseNum: 0 }));
120+
rerender();
121+
122+
expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 7 });
123+
});
124+
125+
it('normalizes a verse-0 reference that names a different chapter (a real chapter jump)', () => {
126+
// A verse-0 reference for a chapter other than the one shown is a genuine chapter navigation, not
127+
// an echo, so it still normalizes to that chapter's first verse.
128+
const { result, setRef, rerender } = renderNavMutable({
129+
book: 'GEN',
130+
chapterNum: 3,
131+
verseNum: 7,
132+
});
133+
134+
act(() => setRef({ book: 'GEN', chapterNum: 4, verseNum: 0 }));
135+
rerender();
136+
137+
expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 1 });
138+
});
139+
108140
it('throws when used outside a provider', () => {
109141
expect(() => renderHook(() => useInterlinearNav())).toThrow(
110142
'useInterlinearNav must be used within an InterlinearNavProvider',

src/__tests__/components/InterlinearizerLoader.test.tsx

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,27 +1190,20 @@ describe('InterlinearizerLoader', () => {
11901190
expect(fadeOpacity()).toBe('0');
11911191
});
11921192

1193-
it('freezes the scrRef handed to Interlinearizer until the new book loads', () => {
1193+
it('shows the Loading curtain (not the old book) during a cross-book swap', () => {
11941194
const { setRef, rerenderNow } = renderLoader({ book: 'GEN', chapterNum: 1, verseNum: 5 });
1195-
expect(capturedInterlinearizerProps?.scrRef).toEqual({
1196-
book: 'GEN',
1197-
chapterNum: 1,
1198-
verseNum: 5,
1199-
});
1195+
expect(screen.getByTestId('interlinearizer')).toBeInTheDocument();
12001196

1201-
// Cross-book jump to MAT while the loaded book is still GEN (the window before the USJ arrives
1202-
// / Interlinearizer remounts). The still-mounted views must keep receiving the last in-book GEN
1203-
// reference, never the out-of-book MAT one — otherwise they reseed focus and scroll toward a
1204-
// verse absent from the mounted book, the pre-fade shuffle this freeze exists to prevent.
1197+
// Cross-book jump to MAT while the loaded book is still GEN (the window before the USJ arrives /
1198+
// Interlinearizer remounts). Rather than leave the previous book's views mounted — where they
1199+
// would show through the fade as the swap happens — the loader shows the Loading curtain, so
1200+
// nothing of either book is visible until the new one mounts and fades in.
12051201
setRef({ book: 'MAT', chapterNum: 5, verseNum: 3 });
12061202
rerenderNow();
1207-
expect(capturedInterlinearizerProps?.scrRef).toEqual({
1208-
book: 'GEN',
1209-
chapterNum: 1,
1210-
verseNum: 5,
1211-
});
1203+
expect(screen.queryByTestId('interlinearizer')).not.toBeInTheDocument();
1204+
expect(screen.getByText('Loading…')).toBeInTheDocument();
12121205

1213-
// Once MAT's book data arrives, Interlinearizer remounts on it and receives the live MAT ref.
1206+
// Once MAT's book data arrives, Interlinearizer mounts on it and receives the live MAT ref.
12141207
mockBookData({ book: { ...GEN_1_1_BOOK, id: 'MAT', bookRef: 'MAT' } });
12151208
rerenderNow();
12161209
expect(capturedInterlinearizerProps?.scrRef).toEqual({

src/components/ContinuousView.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type { LinkSlot, TokenGroup } from '../types/token-layout';
1313
import { buildRenderUnits, groupTokens, resolveFocusContext } from '../utils/token-layout';
1414
import { useArcPaths } from '../hooks/useArcPaths';
1515
import { usePhraseHoverState } from '../hooks/usePhraseHoverState';
16+
import useLatestRef from '../hooks/useLatestRef';
1617
import MemoizedArcOverlay from './ArcOverlay';
1718
import { RECENTER_FADE_MS, RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade';
1819

@@ -330,17 +331,15 @@ export default function ContinuousView({
330331
focusedTokenRef !== undefined ? tokenSegmentMap.get(focusedTokenRef) : undefined;
331332

332333
/** Ref mirror of the target so the post-scroll timeout reads the latest value without a dep. */
333-
const targetActiveSegmentIdRef = useRef(targetActiveSegmentId);
334-
targetActiveSegmentIdRef.current = targetActiveSegmentId;
334+
const targetActiveSegmentIdRef = useLatestRef(targetActiveSegmentId);
335335

336336
/** Snaps the committed active segment to the current target; runs after an internal-nav scroll. */
337337
const commitPendingActiveSegment = useCallback(() => {
338338
setCommittedActiveSegmentId(targetActiveSegmentIdRef.current);
339-
}, []);
339+
}, [targetActiveSegmentIdRef]);
340340

341341
/** Ref mirror of `onFocusedTokenRefChange` so callbacks never need it as a dep. */
342-
const onFocusedTokenRefChangeRef = useRef(onFocusedTokenRefChange);
343-
onFocusedTokenRefChangeRef.current = onFocusedTokenRefChange;
342+
const onFocusedTokenRefChangeRef = useLatestRef(onFocusedTokenRefChange);
344343

345344
/**
346345
* Emits a focus change that originated _inside_ the strip (arrow nav, phrase click, edit-mode
@@ -352,10 +351,13 @@ export default function ContinuousView({
352351
*
353352
* @param ref - The word-token ref to focus.
354353
*/
355-
const emitInternalFocus = useCallback((ref: string) => {
356-
internalFocusedTokenRefRef.current = ref;
357-
onFocusedTokenRefChangeRef.current(ref);
358-
}, []);
354+
const emitInternalFocus = useCallback(
355+
(ref: string) => {
356+
internalFocusedTokenRefRef.current = ref;
357+
onFocusedTokenRefChangeRef.current(ref);
358+
},
359+
[onFocusedTokenRefChangeRef],
360+
);
359361

360362
// Notify the parent of the initially-focused token on mount so the segment list scrolls the
361363
// active verse into view on first render. Only fires when no token was already focused.

src/components/InterlinearNavContext.tsx

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,31 @@ export function InterlinearNavProvider({
165165
}>) {
166166
const [rawScrRef, setScrRef, scrollGroupId, setScrollGroupId] = useWebViewScrollGroupScrRef();
167167

168-
const liveScrRef = useMemo(() => normalizeScrRef(rawScrRef), [rawScrRef]);
168+
/**
169+
* The last committed {@link liveScrRef}, mirrored so the verse-0 stickiness below can compare the
170+
* incoming `rawScrRef` against the verse currently shown.
171+
*/
172+
const liveScrRefRef = useRef<SerializedVerseRef>(normalizeScrRef(rawScrRef));
173+
174+
// After a verse navigation the host re-broadcasts the *chapter* to the scroll group as a separate
175+
// `verseNum: 0` reference (an echo of the current location, not a real move). Normalizing that to
176+
// verse 1 unconditionally would read as a fresh navigation to the chapter's first verse, fading and
177+
// recentering the views off the verse the user is actually on. So a verse-0 reference that names the
178+
// book+chapter already shown is treated as sticky: keep the current `liveScrRef` (its real verse)
179+
// rather than snapping to verse 1. A verse-0 reference for a *different* chapter is a genuine
180+
// chapter jump and normalizes to verse 1 as before.
181+
const liveScrRef = useMemo(() => {
182+
const prev = liveScrRefRef.current;
183+
if (
184+
rawScrRef.verseNum === 0 &&
185+
rawScrRef.book === prev.book &&
186+
rawScrRef.chapterNum === prev.chapterNum
187+
) {
188+
return prev;
189+
}
190+
return normalizeScrRef(rawScrRef);
191+
}, [rawScrRef]);
192+
liveScrRefRef.current = liveScrRef;
169193

170194
/**
171195
* Verse keys of internal navigations still awaiting their host round-trip. A `navigate(ref,

src/components/InterlinearizerLoader.tsx

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { InterlinearProject, TextAnalysis } from 'interlinearizer';
55
import { TabToolbar } from 'platform-bible-react';
66
import type { SelectMenuItemHandler } from 'platform-bible-react';
77
import { isPlatformError } from 'platform-bible-utils';
8-
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
8+
import { useCallback, useEffect, useMemo, useState } from 'react';
99
import useInterlinearizerBookData from '../hooks/useInterlinearizerBookData';
1010
import useOptimisticBooleanSetting from '../hooks/useOptimisticBooleanSetting';
1111
import type { InterlinearProjectSummary } from '../types/interlinear-project-summary';
@@ -221,7 +221,14 @@ function InterlinearizerLoaderInner({
221221
isHideInactiveLinkButtonsLoading ||
222222
isSimplifyPhrasesLoading ||
223223
isChapterLabelInVerseLoading;
224-
const showLoading = isLoading || isAnalysisLoading || isSettingLoading;
224+
// True during a cross-book swap: the live `scrRef` already names the new book but the loaded `book`
225+
// is still the previous one (its USJ hasn't arrived yet). The old `Interlinearizer` is still
226+
// mounted here; showing it (even frozen on its last in-book reference) lets the previous book's
227+
// components stay visible while the new book loads, so the swap is seen before the fade hides it.
228+
// Treating this window as loading swaps the old view for the Loading… curtain immediately, so
229+
// nothing of either book shows until the new one has mounted and fades in.
230+
const isCrossBookSwap = !!book && scrRef.book !== book.bookRef;
231+
const showLoading = isLoading || isAnalysisLoading || isSettingLoading || isCrossBookSwap;
225232
const isLoaded = !hasError && !showLoading && !!book;
226233

227234
// Abort any in-flight cross-book fade when the new book fails to load, so the error is revealed
@@ -230,22 +237,6 @@ function InterlinearizerLoaderInner({
230237
if (hasError) cancelFade();
231238
}, [hasError, cancelFade]);
232239

233-
/**
234-
* The scripture reference handed to {@link Interlinearizer}. While a cross-book navigation is in
235-
* flight, `scrRef` already names the new book but the loaded `book` is still the previous one
236-
* (its USJ hasn't arrived), so the still-mounted views would react to a verse absent from the
237-
* mounted book — reseeding focus and scrolling toward it, a visible shuffle behind/before the
238-
* fade. Gating on `book.bookRef` keeps the views on a reference that matches the book they
239-
* actually render: the live `scrRef` only once the loaded book matches it, otherwise the last
240-
* in-book reference. Once the new book's USJ arrives, `Interlinearizer` remounts on it (it is
241-
* keyed by `book.bookRef`, so a book change tears down the old instance rather than updating it
242-
* in place with carried-over scroll/focus state) and immediately receives the live (new-book)
243-
* reference, so it seeds focus on the intended verse and reports settled.
244-
*/
245-
const lastInBookScrRefRef = useRef(scrRef);
246-
if (book && scrRef.book === book.bookRef) lastInBookScrRefRef.current = scrRef;
247-
const viewScrRef = book && scrRef.book === book.bookRef ? scrRef : lastInBookScrRefRef.current;
248-
249240
const [modal, setModal] = useState<ModalState>('none');
250241

251242
const [phraseMode, setPhraseMode] = useState<PhraseMode>({ kind: 'view' });
@@ -376,7 +367,7 @@ function InterlinearizerLoaderInner({
376367
key={`${activeProject?.id ?? ''}:${book.bookRef}`}
377368
book={book}
378369
continuousScroll={continuousScroll}
379-
scrRef={viewScrRef}
370+
scrRef={scrRef}
380371
analysisLanguage={analysisLanguage}
381372
initialAnalysis={activeProjectAnalysis}
382373
onSaveAnalysis={handleSaveAnalysis}

src/hooks/useLatestRef.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import type { RefObject } from 'react';
2+
import { useRef } from 'react';
3+
4+
/**
5+
* Mirrors a value into a ref whose `.current` always holds the latest render's value, reassigned on
6+
* every render (before effects run). Lets a `useCallback`/effect read the current value through the
7+
* ref without listing it as a dependency, so the callback keeps a stable identity even when the
8+
* value changes identity each render — the PAPI host, for instance, hands `scrRef` back as a fresh
9+
* object on many renders, and closing over it directly would churn the identity of any callback
10+
* that reads it. Reading through the ref decouples that churn.
11+
*
12+
* @param value - The value to mirror; the returned ref's `.current` is set to it on every render.
13+
* @returns A stable ref object whose `.current` tracks the latest `value`.
14+
*/
15+
export default function useLatestRef<T>(value: T): RefObject<T> {
16+
const ref = useRef(value);
17+
ref.current = value;
18+
return ref;
19+
}

0 commit comments

Comments
 (0)