From fb1af72a3a569f977400ebb529c6bd24698d8be6 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 2 Jul 2026 15:38:05 -0600 Subject: [PATCH] Resolve out-of-range verse refs to the nearest preceding segment The host's next-verse button emits verseNum + 1 without clamping, so bumping forward from a chapter's last verse delivered a verse past the chapter's end; the loader only resolved verse-0 refs, and the segment window's anchor fallback jumped to the start of the book. Any unmatched verse now resolves to the nearest preceding segment start in its chapter, so the view holds at the chapter's end (and a verse inside a merged multi-verse segment resolves to its owning segment). --- .../components/InterlinearizerLoader.test.tsx | 96 +++++++++++++++++++ src/components/InterlinearNavContext.tsx | 13 +-- src/components/Interlinearizer.tsx | 13 ++- src/components/InterlinearizerLoader.tsx | 37 +++++-- 4 files changed, 138 insertions(+), 21 deletions(-) diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 54958e97..a2f30210 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. diff --git a/src/components/InterlinearNavContext.tsx b/src/components/InterlinearNavContext.tsx index 134f0470..20f2fcc6 100644 --- a/src/components/InterlinearNavContext.tsx +++ b/src/components/InterlinearNavContext.tsx @@ -117,10 +117,11 @@ export interface InterlinearNav { * value-equal duplicate deliveries so a re-send reads as 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. 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. + * 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. */ liveScrRef: SerializedVerseRef; /** @@ -239,8 +240,8 @@ export function InterlinearNavProvider({ // `liveScrRef` is the active reference the views recenter on. With verse-0 stickiness removed it no // longer transforms `rawScrRef` — every reference (verse 0 included) passes through verbatim, so // the host's `<` (previous-verse) from verse 1 lands on the chapter's superscription, and the - // loader resolves a verse-0 reference with no matching segment to the chapter's first numbered - // verse before rendering. The alias keeps the intent-revealing name and marks the seam where any + // loader resolves a reference with no matching segment (verse 0, or an unclamped host verse bump + // past the chapter's end) before rendering. The alias keeps the intent-revealing name and marks the seam where any // future raw→active mapping would live; `rawScrRef` already reuses the previously committed object // on a value-equal re-send, so a duplicate delivery never reads as a fresh navigation. const liveScrRef = rawScrRef; diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index f4a2b8ca..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; /** diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 0228b293..644c1869 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']; @@ -243,16 +244,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;