Address review findings: navigation edge cases, dropdown stacking, verse-0 dead code

src/__tests__/components/ContinuousView.test.tsx

@@ -793,6 +793,31 @@ describe('ContinuousView arrow navigation', () => {
     expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'tok-1');
     expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'tok-2');
   });
+
+  it('steps from the externally-imposed focus, not the stale pending index, after an external change interrupts an in-flight internal nav', async () => {
+    // Sequence: an external nav (tok-3) starts its fade while tok-1 is still displayed; the user
+    // clicks Next during the fade (internal nav in flight — this parent never echoes it); then a
+    // second external change lands back on the still-displayed tok-1. Because that value equals the
+    // displayed ref, the focus-change effect early-returns without clearing the in-flight marker,
+    // so only the render-phase external-override detection resyncs the pending index. Without it,
+    // the next step would advance from the stale pending index (group 2 → tok-1) instead of the
+    // externally-imposed position (group 1 → tok-0).
+    const book = makeBook();
+    const props = requiredProps(book, { focusedTokenRef: 'tok-1' });
+    const { rerender } = render(<ContinuousView {...props} />, withAnalysisStore);
+
+    // External nav while idle: the fade starts; the displayed focus is still tok-1.
+    rerender(<ContinuousView {...props} focusedTokenRef="tok-3" />);
+    // Internal nav in flight: Next from the displayed group (tok-1) emits tok-2.
+    await userEvent.click(screen.getByRole('button', { name: 'Next token' }));
+    expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'tok-2');
+
+    // The parent imposes an external position (not the tok-2 echo) that matches the displayed ref.
+    rerender(<ContinuousView {...props} focusedTokenRef="tok-1" />);
+
+    await userEvent.click(screen.getByRole('button', { name: 'Previous token' }));
+    expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'tok-0');
+  });
 });
 
 // ---------------------------------------------------------------------------

src/__tests__/components/InterlinearNavContext.test.tsx

@@ -5,7 +5,11 @@
 import type { SerializedVerseRef } from '@sillsdev/scripture';
 import { act, renderHook } from '@testing-library/react';
 import type { ReactNode } from 'react';
-import { InterlinearNavProvider, useInterlinearNav } from '../../components/InterlinearNavContext';
+import {
+  INTERNAL_NAV_TTL_MS,
+  InterlinearNavProvider,
+  useInterlinearNav,
+} from '../../components/InterlinearNavContext';
 import { RECENTER_FADE_MS } from '../../components/recenter-fade';
 
 /** Tuple shape returned by the PAPI scroll-group hook. */
@@ -266,6 +270,33 @@ describe('InterlinearNavContext', () => {
       expect(result.current.consumeInternalNav(b)).toBe(true);
     });
 
