diff --git a/src/__tests__/components/InterlinearNavContext.test.tsx b/src/__tests__/components/InterlinearNavContext.test.tsx index 7f6d3218..ad6aeb07 100644 --- a/src/__tests__/components/InterlinearNavContext.test.tsx +++ b/src/__tests__/components/InterlinearNavContext.test.tsx @@ -29,7 +29,7 @@ function renderNav(hook: () => ScrollGroupTuple) { /** * Renders the nav hook with a scroll-group stub whose reference can be restaged between rerenders, * so a cross-book navigation can be simulated. A fresh object identity is required on each change - * so the provider's `liveScrRef` memo recomputes. + * so the provider adopts it as a new `scrRef`. * * @param initial - The reference reported on the first render. * @returns The render-hook result plus a `setRef` to stage the next reference (call inside `act`, @@ -51,13 +51,13 @@ function renderNavMutable(initial: SerializedVerseRef) { } describe('InterlinearNavContext', () => { - it('exposes the raw reference and scroll-group plumbing verbatim', () => { + it('exposes the host reference and scroll-group plumbing verbatim', () => { const setScrRef = jest.fn(); const setScrollGroupId = jest.fn(); const ref: SerializedVerseRef = { book: 'GEN', chapterNum: 3, verseNum: 4 }; const { result } = renderNav(makeScrollGroupHook(ref, setScrRef, 2, setScrollGroupId)); - expect(result.current.rawScrRef).toEqual(ref); + expect(result.current.scrRef).toEqual(ref); expect(result.current.scrollGroupId).toBe(2); act(() => result.current.navigate({ book: 'MAT', chapterNum: 1, verseNum: 1 })); @@ -67,57 +67,38 @@ describe('InterlinearNavContext', () => { expect(setScrollGroupId).toHaveBeenCalledWith(3); }); - it('passes a verse-level reference through to liveScrRef unchanged', () => { + it('passes a verse-level reference through to scrRef unchanged', () => { const ref: SerializedVerseRef = { book: 'GEN', chapterNum: 3, verseNum: 4 }; const { result } = renderNav(makeScrollGroupHook(ref)); - expect(result.current.liveScrRef).toEqual(ref); + expect(result.current.scrRef).toEqual(ref); }); - it('passes a chapter-level (verse 0) reference through to liveScrRef unchanged', () => { + it('passes a chapter-level (verse 0) reference through to scrRef unchanged', () => { // Verse 0 is a real verse (a Psalm superscription), so it is no longer mapped to verse 1 here. // The loader resolves it to verse 1 only when the loaded book has no verse-0 segment. const ref: SerializedVerseRef = { book: 'GEN', chapterNum: 3, verseNum: 0 }; const { result } = renderNav(makeScrollGroupHook(ref)); - expect(result.current.liveScrRef).toEqual(ref); - expect(result.current.rawScrRef).toEqual(ref); + expect(result.current.scrRef).toEqual(ref); }); - it('keeps the current verse when the host echoes a verse-0 reference for the chapter already shown', () => { - // After a verse navigation the host re-broadcasts the chapter as a separate verse-0 reference. - // Normalizing that to verse 1 would read as a fresh move off the verse the user is on, so a - // verse-0 echo for the same book+chapter must stay sticky on the current verse. + it('passes a same-chapter verse-0 reference through to scrRef (host < from verse 1)', () => { + // Verse 0 is a real, focusable verse (a chapter superscription), so a verse-0 reference for the + // chapter already shown passes through verbatim rather than being held on the current verse. This + // is the host's `<` (previous-verse) from verse 1: it must land on the superscription, which the + // loader resolves from verse 0 (or back to verse 1 when the chapter has no verse-0 segment). const { result, setRef, rerender } = renderNavMutable({ book: 'GEN', chapterNum: 3, - verseNum: 7, + verseNum: 1, }); - expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 7 }); + expect(result.current.scrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 1 }); act(() => setRef({ book: 'GEN', chapterNum: 3, verseNum: 0 })); rerender(); - expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 7 }); - }); - - it('passes a same-chapter verse-0 echo through when it matches a fresh internal-nav marker', () => { - // Selecting a verse-0 superscription navigates the host to verse 0 of the chapter already shown, - // so the host's echo is shaped exactly like the spurious post-nav chapter echo. A fresh - // internal-nav marker for that verse-0 key marks it as our own deliberate move, so the stickiness - // exception lets it through to the superscription rather than holding the prior verse. - const { result, setRef, rerender } = renderNavMutable({ - book: 'GEN', - chapterNum: 3, - verseNum: 7, - }); - - act(() => result.current.navigate({ book: 'GEN', chapterNum: 3, verseNum: 0 }, 'internal')); - - act(() => setRef({ book: 'GEN', chapterNum: 3, verseNum: 0 })); - rerender(); - - expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 0 }); + expect(result.current.scrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 0 }); }); it('passes a verse-0 reference for a different chapter through as a real chapter jump', () => { @@ -133,28 +114,26 @@ describe('InterlinearNavContext', () => { act(() => setRef({ book: 'GEN', chapterNum: 4, verseNum: 0 })); rerender(); - expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 0 }); + expect(result.current.scrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 0 }); }); describe('duplicate host deliveries', () => { - it('keeps rawScrRef and liveScrRef identity when the host re-sends a value-equal reference', () => { + it('keeps scrRef identity when the host re-sends a value-equal reference', () => { // The scripture picker fires each external navigation twice in quick succession, the second // delivery being a fresh object with identical content. The provider must hand back the - // previously adopted objects so the duplicate is invisible to consumers (no context-value + // previously adopted object so the duplicate is invisible to consumers (no context-value // change, no re-render churn mid-recenter). const { result, setRef, rerender } = renderNavMutable({ book: 'GEN', chapterNum: 3, verseNum: 7, }); - const rawBefore = result.current.rawScrRef; - const liveBefore = result.current.liveScrRef; + const before = result.current.scrRef; act(() => setRef({ book: 'GEN', chapterNum: 3, verseNum: 7 })); rerender(); - expect(result.current.rawScrRef).toBe(rawBefore); - expect(result.current.liveScrRef).toBe(liveBefore); + expect(result.current.scrRef).toBe(before); }); it('treats a verse-0 chapter jump and its verse-1 form as two distinct deliveries', () => { @@ -169,13 +148,12 @@ describe('InterlinearNavContext', () => { act(() => setRef({ book: 'GEN', chapterNum: 4, verseNum: 0 })); rerender(); - const liveAfterJump = result.current.liveScrRef; - expect(liveAfterJump).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 0 }); + expect(result.current.scrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 0 }); act(() => setRef({ book: 'GEN', chapterNum: 4, verseNum: 1 })); rerender(); - expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 1 }); + expect(result.current.scrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 1 }); }); it('reuses the previous reference when a duplicate differs only in the verse segment string', () => { @@ -188,7 +166,7 @@ describe('InterlinearNavContext', () => { act(() => setRef({ book: 'GEN', chapterNum: 3, verseNum: 7, verse: '7a' })); rerender(); - expect(result.current.rawScrRef).toBe(initial); + expect(result.current.scrRef).toBe(initial); }); it('reuses the previous reference when a duplicate differs only in versification', () => { @@ -207,7 +185,7 @@ describe('InterlinearNavContext', () => { ); rerender(); - expect(result.current.rawScrRef).toBe(initial); + expect(result.current.scrRef).toBe(initial); }); }); @@ -542,9 +520,11 @@ describe('InterlinearNavContext', () => { expect(result.current.fadePhase).toBe('out'); }); - it('does not re-engage the curtain for a verse-0 echo during the fade-in', () => { - // The host's chapter re-broadcast (verse 0 for the chapter already shown) is sticky — it - // names the verse currently displayed, so it must not read as a fresh navigation mid-reveal. + it('re-engages the curtain for a verse-0 navigation arriving during the fade-in', () => { + // Verse 0 is an ordinary verse (a chapter superscription), so a verse-0 reference naming a + // different verse than the one shown is a real mid-reveal navigation — e.g. the host's `<` from + // verse 1 landing on the superscription. It re-engages the curtain like any other external move + // arriving while the new book is still fading in, rather than fading the fresh content twice. const { result, rerender, setRef } = renderNavMutable({ book: 'GEN', chapterNum: 1, @@ -559,7 +539,7 @@ describe('InterlinearNavContext', () => { act(() => setRef({ book: 'ZEP', chapterNum: 3, verseNum: 0 })); rerender(); - expect(result.current.fadePhase).toBe('in'); + expect(result.current.fadePhase).toBe('out'); }); it('clears the pending fade-in timer on unmount', () => { diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 9a133829..fde0eb24 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -535,8 +535,8 @@ describe('Interlinearizer', () => { it('writes a verse-0 reference to the host when a verse-0 token is selected', () => { // Selecting a superscription token navigates the host to verse 0 like any other verse; the - // internal-nav marker keeps the host's chapter echo from bouncing the view (the stickiness - // exception in InterlinearNavContext). Default scrRef is GEN 1:1, so this is a real verse change. + // 'internal' origin records a nav marker so the segment window skips the recenter fade for our + // own move. Default scrRef is GEN 1:1, so this is a real verse change. const mockNavigate = jest.fn(); renderInterlinearizer({ book: GEN_SUPERSCRIPTION_BOOK, navigate: mockNavigate }); diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 54958e97..6b28da37 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -527,6 +527,102 @@ describe('InterlinearizerLoader', () => { }); }); + it('resolves an out-of-range verse to the last segment of the same chapter', async () => { + // The host's next-verse button emits verseNum + 1 without clamping, so bumping forward from + // the last verse of a chapter delivers a verse past the chapter's end. The loader resolves it + // to the chapter's last segment rather than letting the views fall back to the start of the + // book. Chapter 4 exists so the test proves resolution stays within the requested chapter. + const multiSegmentBook: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + segments: [ + { + id: 'GEN 3:1', + startRef: { book: 'GEN', chapter: 3, verse: 1 }, + endRef: { book: 'GEN', chapter: 3, verse: 1 }, + baselineText: 'First verse.', + tokens: [], + }, + { + id: 'GEN 3:2', + startRef: { book: 'GEN', chapter: 3, verse: 2 }, + endRef: { book: 'GEN', chapter: 3, verse: 2 }, + baselineText: 'Last verse of the chapter.', + tokens: [], + }, + { + id: 'GEN 4:1', + startRef: { book: 'GEN', chapter: 4, verse: 1 }, + endRef: { book: 'GEN', chapter: 4, verse: 1 }, + baselineText: 'Next chapter.', + tokens: [], + }, + ], + }; + mockBookData({ book: multiSegmentBook }); + + await act(async () => { + renderLoader({ + useWebViewScrollGroupScrRef: makeScrollGroupHook({ + book: 'GEN', + chapterNum: 3, + verseNum: 3, + }), + }); + }); + + expect(capturedInterlinearizerProps?.scrRef).toEqual({ + book: 'GEN', + chapterNum: 3, + verseNum: 2, + }); + }); + + it('resolves a mid-segment verse to the owning segment start', async () => { + // A merged segment can span several verses; navigating to a verse inside the span has no + // segment starting at that verse, so the loader resolves to the nearest preceding segment + // start — the segment that contains the verse, never a later segment in the chapter. + const mergedSegmentBook: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + segments: [ + { + id: 'GEN 3:1', + startRef: { book: 'GEN', chapter: 3, verse: 1 }, + endRef: { book: 'GEN', chapter: 3, verse: 2 }, + baselineText: 'Two verses merged into one segment.', + tokens: [], + }, + { + id: 'GEN 3:3', + startRef: { book: 'GEN', chapter: 3, verse: 3 }, + endRef: { book: 'GEN', chapter: 3, verse: 3 }, + baselineText: 'Verse after the merged segment.', + tokens: [], + }, + ], + }; + mockBookData({ book: mergedSegmentBook }); + + await act(async () => { + renderLoader({ + useWebViewScrollGroupScrRef: makeScrollGroupHook({ + book: 'GEN', + chapterNum: 3, + verseNum: 2, + }), + }); + }); + + expect(capturedInterlinearizerProps?.scrRef).toEqual({ + book: 'GEN', + chapterNum: 3, + verseNum: 1, + }); + }); + it('leaves a verse-0 reference untouched while the book is still loading', async () => { // With no book loaded yet, the verse-0 resolution has nothing to consult, so the loader shows // the loading placeholder and does not render the interlinearizer. @@ -1310,7 +1406,7 @@ describe('InterlinearizerLoader', () => { /** * Builds a scroll-group hook whose reference can be restaged between rerenders. A fresh object - * identity is required each change so the provider's `liveScrRef` memo recomputes. + * identity is required each change so the provider adopts it as a new `scrRef`. * * @param initial - The reference reported on the first render. * @returns A `[hook, setRef]` pair. diff --git a/src/components/InterlinearNavContext.tsx b/src/components/InterlinearNavContext.tsx index 9e6b3845..890e1dcd 100644 --- a/src/components/InterlinearNavContext.tsx +++ b/src/components/InterlinearNavContext.tsx @@ -62,7 +62,7 @@ function areScrRefsEqual(a: SerializedVerseRef, b: SerializedVerseRef): boolean /** * Builds a stable string key identifying the verse a reference names. Used to match an internal - * navigation against the `liveScrRef` the host later delivers, so the segment window can tell an + * navigation against the reference the host later delivers, so the segment window can tell an * internally-originated change apart from an external one. Verse 0 is keyed verbatim (as its own * verse) so a deliberate navigation to a chapter's verse-0 superscription is distinct from verse * 1. @@ -105,30 +105,19 @@ function isInternalNavMarkerFresh(stampedAt: number | undefined): boolean { */ export interface InterlinearNav { /** - * The reference from the host scroll-group hook, value-verbatim. Drives the editable book/chapter - * nav controls so a chapter selection (verse 0) is reflected exactly as the user entered it. - * Identity-stable across the host's duplicate deliveries: when the host re-sends a value-equal - * reference (the scripture picker fires each navigation twice), the previously adopted object is - * handed back so consumers see no change. + * The reference from the host scroll-group hook, value-verbatim, driving both the editable + * book/chapter nav controls and the views' recentering. Identity-stable across the host's + * duplicate deliveries: when the host re-sends a value-equal reference (the scripture picker + * fires each navigation twice), the previously adopted object is handed back so consumers see no + * change. Verse 0 is treated as an ordinary verse: a `verseNum: 0` reference passes through + * verbatim, so the host's `<` (previous-verse) from verse 1 lands on the chapter's + * superscription. When it names a chapter with a verse-0 superscription segment, that segment + * becomes the active verse. Which verses actually have segments is unknown here (the book is not + * loaded at this layer), so the loader resolves a reference with no matching segment before + * rendering: verse 0 falls back to the chapter's first numbered verse, any other unmatched verse + * to the nearest preceding segment start in its chapter. */ - rawScrRef: SerializedVerseRef; - /** - * The active reference, equal to `rawScrRef` except that a verse-0 reference naming the chapter - * already shown is held sticky on the current verse — unless it matches a fresh internal-nav - * marker (the extension's own move to that chapter's superscription), which passes through. A - * verse-0 reference is otherwise passed through verbatim: when it names a chapter with a verse-0 - * superscription segment, that segment becomes the active verse. Whether a given chapter actually - * has verse-0 content is unknown here (the book is not loaded at this layer), so the loader - * resolves a verse-0 reference with no matching segment back to the chapter's first numbered - * verse before rendering. - * - * The sticky exception exists because, after a verse navigation, the host re-broadcasts the - * chapter as a separate `verseNum: 0` reference (an echo of the current location, not a real - * move); treating that as a jump to verse 0 would yank the view off the verse the user is on. The - * marker carve-out distinguishes that spurious echo from a deliberate verse-0 navigation the - * extension itself just made (which is shaped identically). - */ - liveScrRef: SerializedVerseRef; + scrRef: SerializedVerseRef; /** * Sets the scripture reference, writing through to the host scroll-group ref, and records the * navigation's {@link NavOrigin}. An `internal` origin marks the target verse so the segment @@ -208,84 +197,51 @@ export function InterlinearNavProvider({ const [hostScrRef, setScrRef, scrollGroupId, setScrollGroupId] = useWebViewScrollGroupScrRef(); /** - * The last host delivery adopted as `rawScrRef`, kept so the duplicate-delivery guard below can - * hand back the same object when the host re-sends an identical reference. + * The last host delivery adopted as `scrRef`, serving two readers each render: the + * duplicate-delivery guard below hands back the same object when the host re-sends an identical + * reference, and the render-phase mid-reveal guard compares the incoming reference against the + * verse last shown. + */ + const prevScrRefRef = useRef(hostScrRef); + + /** + * The {@link scrRef} adopted on the previous render, captured before the write below overwrites + * it, so the mid-reveal navigation guard further down can compare the incoming reference against + * the verse last shown. */ - const stableRawScrRefRef = useRef(hostScrRef); + const prevScrRef = prevScrRefRef.current; // The host delivers each scripture-picker navigation twice in quick succession: two back-to-back // signals carrying the same reference as distinct objects. Passing the second through verbatim - // would change `rawScrRef`'s identity — and with it the context value — for a navigation that + // would change `scrRef`'s identity — and with it the context value — for a navigation that // already happened, re-rendering every nav consumer mid-recenter for nothing (the same // double-fire whose duplicate USJ payload `useInterlinearizerBookData` already stabilizes). Reuse // the previously adopted object when the delivery is value-equal, so a duplicate is invisible - // downstream. - const rawScrRef = areScrRefsEqual(hostScrRef, stableRawScrRefRef.current) - ? stableRawScrRefRef.current - : hostScrRef; - stableRawScrRefRef.current = rawScrRef; - - /** - * The last committed {@link liveScrRef}, mirrored so the verse-0 stickiness below can compare the - * incoming `rawScrRef` against the verse currently shown. - */ - const liveScrRefRef = useRef(rawScrRef); + // downstream. Verse 0 is not special-cased: every reference passes through verbatim, so the + // host's `<` (previous-verse) from verse 1 lands on the chapter's superscription, and the loader + // resolves a reference with no matching segment (verse 0, or an unclamped host verse bump past + // the chapter's end) before rendering. + const scrRef = areScrRefsEqual(hostScrRef, prevScrRef) ? prevScrRef : hostScrRef; + prevScrRefRef.current = scrRef; /** * Verse keys of internal navigations still awaiting their host round-trip, each mapped to its * `Date.now()` stamp. `navigate(ref, 'internal')` records `verseKey(ref)`; `consumeInternalNav` * removes it on match. Keyed (not a single value) so that rapid successive clicks both stay * pending and neither host delivery is misread as external. The stamp gives each marker a TTL - * ({@link INTERNAL_NAV_TTL_MS} — see its doc for why stranded markers must expire), honored by ALL - * readers: `consumeInternalNav` (which also evicts expired markers), the verse-0 stickiness - * exception below, and the render-phase mid-reveal guard (both pure reads — no eviction during - * render). + * ({@link INTERNAL_NAV_TTL_MS} — see its doc for why stranded markers must expire), honored by + * both readers: `consumeInternalNav` (which also evicts expired markers) and the render-phase + * mid-reveal guard (a pure read — no eviction during render). */ const pendingInternalNavRef = useRef>(new Map()); - // After a verse navigation the host re-broadcasts the *chapter* to the scroll group as a separate - // `verseNum: 0` reference (an echo of the current location, not a real move). Treating that as a - // jump to verse 0 would yank the views off the verse the user is actually on, so a verse-0 - // reference that names the book+chapter already shown is held sticky: keep the current `liveScrRef` - // (its real verse). - // - // The exception is a verse-0 reference the extension itself just navigated to — selecting a - // chapter's verse-0 superscription segment writes `verseNum: 0` for the chapter already shown, - // which is shaped exactly like the spurious echo. A fresh internal-nav marker for that verse-0 key - // distinguishes the two: when one exists, this is our own deliberate move to the superscription, so - // it passes through (and `consumeInternalNav` clears the marker downstream). A pure read here — no - // eviction during render, matching the render-phase mid-reveal guard. - // - // Every other reference — including a verse-0 reference for a *different* chapter (a genuine chapter - // jump, which the loader resolves to the verse-0 superscription when one exists, else to verse 1) — - // passes through verbatim. The duplicate-delivery guard reuses the previously committed object when - // a re-send is value-equal, so a duplicate never reads as a fresh navigation. - const liveScrRef = useMemo(() => { - const prev = liveScrRefRef.current; - if ( - rawScrRef.verseNum === 0 && - rawScrRef.book === prev.book && - rawScrRef.chapterNum === prev.chapterNum && - !isInternalNavMarkerFresh(pendingInternalNavRef.current.get(verseKey(rawScrRef))) - ) { - return prev; - } - return areScrRefsEqual(rawScrRef, prev) ? prev : rawScrRef; - }, [rawScrRef]); - /** - * The {@link liveScrRef} committed on the previous render, captured before the mirror update below - * overwrites it, so the mid-reveal navigation guard further down can compare the incoming - * reference against the verse last shown. - */ - const prevLiveScrRef = liveScrRefRef.current; - liveScrRefRef.current = liveScrRef; - const navigate = useCallback( (newScrRef: SerializedVerseRef, origin: NavOrigin = 'external') => { - // Invariant: a marker write is ALWAYS paired with the `setScrRef` below. The `liveScrRef` memo - // reads this marker but lists only `[rawScrRef]` as a dependency, relying on every marker write - // also pushing a new ref through the host (which re-runs the memo). Never set a marker here - // without the accompanying `setScrRef`, or the memo could read a stale marker. + // Invariant: a marker write is ALWAYS paired with the `setScrRef` below. The marker readers + // that run during render — the render-phase mid-reveal guard, which lists only `[scrRef]` + // in the reactive path that reaches it — rely on every marker write also pushing a new ref + // through the host, so the render that reads the marker is the one that sees it fresh. Never + // set a marker here without the accompanying `setScrRef`, or a reader could see a stale marker. if (origin === 'internal') pendingInternalNavRef.current.set(verseKey(newScrRef), Date.now()); setScrRef(newScrRef); }, @@ -311,10 +267,10 @@ export function InterlinearNavProvider({ /** * Book code the curtain currently shows fully faded-in. A book change is detected by comparing - * `liveScrRef.book` against this; `reportSettled` advances it to the new book once the view has - * laid out. Seeded to the book at mount so the initial load shows no fade. + * `scrRef.book` against this; `reportSettled` advances it to the new book once the view has laid + * out. Seeded to the book at mount so the initial load shows no fade. */ - const displayedBookRef = useRef(liveScrRef.book); + const displayedBookRef = useRef(scrRef.book); /** * `true` between detecting a book change and the view reporting settled. Gates `reportSettled` so @@ -325,15 +281,18 @@ export function InterlinearNavProvider({ /** Handle of the in-flight fade-in→idle timer, or `undefined` when none is pending. */ const fadeInTimeoutRef = useRef | undefined>(undefined); - /** Verse key of the live ref, computed once for the cross-book / mid-reveal render guards below. */ - const liveKey = verseKey(liveScrRef); + /** + * Verse key of the adopted ref, computed once for the cross-book / mid-reveal render guards + * below. + */ + const scrRefKey = verseKey(scrRef); // Detect a cross-book navigation *during render* and start the fade-out synchronously, so the // curtain drops in the same commit the book ref changes — never a paint later (an effect-driven // fade-out would let the mounted views render one frame against the new book's ref before the // curtain covers them). Setting state during render is the guarded React pattern: `awaitingSettle` // flips true so this fires once per book change, and `setFadePhase` is batched into this commit. - if (liveScrRef.book !== displayedBookRef.current && !awaitingSettleRef.current) { + if (scrRef.book !== displayedBookRef.current && !awaitingSettleRef.current) { if (fadeInTimeoutRef.current !== undefined) { clearTimeout(fadeInTimeoutRef.current); fadeInTimeoutRef.current = undefined; @@ -342,8 +301,8 @@ export function InterlinearNavProvider({ setFadePhase('out'); } else if ( fadePhase === 'in' && - liveKey !== verseKey(prevLiveScrRef) && - !isInternalNavMarkerFresh(pendingInternalNavRef.current.get(liveKey)) + scrRefKey !== verseKey(prevScrRef) && + !isInternalNavMarkerFresh(pendingInternalNavRef.current.get(scrRefKey)) ) { // A follow-up external navigation landing mid-fade-in: the host resolves one picker selection // as two navigations (book change, then precise target), so the second routinely arrives while @@ -363,7 +322,7 @@ export function InterlinearNavProvider({ const reportSettled = useCallback(() => { if (!awaitingSettleRef.current) return; awaitingSettleRef.current = false; - displayedBookRef.current = liveScrRef.book; + displayedBookRef.current = scrRef.book; setFadePhase('in'); /* v8 ignore next -- defensive: render-time book-change guard always clears any stale timer first */ if (fadeInTimeoutRef.current !== undefined) clearTimeout(fadeInTimeoutRef.current); @@ -371,7 +330,7 @@ export function InterlinearNavProvider({ fadeInTimeoutRef.current = undefined; setFadePhase('idle'); }, RECENTER_FADE_MS); - }, [liveScrRef.book]); + }, [scrRef.book]); const cancelFade = useCallback(() => { if (fadeInTimeoutRef.current !== undefined) { @@ -379,9 +338,9 @@ export function InterlinearNavProvider({ fadeInTimeoutRef.current = undefined; } awaitingSettleRef.current = false; - displayedBookRef.current = liveScrRef.book; + displayedBookRef.current = scrRef.book; setFadePhase('idle'); - }, [liveScrRef.book]); + }, [scrRef.book]); // Clear any pending fade-in timer on unmount so a deferred state update doesn't run on a torn-down // tree. @@ -394,8 +353,7 @@ export function InterlinearNavProvider({ const value = useMemo( () => ({ - rawScrRef, - liveScrRef, + scrRef, navigate, consumeInternalNav, scrollGroupId, @@ -405,8 +363,7 @@ export function InterlinearNavProvider({ cancelFade, }), [ - rawScrRef, - liveScrRef, + scrRef, navigate, consumeInternalNav, scrollGroupId, diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index b5c7338e..5bf1c6b7 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -35,11 +35,14 @@ type InterlinearizerProps = Readonly<{ /** When true, the horizontal token strip is shown above the segment list. */ continuousScroll: boolean; /** - * Current scripture reference used to highlight the active verse. Always names a verse that has a - * segment in `book`: the loader keeps `verseNum: 0` when the active chapter has a verse-0 - * superscription segment (so the superscription becomes the active verse) and otherwise resolves - * a whole-chapter (verse 0) selection to the chapter's first numbered verse. So this is normally - * verse >= 1, and is verse 0 only when a matching verse-0 segment exists. + * Current scripture reference used to highlight the active verse. Always names a verse some + * segment in `book` starts at, provided the referenced chapter has segments: the loader keeps + * `verseNum: 0` when the active chapter has a verse-0 superscription segment (so the + * superscription becomes the active verse), resolves a whole-chapter (verse 0) selection to the + * chapter's first numbered verse, and resolves any other unmatched verse — the host's next-verse + * button past the chapter's end, or a verse inside a multi-verse segment — to the nearest + * preceding segment start in the chapter. So this is normally verse >= 1, and is verse 0 only + * when a matching verse-0 segment exists. */ scrRef: SerializedVerseRef; /** @@ -220,9 +223,9 @@ function InterlinearizerInner({ * reference. (The loader's `viewScrRef` freeze normally keeps the two in sync, so this guards a * transient.) * - * A verse-0 segment (a chapter superscription) navigates like any other: the write records an - * internal-nav marker so the host's chapter echo is recognized as our own move rather than - * bouncing the view back. (See the verse-0 stickiness exception in `InterlinearNavContext`.) + * A verse-0 segment (a chapter superscription) navigates like any other verse: the `'internal'` + * origin records a nav marker so the segment window recognizes the host's echo as our own move + * and skips the recenter fade (the target is already on screen). * * @param tokenRef - The word-token ref to focus. */ @@ -245,8 +248,8 @@ function InterlinearizerInner({ * Updates the active scripture reference (when the verse actually changed) and, when a specific * token was clicked, focuses that token. Skips the write to PAPI when the clicked verse matches * the current one, avoiding a gratuitous echo round-trip. A verse-0 segment (a chapter - * superscription) writes like any other verse — the internal-nav marker keeps the host's chapter - * echo from bouncing the view (see the verse-0 stickiness exception in `InterlinearNavContext`). + * superscription) writes like any other verse — the `'internal'` origin records a nav marker so + * the segment window skips the recenter fade for our own move. * * @param ref - The verse coordinate that was selected. * @param tokenRef - The token that was clicked; omitted when the whole segment was selected. diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 0228b293..a1b78792 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -7,6 +7,7 @@ import papi, { logger } from '@papi/frontend'; import { useData, useSetting } from '@papi/frontend/react'; import { TabToolbar } from 'platform-bible-react'; import type { SelectMenuItemHandler } from 'platform-bible-react'; +import type { ScriptureRef } from 'interlinearizer'; import { isPlatformError } from 'platform-bible-utils'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import useDraftProject from '../hooks/useDraftProject'; @@ -21,7 +22,7 @@ import { WipeModal, type WipeScope } from './modals/WipeModal'; import ScriptureNavControls from './controls/ScriptureNavControls'; import { InterlinearNavProvider, useInterlinearNav } from './InterlinearNavContext'; import { RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; -import { isSameVerse } from '../utils/verse-ref'; +import { isSameVerse, toSerializedVerseRef } from '../utils/verse-ref'; /** Host-injected callback to update this WebView's definition (used to toggle the tab title). */ type UpdateWebViewDefinition = WebViewProps['updateWebViewDefinition']; @@ -107,15 +108,8 @@ function InterlinearizerLoaderInner({ useWebViewState: UseWebViewStateHook; updateWebViewDefinition: UpdateWebViewDefinition; }>) { - const { - rawScrRef, - liveScrRef: scrRef, - navigate, - scrollGroupId, - setScrollGroupId, - fadePhase, - cancelFade, - } = useInterlinearNav(); + const { scrRef, navigate, scrollGroupId, setScrollGroupId, fadePhase, cancelFade } = + useInterlinearNav(); const [interfaceMode] = useSetting('platform.interfaceMode', 'simple'); const [interfaceLanguages] = useSetting('platform.interfaceLanguage', ['und']); @@ -243,16 +237,32 @@ function InterlinearizerLoaderInner({ scrRef, }); - // The active reference handed to the interlinearizer. The host emits `verseNum: 0` both for a - // chapter's verse-0 superscription (which has its own segment) and for a plain whole-chapter - // selection (which does not). Keep verse 0 when the loaded book actually has a verse-0 segment for - // that chapter — so a Psalm superscription becomes the active verse — and otherwise fall back to - // the chapter's first numbered verse, so an ordinary chapter selection still lands on verse 1 - // rather than leaving nothing highlighted. Non-verse-0 references pass through unchanged. + // The active reference handed to the interlinearizer, resolved so it always names a verse some + // segment starts at (when the requested chapter has segments at all). A reference whose verse is + // a segment start passes through unchanged. Otherwise: + // + // - Verse 0 with no verse-0 segment is a plain whole-chapter selection (the host emits + // `verseNum: 0` for those as well as for superscriptions), so it falls back to the chapter's + // first numbered verse rather than leaving nothing highlighted. + // - Any other unmatched verse resolves to the nearest preceding segment start in the same + // chapter. This covers the host's next-verse button, which emits `verseNum + 1` without + // clamping — bumping forward from a chapter's last verse delivers a verse past the chapter's + // end, which must land on the chapter's last segment, not fall through to the views' anchor + // fallback (the start of the book). It also covers verses inside a multi-verse segment, which + // resolve to the segment that contains them. + // - A reference to a chapter with no segments (including any book mismatch during a cross-book + // swap) passes through unchanged; the views' own fallbacks handle that transient. const activeScrRef = useMemo(() => { - if (scrRef.verseNum !== 0 || !book) return scrRef; - const hasVerseZero = book.segments.some((segment) => isSameVerse(segment.startRef, scrRef)); - return hasVerseZero ? scrRef : { ...scrRef, verseNum: 1 }; + if (!book) return scrRef; + if (book.segments.some((segment) => isSameVerse(segment.startRef, scrRef))) return scrRef; + if (scrRef.verseNum === 0) return { ...scrRef, verseNum: 1 }; + let preceding: ScriptureRef | undefined; + book.segments.forEach(({ startRef }) => { + if (startRef.book !== scrRef.book || startRef.chapter !== scrRef.chapterNum) return; + if (startRef.verse > scrRef.verseNum) return; + if (!preceding || startRef.verse > preceding.verse) preceding = startRef; + }); + return preceding ? toSerializedVerseRef(preceding) : scrRef; }, [scrRef, book]); const hasError = !!bookError || !!tokenizeError; @@ -420,7 +430,7 @@ function InterlinearizerLoaderInner({ startAreaChildren={ interfaceMode === 'power' ? (