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;