+    it('expires a stranded internal mark after the TTL so a later external navigation fades', () => {
+      // When React batches two rapid internal clicks (verse A then B in one frame), the host
+      // echoes only the final value: B's marker is consumed but A's is stranded. Once the TTL has
+      // passed, a later external navigation to A must classify as external (consume returns
+      // false), not be misread as internal by the stale marker.
+      jest.useFakeTimers();
+      try {
+        const { result } = renderNav(
+          makeScrollGroupHook({ book: 'GEN', chapterNum: 1, verseNum: 1 }),
+        );
+        const a: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 5 };
+        const b: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 10 };
+
+        act(() => {
+          result.current.navigate(a, 'internal');
+          result.current.navigate(b, 'internal');
+        });
+        // The host coalesces the batched navigations and echoes only the final value.
+        expect(result.current.consumeInternalNav(b)).toBe(true);
+
+        jest.advanceTimersByTime(INTERNAL_NAV_TTL_MS + 1);
+        expect(result.current.consumeInternalNav(a)).toBe(false);
+      } finally {
+        jest.useRealTimers();
+      }
+    });
+
     it('matches a verse-0 internal mark against the host-normalized verse-1 reference', () => {
       // An internal navigation stamped at chapter granularity (verse 0) must still be consumable
       // when the host echoes it back normalized to the chapter's first verse (verse 1) — the keys
@@ -482,6 +513,36 @@ describe('InterlinearNavContext', () => {
       expect(result.current.fadePhase).toBe('in');
     });
 
+    it('re-engages the curtain when an expired stranded internal mark names the mid-reveal target', () => {
+      // A stranded internal marker (its echo never arrived) must stop exempting the verse once the
+      // TTL passes: a later external navigation to that verse landing mid-reveal still re-engages
+      // the curtain. Markers stamp at navigate time and RECENTER_FADE_MS (500ms) is far shorter
+      // than the TTL (3000ms), so the clock is advanced past the TTL *before* the reveal begins —
+      // advancing during the 'in' phase would fire the fade-in timer to 'idle' first.
+      const { result, rerender, setRef } = renderNavMutable({
+        book: 'GEN',
+        chapterNum: 1,
+        verseNum: 1,
+      });
+
+      // Strand a marker for the eventual target: an internal navigation whose echo never arrives.
+      act(() => result.current.navigate({ book: 'ZEP', chapterNum: 3, verseNum: 1 }, 'internal'));
+      // Let the marker expire while the clock is still idle.
+      act(() => jest.advanceTimersByTime(INTERNAL_NAV_TTL_MS + 1));
+
+      // Cross-book navigation, then settle: the curtain is mid-reveal ('in').
+      act(() => setRef({ book: 'ZEP', chapterNum: 1, verseNum: 1 }));
+      rerender();
+      act(() => result.current.reportSettled());
+      expect(result.current.fadePhase).toBe('in');
+
+      // An external navigation to the stranded verse lands mid-reveal. The expired marker must not
+      // exempt it: the curtain re-engages.
+      act(() => setRef({ book: 'ZEP', chapterNum: 3, verseNum: 1 }));
+      rerender();
+      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.

src/__tests__/components/Interlinearizer.test.tsx

@@ -419,13 +419,6 @@ describe('Interlinearizer', () => {
     expect(capturedSegmentViewPropsList[1].isActive).toBeFalsy();
   });
 
-  it('renders all segments when navigating to a title reference (verse 0)', () => {
-    const titleRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 0 };
-    renderInterlinearizer({ book: GEN_1_MULTI_BOOK, scrRef: titleRef });
-
-    expect(screen.getAllByTestId('segment-view')).toHaveLength(2);
-  });
-
   it('calls setScrRef with the segment ref when a segment fires onSelect', () => {
     const mockNavigate = jest.fn();
     renderInterlinearizer({ book: GEN_1_MULTI_BOOK, navigate: mockNavigate });
@@ -838,45 +831,6 @@ describe('Interlinearizer', () => {
     expect(screen.queryByText('Chapter 2')).not.toBeInTheDocument();
   });
 
-  it('tags each inline chapter header with its chapter number for snap targeting', () => {
-    renderInterlinearizer({
-      book: GEN_TWO_CHAPTER_BOOK,
-      scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 },
-      continuousScroll: false,
-    });
-
-    expect(screen.getByText('Chapter 1')).toHaveAttribute('data-chapter-start', '1');
-    expect(screen.getByText('Chapter 2')).toHaveAttribute('data-chapter-start', '2');
-  });
-
-  it('highlights the first verse of a chapter when the reference names verse 0 (the heading)', () => {
-    renderInterlinearizer({
-      book: GEN_TWO_CHAPTER_BOOK,
-      scrRef: { book: 'GEN', chapterNum: 2, verseNum: 0 },
-      continuousScroll: false,
-    });
-
-    // Verse 0 names the chapter heading, which is not a verse; the active-verse marker still lands
-    // on the chapter's first segment so a verse stays highlighted.
-    const activeIds = new Set(
-      capturedSegmentViewPropsList.filter((p) => p.isActive).map((p) => p.segment.id),
-    );
-    expect([...activeIds]).toEqual(['GEN 2:1']);
-  });
-
-  it('scrolls the chapter heading to the top when the reference names verse 0', () => {
-    const scrollSpy = jest.spyOn(Element.prototype, 'scrollIntoView');
-    renderInterlinearizer({
-      book: GEN_TWO_CHAPTER_BOOK,
-      scrRef: { book: 'GEN', chapterNum: 2, verseNum: 0 },
-      continuousScroll: false,
-    });
-
-    // The snap targets the chapter-2 heading element rather than the active verse-1 segment.
-    const heading = screen.getByText('Chapter 2');
-    expect(scrollSpy.mock.contexts).toContain(heading);
-  });
-
   it('renders the snap-to-active-verse button when segments are present', () => {
     renderInterlinearizer({ book: GEN_1_MULTI_BOOK });
 

src/__tests__/hooks/useSegmentWindow.test.ts

@@ -670,52 +670,6 @@ describe('useSegmentWindow', () => {
     expect(scrollIntoView).toHaveBeenCalledWith({ behavior: 'auto', block: 'start' });
   });
 
-  it('snaps the chapter heading (not the active verse) when the reference names verse 0', () => {
-    const book = makeBook(60, 60);
-    const scrollIntoView = jest.spyOn(Element.prototype, 'scrollIntoView');
-    const { container, rerender } = renderSegmentWindow(book, {
-      book: 'GEN',
-      chapterNum: 1,
-      verseNum: 1,
-    });
-
-    // Mount both candidate targets: the active verse-1 segment and the chapter-2 heading. A verse-0
-    // reference should snap the heading, leaving the active verse highlighted below it.
-    const active = document.createElement('div');
-    active.setAttribute('aria-current', 'true');
-    container.appendChild(active);
-    const heading = document.createElement('span');
-    heading.setAttribute('data-chapter-start', '2');
-    container.appendChild(heading);
-
-    act(() => rerender({ b: book, ref: { book: 'GEN', chapterNum: 2, verseNum: 0 } }));
-    act(() => jest.advanceTimersByTime(RECENTER_FADE_MS));
-
-    expect(scrollIntoView.mock.contexts).toContain(heading);
-    expect(scrollIntoView.mock.contexts).not.toContain(active);
-  });
-
-  it('falls back to the active verse for a verse-0 reference when no heading is mounted', () => {
-    const book = makeBook(60, 60);
-    const scrollIntoView = jest.spyOn(Element.prototype, 'scrollIntoView');
-    const { container, rerender } = renderSegmentWindow(book, {
-      book: 'GEN',
-      chapterNum: 1,
-      verseNum: 1,
-    });
-
-    // No `[data-chapter-start]` element (headings suppressed by chapterLabelInVerse), so the snap
-    // falls back to the active verse.
-    const active = document.createElement('div');
-    active.setAttribute('aria-current', 'true');
-    container.appendChild(active);
-
-    act(() => rerender({ b: book, ref: { book: 'GEN', chapterNum: 2, verseNum: 0 } }));
-    act(() => jest.advanceTimersByTime(RECENTER_FADE_MS));
-
-    expect(scrollIntoView.mock.contexts).toContain(active);
-  });
-
   it('grows the snap spacer when scrollIntoView cannot reach the top', () => {
     const book = makeBook(60, 0);
     const scrollIntoView = jest.fn();

src/components/ContinuousView.tsx

@@ -259,6 +259,28 @@ export default function ContinuousView({
    * group instead of two.
    */
   const pendingPhraseIndexRef = useRef(0);
+
+  /**
+   * `focusedTokenRef` prop value from the previous render. Lets the sync block below distinguish a
+   * prop that merely hasn't echoed an in-flight internal nav yet (unchanged since last render) from
+   * one the parent changed to an external position (changed to something other than the in-flight
+   * ref).
+   */
+  const prevFocusedTokenRefPropRef = useRef(focusedTokenRef);
+  // If the prop changed to anything other than the in-flight internal ref, the parent imposed an
+  // external position instead of echoing the nav. Clear the in-flight marker so the pending index
+  // resyncs below; otherwise the next step() would advance from the stale pending index rather
+  // than the externally-imposed position. The focus-change effect can't cover this case: it
+  // early-returns without clearing the marker when the external value already matches the
+  // displayed ref.
+  if (
+    internalFocusedTokenRefRef.current !== undefined &&
+    focusedTokenRef !== prevFocusedTokenRefPropRef.current &&
+    focusedTokenRef !== internalFocusedTokenRefRef.current
+  ) {
+    internalFocusedTokenRefRef.current = undefined;
+  }
+  prevFocusedTokenRefPropRef.current = focusedTokenRef;
   // Keep in sync with the rendered value so external jumps reset the pending index. When an
   // internal nav is still in flight (the parent hasn't echoed back yet), do not overwrite: a rapid
   // second click needs to read the already-advanced pending index rather than the stale rendered

src/components/InterlinearNavContext.tsx

@@ -88,6 +88,28 @@ export function verseKey(ref: SerializedVerseRef): string {
   return `${normalized.book}:${normalized.chapterNum}:${normalized.verseNum}`;
 }
 
+/**
+ * How long an unconsumed internal-navigation marker stays valid, in milliseconds. The host's echo
+ * normally consumes a marker within milliseconds, but when React batches rapid clicks (verse A then
+ * B in one frame) the host echoes only the final value, stranding A's marker. Without expiry, a
+ * much-later external navigation to A would consume the stale marker and skip its recenter fade. 3s
+ * is far beyond any echo round-trip yet well before the user could plausibly navigate back.
+ */
+export const INTERNAL_NAV_TTL_MS = 3000;
+
+/**
+ * The single freshness definition for an internal-navigation marker, shared by both marker readers
+ * — `consumeInternalNav` and the render-phase mid-reveal guard — so the two can never drift on what
+ * counts as expired. The boundary is consistent with eviction: a marker is fresh iff its age is at
+ * most {@link INTERNAL_NAV_TTL_MS}.
+ *
+ * @param stampedAt - The marker's `Date.now()` stamp, or `undefined` when no marker exists.
+ * @returns `true` when a marker exists and is within the TTL.
+ */
+function isInternalNavMarkerFresh(stampedAt: number | undefined): boolean {
+  return stampedAt !== undefined && Date.now() - stampedAt <= INTERNAL_NAV_TTL_MS;
+}
+
 /**
  * The single navigation surface for the Interlinearizer WebView. Owns the scripture reference, the
  * scroll-group linkage, the cross-book fade clock, and the internal/external classification of each
@@ -128,7 +150,10 @@ export interface InterlinearNav {
    * Consumes a pending internal-navigation marker for `ref`: returns `true` (and clears the marker)
    * when the most recent {@link navigate} to this verse was `internal`, else `false`. The segment
    * window calls this when an anchor change arrives to decide whether to fade. Consuming clears the
-   * marker so a later _external_ navigation to the same verse still fades.
+   * marker so a later _external_ navigation to the same verse still fades. Markers older than
+   * {@link INTERNAL_NAV_TTL_MS} are ignored (and discarded): a marker stranded by React batching
+   * rapid clicks — where the host echoes only the last of several internal navigations — must not
+   * misclassify a later external navigation to the un-echoed verse.
    *
    * @param ref - The reference whose pending classification to consume.
    * @returns `true` if the navigation to `ref` was internal (skip the fade), else `false`.
@@ -242,26 +267,36 @@ export function InterlinearNavProvider({
   liveScrRefRef.current = liveScrRef;
 
   /**
-   * Verse keys of internal navigations still awaiting their host round-trip. A `navigate(ref,
-   * 'internal')` adds `verseKey(ref)`; `consumeInternalNav` removes it on match. A set (not a
-   * single value) handles rapid successive internal clicks: if two verses are clicked before the
-   * host delivers the first `liveScrRef`, both keys stay pending so neither delivery is misread as
-   * external.
+   * 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
+   * 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<Set<string>>(new Set());
+  const pendingInternalNavRef = useRef<Map<string, number>>(new Map());
 
   const navigate = useCallback(
     (newScrRef: SerializedVerseRef, origin: NavOrigin = 'external') => {
-      if (origin === 'internal') pendingInternalNavRef.current.add(verseKey(newScrRef));
+      if (origin === 'internal') pendingInternalNavRef.current.set(verseKey(newScrRef), Date.now());
       setScrRef(newScrRef);
     },
     [setScrRef],
   );
 
   const consumeInternalNav = useCallback((ref: SerializedVerseRef) => {
+    const pending = pendingInternalNavRef.current;
+    // Evict expired markers before matching, so a marker stranded by a batched rapid-click (its
+    // echo never arrived) cannot be consumed by a later external navigation to the same verse.
+    // Eviction also bounds the map's size; freshness is the shared `isInternalNavMarkerFresh`
+    // definition so this reader and the mid-reveal guard cannot drift.
+    pending.forEach((stampedAt, pendingKey) => {
+      if (!isInternalNavMarkerFresh(stampedAt)) pending.delete(pendingKey);
+    });
     const key = verseKey(ref);
-    if (!pendingInternalNavRef.current.has(key)) return false;
-    pendingInternalNavRef.current.delete(key);
+    if (!pending.has(key)) return false;
+    pending.delete(key);
     return true;
   }, []);
 
@@ -298,17 +333,16 @@ export function InterlinearNavProvider({
   } else if (
     fadePhase === 'in' &&
     verseKey(liveScrRef) !== verseKey(prevLiveScrRef) &&
-    !pendingInternalNavRef.current.has(verseKey(liveScrRef))
+    !isInternalNavMarkerFresh(pendingInternalNavRef.current.get(verseKey(liveScrRef)))
   ) {
-    // A follow-up external navigation landing while the cross-book reveal is still animating. The
-    // host resolves one picker selection as two navigations — the book change first, the precise
-    // target a beat later — so the second routinely arrives mid-fade-in. Left alone it would fade
-    // the freshly revealed content a second time (curtain up, content out, content in: the "double
-    // fade"). Instead, fold it into the same curtain cycle: re-engage the curtain (the CSS
-    // transition carries the opacity smoothly down from wherever the rise had reached), let the
-    // views re-anchor on the new verse behind it, and lift once on their settle. Internal echoes
-    // (a click made during the reveal) are exempt — the curtain must not drop over a navigation
-    // whose target is already on screen.
+    // 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
+    // the reveal is still animating and would fade the fresh content a second time. Instead,
+    // re-engage the curtain (the CSS transition carries opacity smoothly down from wherever the
+    // rise reached) and lift once when the views settle on the new verse. Internal echoes (a click
+    // made during the reveal) are exempt — their target is already on screen — but the exemption
+    // honors the marker TTL, so a stranded marker cannot suppress the re-engage. Pure read, no
+    // eviction: this runs during render; `consumeInternalNav` handles eviction.
     /* v8 ignore next -- defensive: reportSettled always arms the fade-in timer alongside 'in' */
     if (fadeInTimeoutRef.current !== undefined) clearTimeout(fadeInTimeoutRef.current);
     fadeInTimeoutRef.current = undefined;