From 64e82bd49839e46756528b580c00ce02c9c43354 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 21 Aug 2026 16:20:22 -0400 Subject: [PATCH 1/9] Hoist focus state into a dedicated store with origin at the call site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The focused word token was `useState` in `Interlinearizer`, threaded as props into both views, with six writers — three of them coordinated by hook declaration order — and each consumer reconstructing where a change came from by comparing prop values against refs kept for the purpose. Move it to a `FocusProvider` mounted inside `Interlinearizer` below the book indexes: - Every write records a `FocusOrigin` at the call site, and one table documents what each origin asks of the strip and of the list. The two readings of "internal" that were explained twice, in prose, in two files are now one table with two columns. - The seed and the three reseed rules collapse into a single effect that branches in priority order, so an outside focus request outranks both reseeds by rule rather than by source-file layout. Both "keep this declared above" comments are gone, as is the closure-age argument the claim depended on. - Focus rides `useSyncExternalStore`, so an event-time reader takes the current value through a getter without subscribing, and a focus move re-renders only the views that read it — not `Interlinearizer`, and not the loader above it. Deleted: the `onFocusedTokenRefChange` prop and its ref mirror, the in-flight internal marker, and the render-phase block that cleared it. `lastDisplayUpdateWasInternalRef` stays, since something still has to carry the fade/glide decision across the fade timer, but it now carries a declared origin instead of inferring one. Behavior is unchanged, including the reseed guard that leaves the strip still for an external navigation within the focused segment. Two strip tests were rewritten rather than ported: both asserted on a state only the prop round-trip could produce (an internal nav in flight that the parent never echoed). They now cover the same invariants — resetting the step count on a focus the strip did not choose, and the window falling back to the live focus when the displayed ref names a token this book lacks — through states the store can actually reach. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/ContinuousView.test.tsx | 337 ++++++++----- src/__tests__/components/FocusStore.test.tsx | 460 ++++++++++++++++++ .../components/Interlinearizer.test.tsx | 43 +- src/components/ContinuousView.tsx | 214 ++++---- src/components/FocusStore.tsx | 342 +++++++++++++ src/components/Interlinearizer.tsx | 233 ++------- src/components/SegmentListView.tsx | 16 +- src/hooks/useSegmentWindow.ts | 6 +- 8 files changed, 1190 insertions(+), 461 deletions(-) create mode 100644 src/__tests__/components/FocusStore.test.tsx create mode 100644 src/components/FocusStore.tsx diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index ed68e273..63584d1a 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -4,17 +4,23 @@ import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { Book, PhraseAnalysisLink, Token } from 'interlinearizer'; -import { useState, type ReactNode } from 'react'; +import { useEffect, useMemo, useRef, useState, type ComponentProps, type ReactNode } from 'react'; import { resegmentBook } from 'parsers/papi/resegmentBook'; import type { PhraseDispatch } from '../../components/AnalysisStore'; import { AltHeldProvider } from '../../components/AltHeldContext'; import ContinuousView from '../../components/ContinuousView'; +import { + createFocusStore, + FocusStoreProvider, + type FocusActions, + type FocusStore, +} from '../../components/FocusStore'; import { SegmentationProvider, type SegmentationContextValue, } from '../../components/SegmentationStore'; +import { RECENTER_FADE_MS } from '../../components/recenter-fade'; import { isWordToken } from '../../types/type-guards'; -import type { ViewOptions } from '../../types/view-options'; import { FIXTURE_STAMPS, makePunctToken, makeSegment, makeWordToken } from '../test-helpers'; import { allFalseViewOptions, @@ -270,26 +276,68 @@ function buildLookups(book: Book): { return { tokenSegmentMap, tokenDocOrder, wordTokenByRef }; } +/** Props for {@link ContinuousViewHarness}. */ +type ContinuousViewHarnessProps = ComponentProps & + Readonly<{ + /** + * Focus the store is seeded with; each later change is applied as a focus the strip did not + * choose. + */ + focusedTokenRef: string | undefined; + /** Called with every focus the strip writes, so a test can spy on or echo back its moves. */ + onFocusedTokenRefChange: (ref: string) => void; + }>; + +/** + * Mounts the strip over a real focus store driven by the harness props, so a test can drive focus + * from outside the strip and observe the moves the strip makes without standing up the provider + * that resolves focus from a book and a reference. + */ +function ContinuousViewHarness({ + focusedTokenRef, + onFocusedTokenRefChange, + ...stripProps +}: ContinuousViewHarnessProps) { + const storeRef = useRef(undefined); + if (storeRef.current === undefined) storeRef.current = createFocusStore(focusedTokenRef); + const store = storeRef.current; + + const actions = useMemo( + () => ({ + focusToken: (ref, origin) => { + store.write(ref, origin); + onFocusedTokenRefChange(ref); + }, + selectSegment: () => {}, + }), + [store, onFocusedTokenRefChange], + ); + + // Mirrors only an actual prop change: applying the seed again on mount would clobber the focus the + // strip names for itself when nothing resolved one, since child effects run first. + const prevFocusRef = useRef(focusedTokenRef); + useEffect(() => { + if (focusedTokenRef === prevFocusRef.current) return; + prevFocusRef.current = focusedTokenRef; + store.write(focusedTokenRef, 'reseed'); + }, [store, focusedTokenRef]); + + return ( + + + + ); +} + /** - * Minimal required props for ContinuousView. Spread into render calls so tests only need to + * Minimal required props for the strip harness. Spread into render calls so tests only need to * override what they actually care about. The lookup maps are derived from `book` so they always * agree with what's rendered. */ function requiredProps( book: Book, overrides?: { focusedTokenRef?: string | undefined }, -): { - book: Book; - editPhraseSegmentId: string | undefined; - focusedTokenRef: string | undefined; - onFocusedTokenRefChange: jest.Mock; - phraseMode: { kind: 'view' }; - setPhraseMode: jest.Mock; - tokenSegmentMap: ReadonlyMap; - tokenDocOrder: ReadonlyMap; - wordTokenByRef: ReadonlyMap; - viewOptions: ViewOptions; -} { +): ContinuousViewHarnessProps & { onFocusedTokenRefChange: jest.Mock; setPhraseMode: jest.Mock } { const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); return { book, @@ -363,7 +411,7 @@ beforeEach(() => { describe('ContinuousView initial render', () => { it('renders all tokens from all segments as a flat list', () => { const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('the')).toBeInTheDocument(); @@ -373,7 +421,7 @@ describe('ContinuousView initial render', () => { it('renders an inline verse-number superscript at each verse start', () => { const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); // Verses 1 and 2; the first opens chapter 1, so its label is chapter-qualified (`1:1`). const sups = screen.getAllByTestId('verse-superscript'); @@ -402,7 +450,7 @@ describe('ContinuousView initial render', () => { }, ], }); - render(, withAnalysisStore); + render(, withAnalysisStore); const sups = screen.getAllByTestId('verse-superscript'); expect(sups.map((s) => s.textContent)).toEqual(['1:1']); @@ -410,14 +458,14 @@ describe('ContinuousView initial render', () => { it('does not render an extension-generated segment separator', () => { const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(screen.queryByText('GEN 1:1')).not.toBeInTheDocument(); }); it('renders a Previous token button and a Next token button', () => { const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -429,7 +477,7 @@ describe('ContinuousView initial render', () => { it('renders a non-word token via InertTokenChip within the strip', () => { const book = makeMixedBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('.')).toBeInTheDocument(); @@ -437,7 +485,7 @@ describe('ContinuousView initial render', () => { it('renders without crashing when book has no word tokens', () => { const book = makeWordFreeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(screen.getByText('.')).toBeInTheDocument(); }); @@ -445,7 +493,7 @@ describe('ContinuousView initial render', () => { it('notifies the parent of the initially-focused token on mount when no focus prop is set', () => { const book = makeBook(); const props = requiredProps(book); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('tok-0'); }); @@ -453,7 +501,7 @@ describe('ContinuousView initial render', () => { it('does not notify the parent on mount when focusedTokenRef is already set', () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(props.onFocusedTokenRefChange).not.toHaveBeenCalled(); }); @@ -461,7 +509,7 @@ describe('ContinuousView initial render', () => { it('marks the phrase containing focusedTokenRef as focused', () => { const book = makeBook(); render( - , + , withAnalysisStore, ); @@ -469,17 +517,38 @@ describe('ContinuousView initial render', () => { expect(focusedBox).toHaveAttribute('data-focus-state', 'focused'); }); - it('falls back to focusedTokenRef when the lagging displayed ref is from another book', () => { - // During a book change displayFocusedTokenRef lags by one fade, briefly naming a token absent - // from the new book; focus must follow the live focusedTokenRef, not collapse to phrase 0. - const book = makeBook(); - const { rerender } = render( - , - withAnalysisStore, + it('falls back to the live focus while the displayed ref names a token this book lacks', () => { + // Through a book change the displayed ref lags by one fade, briefly naming a token absent from + // the mounted book. The window must follow the live focus rather than collapsing to phrase 0. + // Seeded with a foreign ref so the mounted book cannot resolve the displayed value, which is the + // state that fade leaves behind. + const otherBook: Book = { + id: 'MAT', + bookRef: 'MAT', + textVersion: '1', + segments: [ + makeSegment('MAT 1:1', 'Alpha', [makeWordToken('mat-tok-0', 'Alpha')]), + makeSegment('MAT 1:2', 'Beta', [makeWordToken('mat-tok-1', 'Beta')]), + ], + }; + const props = requiredProps(otherBook, { focusedTokenRef: 'tok-2' }); + const { rerender } = render(, withAnalysisStore); + + scrollIntoViewMock.mockClear(); + rerender(); + + // The scroll lands on "Beta" (the live focus), never "Alpha" (phrase 0). + const scrolledTexts = scrollIntoViewMock.mock.contexts.map((el) => + el instanceof HTMLElement ? el.textContent : undefined, ); + expect(scrolledTexts.some((t) => t?.includes('Beta'))).toBe(true); + expect(scrolledTexts.some((t) => t?.includes('Alpha'))).toBe(false); + }); - // A different book sharing no token refs: the displayed 'tok-2' is now absent, and - // focusedTokenRef points at the new book's second phrase. + it('centers the focused group after a book swap', () => { + // A book swap drops the group ref setters, which are keyed by absolute group index and so name + // different groups in the new book. Centering afterwards proves the new book's groups took + // setters of their own rather than inheriting dead ones. const otherBook: Book = { id: 'MAT', bookRef: 'MAT', @@ -489,16 +558,20 @@ describe('ContinuousView initial render', () => { makeSegment('MAT 1:2', 'Beta', [makeWordToken('mat-tok-1', 'Beta')]), ], }; + const { rerender } = render( + , + withAnalysisStore, + ); scrollIntoViewMock.mockClear(); - rerender(); + rerender( + , + ); - // The scroll lands on "Beta" (the focusedTokenRef phrase), never "Alpha" (phrase 0). const scrolledTexts = scrollIntoViewMock.mock.contexts.map((el) => el instanceof HTMLElement ? el.textContent : undefined, ); expect(scrolledTexts.some((t) => t?.includes('Beta'))).toBe(true); - expect(scrolledTexts.some((t) => t?.includes('Alpha'))).toBe(false); }); }); @@ -506,7 +579,7 @@ describe('ContinuousView focus changes', () => { it('notifies the parent when an out-of-focus phrase box is clicked', async () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + render(, withAnalysisStore); const clickedPhraseBox = screen.getByText('beginning').closest('[data-phrase-box="true"]'); if (!clickedPhraseBox) throw new Error('Expected phrase box wrapper for token'); @@ -519,7 +592,7 @@ describe('ContinuousView focus changes', () => { it('does not notify the parent when clicking the already-focused phrase box', async () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + render(, withAnalysisStore); const firstPhraseBox = screen.getByText('In').closest('[data-phrase-box="true"]'); if (!firstPhraseBox) throw new Error('Expected phrase box wrapper for token'); @@ -545,7 +618,7 @@ describe('ContinuousView focus changes', () => { phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + render(, withAnalysisStore); const groupedBox = screen.getByText('In').closest('[data-phrase-box="true"]'); if (!groupedBox) throw new Error('Expected phrase box wrapper for grouped tokens'); @@ -558,7 +631,7 @@ describe('ContinuousView focus changes', () => { it('notifies the parent when clicking a phrase box while nothing is focused', async () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: undefined }); - render(, withAnalysisStore); + render(, withAnalysisStore); const firstPhraseBox = screen.getByText('In').closest('[data-phrase-box="true"]'); if (!firstPhraseBox) throw new Error('Expected phrase box wrapper for token'); @@ -573,7 +646,7 @@ describe('ContinuousView arrow disabled states', () => { it('disables the prev arrow when focus is on the first phrase', () => { const book = makeBook(); render( - , + , withAnalysisStore, ); @@ -585,7 +658,7 @@ describe('ContinuousView arrow disabled states', () => { it('enables the prev arrow when focus is on a non-first phrase', () => { const book = makeBook(); render( - , + , withAnalysisStore, ); @@ -597,7 +670,7 @@ describe('ContinuousView arrow disabled states', () => { it('disables the next arrow when focus is on the last phrase', () => { const book = makeBook(); render( - , + , withAnalysisStore, ); @@ -609,7 +682,7 @@ describe('ContinuousView arrow disabled states', () => { it('enables the next arrow when focus is on a non-last phrase', () => { const book = makeBook(); render( - , + , withAnalysisStore, ); @@ -621,7 +694,7 @@ describe('ContinuousView arrow disabled states', () => { it('disables both arrows when the book has a single token', () => { const book = makeSingleTokenBook(); render( - , + , withAnalysisStore, ); @@ -635,7 +708,7 @@ describe('ContinuousView arrow disabled states', () => { it('disables both arrows when the book has no word tokens', () => { const book = makeWordFreeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -650,7 +723,7 @@ describe('ContinuousView arrow navigation', () => { it('notifies the parent of the next phrase ref when Next is clicked', async () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + render(, withAnalysisStore); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), @@ -662,7 +735,7 @@ describe('ContinuousView arrow navigation', () => { it('notifies the parent of the previous phrase ref when Previous is clicked', async () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + render(, withAnalysisStore); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -674,7 +747,7 @@ describe('ContinuousView arrow navigation', () => { it('crosses verse boundaries via the Next arrow', async () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + render(, withAnalysisStore); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), @@ -686,7 +759,7 @@ describe('ContinuousView arrow navigation', () => { it('crosses chapter boundaries via the Next arrow', async () => { const book = makeTwoChapterBook(); const props = requiredProps(book, { focusedTokenRef: 'ch1-tok-0' }); - render(, withAnalysisStore); + render(, withAnalysisStore); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), @@ -698,7 +771,7 @@ describe('ContinuousView arrow navigation', () => { it('advances two groups on rapid double-click before re-render', async () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + render(, withAnalysisStore); const next = screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }); await userEvent.click(next); @@ -708,38 +781,41 @@ describe('ContinuousView arrow navigation', () => { 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 () => { - // An external nav (tok-3) fades while tok-1 is displayed; the user clicks Next mid-fade (internal - // nav in flight, never echoed); a second external change lands back on the still-displayed tok-1. - // Since that equals the displayed ref, the focus-change effect early-returns without clearing the - // in-flight marker, so render-phase external-override detection must resync the pending index — - // otherwise the next step advances from the stale pending index instead of the imposed position. - const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - const { rerender } = render(, withAnalysisStore); + it('steps from a focus it did not choose rather than from its own last target', async () => { + // A step counts from where the strip is heading, so rapid presses accumulate. A focus the strip + // did not choose has to reset that count: otherwise the press after one lands a group away from + // what the reader is looking at. + jest.useFakeTimers(); + try { + const book = makeBook(); + const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); + const { rerender } = render(, withAnalysisStore); - // External nav while idle: the fade starts; the displayed focus is still tok-1. - rerender(); - // Internal nav in flight: Next from the displayed group (tok-1) emits tok-2. - await userEvent.click( - screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), - ); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'tok-2'); + fireEvent.click( + screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), + ); + expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'tok-2'); - // The parent imposes an external position (not the tok-2 echo) that matches the displayed ref. - rerender(); + // A focus from outside the strip, given the fade it takes to arrive. + rerender(); + act(() => { + jest.advanceTimersByTime(RECENTER_FADE_MS); + }); - await userEvent.click( - screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), - ); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'tok-0'); + fireEvent.click( + screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), + ); + expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'tok-2'); + } finally { + jest.useRealTimers(); + } }); }); describe('ContinuousView scroll behavior', () => { it('calls scrollIntoView on initial mount', () => { const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(scrollIntoViewMock).toHaveBeenCalledWith({ behavior: 'auto', @@ -751,13 +827,13 @@ describe('ContinuousView scroll behavior', () => { it('uses instant scroll when focusedTokenRef changes externally', () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); scrollIntoViewMock.mockClear(); act(() => { jest.useFakeTimers(); }); - rerender(); + rerender(); act(() => { jest.advanceTimersByTime(600); jest.useRealTimers(); @@ -772,14 +848,14 @@ describe('ContinuousView scroll behavior', () => { // settling on the slid window) shifts the strip after the instant snap. const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); act(() => { jest.useFakeTimers(); }); try { // tok-1 shares GEN 1:1 with tok-0, so the active segment is unchanged by this jump. - rerender(); + rerender(); // Complete the fade-out (RECENTER_FADE_MS) so the displayed focus updates and the instant // snap fires. scrollIntoViewMock.mockClear(); @@ -824,7 +900,7 @@ describe('ContinuousView scroll behavior', () => { try { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); act(() => { jest.useFakeTimers(); @@ -832,7 +908,7 @@ describe('ContinuousView scroll behavior', () => { try { // tok-1 shares GEN 1:1 with tok-0, so the active segment is unchanged: only the scroll // effect's own hold loop keeps the group pinned. - rerender(); + rerender(); act(() => { jest.advanceTimersByTime(510); }); @@ -873,7 +949,7 @@ describe('ContinuousView scroll behavior', () => { try { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); act(() => { jest.useFakeTimers(); @@ -883,7 +959,7 @@ describe('ContinuousView scroll behavior', () => { // the initial real-timer mount is not deterministically advanceable). tok-1 shares GEN 1:1 // with tok-0, so there's no committed-active-segment flip: only the scroll effect's own hold // pins the group, exercising the same observer lifetime as the initial mount. - rerender(); + rerender(); // Complete the fade-out (RECENTER_FADE_MS) so the instant snap + hold start. act(() => { jest.advanceTimersByTime(510); @@ -922,14 +998,14 @@ describe('ContinuousView scroll behavior', () => { it('snaps the link slots (no transition) during an external jump so they do not slide after the fade-in', () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { container, rerender } = render(, withAnalysisStore); + const { container, rerender } = render(, withAnalysisStore); act(() => { jest.useFakeTimers(); }); // External nav into the other verse: the active segment commits instantly behind the fade, so // the slots snap to their new widths rather than animating (which would slide the boxes). - rerender(); + rerender(); const slotWrapper = container.querySelector('[data-testid="link-slot-icon"]'); if (!(slotWrapper instanceof HTMLElement)) throw new Error('Expected a link-slot icon wrapper'); @@ -950,7 +1026,7 @@ describe('ContinuousView scroll behavior', () => { function Parent() { const [ref, setRef] = useState('tok-0'); return ( - { function Parent() { const [ref, setRef] = useState('tok-1'); return ( - { applyBoundaryEdit = () => setEdited(true); const lookups = edited ? mergedLookups : { tokenSegmentMap, tokenDocOrder, wordTokenByRef }; return ( - { it('scrolls with the nearest-block, center-inline placement', () => { const book = makeBook(); render( - , + , withAnalysisStore, ); @@ -1235,11 +1311,11 @@ describe('ContinuousView scroll behavior', () => { // hideInactiveLinkButtons doesn't shift layout; simplifyPhrases does, so it re-centers once. const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); scrollIntoViewMock.mockClear(); rerender( - , @@ -1247,7 +1323,7 @@ describe('ContinuousView scroll behavior', () => { expect(scrollIntoViewMock).not.toHaveBeenCalled(); rerender( - , @@ -1263,11 +1339,14 @@ describe('ContinuousView scroll behavior', () => { // focused group must be snapped back to center when the toggle flips. const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); scrollIntoViewMock.mockClear(); rerender( - , + , ); expect(scrollIntoViewMock).toHaveBeenCalledWith( expect.objectContaining({ behavior: 'auto', inline: 'center' }), @@ -1292,9 +1371,11 @@ describe('ContinuousView segmentation edits', () => { it('keeps the focused segment link buttons active when a merge changes the focused token segment id', () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-2' }); - props.viewOptions = { ...allFalseViewOptions, hideInactiveLinkButtons: true }; - const { container, rerender } = render(, withAnalysisStore); + const props = { + ...requiredProps(book, { focusedTokenRef: 'tok-2' }), + viewOptions: { ...allFalseViewOptions, hideInactiveLinkButtons: true }, + }; + const { container, rerender } = render(, withAnalysisStore); // Focus sits in GEN 1:2, so the slot between its two tokens is active and visible. expect(slotOpacity(container, 'tok-2', 'tok-3')).toBe('1'); @@ -1302,7 +1383,7 @@ describe('ContinuousView segmentation edits', () => { // Merge GEN 1:2 into GEN 1:1. Token refs survive, so focus stays put, but the focused token's // segment id changes. const merged = resegmentBook(book, { removedVerseStarts: ['tok-2'], addedStarts: [] }); - rerender(); + rerender(); // The committed active segment must follow the merge; a stale id would suppress every link // button until the next navigation. @@ -1332,7 +1413,7 @@ describe('ContinuousView split marker', () => { render( - + , withAnalysisStore, @@ -1372,7 +1453,7 @@ describe('ContinuousView RTL layout', () => { it('uses right-pointing arrow for Previous in RTL', () => { document.documentElement.dir = 'rtl'; const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); const prev = screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%', @@ -1383,7 +1464,7 @@ describe('ContinuousView RTL layout', () => { it('uses left-pointing arrow for Next in RTL', () => { document.documentElement.dir = 'rtl'; const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); const next = screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }); expect(next.textContent).toContain('←'); @@ -1392,7 +1473,7 @@ describe('ContinuousView RTL layout', () => { it('uses left-pointing arrow for Previous in LTR', () => { document.documentElement.dir = 'ltr'; const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); const prev = screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%', @@ -1405,7 +1486,7 @@ describe('ContinuousView phrase window', () => { it('renders the focused phrase from a large book', () => { const book = makeLargeBook(300); render( - , + , withAnalysisStore, ); @@ -1415,7 +1496,7 @@ describe('ContinuousView phrase window', () => { it('does not render tokens that fall outside the rendered window', () => { const book = makeLargeBook(300); render( - , + , withAnalysisStore, ); @@ -1444,7 +1525,7 @@ describe('ContinuousView phrase window', () => { linkFarApartTokens(); const book = makeLargeBook(300); render( - , + , withAnalysisStore, ); @@ -1455,7 +1536,7 @@ describe('ContinuousView phrase window', () => { linkFarApartTokens(); const book = makeLargeBook(300); render( - , + , withAnalysisStore, ); @@ -1467,7 +1548,7 @@ describe('ContinuousView phrase window', () => { // shift a resize causes — but the window size is unchanged, so a size-keyed correction misses it. const book = makeLargeBook(300); const props = requiredProps(book, { focusedTokenRef: 'large-tok-150' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); scrollIntoViewMock.mockClear(); addPhraseLinkWithNewIdentity({ @@ -1479,7 +1560,7 @@ describe('ContinuousView phrase window', () => { { tokenRef: 'large-tok-145', surfaceText: 'word145' }, ], }); - rerender(); + rerender(); expect(screen.getByText('word110')).toBeInTheDocument(); expect(scrollIntoViewMock).toHaveBeenCalledWith(expect.objectContaining({ behavior: 'auto' })); @@ -1488,7 +1569,7 @@ describe('ContinuousView phrase window', () => { it('leaves an unlinked token at the same distance outside the window', () => { const book = makeLargeBook(300); render( - , + , withAnalysisStore, ); @@ -1505,7 +1586,7 @@ describe('ContinuousView phrase window', () => { try { const book = makeLargeBook(300); render( - , + , withAnalysisStore, ); @@ -1567,7 +1648,7 @@ describe('ContinuousView phrase window', () => { const book = makeLargeBook(300); // Focused close enough to the book start that the widened window's start clamps to 0. const props = requiredProps(book, { focusedTokenRef: 'large-tok-12' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); const viewport = screen.getByTestId('strip-scroll-viewport'); const stripRow = screen.getByTestId('token-strip'); @@ -1619,7 +1700,7 @@ describe('ContinuousView phrase window', () => { const emitted = props.onFocusedTokenRefChange.mock.calls.at(-1)?.[0]; const nextRef = typeof emitted === 'string' ? emitted : undefined; expect(nextRef).toBe('large-tok-13'); - rerender(); + rerender(); layOutGroups(); // Let the step's own glide settle, so the deferred-correction path is not what answers the @@ -1663,7 +1744,7 @@ describe('ContinuousView phrase window', () => { try { const book = makeLargeBook(300); const props = requiredProps(book, { focusedTokenRef: 'large-tok-150' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); // jsdom lays nothing out, so the geometry the window measures is supplied: a viewport wide // enough to ask for more groups, over groups spaced a fixed pitch apart. @@ -1707,7 +1788,7 @@ describe('ContinuousView phrase window', () => { const emitted = props.onFocusedTokenRefChange.mock.calls.at(-1)?.[0]; const nextRef = typeof emitted === 'string' ? emitted : undefined; expect(nextRef).toBe('large-tok-151'); - rerender(); + rerender(); scrollIntoViewMock.mockClear(); // Report the window slide's own reflow, then run out the frames a restarted hold would use. @@ -1749,7 +1830,7 @@ describe('ContinuousView phrase window', () => { try { const book = makeLargeBook(300); const props = requiredProps(book, { focusedTokenRef: 'large-tok-150' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); // jsdom lays nothing out, so the geometry the window measures is supplied: a viewport wide // enough to ask for more groups, over groups spaced a fixed pitch apart. @@ -1781,7 +1862,7 @@ describe('ContinuousView phrase window', () => { ); const emitted = props.onFocusedTokenRefChange.mock.calls.at(-1)?.[0]; const nextRef = typeof emitted === 'string' ? emitted : undefined; - rerender(); + rerender(); scrollIntoViewMock.mockClear(); const windowObserver = resizeObserverInstances.find((o) => o.targets.includes(viewport)); @@ -1834,7 +1915,7 @@ describe('ContinuousView phrase grouping', () => { ], }); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); const phraseBoxes = document.querySelectorAll('[data-phrase-box="true"]'); // Two tokens grouped → one box; plus the two free tokens from segment 2 → 3 total. @@ -1854,7 +1935,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-2', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); const phraseBoxes = document.querySelectorAll('[data-phrase-id="phrase-1"]'); expect(phraseBoxes).toHaveLength(2); @@ -1877,7 +1958,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); // Hover the phrase group to set hoveredPhraseId='phrase-1'. const phraseGroupSpan = document.querySelector('[data-phrase-box="true"]')?.parentElement; @@ -1899,7 +1980,7 @@ describe('ContinuousView phrase grouping', () => { // display update behind a fade timeout, leaving 'In' (tok-0) focused right after the rerender. const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const { rerender } = render(, withAnalysisStore); // Sanity: tok-0's box ('In') is focused before the click. expect(screen.getByText('In').closest('[data-phrase-box="true"]')).toHaveAttribute( @@ -1911,7 +1992,7 @@ describe('ContinuousView phrase grouping', () => { screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); // Reflect the new ref back as a prop change; the click stamped it internal, so it applies at once. - rerender(); + rerender(); // The displayed focus moved synchronously to tok-1's box ('the') — the internal path. expect(screen.getByText('the').closest('[data-phrase-box="true"]')).toHaveAttribute( @@ -1939,7 +2020,7 @@ describe('ContinuousView phrase grouping', () => { const book = makeBook(); const onFocusedTokenRefChange = jest.fn(); const { rerender } = render( - { // Switch to edit mode for phrase-1. rerender( - { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); // The PhraseGroup wrapper span contains the phrase box. const phraseBox = document.querySelector('[data-phrase-box="true"]'); @@ -2009,7 +2090,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); await userEvent.click(screen.getByTestId('arc-split-btn')); expect(deletePhrase).toHaveBeenCalledWith('phrase-1'); }); @@ -2023,7 +2104,7 @@ describe('ContinuousView phrase grouping', () => { mergePhrases: jest.fn(), }); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); await userEvent.click(screen.getByTestId('arc-split-btn')); expect(deletePhrase).not.toHaveBeenCalled(); }); @@ -2038,7 +2119,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); mockCandidateTokenRefs.current = new Set(['tok-0']); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); // The hovered candidate ref (tok-0) resolves to its phrase, forwarded to ArcOverlay. expect(screen.getByTestId('arc-split-btn')).toHaveAttribute( 'data-candidate-phrase-ids', @@ -2057,7 +2138,7 @@ describe('ContinuousView phrase grouping', () => { // No hovered candidate refs: the phrase exists, but nothing should resolve to it. mockCandidateTokenRefs.current = new Set(); const book = makeBook(); - render(, withAnalysisStore); + render(, withAnalysisStore); expect(screen.getByTestId('arc-split-btn')).toHaveAttribute('data-candidate-phrase-ids', ''); }); }); diff --git a/src/__tests__/components/FocusStore.test.tsx b/src/__tests__/components/FocusStore.test.tsx new file mode 100644 index 00000000..f2bc5323 --- /dev/null +++ b/src/__tests__/components/FocusStore.test.tsx @@ -0,0 +1,460 @@ +/// +/// + +import { logger } from '@papi/frontend'; +import type { SerializedVerseRef } from '@sillsdev/scripture'; +import { act, render, renderHook, screen } from '@testing-library/react'; +import type { Book, Segment, Token } from 'interlinearizer'; +import type { ReactNode } from 'react'; +import { + createFocusStore, + FocusProvider, + FocusStoreProvider, + useFocus, + useFocusActions, + useFocusGetter, + type Focus, + type FocusActions, +} from '../../components/FocusStore'; +import { InterlinearNavProvider, useInterlinearNav } from '../../components/InterlinearNavContext'; +import { isWordToken } from '../../types/type-guards'; +import { makeSegment, makeWordToken, type ScrollGroupTuple } from '../test-helpers'; + +/** + * A two-verse GEN book whose first verse holds two word tokens, so a focus can sit on a non-first + * token of the segment that owns the active verse. + */ +function makeBook(): Book { + return { + id: 'GEN', + bookRef: 'GEN', + textVersion: '1', + segments: [ + makeSegment('GEN 1:1', 'In beginning', [ + makeWordToken('GEN 1:1:0', 'In'), + makeWordToken('GEN 1:1:1', 'beginning', 3), + ]), + makeSegment('GEN 1:2', 'And', [makeWordToken('GEN 1:2:0', 'And')]), + ], + }; +} + +/** The lookups {@link FocusProvider} resolves a focus against, derived from `book`. */ +function buildLookups(book: Book) { + const segmentById = new Map(); + const tokenSegmentMap = new Map(); + const wordTokenByRef = new Map(); + book.segments.forEach((seg) => { + segmentById.set(seg.id, seg); + seg.tokens.forEach((t) => { + tokenSegmentMap.set(t.ref, seg.id); + if (isWordToken(t)) wordTokenByRef.set(t.ref, t); + }); + }); + return { segmentById, tokenSegmentMap, wordTokenByRef }; +} + +/** + * Mounts a {@link FocusProvider} over a scroll-group stub whose reference the test controls, and + * exposes the focus and navigation surfaces plus a `setBook` / `setScrRef` pair for restaging the + * inputs the resolution rules classify on. A fresh reference object is required on each change so + * the nav provider adopts it. + */ +function renderFocus(initialBook: Book, initialScrRef: SerializedVerseRef) { + let book = initialBook; + let hostScrRef = initialScrRef; + const setScrRefSpy = jest.fn((next: SerializedVerseRef) => { + hostScrRef = next; + }); + const scrollGroupHook = (): ScrollGroupTuple => [ + hostScrRef, + setScrRefSpy, + undefined, + () => {}, + undefined, + ]; + + let focus: Focus | undefined; + let actions: FocusActions | undefined; + let nav: ReturnType | undefined; + + function Probe() { + focus = useFocus(); + actions = useFocusActions(); + return
; + } + + function Tree() { + nav = useInterlinearNav(); + return ( + + + + ); + } + + const view = render( + + + , + ); + + return { + /** The focus, its origin, and the surfaces a test drives it through. */ + read: () => { + if (!focus || !actions || !nav) throw new Error('The focus harness has not rendered'); + return { ...focus, actions, nav }; + }, + setScrRefSpy, + setBook: (next: Book) => { + book = next; + view.rerender( + + + , + ); + }, + setScrRef: (next: SerializedVerseRef) => { + hostScrRef = next; + view.rerender( + + + , + ); + }, + }; +} + +const GEN_1_1: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 1 }; +const GEN_1_2: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 2 }; + +beforeEach(() => { + jest.mocked(logger.warn).mockClear(); +}); + +describe('createFocusStore', () => { + it('seeds the focus it is given as a seed origin', () => { + expect(createFocusStore('GEN 1:1:1').getFocus()).toEqual({ + tokenRef: 'GEN 1:1:1', + origin: 'seed', + }); + }); + + it('applies a write and notifies every subscriber', () => { + const store = createFocusStore(undefined); + const first = jest.fn(); + const second = jest.fn(); + store.subscribe(first); + store.subscribe(second); + + store.write('GEN 1:2:0', 'list'); + + expect(store.getFocus()).toEqual({ tokenRef: 'GEN 1:2:0', origin: 'list' }); + expect(first).toHaveBeenCalledTimes(1); + expect(second).toHaveBeenCalledTimes(1); + }); + + it('drops a write naming the token already focused, so a reseed onto it wakes nobody', () => { + const store = createFocusStore('GEN 1:1:1'); + const listener = jest.fn(); + store.subscribe(listener); + + store.write('GEN 1:1:1', 'reseed'); + + expect(store.getFocus().origin).toBe('seed'); + expect(listener).not.toHaveBeenCalled(); + }); + + it('stops notifying once unsubscribed', () => { + const store = createFocusStore(undefined); + const listener = jest.fn(); + const unsubscribe = store.subscribe(listener); + + unsubscribe(); + store.write('GEN 1:1:0', 'strip'); + + expect(listener).not.toHaveBeenCalled(); + }); +}); + +describe('focus hooks', () => { + /** Wraps children in a provider over `store`, with inert actions. */ + function withStore(store = createFocusStore('GEN 1:1:0')) { + const actions: FocusActions = { focusToken: () => {}, selectSegment: () => {} }; + return { + store, + wrapper: ({ children }: { children: ReactNode }) => ( + + {children} + + ), + }; + } + + it('re-renders a subscriber on a focus move', () => { + const { store, wrapper } = withStore(); + const renders = jest.fn(); + const { result } = renderHook( + () => { + renders(); + return useFocus(); + }, + { wrapper }, + ); + + act(() => store.write('GEN 1:2:0', 'list')); + + expect(result.current).toEqual({ tokenRef: 'GEN 1:2:0', origin: 'list' }); + expect(renders).toHaveBeenCalledTimes(2); + }); + + it('leaves a getter-only reader unrendered by a focus move', () => { + const { store, wrapper } = withStore(); + const renders = jest.fn(); + const { result } = renderHook( + () => { + renders(); + return useFocusGetter(); + }, + { wrapper }, + ); + + act(() => store.write('GEN 1:2:0', 'list')); + + expect(result.current().tokenRef).toBe('GEN 1:2:0'); + expect(renders).toHaveBeenCalledTimes(1); + }); + + it('hands back the provider actions', () => { + const { wrapper } = withStore(); + const { result } = renderHook(() => useFocusActions(), { wrapper }); + + expect(result.current.focusToken).toBeInstanceOf(Function); + expect(result.current.selectSegment).toBeInstanceOf(Function); + }); + + it.each([ + ['useFocus', useFocus], + ['useFocusGetter', useFocusGetter], + ['useFocusActions', useFocusActions], + ])('%s throws outside a provider', (name, hook) => { + expect(() => renderHook(() => hook())).toThrow(`${name} must be used within a FocusProvider`); + }); +}); + +describe('FocusProvider seeding', () => { + it('seeds the first word token of the segment that owns the active verse', () => { + const harness = renderFocus(makeBook(), GEN_1_2); + + expect(harness.read().tokenRef).toBe('GEN 1:2:0'); + expect(harness.read().origin).toBe('seed'); + }); + + it('seeds nothing when no segment owns the active verse', () => { + const harness = renderFocus(makeBook(), { book: 'GEN', chapterNum: 9, verseNum: 9 }); + + expect(harness.read().tokenRef).toBeUndefined(); + expect(screen.getByTestId('probe')).toHaveAttribute('data-focus', ''); + }); + + it('seeds nothing when the active segment has no word token', () => { + const punctuationOnly: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: '1', + segments: [makeSegment('GEN 1:1', '', [])], + }; + + expect(renderFocus(punctuationOnly, GEN_1_1).read().tokenRef).toBeUndefined(); + }); +}); + +describe('FocusProvider focusToken', () => { + it('navigates to the target segment when it does not hold the active verse', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => harness.read().actions.focusToken('GEN 1:2:0', 'strip')); + + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:2:0', origin: 'strip' }); + expect(harness.setScrRefSpy).toHaveBeenCalledWith(GEN_1_2); + }); + + it('focuses without navigating within the segment that already holds the active verse', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => harness.read().actions.focusToken('GEN 1:1:1', 'strip')); + + expect(harness.read().tokenRef).toBe('GEN 1:1:1'); + expect(harness.setScrRefSpy).not.toHaveBeenCalled(); + }); + + it('does not echo a verse from a book the reference has already left', () => { + // Mid cross-book navigation the reference names the new book while the mounted book — and so + // this token — still belong to the previous one; echoing that verse would overwrite the target. + const harness = renderFocus(makeBook(), { book: 'MAT', chapterNum: 1, verseNum: 1 }); + + act(() => harness.read().actions.focusToken('GEN 1:2:0', 'strip')); + + expect(harness.read().tokenRef).toBe('GEN 1:2:0'); + expect(harness.setScrRefSpy).not.toHaveBeenCalled(); + }); +}); + +describe('FocusProvider selectSegment', () => { + it('navigates and focuses the clicked token', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => + harness.read().actions.selectSegment({ book: 'GEN', chapter: 1, verse: 2 }, 'GEN 1:2:0'), + ); + + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:2:0', origin: 'list' }); + expect(harness.setScrRefSpy).toHaveBeenCalledWith(GEN_1_2); + }); + + it('skips the navigation when the selected verse is already active', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => + harness.read().actions.selectSegment({ book: 'GEN', chapter: 1, verse: 1 }, 'GEN 1:1:1'), + ); + + expect(harness.read().tokenRef).toBe('GEN 1:1:1'); + expect(harness.setScrRefSpy).not.toHaveBeenCalled(); + }); + + it('leaves the focus alone when the whole segment was selected', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => harness.read().actions.selectSegment({ book: 'GEN', chapter: 1, verse: 2 })); + + expect(harness.read().tokenRef).toBe('GEN 1:1:0'); + expect(harness.setScrRefSpy).toHaveBeenCalledWith(GEN_1_2); + }); +}); + +describe('FocusProvider resolution rules', () => { + it('keeps a focus the re-segmented book still resolves', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + act(() => harness.read().actions.focusToken('GEN 1:1:1', 'strip')); + + // A boundary edit produces a fresh book carrying the same token refs. + const merged: Book = { ...makeBook(), segments: [...makeBook().segments] }; + harness.setBook(merged); + + expect(harness.read().tokenRef).toBe('GEN 1:1:1'); + }); + + it('reseeds to the active verse when the new book cannot resolve the focus', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + act(() => harness.read().actions.focusToken('GEN 1:1:1', 'strip')); + + const retokenized: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: '2', + segments: [makeSegment('GEN 1:1', 'Anew', [makeWordToken('GEN 1:1:9', 'Anew')])], + }; + harness.setBook(retokenized); + + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:1:9', origin: 'reseed' }); + }); + + it('keeps the focus when its own segment contains the new verse', () => { + const spanning: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: '1', + segments: [ + { + ...makeSegment('GEN 1:1', 'In beginning', [ + makeWordToken('GEN 1:1:0', 'In'), + makeWordToken('GEN 1:1:1', 'beginning', 3), + ]), + verseStarts: [ + { charStart: 0, number: '1', chapter: 1 }, + { charStart: 3, number: '2', chapter: 1 }, + ], + }, + ], + }; + const harness = renderFocus(spanning, GEN_1_1); + act(() => harness.read().actions.focusToken('GEN 1:1:1', 'strip')); + + harness.setScrRef(GEN_1_2); + + expect(harness.read().tokenRef).toBe('GEN 1:1:1'); + }); + + it('reseeds to the new verse when nothing is focused yet', () => { + // The active verse resolves no word token, so the seed leaves focus unset and the verse change + // has no focused segment to test against. + const noWordToken: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: '1', + segments: [makeSegment('GEN 1:1', '', []), ...makeBook().segments.slice(1)], + }; + const harness = renderFocus(noWordToken, GEN_1_1); + expect(harness.read().tokenRef).toBeUndefined(); + + harness.setScrRef(GEN_1_2); + + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:2:0', origin: 'reseed' }); + }); + + it('reseeds to the new verse when the focused segment does not contain it', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + harness.setScrRef(GEN_1_2); + + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:2:0', origin: 'reseed' }); + }); + + it('claims a focus request over the verse reseed landing in the same commit', () => { + // A request moves focus and nothing else, so its caller navigates too; both land in one commit, + // which is exactly where the two rules would otherwise race. + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => { + harness.read().nav.requestFocusToken('GEN 1:1:1'); + harness.read().nav.navigate(GEN_1_2, 'internal'); + }); + + // The verse change alone would have reseeded to the new verse's first word. + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:1:1', origin: 'request' }); + }); + + it('warns and falls through to the reseed when the book cannot resolve the request', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => { + harness.read().nav.requestFocusToken('GEN 1:1:99'); + harness.read().nav.navigate(GEN_1_2, 'internal'); + }); + + expect(jest.mocked(logger.warn)).toHaveBeenCalledWith( + 'Interlinearizer: focus request "GEN 1:1:99" matched no word token', + ); + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:2:0', origin: 'reseed' }); + }); + + it('claims a request naming the verse already on screen', () => { + // The request count is the only signal such a request gives, since nothing else about + // navigation changes. + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => harness.read().nav.requestFocusToken('GEN 1:1:1')); + + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:1:1', origin: 'request' }); + }); + + it('leaves a request naming another book pending', () => { + const harness = renderFocus(makeBook(), GEN_1_1); + + act(() => harness.read().nav.requestFocusToken('MAT 1:1:0')); + + expect(harness.read().tokenRef).toBe('GEN 1:1:0'); + expect(jest.mocked(logger.warn)).not.toHaveBeenCalled(); + }); +}); diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 5c572522..fee1a06f 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -98,6 +98,12 @@ const mockDeletePhrase = jest.fn(); */ const mockPhraseLinkById = new Map(); +/** + * How many times the phrase-link-by-id map has been read. `Interlinearizer` reads it once per + * render, so this doubles as a render counter for the component under test. + */ +let phraseLinkByIdMapReads = 0; + jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, /** @@ -115,7 +121,10 @@ jest.mock('../../components/AnalysisStore', () => ({ * Returns the test-owned phrase-link map so straddled-boundary tests can seed phrases the * component's `straddledBoundaryRefs` memo sees. */ - usePhraseLinkByIdMap: () => mockPhraseLinkById, + usePhraseLinkByIdMap: () => { + phraseLinkByIdMapReads += 1; + return mockPhraseLinkById; + }, /** * Returns a getter over the test-owned phrase-link map so force-break tests can seed straddling * phrases. @@ -134,12 +143,21 @@ jest.mock('../../components/ContinuousView', () => ({ * ContinuousView stub; captures its props and the segmentation context (the wrapped, * force-breaking dispatch) so tests can invoke the dispatch directly. */ - default: function ContinuousViewStub(props: CapturedContinuousViewProps) { - capturedContinuousViewProps = props; + default: function ContinuousViewStub( + props: Omit, + ) { + // Read lazily (not via an outer import) because jest.mock factories are hoisted. + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { useFocus, useFocusActions } = require('../../components/FocusStore'); + const focusedTokenRef: string | undefined = useFocus().tokenRef; + const { focusToken } = useFocusActions(); + capturedContinuousViewProps = { + ...props, + focusedTokenRef, + onFocusedTokenRefChange: (ref: string) => focusToken(ref, 'strip'), + }; capturedSegmentation = useSegmentation(); - return ( -
- ); + return
; }, })); @@ -1709,6 +1727,19 @@ describe('focus preservation across segmentation edits', () => { expect(capturedContinuousViewProps?.focusedTokenRef).toBe('GEN 1:2:0'); }); + it('leaves Interlinearizer unrendered by a focus move inside the active verse', () => { + // The focus store exists so a move at arrow-step rate re-renders only the views that read it; + // an owner that re-rendered too would put the cost back where hoisting removed it. + const scrRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 1 }; + render(interlinearizerEl(GEN_TWO_TOKEN_V1_BOOK, scrRef)); + const rendersBefore = phraseLinkByIdMapReads; + + act(() => capturedContinuousViewProps?.onFocusedTokenRefChange('GEN 1:1:3')); + + expect(capturedContinuousViewProps?.focusedTokenRef).toBe('GEN 1:1:3'); + expect(phraseLinkByIdMapReads).toBe(rendersBefore); + }); + it('keeps a deliberately-focused token across a merge into the active verse', () => { const scrRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 1 }; const { rerender } = render(interlinearizerEl(GEN_TWO_TOKEN_V1_BOOK, scrRef)); diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index 61ce1e6b..92e745de 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -23,6 +23,7 @@ import { import useLatestRef from '../hooks/useLatestRef'; import usePhraseWindowHalf from '../hooks/usePhraseWindowHalf'; import MemoizedArcOverlay from './ArcOverlay'; +import { useFocus, useFocusActions, useFocusGetter } from './FocusStore'; import { RECENTER_FADE_MS, RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; /** Clamps `index` to `[0, len - 1]`, returning `0` when `len` is zero. */ @@ -100,18 +101,6 @@ type ContinuousViewProps = Readonly<{ book: Book; /** Segment id of the phrase being edited, or `undefined` outside edit mode. */ editPhraseSegmentId: string | undefined; - /** - * Token ref of the currently focused word token, or `undefined` when nothing is focused. The - * strip jumps to the group containing this token and uses it as the single source of truth for - * highlight + slot rules. All scroll position is derived from this value. - */ - focusedTokenRef: string | undefined; - /** - * Called when arrow navigation or a click in the strip should change which token is focused. The - * parent echoes the value back through `focusedTokenRef`; the strip then re-renders with the new - * focus and scrolls into view. - */ - onFocusedTokenRefChange: (ref: string) => void; /** Current phrase-interaction mode; controls token click behavior in the strip. */ phraseMode: PhraseMode; /** Setter for `phraseMode`; passed to phrase boxes so they can transition modes. */ @@ -120,7 +109,7 @@ type ContinuousViewProps = Readonly<{ tokenSegmentMap: ReadonlyMap; /** Word token ref → flat book-level index; used to sort phrase tokens in document order. */ tokenDocOrder: ReadonlyMap; - /** Word token ref → token lookup; used to resolve the focused token from `focusedTokenRef`. */ + /** Word token ref → token lookup; used to resolve the focused word token. */ wordTokenByRef: ReadonlyMap; /** Bundled display toggles forwarded to the strip. */ viewOptions: ViewOptions; @@ -133,16 +122,13 @@ type ContinuousViewProps = Readonly<{ * by one phrase group at a time with smooth scrolling animation. No segment markers, verse labels, * or chapter boundaries are shown — the strip is fully continuous. * - * Scroll position is derived from `focusedTokenRef`: the strip always centers the group containing - * that token. Arrow buttons advance or retreat focus by one group and notify the parent; the parent - * echoes the new ref back through `focusedTokenRef`. The previous/next arrows are disabled when the - * first/last phrase is focused. + * Scroll position is derived from the focused token: the strip always centers the group containing + * it. Arrow buttons advance or retreat focus by one group; scroll and highlight follow the store + * write. The previous/next arrows are disabled when the first/last phrase is focused. */ export default function ContinuousView({ book, editPhraseSegmentId, - focusedTokenRef, - onFocusedTokenRefChange, phraseMode, setPhraseMode, tokenSegmentMap, @@ -150,6 +136,12 @@ export default function ContinuousView({ wordTokenByRef, viewOptions, }: ContinuousViewProps) { + // Focus drives every scroll, highlight and slot decision here, and its origin decides whether a + // change glides or fades. See FocusOrigin for what each origin asks of this view. + const { tokenRef: focusedTokenRef, origin: focusOrigin } = useFocus(); + const getFocus = useFocusGetter(); + const { focusToken } = useFocusActions(); + const { hideInactiveLinkButtons, simplifyPhrases, showMorphology } = viewOptions; const isRtl = document.documentElement.dir === 'rtl'; @@ -193,10 +185,10 @@ export default function ContinuousView({ }, [phraseGroups]); /** - * Token ref that the strip is currently displaying as focused. Lags `focusedTokenRef` during the - * fade-out for external jumps so the window/scroll/highlight don't shift until the strip has - * faded out. For internal nav (arrow buttons, phrase clicks) this is updated immediately so the - * smooth scroll starts on the same frame. + * Token ref that the strip is currently displaying as focused. Lags the live focus through the + * fade-out for a jump it has to travel, so the window/scroll/highlight don't shift until the + * strip has faded out. For its own arrow/click moves this is updated immediately so the smooth + * scroll starts on the same frame. */ const [displayFocusedTokenRef, setDisplayFocusedTokenRef] = useState( focusedTokenRef, @@ -210,8 +202,8 @@ export default function ContinuousView({ * when the fade timeout fires), so for a few frames it names a token from the previous book that * no longer exists in this book's `groupIndexByTokenRef`. Falling straight back to `0` then parks * the strip on the new book's very first phrase instead of the verse the user navigated to. Fall - * back to the live `focusedTokenRef` first — the parent reseeds it to the new book's active verse - * on the book change — so the transient lands on the intended verse rather than book start. + * back to the live focus first — it is reseeded to the new book's active verse on the book change + * — so the transient lands on the intended verse rather than book start. */ const focusPhraseIndex = useMemo(() => { const resolved = @@ -239,13 +231,10 @@ export default function ContinuousView({ const isInitialLoadInProgressRef = useRef(true); /** - * Token ref that the strip set via `onFocusedTokenRefChange` from internal arrow nav or click. - * When the parent echoes the same value back as `focusedTokenRef`, the focus-change effect - * applies the new ref immediately and smooth-scrolls instead of fade-then-snap. + * Whether the displayed-focus update just applied (or still pending behind the fade) came from a + * move this strip made. Carried from the focus-change effect to the scroll effect, which the fade + * timer can separate by half a second. */ - const internalFocusedTokenRefRef = useRef(undefined); - - /** True when the last displayFocusedTokenRef update was triggered by internal navigation. */ const lastDisplayUpdateWasInternalRef = useRef(false); /** @@ -256,32 +245,11 @@ export default function ContinuousView({ */ 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 - // focusPhraseIndex. - if (internalFocusedTokenRefRef.current === undefined) { + // Keep the pending index on the rendered value, so a focus the strip did not choose resets where + // the next step counts from. A move the strip made itself is excluded: it sets the pending index + // at the call site, and a rapid second click must read that already-advanced value rather than + // the rendered index, which has yet to catch up. + if (focusOrigin !== 'strip') { pendingPhraseIndexRef.current = focusPhraseIndex; } @@ -452,11 +420,11 @@ export default function ContinuousView({ const scrollViewportRef = useRef(null); /** - * Segment id whose link buttons are currently treated as active, lagging `focusedTokenRef` during - * internal navigation. Toggling this adds/removes inactive link icons, which re-lays out the - * whole strip; deferring it until the smooth scroll settles keeps the animation a pure one-token - * glide with no mid-flight box shifts. For external jumps and the initial mount it tracks the - * focus immediately (the strip is faded out or static, so there is no animation to disturb). + * Segment id whose link buttons are currently treated as active, lagging the live focus while the + * strip glides. Toggling this adds/removes inactive link icons, which re-lays out the whole + * strip; deferring it until the smooth scroll settles keeps the animation a pure one-token glide + * with no mid-flight box shifts. For a jump and for the initial mount it tracks the focus + * immediately (the strip is faded out or static, so there is no animation to disturb). */ const [committedActiveSegmentId, setCommittedActiveSegmentId] = useState( () => (focusedTokenRef !== undefined ? tokenSegmentMap.get(focusedTokenRef) : undefined), @@ -479,9 +447,9 @@ export default function ContinuousView({ }, [targetActiveSegmentIdRef]); /** - * `focusedTokenRef` last seen by the segmentation-reconcile effect below, so it can distinguish - * "the focused token's segment id changed because the segmentation changed" (commit immediately) - * from "focus moved" (the focus-change machinery owns the commit timing). + * The focus last seen by the segmentation-reconcile effect below, so it can distinguish "the + * focused token's segment id changed because the segmentation changed" (commit immediately) from + * "focus moved" (the focus-change machinery owns the commit timing). */ const prevFocusForSegmentationRef = useRef(focusedTokenRef); @@ -522,32 +490,26 @@ export default function ContinuousView({ commitPendingActiveSegment(); }, [tokenSegmentMap, focusedTokenRef, commitPendingActiveSegment]); - /** Ref mirror of `onFocusedTokenRefChange` so callbacks never need it as a dep. */ - const onFocusedTokenRefChangeRef = useLatestRef(onFocusedTokenRefChange); - /** - * Emits a focus change that originated _inside_ the strip (arrow nav, phrase click, edit-mode - * jump). Records the ref as internally-originated, then notifies the parent. When the parent - * echoes the same ref back through `focusedTokenRef`, the focus-change effect recognizes the - * match and applies it immediately with a smooth scroll instead of the fade-then-snap used for - * external jumps. Folds the stamp and the notify into one call so the "this is an internal emit" - * intent lives in a single place rather than being restated at each call site. + * Focuses `ref` as a move this strip made, and records `groupIndex` as the position the next + * arrow step counts from. Folds the two into one call so no caller can advance focus while + * leaving the step origin behind. */ - const emitInternalFocus = useCallback( - (ref: string) => { - internalFocusedTokenRefRef.current = ref; - onFocusedTokenRefChangeRef.current(ref); + const stepFocusTo = useCallback( + (groupIndex: number, ref: string) => { + pendingPhraseIndexRef.current = groupIndex; + focusToken(ref, 'strip'); }, - [onFocusedTokenRefChangeRef], + [focusToken], ); - // Notify the parent of the initially-focused token on mount so the segment list scrolls the - // active verse into view on first render. Only fires when no token was already focused. + // Name the initially-focused token on mount so the segment list scrolls the active verse into + // view on first render. Only fires when nothing has resolved a focus already. useEffect(() => { if (focusedTokenRef !== undefined) return; const initialGroup = phraseGroups[focusPhraseIndex]; const initialRef = initialGroup?.tokens[0]?.ref; - if (initialRef !== undefined) onFocusedTokenRefChangeRef.current(initialRef); + if (initialRef !== undefined) focusToken(initialRef, 'seed'); // Intentionally runs only on mount; do not add deps. // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -633,9 +595,9 @@ export default function ContinuousView({ : allTokens.length; /** - * Advances focus by `delta` phrases by notifying the parent of the new focused token ref. The - * parent echoes the change back through `focusedTokenRef`, which re-derives `focusPhraseIndex` - * and triggers the scroll effect. Marks the change as internal so the fade is suppressed. + * Advances focus by `delta` phrases, which re-derives `focusPhraseIndex` and triggers the scroll + * effect. Counts from the pending index rather than the rendered one, so a rapid second press + * moves two groups instead of repeating the first. * * @param delta - Number of phrases to move (positive = forward, negative = backward). */ @@ -648,11 +610,10 @@ export default function ContinuousView({ const clamped = nextIndex < 0 ? 0 : Math.min(nextIndex, phraseGroups.length - 1); /* v8 ignore next -- disabled buttons prevent clicking when already at boundary */ if (clamped === pendingPhraseIndexRef.current) return; - pendingPhraseIndexRef.current = clamped; const nextRef = phraseGroups[clamped]?.tokens[0]?.ref; - if (nextRef !== undefined) emitInternalFocus(nextRef); + if (nextRef !== undefined) stepFocusTo(clamped, nextRef); }, - [phraseGroups, emitInternalFocus], + [phraseGroups, stepFocusTo], ); /** Moves focus one phrase backward. */ @@ -661,42 +622,40 @@ export default function ContinuousView({ /** Moves focus one phrase forward. */ const stepNext = useCallback(() => step(1), [step]); - /** Ref mirror of the focus so the select handler can compare against it without a dep on it. */ - const focusedTokenRefRef = useLatestRef(focusedTokenRef); - /** - * Notifies the parent that the user selected the phrase whose first token is `ref`. The parent - * echoes the new token ref back through `focusedTokenRef`; scroll + highlight follow - * automatically. Selecting the already-focused phrase is a no-op. + * Focuses the phrase whose first token is `ref`; scroll and highlight follow. Selecting the + * already-focused phrase is a no-op. * - * Reads the current focus through a ref rather than closing over it, so the handler keeps one - * identity across focus moves and passing it down cannot invalidate a memoized child. + * Reads the focus at click time rather than closing over it, so the handler keeps one identity + * across focus moves and passing it down cannot invalidate a memoized child. * * @param ref - First-token ref (group key) of the selected phrase. */ const handlePhraseSelect = useCallback( (ref: string) => { const targetGroupIndex = groupIndexByTokenRef.get(ref); - const currentFocus = focusedTokenRefRef.current; + /* v8 ignore next -- ref is a group key, which the group-index map always resolves */ + if (targetGroupIndex === undefined) return; + const currentFocus = getFocus().tokenRef; + /* v8 ignore next 2 -- a focus is always resolved before a phrase box can be clicked */ const currentGroupIndex = currentFocus === undefined ? undefined : groupIndexByTokenRef.get(currentFocus); - if (targetGroupIndex !== undefined && targetGroupIndex === currentGroupIndex) return; - emitInternalFocus(ref); + if (targetGroupIndex === currentGroupIndex) return; + stepFocusTo(targetGroupIndex, ref); }, - [focusedTokenRefRef, groupIndexByTokenRef, emitInternalFocus], + [getFocus, groupIndexByTokenRef, stepFocusTo], ); /** Splits a phrase arc at a token boundary and dispatches the resulting phrase-store writes. */ const handleArcSplit = useArcSplitHandler(tokenDocOrder); - // React to changes in the prop `focusedTokenRef`. For internal nav (arrow/click in this view), - // apply the change immediately and smooth-scroll. For external jumps (segment-mode click, - // Paratext verse selector, mode switch), fade the strip out, wait for the fade to complete, - // then snap the displayed focus into place so the scroll happens behind the curtain. + // React to focus moves. For a move this strip made (arrow, phrase click, mode entry), apply the + // change immediately and smooth-scroll. For every other origin — segment-list click, Paratext + // verse selector, mode switch — fade the strip out, wait for the fade to complete, then snap the + // displayed focus into place so the scroll happens behind the curtain. useEffect(() => { if (focusedTokenRef === displayFocusedTokenRef) return undefined; - const isInternal = internalFocusedTokenRefRef.current === focusedTokenRef; - internalFocusedTokenRefRef.current = undefined; + const isInternal = focusOrigin === 'strip'; if (isInternal) { lastDisplayUpdateWasInternalRef.current = true; setDisplayFocusedTokenRef(focusedTokenRef); @@ -708,16 +667,18 @@ export default function ContinuousView({ setDisplayFocusedTokenRef(focusedTokenRef); }, RECENTER_FADE_MS); return () => clearTimeout(timeout); + // focusOrigin classifies the move that changed focusedTokenRef, so it is never itself a reason + // to re-run: listing it would re-fade for an origin change that moved no focus. + // eslint-disable-next-line react-hooks/exhaustive-deps }, [focusedTokenRef, displayFocusedTokenRef]); // Scroll the focused phrase into view whenever the displayed focus changes. Smooth-scroll for // internal nav (the displayed ref was updated immediately, so the prop and display agree); snap // for external jumps (the displayed ref was just updated post-fade) and for the initial mount. // - // "Internal" here means *this strip* emitted the change. `useSegmentWindow`'s `consumeInternalNav` - // answers a different question — whether any view in the tree originated the nav — so the two - // classify the same event differently: a segment-list click is internal to the list (it does not - // fade) and external to this strip (it fades, since the strip still has to travel). + // "Internal" here means the move carried this strip's own origin. The segment window asks a + // different question — whether any view in the tree originated the nav — so the two classify the + // same event differently by design; FocusOrigin is where the two readings are set side by side. useEffect(() => { // Drop any hold still running: it pins the group the focus just left. Such a hold is armed by // something the focus does not control (a window resize, an active-segment flip) and keeps @@ -901,18 +862,17 @@ export default function ContinuousView({ }, [renderWindowStart, focusPhraseIndex]); // When entering edit or confirm-unlink mode, smooth-scroll to the first group of the active - // phrase by notifying the parent of the new focused token. Scroll then follows automatically - // through focusedTokenRef → focusPhraseIndex. + // phrase by focusing its first token. Scroll then follows through focusPhraseIndex. useEffect(() => { if (phraseMode.kind === 'view') return; const targetPhraseId = phraseMode.phraseId; - const group = phraseGroups.find((g) => g.phraseLink?.analysisId === targetPhraseId); - const nextRef = group?.tokens[0]?.ref; - /* v8 ignore next -- phrase always has tokens; focusedTokenRef differs at mode entry */ + const groupIndex = phraseGroups.findIndex((g) => g.phraseLink?.analysisId === targetPhraseId); + const nextRef = phraseGroups[groupIndex]?.tokens[0]?.ref; + /* v8 ignore next -- phrase always has tokens; the focus differs at mode entry */ if (nextRef === undefined || nextRef === focusedTokenRef) return; - emitInternalFocus(nextRef); - // phraseGroups and focusedTokenRef are read once per mode change; intentionally not deps so the - // effect only fires on actual mode transitions. emitInternalFocus has a stable identity. + stepFocusTo(groupIndex, nextRef); + // phraseGroups and the focus are read once per mode change; intentionally not deps so the + // effect only fires on actual mode transitions. stepFocusTo has a stable identity. // eslint-disable-next-line react-hooks/exhaustive-deps }, [phraseMode]); @@ -987,9 +947,9 @@ export default function ContinuousView({ }); /** - * Group index of the focused token, derived from `focusedTokenRef`. Used per-slot to compute - * `focusedSideIsPrev` from the same source as `focus.focusedPhraseLink` / - * `focus.focusedFreeToken` so link direction and link target can never disagree. + * Group index of the live focused token. Used per-slot to compute `focusedSideIsPrev` from the + * same source as `focus.focusedPhraseLink` / `focus.focusedFreeToken` so link direction and link + * target can never disagree. */ const focusedGroupIndex = useMemo( () => (focusedTokenRef !== undefined ? groupIndexByTokenRef.get(focusedTokenRef) : undefined), @@ -998,12 +958,12 @@ export default function ContinuousView({ /** * Resolved focus context — what's focused, what segment it's in, what phrase it belongs to. Built - * from the fade-gated `displayFocusedTokenRef` (not the live `focusedTokenRef`) so every - * highlight and link-button active/disabled decision moves only at the recenter midpoint, behind - * the fade — never re-evaluating (and dimming the buttons) on the still-visible old strip the - * instant an external nav reseeds the live focus. The scroll target (`focusedGroupIndex`) still - * uses the live ref so the jump lands on the new verse behind the curtain. Mirrors SegmentView, - * which is fed the segment window's own gated display ref. + * from the fade-gated `displayFocusedTokenRef` (not the live focus) so every highlight and + * link-button active/disabled decision moves only at the recenter midpoint, behind the fade — + * never re-evaluating (and dimming the buttons) on the still-visible old strip the instant an + * external nav reseeds the live focus. The scroll target (`focusedGroupIndex`) still uses the + * live ref so the jump lands on the new verse behind the curtain. Mirrors SegmentView, which is + * fed the segment window's own gated display ref. */ const focus = useMemo( () => diff --git a/src/components/FocusStore.tsx b/src/components/FocusStore.tsx new file mode 100644 index 00000000..36c05681 --- /dev/null +++ b/src/components/FocusStore.tsx @@ -0,0 +1,342 @@ +import { logger } from '@papi/frontend'; +import type { SerializedVerseRef } from '@sillsdev/scripture'; +import type { Book, ScriptureRef, Segment, Token } from 'interlinearizer'; +import { createContext, useContext, useEffect, useMemo, useRef, useSyncExternalStore } from 'react'; +import type { ReactNode } from 'react'; +import useLatestRef from '../hooks/useLatestRef'; +import { isWordToken } from '../types/type-guards'; +import { isSameVerse, segmentContainsVerse, toSerializedVerseRef } from '../utils/verse-ref'; +import { useInterlinearNav, verseKey } from './InterlinearNavContext'; + +/** + * Where a focus change came from, recorded _at the call site_ rather than reconstructed by the + * consumers reacting to it. Every consumer maps origin to behavior itself, and the mappings + * deliberately disagree — a click in the segment list is `list`, which the list treats as its own + * doing and the strip treats as a jump it still has to travel: + * + * - `seed` — no user act: the initial resolution of the active verse, or the strip naming its first + * token when nothing resolved. The view is static or invisible, so the move is instant. + * - `strip` — the continuous strip emitted it (arrow step, phrase click, phrase-mode entry). The one + * origin the strip animates through rather than fading over, since it owns the scroll and the + * target is already on screen. + * - `list` — a click or focus inside the segment list. Already on screen for the list; a jump the + * strip has to fade for. + * - `reseed` — focus following the reference rather than driving it, when an external navigation + * lands somewhere the focused token cannot stay. + * - `request` — a focus asked for from outside the views and claimed by the book that resolves it. + */ +export type FocusOrigin = 'seed' | 'strip' | 'list' | 'reseed' | 'request'; + +/** The focused word token together with the origin of the write that put it there. */ +export type Focus = Readonly<{ + /** Token ref of the focused word token, or `undefined` when nothing is focused. */ + tokenRef: string | undefined; + /** Where the write that set {@link Focus.tokenRef} came from. */ + origin: FocusOrigin; +}>; + +/** + * Holds the focused token outside React so an event-time reader can take the current value without + * subscribing, and a subscriber re-renders only for focus. + */ +export interface FocusStore { + /** The focus as of now, including writes made earlier in the current event. */ + getFocus: () => Focus; + /** + * Registers `onFocusChange` for every write that changes the focused token. + * + * @returns The unsubscribe function. + */ + subscribe: (onFocusChange: () => void) => () => void; + /** + * Sets the focused token and its origin. A write naming the token already focused is dropped, so + * a reseed or a claim that resolves to the standing focus wakes nobody. + */ + write: (tokenRef: string | undefined, origin: FocusOrigin) => void; +} + +/** The write paths that own how focus moves; identities are stable for the provider's lifetime. */ +export interface FocusActions { + /** + * Focuses `tokenRef` and, when it lives in a different verse than the active one, navigates + * there. The single explicit focus-move operation behind strip arrow nav and phrase clicks: it + * sets the focused token and pushes the verse change as an _internal_ navigation (so the segment + * window tracks along without a recenter fade). A verse-0 segment (a chapter superscription) + * navigates like any other verse. + * + * Never navigates when the focused token's book differs from the active reference's book: during + * an external book change the reference can briefly name the new book while the mounted book (and + * this token) still belong to the previous one, and echoing that stale verse would overwrite the + * new reference. + */ + focusToken: (tokenRef: string, origin: FocusOrigin) => void; + /** + * 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. + * + * @param ref - The verse coordinate that was selected. + * @param tokenRef - The token that was clicked; omitted when the whole segment was selected. + */ + selectSegment: (ref: ScriptureRef, tokenRef?: string) => void; +} + +/** What {@link FocusStoreProvider} carries; stable, so reading it never re-renders a consumer. */ +type FocusContextValue = Readonly<{ + store: FocusStore; + actions: FocusActions; +}>; + +/** + * React context carrying the focus surface. Undefined outside a provider so the hooks can throw a + * clear error rather than handing back a silently-empty object. + */ +const FocusContext = createContext(undefined); + +/** + * Returns the ref of the first word token in `segment`, or `undefined` when the segment has none. + * The resolution behind every focus that follows the active verse rather than a click. + * + * @param segment - The segment to read, or `undefined` when no active segment is resolved. + */ +function firstWordTokenRefOf(segment: Segment | undefined): string | undefined { + return segment?.tokens.find(isWordToken)?.ref; +} + +/** Builds a store seeded with `tokenRef` as a {@link FocusOrigin} `seed`. */ +export function createFocusStore(tokenRef: string | undefined): FocusStore { + let focus: Focus = { tokenRef, origin: 'seed' }; + const listeners = new Set<() => void>(); + return { + getFocus: () => focus, + subscribe: (onFocusChange) => { + listeners.add(onFocusChange); + return () => { + listeners.delete(onFocusChange); + }; + }, + write: (nextTokenRef, origin) => { + if (nextTokenRef === focus.tokenRef) return; + focus = { tokenRef: nextTokenRef, origin }; + listeners.forEach((listener) => listener()); + }, + }; +} + +/** Props for {@link FocusStoreProvider}. */ +type FocusStoreProviderProps = Readonly<{ + /** The store the subtree reads focus from. */ + store: FocusStore; + /** The write paths the subtree calls to move focus. */ + actions: FocusActions; + /** The subtree that reads and moves focus. */ + children: ReactNode; +}>; + +/** + * Publishes an already-built store and action set to the subtree. Separate from + * {@link FocusProvider} so a view can be exercised against a store driven directly, without the book + * indexes and navigation surface the real provider resolves focus from. + */ +export function FocusStoreProvider({ store, actions, children }: FocusStoreProviderProps) { + const value = useMemo(() => ({ store, actions }), [store, actions]); + return {children}; +} + +/** Props for {@link FocusProvider}. */ +type FocusProviderProps = Readonly<{ + /** Tokenized book the focused token must resolve within. */ + book: Book; + /** + * Current scripture reference. Resolved by the loader to a verse contained in some segment of + * `book` (when the chapter has segments), so the active segment behind a reseed is normally + * found. + */ + scrRef: SerializedVerseRef; + /** Maps every segment id to its segment; resolves the focused token's own verse range. */ + segmentById: ReadonlyMap; + /** Maps every token ref to the id of the segment that contains it. */ + tokenSegmentMap: ReadonlyMap; + /** Maps every word token ref to the token; decides whether this book can hold a given focus. */ + wordTokenByRef: ReadonlyMap; + /** The views that read and move focus. */ + children: ReactNode; +}>; + +/** + * Owns the focused word token for one mounted book, and resolves it against the book and the active + * verse in one ordered rule set, so no two rules can race on which reseed wins. + * + * Seeded so focus is never `undefined` while the active verse has a word token: an undefined focus + * disables every link button, since the active-segment test reads the focused segment. + */ +export function FocusProvider({ + book, + scrRef, + segmentById, + tokenSegmentMap, + wordTokenByRef, + children, +}: FocusProviderProps) { + // `navigate` writes the reference (classifying internal vs external at the call site), and + // `consumeFocusRequest` / `focusRequestCount` collect a token focus asked for from outside the + // views. + const { navigate, consumeFocusRequest, focusRequestCount } = useInterlinearNav(); + + /** + * Finds the book segment that owns the active verse: the first segment in document order whose + * verse range contains it. Containment (rather than an exact start-verse match) matters after + * boundary edits — a verse absorbed into a multi-verse segment, or named by a later portion of a + * split verse, still resolves to the segment that holds its text. The containment test also + * matches the book, so during a cross-book navigation (where the reference names the new book + * before its data loads, leaving `book` still the previous one) this finds nothing rather than + * resolving to the wrong book's verse. + */ + const findActiveSegment = () => book.segments.find((seg) => segmentContainsVerse(seg, scrRef)); + + const storeRef = useRef(undefined); + if (storeRef.current === undefined) { + storeRef.current = createFocusStore(firstWordTokenRefOf(findActiveSegment())); + } + const store = storeRef.current; + + // Mirrored so the actions below keep one identity for the provider's lifetime: a focus handler + // passed to a memoized child must not churn when the book's indexes are rebuilt. + const navigateRef = useLatestRef(navigate); + const scrRefRef = useLatestRef(scrRef); + const segmentByIdRef = useLatestRef(segmentById); + const tokenSegmentMapRef = useLatestRef(tokenSegmentMap); + + const actions = useMemo( + () => ({ + focusToken: (tokenRef, origin) => { + store.write(tokenRef, origin); + const segId = tokenSegmentMapRef.current.get(tokenRef); + /* v8 ignore next 2 -- tokenRef always resolves to a segment in the mounted book */ + const seg = segId === undefined ? undefined : segmentByIdRef.current.get(segId); + if (!seg) return; + const { current } = scrRefRef; + if (seg.startRef.book !== current.book) return; + // Containment check (not exact start-verse match): focusing another token of the segment + // that already holds the active verse must not renavigate to the segment's start verse. + if (segmentContainsVerse(seg, current)) return; + navigateRef.current(toSerializedVerseRef(seg.startRef), 'internal'); + }, + selectSegment: (ref, tokenRef) => { + const { current } = scrRefRef; + if (!isSameVerse(ref, current)) { + navigateRef.current(toSerializedVerseRef(ref), 'internal'); + } + if (tokenRef) store.write(tokenRef, 'list'); + }, + }), + [store, navigateRef, scrRefRef, segmentByIdRef, tokenSegmentMapRef], + ); + + /** + * The inputs the resolution below classifies on, as of its last run. Compared rather than + * consumed from the dependency list because the rules need to know _which_ input moved: a book + * that no longer holds the focused token and a verse the focused segment no longer covers reseed + * on different tests. + */ + const prevInputsRef = useRef({ book, verse: verseKey(scrRef) }); + + // Resolve focus against the book and the active verse, in priority order. Ordering the rules + // inside one effect is what makes the precedence explicit: an outside request outranks both + // reseeds, and reordering hooks cannot change that. Runs after commit rather than during render + // so a claim is never made in a render React may discard. + useEffect(() => { + const prev = prevInputsRef.current; + const verse = verseKey(scrRef); + // Refreshed up front so no early return leaves an input stale for a later comparison. + prevInputsRef.current = { book, verse }; + + // Attempted on every run, not only when the count moves: the count is the only signal when a + // request names the verse already on screen, and the book the only one when it named a book + // that had yet to load. Claiming clears the request, so a run that finds nothing left is a + // no-op. A ref this book cannot resolve is dropped rather than held for a later attempt: one + // that outlived the load it was made for would fire on an unrelated navigation, long after the + // click that raised it. Logged because the drop is otherwise invisible. + const requested = consumeFocusRequest(book.bookRef); + if (requested !== undefined) { + if (wordTokenByRef.has(requested)) { + store.write(requested, 'request'); + return; + } + logger.warn(`Interlinearizer: focus request "${requested}" matched no word token`); + } + + const { tokenRef: current } = store.getFocus(); + const resolvesInBook = current !== undefined && wordTokenByRef.has(current); + + // A boundary edit (merge/split) produces a fresh book too, but token refs survive + // re-segmentation, so a still-resolving focus is kept rather than snapped back to the active + // verse's first word — and left to the verse rule below, which a re-tokenization arriving + // alongside a navigation still has to answer. + if (book !== prev.book && !resolvesInBook) { + store.write(firstWordTokenRefOf(findActiveSegment()), 'reseed'); + return; + } + + // Skip when the focused token's *own* segment already contains the new verse — that means the + // change came from a token click or strip nav here, and reseeding would clobber the + // deliberately-focused token. Testing the focused token's own segment (not the active segment's + // id) is what lets a click on a non-first portion of a split verse stay put instead of being + // reseeded to the verse's first portion. + if (verse !== prev.verse) { + const focusedSegId = current ? tokenSegmentMap.get(current) : undefined; + const focusedSeg = focusedSegId ? segmentById.get(focusedSegId) : undefined; + if (focusedSeg && segmentContainsVerse(focusedSeg, scrRef)) return; + /* v8 ignore next -- the active segment is always found when the book includes the verse */ + store.write(firstWordTokenRefOf(findActiveSegment()), 'reseed'); + } + // findActiveSegment closes over the reactive inputs already listed; the lookup maps and + // consumeFocusRequest are read only as resolvers, and listing them would re-run the rules on + // a phrase edit that moved no focus. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [book, scrRef.book, scrRef.chapterNum, scrRef.verseNum, focusRequestCount]); + + return ( + + {children} + + ); +} + +/** Reads the nearest provider's focus surface, or throws when there is none. */ +function useFocusContext(hookName: string): FocusContextValue { + const context = useContext(FocusContext); + if (!context) throw new Error(`${hookName} must be used within a FocusProvider`); + return context; +} + +/** + * Subscribes to the focused token: the caller re-renders on every focus move and on nothing else. + * + * @throws {Error} When called outside a {@link FocusProvider}. + */ +export function useFocus(): Focus { + const { store } = useFocusContext('useFocus'); + return useSyncExternalStore(store.subscribe, store.getFocus); +} + +/** + * Returns a stable getter for the focus as of the moment it is called, for event-time reads that + * must not subscribe the caller to focus moves. + * + * @throws {Error} When called outside a {@link FocusProvider}. + */ +export function useFocusGetter(): () => Focus { + const { store } = useFocusContext('useFocusGetter'); + return store.getFocus; +} + +/** + * Returns the focus write paths. Never re-renders the caller. + * + * @throws {Error} When called outside a {@link FocusProvider}. + */ +export function useFocusActions(): FocusActions { + return useFocusContext('useFocusActions').actions; +} diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index 9c0ea579..14978f53 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -1,6 +1,5 @@ -import { logger } from '@papi/frontend'; import type { SerializedVerseRef } from '@sillsdev/scripture'; -import type { Book, ScriptureRef, Segment } from 'interlinearizer'; +import type { Book } from 'interlinearizer'; import { TooltipProvider } from 'platform-bible-react'; import { useCallback, useEffect, useMemo, useState } from 'react'; import type { Dispatch, SetStateAction } from 'react'; @@ -16,27 +15,15 @@ import { AltHeldProvider } from './AltHeldContext'; import EditPhraseControls from './controls/EditPhraseControls'; import useBookIndexes from '../hooks/useBookIndexes'; import { useAltHeld } from '../hooks/useAltHeld'; -import useLatestRef from '../hooks/useLatestRef'; import type { PhraseMode } from '../types/phrase-mode'; import type { ViewOptions } from '../types/view-options'; -import { isWordToken } from '../types/type-guards'; import { phrasesStraddlingBoundary, splitPhraseAtBoundary } from '../utils/phrase-arc'; -import { isSameVerse, segmentContainsVerse, toSerializedVerseRef } from '../utils/verse-ref'; import SegmentListView from './SegmentListView'; import UnlinkPhraseConfirm from './modals/UnlinkPhraseConfirm'; +import { FocusProvider } from './FocusStore'; import { useInterlinearNav } from './InterlinearNavContext'; import { RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; -/** - * Returns the ref of the first word token in `segment`, or `undefined` when the segment has none. - * Used to seed `focusedTokenRef` from the active verse's leading word. - * - * @param segment - The segment to read, or `undefined` when no active segment is resolved. - */ -function firstWordTokenRefOf(segment: Segment | undefined): string | undefined { - return segment?.tokens.find(isWordToken)?.ref; -} - /** Stable empty map used as the `formerBoundaries` default so memoization holds. */ const EMPTY_FORMER_BOUNDARIES: ReadonlyMap = new Map(); @@ -95,39 +82,15 @@ export default function Interlinearizer({ formerBoundaries = EMPTY_FORMER_BOUNDARIES, segmentationVersion = 0, }: InterlinearizerProps) { - // Navigation surface from the context: `navigate` writes the reference (classifying internal vs - // external at the call site), `consumeInternalNav` lets the segment window suppress the fade for - // internal moves, `reportSettled` lifts the cross-book curtain once the new book is laid out, and - // `consumeFocusRequest` / `focusRequestCount` collect a token focus asked for from outside the - // views. - const { navigate, consumeInternalNav, reportSettled, consumeFocusRequest, focusRequestCount } = - useInterlinearNav(); + // Navigation surface from the context: `consumeInternalNav` lets the segment window suppress the + // fade for internal moves, and `reportSettled` lifts the cross-book curtain once the new book is + // laid out. + const { consumeInternalNav, reportSettled } = useInterlinearNav(); // Whether Alt is currently held. Provided through a dedicated context (not the memoized // SegmentationContext) so an Alt press re-renders only the split-gap markers that consume it. const altHeld = useAltHeld(); - /** - * Finds the book segment that owns the active verse named by `scrRef`: the first segment in - * document order whose verse range contains it. Containment (rather than an exact start-verse - * match) matters after boundary edits — a verse absorbed into a multi-verse segment, or named by - * a later portion of a split verse, still resolves to the segment that holds its text. - * `segmentContainsVerse` also matches the book, so during a cross-book navigation (where `scrRef` - * names the new book before its data loads, leaving `book` still the previous one) this returns - * `undefined` rather than resolving to the wrong book's verse. - */ - const findActiveSegment = useCallback( - () => book.segments.find((seg) => segmentContainsVerse(seg, scrRef)), - [book.segments, scrRef], - ); - - // Seed focusedTokenRef from the active verse on first render so it is never undefined: an - // undefined focusedTokenRef would disable all link buttons (isSameSegmentAsFocus checks - // focus.focusedSegmentId). - const [focusedTokenRef, setFocusedTokenRef] = useState(() => - firstWordTokenRefOf(findActiveSegment()), - ); - // Book-wide lookup indexes. const { segmentById, @@ -139,22 +102,6 @@ export default function Interlinearizer({ wordRefByOrder, } = useBookIndexes(book); - // Reseed only when the new book no longer resolves the focused token — a book change, or a - // re-tokenization that dropped the token. A boundary edit (merge/split) also produces a fresh - // `book`, but token refs survive re-segmentation, so a still-resolving focus is kept to avoid - // snapping the strip back to the active verse's first word. Keep this declared above the - // focus-request claim below: that claim wins only by running last in the same commit. - useEffect(() => { - setFocusedTokenRef((current) => - current !== undefined && wordTokenByRef.has(current) - ? current - : firstWordTokenRefOf(findActiveSegment()), - ); - // findActiveSegment changes with scrRef too, and wordTokenByRef derives from book; only re-seed - // on book change. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [book]); - const phraseDispatch = usePhraseDispatch(); const getPhraseLinkById = usePhraseLinkByIdGetter(); const phraseLinkById = usePhraseLinkByIdMap(); @@ -273,102 +220,6 @@ export default function Interlinearizer({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [isRevert, updatePhrase, setPhraseMode]); - // Reseed focusedTokenRef when scrRef changes externally (e.g. Paratext verse selector). Skip when - // the focused token's *own* segment already contains the new verse — that means the change came - // from a token click / strip nav here, and reseeding would clobber the deliberately-focused token. - // The guard tests the focused token's own segment (not the active segment's id) so that clicking a - // non-first portion of a split verse doesn't get reseeded to the verse's first portion. Internal - // navigation always hits the skip branch because the handler has already set focus into the target - // verse. Keep this declared above the focus-request claim below: that claim wins only by running - // last in the same commit. - useEffect(() => { - const focusedSegId = focusedTokenRef ? tokenSegmentMap.get(focusedTokenRef) : undefined; - const focusedSeg = focusedSegId ? segmentById.get(focusedSegId) : undefined; - if (focusedSeg && segmentContainsVerse(focusedSeg, scrRef)) return; - /* v8 ignore next -- activeSeg is always defined when the book includes the active verse */ - setFocusedTokenRef(firstWordTokenRefOf(findActiveSegment())); - // findActiveSegment is intentionally excluded: the verse-coordinate deps already capture the - // change we care about, and it changes identity on every scrRef update. focusedTokenRef, - // tokenSegmentMap, and segmentById are excluded too — they are read only as guards; as deps they - // would re-run this effect on every focus move and clobber the deliberately-focused token with - // the verse's first word. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [scrRef.book, scrRef.chapterNum, scrRef.verseNum]); - - // Claim a focus asked for from outside the views. Declared after both reseed effects above so it - // wins: they run first in the same commit, and the scrRef reseed reads a focus closure that - // predates the book reseed, so its own guard cannot see this claim. Claimed in an effect rather - // than the seed above so the claim lands after commit, never during a render React may discard. - // It re-runs whenever the book object changes identity — a boundary edit re-segments it — which is - // harmless: claiming clears the request, so a repeat call finds nothing left to claim. - useEffect(() => { - const requested = consumeFocusRequest(book.bookRef); - if (requested === undefined) return; - if (wordTokenByRef.has(requested)) { - setFocusedTokenRef(requested); - return; - } - // A ref this book cannot resolve is dropped rather than held for a later attempt: one that - // outlived the load it was made for would fire on an unrelated navigation, long after the - // click that raised it. Logged because the drop is otherwise invisible. - logger.warn(`Interlinearizer: focus request "${requested}" matched no word token`); - // The count is the only signal when a request names the verse already on screen, the book the - // only one when it named a book that had yet to load. wordTokenByRef is excluded because it - // derives from book, consumeFocusRequest because its identity is stable. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [book, focusRequestCount]); - - const scrRefRef = useLatestRef(scrRef); - - /** - * Focuses `tokenRef` and, when it lives in a different verse than the active one, navigates - * there. The single explicit focus-move operation behind strip arrow nav and phrase clicks: it - * sets the focused token and pushes the verse change as an _internal_ navigation (so the segment - * window tracks along without a recenter fade). A verse-0 segment (a chapter superscription) - * navigates like any other verse. - * - * Never navigates when the focused token's book differs from the active `scrRef`'s book: during - * an external book change `scrRef` can briefly name the new book while the mounted book (and this - * token) still belong to the previous one, and echoing that stale verse would overwrite the new - * reference. - */ - const focusToken = useCallback( - (tokenRef: string) => { - setFocusedTokenRef(tokenRef); - const segId = tokenSegmentMap.get(tokenRef); - /* v8 ignore next 2 -- tokenRef always resolves to a segment in the mounted book */ - const seg = segId === undefined ? undefined : segmentById.get(segId); - if (!seg) return; - const { current } = scrRefRef; - if (seg.startRef.book !== current.book) return; - // Containment check (not exact start-verse match): focusing another token of the segment that - // already holds the active verse must not renavigate to the segment's start verse. - if (segmentContainsVerse(seg, current)) return; - navigate(toSerializedVerseRef(seg.startRef), 'internal'); - }, - [segmentById, tokenSegmentMap, navigate, scrRefRef], - ); - - /** - * 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. - * - * @param ref - The verse coordinate that was selected. - * @param tokenRef - The token that was clicked; omitted when the whole segment was selected. - */ - const handleSegmentSelect = useCallback( - (ref: ScriptureRef, tokenRef?: string) => { - const { current } = scrRefRef; - if (!isSameVerse(ref, current)) { - navigate(toSerializedVerseRef(ref), 'internal'); - } - if (tokenRef) setFocusedTokenRef(tokenRef); - }, - [navigate, scrRefRef], - ); - return ( @@ -390,44 +241,48 @@ export default function Interlinearizer({ className="tw:flex tw:flex-col tw:flex-1 tw:min-h-0 tw:transition-opacity" style={{ opacity: isModeToggleFading ? 0 : 1, ...RECENTER_FADE_TRANSITION_STYLE }} > - {displayContinuousScroll && ( -
- -
- )} - - + > + {displayContinuousScroll && ( +
+ +
+ )} + + +
diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index 37b691a3..771a017e 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -1,6 +1,6 @@ import { useLocalizedStrings } from '@papi/frontend/react'; import { Canon, type SerializedVerseRef } from '@sillsdev/scripture'; -import type { Book, ScriptureRef, Segment, Token } from 'interlinearizer'; +import type { Book, Segment, Token } from 'interlinearizer'; import { LocateFixed, Merge } from 'lucide-react'; import { Button, Tooltip, TooltipContent, TooltipTrigger } from 'platform-bible-react'; import { formatReplacementString } from 'platform-bible-utils'; @@ -15,6 +15,7 @@ import { buildSegmentLabels } from '../utils/segment-labels'; import { segmentContainsVerse } from '../utils/verse-ref'; import { buildVerseStartLabels } from '../utils/verse-superscripts'; import { useAltHeldValue } from './AltHeldContext'; +import { useFocus, useFocusActions } from './FocusStore'; import { useSegmentation } from './SegmentationStore'; import MemoizedSegmentView from './SegmentView'; import { RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; @@ -124,8 +125,6 @@ type SegmentListViewProps = Readonly<{ * (recenter with a fade). */ segmentationVersion: number; - /** Token ref of the currently focused word token, or `undefined` when nothing is focused. */ - focusedTokenRef: string | undefined; /** When true, the horizontal token strip is shown above this list (changes display mode). */ continuousScroll: boolean; /** @@ -159,8 +158,6 @@ type SegmentListViewProps = Readonly<{ setHoveredPhraseId: (phraseId: string | undefined) => void; /** Segment id that contains the phrase currently being edited, or `undefined`. */ editPhraseSegmentId: string | undefined; - /** Called when a segment or one of its word tokens is selected. */ - onSelect: (ref: ScriptureRef, tokenRef?: string) => void; /** Maps every token ref to the id of the segment that contains it. */ tokenSegmentMap: ReadonlyMap; /** Maps every word token ref to its flat book-level index; used to sort phrase tokens. */ @@ -179,7 +176,6 @@ export default function SegmentListView({ book, scrRef, segmentationVersion, - focusedTokenRef, continuousScroll, displayContinuousScroll, onDisplayContinuousScrollChange, @@ -191,11 +187,15 @@ export default function SegmentListView({ hoveredPhraseId, setHoveredPhraseId, editPhraseSegmentId, - onSelect, tokenSegmentMap, tokenDocOrder, wordTokenByRef, }: SegmentListViewProps) { + // The list gates the focus highlight on its own recenter clock, so it needs the live focus but not + // the origin behind it. + const { tokenRef: focusedTokenRef } = useFocus(); + const { selectSegment } = useFocusActions(); + const [localizedStrings] = useLocalizedStrings(HEADER_STRING_KEYS); /** * Inline verse-superscript labels for every segment (chapter-qualified where a verse start opens @@ -431,7 +431,7 @@ export default function SegmentListView({ : segmentContainsVerse(seg, displayScrRef) } onHoverPhrase={setHoveredPhraseId} - onSelect={onSelect} + onSelect={selectSegment} phraseMode={phraseMode} setPhraseMode={setPhraseMode} segment={seg} diff --git a/src/hooks/useSegmentWindow.ts b/src/hooks/useSegmentWindow.ts index f2d4f8ff..d71a9d5a 100644 --- a/src/hooks/useSegmentWindow.ts +++ b/src/hooks/useSegmentWindow.ts @@ -514,9 +514,9 @@ export default function useSegmentWindow({ // selector, scroll group) and recenters with the fade. // // "Internal" here means some view in the tree originated the nav — a wider question than the one - // `ContinuousView`'s own internal-nav check asks, which is whether the strip itself emitted it. - // The two therefore classify the same event differently by design; a click in this list is - // internal by this test and external by that one. + // the strip asks of a focus move, which is whether it emitted that move itself. The two therefore + // classify the same event differently by design; FocusOrigin is where the two readings are set + // side by side. // // A segments-identity change carrying a `segmentationVersion` bump is NOT a navigation: it is a // boundary edit (merge/split from the mounted controls). The window slice already re-renders the From cc38b665bed7b192e2e7b82f4c07fba5bd352f76 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 24 Aug 2026 08:43:23 -0400 Subject: [PATCH 2/9] Count arrow steps from the live focus, not a mirrored index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gating the pending-index resync on `origin !== 'strip'` never reopened: after a strip move the origin stays `strip` until something else writes focus, so the mirror froze. A phrase-link edit regroups the strip without moving focus, which shifts the focused token's group index — and the next arrow press then counted from an index naming a different group and skipped one. The old gate was the in-flight emit marker, which the focus-change effect cleared unconditionally, so the mirror resumed tracking after every echo and absorbed regrouping. Origin only answers who wrote the focus, not whether the rendered index has caught up, so it cannot stand in for that marker. Drop the mirror instead of re-deriving it. Because the store applies a write synchronously, `step` can read the focus as of the press and take its group index from the current map: a second press before the re-render still accumulates, a focus set anywhere else is stepped from where it actually is, and regrouping needs no bookkeeping at all. `stepFocusTo` goes with it — a strip move is now a plain `focusToken(ref, 'strip')`. Also from review: - Pin the mount ordering the provider relies on. Child effects run first, so a view that names its own focus on mount writes before the resolution rules run; they must leave it standing, since neither the book nor the verse has moved. Previously the `scrRef` reseed clobbered it from a stale closure, so this is a behavior change worth a test. - Name the invariant behind the focus-change effect's omitted `focusOrigin` dependency: it is safe only because a write naming the standing focus is dropped, which is what stops the origin from moving while the token ref holds still. Stated at both ends. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/ContinuousView.test.tsx | 31 +++++++- src/__tests__/components/FocusStore.test.tsx | 39 +++++++++- src/components/ContinuousView.tsx | 71 ++++++++----------- src/components/FocusStore.tsx | 4 +- 4 files changed, 100 insertions(+), 45 deletions(-) diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index 63584d1a..e0b871ef 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -21,7 +21,13 @@ import { } from '../../components/SegmentationStore'; import { RECENTER_FADE_MS } from '../../components/recenter-fade'; import { isWordToken } from '../../types/type-guards'; -import { FIXTURE_STAMPS, makePunctToken, makeSegment, makeWordToken } from '../test-helpers'; +import { + FIXTURE_STAMPS, + makePhraseLink, + makePunctToken, + makeSegment, + makeWordToken, +} from '../test-helpers'; import { allFalseViewOptions, mockKeyAsValueLocalizedStrings, @@ -781,6 +787,29 @@ describe('ContinuousView arrow navigation', () => { expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'tok-2'); }); + it('steps from the regrouped index after a phrase link moves the focused group', async () => { + // Linking earlier tokens into one phrase shifts every later group index while moving no focus. + // A step counting from its own last target would then skip the group beside the focused one. + const book = makeLargeBook(5); + const props = requiredProps(book, { focusedTokenRef: 'large-tok-1' }); + const { rerender } = render(, withAnalysisStore); + const next = screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }); + + // A move the strip made itself, so it is its own last target that a later step could count from. + await userEvent.click(next); + expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'large-tok-2'); + + // Joining the first two tokens pulls the focused token's group index down by one. + addPhraseLinkWithNewIdentity( + makePhraseLink('phrase-1', ['large-tok-0', 'large-tok-1'], ['word0', 'word1']), + ); + rerender(); + + await userEvent.click(next); + + expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'large-tok-3'); + }); + it('steps from a focus it did not choose rather than from its own last target', async () => { // A step counts from where the strip is heading, so rapid presses accumulate. A focus the strip // did not choose has to reset that count: otherwise the press after one lands a group away from diff --git a/src/__tests__/components/FocusStore.test.tsx b/src/__tests__/components/FocusStore.test.tsx index f2bc5323..f5282e11 100644 --- a/src/__tests__/components/FocusStore.test.tsx +++ b/src/__tests__/components/FocusStore.test.tsx @@ -5,7 +5,7 @@ import { logger } from '@papi/frontend'; import type { SerializedVerseRef } from '@sillsdev/scripture'; import { act, render, renderHook, screen } from '@testing-library/react'; import type { Book, Segment, Token } from 'interlinearizer'; -import type { ReactNode } from 'react'; +import { useEffect, type ReactNode } from 'react'; import { createFocusStore, FocusProvider, @@ -269,6 +269,43 @@ describe('FocusProvider seeding', () => { }); }); +describe('FocusProvider seeding from a child', () => { + /** Names its own focus on mount when nothing resolved one, as the continuous strip does. */ + function SeedingChild() { + const { tokenRef } = useFocus(); + const { focusToken } = useFocusActions(); + useEffect(() => { + if (tokenRef === undefined) focusToken('GEN 1:2:0', 'seed'); + // Intentionally runs only on mount; do not add deps. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + return
; + } + + it('leaves a focus a child named on mount standing', () => { + // Child effects run before the provider's, so the resolution rules see the child's write. They + // must leave it alone: nothing about the book or the verse has changed since the mount. + const noWordToken: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: '1', + segments: [makeSegment('GEN 1:1', '', []), ...makeBook().segments.slice(1)], + }; + + render( + [GEN_1_1, () => {}, undefined, () => {}, undefined]} + > + + + + , + ); + + expect(screen.getByTestId('child')).toHaveAttribute('data-focus', 'GEN 1:2:0'); + }); +}); + describe('FocusProvider focusToken', () => { it('navigates to the target segment when it does not hold the active verse', () => { const harness = renderFocus(makeBook(), GEN_1_1); diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index 92e745de..b3b30f37 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -238,20 +238,10 @@ export default function ContinuousView({ const lastDisplayUpdateWasInternalRef = useRef(false); /** - * Tracks the "pending" phrase index for sequential arrow-button presses. Written synchronously by - * `step()` so that a second click before re-render reads the already-advanced value instead of - * the stale rendered `focusPhraseIndex`, preventing rapid double-clicks from advancing only one - * group instead of two. + * Ref mirror of the rendered focus index, read only as the fallback for a step whose live focus + * this book cannot place, so a step keeps one identity across focus moves. */ - const pendingPhraseIndexRef = useRef(0); - - // Keep the pending index on the rendered value, so a focus the strip did not choose resets where - // the next step counts from. A move the strip made itself is excluded: it sets the pending index - // at the call site, and a rapid second click must read that already-advanced value rather than - // the rendered index, which has yet to catch up. - if (focusOrigin !== 'strip') { - pendingPhraseIndexRef.current = focusPhraseIndex; - } + const focusPhraseIndexRef = useLatestRef(focusPhraseIndex); /** DOM ref array indexed by group index; used to scroll the focused phrase box into view. */ const phraseRefs = useRef<(HTMLSpanElement | null)[]>([]); @@ -490,19 +480,6 @@ export default function ContinuousView({ commitPendingActiveSegment(); }, [tokenSegmentMap, focusedTokenRef, commitPendingActiveSegment]); - /** - * Focuses `ref` as a move this strip made, and records `groupIndex` as the position the next - * arrow step counts from. Folds the two into one call so no caller can advance focus while - * leaving the step origin behind. - */ - const stepFocusTo = useCallback( - (groupIndex: number, ref: string) => { - pendingPhraseIndexRef.current = groupIndex; - focusToken(ref, 'strip'); - }, - [focusToken], - ); - // Name the initially-focused token on mount so the segment list scrolls the active verse into // view on first render. Only fires when nothing has resolved a focus already. useEffect(() => { @@ -596,8 +573,13 @@ export default function ContinuousView({ /** * Advances focus by `delta` phrases, which re-derives `focusPhraseIndex` and triggers the scroll - * effect. Counts from the pending index rather than the rendered one, so a rapid second press - * moves two groups instead of repeating the first. + * effect. + * + * Counts from the focus as of the press, taken from the store rather than from the rendered + * index. That is what makes a second press before the re-render accumulate instead of repeating + * the first, and what keeps a step correct after anything else has moved the focused token's + * group — a focus set elsewhere, or a phrase-link edit that regrouped the strip without moving + * focus at all. * * @param delta - Number of phrases to move (positive = forward, negative = backward). */ @@ -605,15 +587,20 @@ export default function ContinuousView({ (delta: number) => { /* v8 ignore next -- arrow buttons are disabled when phraseGroups is empty */ if (phraseGroups.length === 0) return; - const nextIndex = pendingPhraseIndexRef.current + delta; + const currentRef = getFocus().tokenRef; + /* v8 ignore next 2 -- a focus always resolves while the strip has groups to step through */ + const from = + (currentRef === undefined ? undefined : groupIndexByTokenRef.get(currentRef)) ?? + focusPhraseIndexRef.current; + const nextIndex = from + delta; /* v8 ignore next -- disabled buttons prevent under/overflow */ const clamped = nextIndex < 0 ? 0 : Math.min(nextIndex, phraseGroups.length - 1); /* v8 ignore next -- disabled buttons prevent clicking when already at boundary */ - if (clamped === pendingPhraseIndexRef.current) return; + if (clamped === from) return; const nextRef = phraseGroups[clamped]?.tokens[0]?.ref; - if (nextRef !== undefined) stepFocusTo(clamped, nextRef); + if (nextRef !== undefined) focusToken(nextRef, 'strip'); }, - [phraseGroups, stepFocusTo], + [phraseGroups, groupIndexByTokenRef, getFocus, focusToken, focusPhraseIndexRef], ); /** Moves focus one phrase backward. */ @@ -634,16 +621,14 @@ export default function ContinuousView({ const handlePhraseSelect = useCallback( (ref: string) => { const targetGroupIndex = groupIndexByTokenRef.get(ref); - /* v8 ignore next -- ref is a group key, which the group-index map always resolves */ - if (targetGroupIndex === undefined) return; const currentFocus = getFocus().tokenRef; /* v8 ignore next 2 -- a focus is always resolved before a phrase box can be clicked */ const currentGroupIndex = currentFocus === undefined ? undefined : groupIndexByTokenRef.get(currentFocus); - if (targetGroupIndex === currentGroupIndex) return; - stepFocusTo(targetGroupIndex, ref); + if (targetGroupIndex !== undefined && targetGroupIndex === currentGroupIndex) return; + focusToken(ref, 'strip'); }, - [getFocus, groupIndexByTokenRef, stepFocusTo], + [getFocus, groupIndexByTokenRef, focusToken], ); /** Splits a phrase arc at a token boundary and dispatches the resulting phrase-store writes. */ @@ -668,7 +653,9 @@ export default function ContinuousView({ }, RECENTER_FADE_MS); return () => clearTimeout(timeout); // focusOrigin classifies the move that changed focusedTokenRef, so it is never itself a reason - // to re-run: listing it would re-fade for an origin change that moved no focus. + // to re-run: listing it would re-fade for an origin change that moved no focus. Reading it + // without listing it is safe only because a write naming the standing focus is dropped, which + // is what keeps origin from moving while the token ref holds still — see FocusStore.write. // eslint-disable-next-line react-hooks/exhaustive-deps }, [focusedTokenRef, displayFocusedTokenRef]); @@ -866,13 +853,13 @@ export default function ContinuousView({ useEffect(() => { if (phraseMode.kind === 'view') return; const targetPhraseId = phraseMode.phraseId; - const groupIndex = phraseGroups.findIndex((g) => g.phraseLink?.analysisId === targetPhraseId); - const nextRef = phraseGroups[groupIndex]?.tokens[0]?.ref; + const group = phraseGroups.find((g) => g.phraseLink?.analysisId === targetPhraseId); + const nextRef = group?.tokens[0]?.ref; /* v8 ignore next -- phrase always has tokens; the focus differs at mode entry */ if (nextRef === undefined || nextRef === focusedTokenRef) return; - stepFocusTo(groupIndex, nextRef); + focusToken(nextRef, 'strip'); // phraseGroups and the focus are read once per mode change; intentionally not deps so the - // effect only fires on actual mode transitions. stepFocusTo has a stable identity. + // effect only fires on actual mode transitions. focusToken has a stable identity. // eslint-disable-next-line react-hooks/exhaustive-deps }, [phraseMode]); diff --git a/src/components/FocusStore.tsx b/src/components/FocusStore.tsx index 36c05681..025df11f 100644 --- a/src/components/FocusStore.tsx +++ b/src/components/FocusStore.tsx @@ -50,7 +50,9 @@ export interface FocusStore { subscribe: (onFocusChange: () => void) => () => void; /** * Sets the focused token and its origin. A write naming the token already focused is dropped, so - * a reseed or a claim that resolves to the standing focus wakes nobody. + * a reseed or a claim that resolves to the standing focus wakes nobody — and so the origin never + * moves while the token ref holds still, which is what lets a reader treat the origin as the + * classification of the change it is already reacting to. */ write: (tokenRef: string | undefined, origin: FocusOrigin) => void; } From 286160600fdf56bc219b2be6b46b1a9d54437f84 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 24 Aug 2026 08:52:46 -0400 Subject: [PATCH 3/9] =?UTF-8?q?Drive=20the=20strip=20tests=20through=20the?= =?UTF-8?q?=20focus=20store,=20not=20a=20prop=20fa=C3=A7ade?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The strip harness took `focusedTokenRef` and `onFocusedTokenRefChange` as props and mirrored them into a store, which rebuilt the prop round-trip this branch deletes. The strip was then exercised through a translation layer production does not have, and the layer cost more than it saved: - it hardcoded `reseed` as the origin of every externally-driven focus, so no strip test could reach `list` or `request` — separate rows in the origin table, and untestable by construction - it needed a `prevFocusRef` guard with no production counterpart, purely because its mirror ran in an effect after child effects and would otherwise clobber the focus the strip names for itself on mount - three tests stood up stateful parents whose only job was echoing a ref back, which is what the store does by being the state Replace it with `renderStrip(book, { focus, props })`, which mounts the strip over a real store and real-shaped actions and hands back `focusToken` (the strip's own moves), `setFocus(ref, origin)` (a focus from anywhere else), `update(props)` and `container`. Focus now arrives and leaves the way it does in the app, with no view above the strip relaying it. What that turned up, all of it now asserted rather than implied: - every spy assertion states the origin, which caught two tests claiming `strip` for what is really the mount `seed` - `setFocus` calls spread across `list`, `request` and `reseed`, so each is exercised - one test's remaining echo line was a no-op the store already covered - test titles and comments no longer describe a parent that no longer exists The split-marker helper's own `renderStrip` is renamed to `renderSplitMarker`, and it mounts the store inline since it already nests its own providers. Net 165 lines lighter, same 71 tests. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/ContinuousView.test.tsx | 591 +++++++----------- 1 file changed, 213 insertions(+), 378 deletions(-) diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index e0b871ef..cafcfb03 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -4,7 +4,7 @@ import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { Book, PhraseAnalysisLink, Token } from 'interlinearizer'; -import { useEffect, useMemo, useRef, useState, type ComponentProps, type ReactNode } from 'react'; +import type { ComponentProps, ReactNode } from 'react'; import { resegmentBook } from 'parsers/papi/resegmentBook'; import type { PhraseDispatch } from '../../components/AnalysisStore'; import { AltHeldProvider } from '../../components/AltHeldContext'; @@ -13,7 +13,7 @@ import { createFocusStore, FocusStoreProvider, type FocusActions, - type FocusStore, + type FocusOrigin, } from '../../components/FocusStore'; import { SegmentationProvider, @@ -259,7 +259,7 @@ function makeLargeBook(count: number): Book { const scrollIntoViewMock = jest.fn(); -/** Builds the lookup maps that ContinuousView's parent supplies, derived from a Book. */ +/** Builds the lookup maps the strip is handed, derived from a Book. */ function buildLookups(book: Book): { tokenSegmentMap: ReadonlyMap; tokenDocOrder: ReadonlyMap; @@ -282,74 +282,15 @@ function buildLookups(book: Book): { return { tokenSegmentMap, tokenDocOrder, wordTokenByRef }; } -/** Props for {@link ContinuousViewHarness}. */ -type ContinuousViewHarnessProps = ComponentProps & - Readonly<{ - /** - * Focus the store is seeded with; each later change is applied as a focus the strip did not - * choose. - */ - focusedTokenRef: string | undefined; - /** Called with every focus the strip writes, so a test can spy on or echo back its moves. */ - onFocusedTokenRefChange: (ref: string) => void; - }>; +/** The strip props a test does not care about, with the lookup maps derived from `book`. */ +type StripProps = ComponentProps; -/** - * Mounts the strip over a real focus store driven by the harness props, so a test can drive focus - * from outside the strip and observe the moves the strip makes without standing up the provider - * that resolves focus from a book and a reference. - */ -function ContinuousViewHarness({ - focusedTokenRef, - onFocusedTokenRefChange, - ...stripProps -}: ContinuousViewHarnessProps) { - const storeRef = useRef(undefined); - if (storeRef.current === undefined) storeRef.current = createFocusStore(focusedTokenRef); - const store = storeRef.current; - - const actions = useMemo( - () => ({ - focusToken: (ref, origin) => { - store.write(ref, origin); - onFocusedTokenRefChange(ref); - }, - selectSegment: () => {}, - }), - [store, onFocusedTokenRefChange], - ); - - // Mirrors only an actual prop change: applying the seed again on mount would clobber the focus the - // strip names for itself when nothing resolved one, since child effects run first. - const prevFocusRef = useRef(focusedTokenRef); - useEffect(() => { - if (focusedTokenRef === prevFocusRef.current) return; - prevFocusRef.current = focusedTokenRef; - store.write(focusedTokenRef, 'reseed'); - }, [store, focusedTokenRef]); - - return ( - - - - ); -} - -/** - * Minimal required props for the strip harness. Spread into render calls so tests only need to - * override what they actually care about. The lookup maps are derived from `book` so they always - * agree with what's rendered. - */ -function requiredProps( - book: Book, - overrides?: { focusedTokenRef?: string | undefined }, -): ContinuousViewHarnessProps & { onFocusedTokenRefChange: jest.Mock; setPhraseMode: jest.Mock } { +/** Minimal strip props, so a test states only what it actually varies. */ +function requiredProps(book: Book): StripProps { const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); return { book, editPhraseSegmentId: undefined, - focusedTokenRef: overrides?.focusedTokenRef, - onFocusedTokenRefChange: jest.fn(), phraseMode: { kind: 'view' }, setPhraseMode: jest.fn(), tokenSegmentMap, @@ -359,6 +300,62 @@ function requiredProps( }; } +/** What {@link renderStrip} hands back for driving and observing the mounted strip. */ +type Strip = { + /** + * Every focus the strip wrote, as `(tokenRef, origin)` — the strip's side of the store, which + * would otherwise be invisible. + */ + focusToken: jest.Mock; + /** + * Applies a focus from outside the strip, the way the resolution rules, the segment list or an + * outside request would. The origin is stated per call, since that is what the strip classifies + * on. + */ + setFocus: (tokenRef: string | undefined, origin: FocusOrigin) => void; + /** Re-renders with `next` merged over the strip's props; call with nothing for a plain re-render. */ + update: (next?: Partial) => void; + /** The rendered strip.container, for the tests that query the DOM directly. */ + container: HTMLElement; +}; + +/** + * Mounts the strip over a real focus store seeded with `focus`. Focus arrives and leaves exactly as + * it does in the app — through the store and the actions it publishes — with no view above the + * strip relaying it, so a test states the origin of every focus it drives and reads the strip's own + * moves off {@link Strip.focusToken}. + */ +function renderStrip( + book: Book, + options?: Readonly<{ focus?: string; props?: Partial }>, +): Strip { + const store = createFocusStore(options?.focus); + const focusToken = jest.fn((tokenRef: string, origin: FocusOrigin) => + store.write(tokenRef, origin), + ); + const actions: FocusActions = { focusToken, selectSegment: jest.fn() }; + let props: StripProps = { ...requiredProps(book), ...options?.props }; + const element = () => ( + + + + ); + + const view = render(element(), withAnalysisStore); + + return { + focusToken, + container: view.container, + setFocus: (tokenRef, origin) => { + act(() => store.write(tokenRef, origin)); + }, + update: (next) => { + props = { ...props, ...next }; + view.rerender(element()); + }, + }; +} + /** Every {@link TrackingResizeObserver} created since the last reset, newest last. */ let resizeObserverInstances: TrackingResizeObserver[] = []; @@ -417,7 +414,7 @@ beforeEach(() => { describe('ContinuousView initial render', () => { it('renders all tokens from all segments as a flat list', () => { const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('the')).toBeInTheDocument(); @@ -427,7 +424,7 @@ describe('ContinuousView initial render', () => { it('renders an inline verse-number superscript at each verse start', () => { const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); // Verses 1 and 2; the first opens chapter 1, so its label is chapter-qualified (`1:1`). const sups = screen.getAllByTestId('verse-superscript'); @@ -456,7 +453,7 @@ describe('ContinuousView initial render', () => { }, ], }); - render(, withAnalysisStore); + renderStrip(splitBook); const sups = screen.getAllByTestId('verse-superscript'); expect(sups.map((s) => s.textContent)).toEqual(['1:1']); @@ -464,14 +461,14 @@ describe('ContinuousView initial render', () => { it('does not render an extension-generated segment separator', () => { const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); expect(screen.queryByText('GEN 1:1')).not.toBeInTheDocument(); }); it('renders a Previous token button and a Next token button', () => { const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -483,7 +480,7 @@ describe('ContinuousView initial render', () => { it('renders a non-word token via InertTokenChip within the strip', () => { const book = makeMixedBook(); - render(, withAnalysisStore); + renderStrip(book); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('.')).toBeInTheDocument(); @@ -491,33 +488,28 @@ describe('ContinuousView initial render', () => { it('renders without crashing when book has no word tokens', () => { const book = makeWordFreeBook(); - render(, withAnalysisStore); + renderStrip(book); expect(screen.getByText('.')).toBeInTheDocument(); }); - it('notifies the parent of the initially-focused token on mount when no focus prop is set', () => { + it('names its own focus on mount when nothing resolved one', () => { const book = makeBook(); - const props = requiredProps(book); - render(, withAnalysisStore); + const strip = renderStrip(book); - expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('tok-0'); + expect(strip.focusToken).toHaveBeenCalledWith('tok-0', 'seed'); }); - it('does not notify the parent on mount when focusedTokenRef is already set', () => { + it('names no focus on mount when one is already resolved', () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-1' }); - expect(props.onFocusedTokenRefChange).not.toHaveBeenCalled(); + expect(strip.focusToken).not.toHaveBeenCalled(); }); - it('marks the phrase containing focusedTokenRef as focused', () => { + it('marks the phrase containing the focused token as focused', () => { const book = makeBook(); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'tok-2' }); const focusedBox = screen.getByText('beginning').closest('[data-phrase-box="true"]'); expect(focusedBox).toHaveAttribute('data-focus-state', 'focused'); @@ -537,11 +529,10 @@ describe('ContinuousView initial render', () => { makeSegment('MAT 1:2', 'Beta', [makeWordToken('mat-tok-1', 'Beta')]), ], }; - const props = requiredProps(otherBook, { focusedTokenRef: 'tok-2' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(otherBook, { focus: 'tok-2' }); scrollIntoViewMock.mockClear(); - rerender(); + strip.setFocus('mat-tok-1', 'reseed'); // The scroll lands on "Beta" (the live focus), never "Alpha" (phrase 0). const scrolledTexts = scrollIntoViewMock.mock.contexts.map((el) => @@ -564,15 +555,11 @@ describe('ContinuousView initial render', () => { makeSegment('MAT 1:2', 'Beta', [makeWordToken('mat-tok-1', 'Beta')]), ], }; - const { rerender } = render( - , - withAnalysisStore, - ); + const strip = renderStrip(makeBook(), { focus: 'tok-2' }); scrollIntoViewMock.mockClear(); - rerender( - , - ); + strip.update({ book: otherBook, ...buildLookups(otherBook) }); + strip.setFocus('mat-tok-1', 'reseed'); const scrolledTexts = scrollIntoViewMock.mock.contexts.map((el) => el instanceof HTMLElement ? el.textContent : undefined, @@ -582,35 +569,33 @@ describe('ContinuousView initial render', () => { }); describe('ContinuousView focus changes', () => { - it('notifies the parent when an out-of-focus phrase box is clicked', async () => { + it('focuses an out-of-focus phrase box when it is clicked', async () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); const clickedPhraseBox = screen.getByText('beginning').closest('[data-phrase-box="true"]'); if (!clickedPhraseBox) throw new Error('Expected phrase box wrapper for token'); await userEvent.click(clickedPhraseBox); - expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('tok-2'); + expect(strip.focusToken).toHaveBeenCalledWith('tok-2', 'strip'); }); - it('does not notify the parent when clicking the already-focused phrase box', async () => { + it('moves no focus when the already-focused phrase box is clicked', async () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); const firstPhraseBox = screen.getByText('In').closest('[data-phrase-box="true"]'); if (!firstPhraseBox) throw new Error('Expected phrase box wrapper for token'); await userEvent.click(firstPhraseBox); - expect(props.onFocusedTokenRefChange).not.toHaveBeenCalled(); + expect(strip.focusToken).not.toHaveBeenCalled(); }); - it('does not notify the parent when clicking the group of an already-focused non-first token', async () => { + it('moves no focus when clicking the group of an already-focused non-first token', async () => { // tok-0/tok-1 grouped into one box (keyed by tok-0) with focus on tok-1: clicking the box stays - // a no-op even though its groupKey differs from focusedTokenRef. + // a no-op even though its group key differs from the focused token. const phraseLink: PhraseAnalysisLink = { ...FIXTURE_STAMPS, analysisId: 'phrase-1', @@ -623,38 +608,35 @@ describe('ContinuousView focus changes', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-1' }); const groupedBox = screen.getByText('In').closest('[data-phrase-box="true"]'); if (!groupedBox) throw new Error('Expected phrase box wrapper for grouped tokens'); await userEvent.click(groupedBox); - expect(props.onFocusedTokenRefChange).not.toHaveBeenCalled(); + expect(strip.focusToken).not.toHaveBeenCalled(); }); - it('notifies the parent when clicking a phrase box while nothing is focused', async () => { + it('focuses the clicked phrase box when nothing was focused', async () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: undefined }); - render(, withAnalysisStore); + const strip = renderStrip(book); const firstPhraseBox = screen.getByText('In').closest('[data-phrase-box="true"]'); if (!firstPhraseBox) throw new Error('Expected phrase box wrapper for token'); await userEvent.click(firstPhraseBox); - expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('tok-0'); + // The mount seed already put focus on tok-0, so the click adds no move of its own. + expect(strip.focusToken).toHaveBeenCalledTimes(1); + expect(strip.focusToken).toHaveBeenCalledWith('tok-0', 'seed'); }); }); describe('ContinuousView arrow disabled states', () => { it('disables the prev arrow when focus is on the first phrase', () => { const book = makeBook(); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'tok-0' }); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -663,10 +645,7 @@ describe('ContinuousView arrow disabled states', () => { it('enables the prev arrow when focus is on a non-first phrase', () => { const book = makeBook(); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'tok-2' }); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -675,10 +654,7 @@ describe('ContinuousView arrow disabled states', () => { it('disables the next arrow when focus is on the last phrase', () => { const book = makeBook(); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'tok-3' }); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), @@ -687,10 +663,7 @@ describe('ContinuousView arrow disabled states', () => { it('enables the next arrow when focus is on a non-last phrase', () => { const book = makeBook(); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'tok-0' }); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), @@ -699,10 +672,7 @@ describe('ContinuousView arrow disabled states', () => { it('disables both arrows when the book has a single token', () => { const book = makeSingleTokenBook(); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'tok-only' }); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -714,7 +684,7 @@ describe('ContinuousView arrow disabled states', () => { it('disables both arrows when the book has no word tokens', () => { const book = makeWordFreeBook(); - render(, withAnalysisStore); + renderStrip(book); expect( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), @@ -726,88 +696,82 @@ describe('ContinuousView arrow disabled states', () => { }); describe('ContinuousView arrow navigation', () => { - it('notifies the parent of the next phrase ref when Next is clicked', async () => { + it('focuses the next phrase when Next is clicked', async () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('tok-1'); + expect(strip.focusToken).toHaveBeenCalledWith('tok-1', 'strip'); }); - it('notifies the parent of the previous phrase ref when Previous is clicked', async () => { + it('focuses the previous phrase when Previous is clicked', async () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-1' }); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), ); - expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('tok-0'); + expect(strip.focusToken).toHaveBeenCalledWith('tok-0', 'strip'); }); it('crosses verse boundaries via the Next arrow', async () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-1' }); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('tok-2'); + expect(strip.focusToken).toHaveBeenCalledWith('tok-2', 'strip'); }); it('crosses chapter boundaries via the Next arrow', async () => { const book = makeTwoChapterBook(); - const props = requiredProps(book, { focusedTokenRef: 'ch1-tok-0' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'ch1-tok-0' }); await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - expect(props.onFocusedTokenRefChange).toHaveBeenCalledWith('ch2-tok-0'); + expect(strip.focusToken).toHaveBeenCalledWith('ch2-tok-0', 'strip'); }); it('advances two groups on rapid double-click before re-render', async () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); const next = screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }); await userEvent.click(next); await userEvent.click(next); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'tok-1'); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'tok-2'); + expect(strip.focusToken).toHaveBeenNthCalledWith(1, 'tok-1', 'strip'); + expect(strip.focusToken).toHaveBeenNthCalledWith(2, 'tok-2', 'strip'); }); it('steps from the regrouped index after a phrase link moves the focused group', async () => { // Linking earlier tokens into one phrase shifts every later group index while moving no focus. // A step counting from its own last target would then skip the group beside the focused one. const book = makeLargeBook(5); - const props = requiredProps(book, { focusedTokenRef: 'large-tok-1' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'large-tok-1' }); const next = screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }); // A move the strip made itself, so it is its own last target that a later step could count from. await userEvent.click(next); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'large-tok-2'); + expect(strip.focusToken).toHaveBeenNthCalledWith(1, 'large-tok-2', 'strip'); // Joining the first two tokens pulls the focused token's group index down by one. addPhraseLinkWithNewIdentity( makePhraseLink('phrase-1', ['large-tok-0', 'large-tok-1'], ['word0', 'word1']), ); - rerender(); + strip.update(); await userEvent.click(next); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'large-tok-3'); + expect(strip.focusToken).toHaveBeenNthCalledWith(2, 'large-tok-3', 'strip'); }); it('steps from a focus it did not choose rather than from its own last target', async () => { @@ -817,16 +781,15 @@ describe('ContinuousView arrow navigation', () => { jest.useFakeTimers(); try { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-1' }); fireEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(1, 'tok-2'); + expect(strip.focusToken).toHaveBeenNthCalledWith(1, 'tok-2', 'strip'); // A focus from outside the strip, given the fade it takes to arrive. - rerender(); + strip.setFocus('tok-3', 'list'); act(() => { jest.advanceTimersByTime(RECENTER_FADE_MS); }); @@ -834,7 +797,7 @@ describe('ContinuousView arrow navigation', () => { fireEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }), ); - expect(props.onFocusedTokenRefChange).toHaveBeenNthCalledWith(2, 'tok-2'); + expect(strip.focusToken).toHaveBeenNthCalledWith(2, 'tok-2', 'strip'); } finally { jest.useRealTimers(); } @@ -844,7 +807,7 @@ describe('ContinuousView arrow navigation', () => { describe('ContinuousView scroll behavior', () => { it('calls scrollIntoView on initial mount', () => { const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); expect(scrollIntoViewMock).toHaveBeenCalledWith({ behavior: 'auto', @@ -853,16 +816,15 @@ describe('ContinuousView scroll behavior', () => { }); }); - it('uses instant scroll when focusedTokenRef changes externally', () => { + it('uses instant scroll for a focus it did not choose', () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); scrollIntoViewMock.mockClear(); act(() => { jest.useFakeTimers(); }); - rerender(); + strip.setFocus('tok-3', 'reseed'); act(() => { jest.advanceTimersByTime(600); jest.useRealTimers(); @@ -876,15 +838,14 @@ describe('ContinuousView scroll behavior', () => { // scroll effect's own hold loop must keep the group pinned while late layout (arc padding // settling on the slid window) shifts the strip after the instant snap. const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); act(() => { jest.useFakeTimers(); }); try { // tok-1 shares GEN 1:1 with tok-0, so the active segment is unchanged by this jump. - rerender(); + strip.setFocus('tok-1', 'list'); // Complete the fade-out (RECENTER_FADE_MS) so the displayed focus updates and the instant // snap fires. scrollIntoViewMock.mockClear(); @@ -928,8 +889,7 @@ describe('ContinuousView scroll behavior', () => { try { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); act(() => { jest.useFakeTimers(); @@ -937,7 +897,7 @@ describe('ContinuousView scroll behavior', () => { try { // tok-1 shares GEN 1:1 with tok-0, so the active segment is unchanged: only the scroll // effect's own hold loop keeps the group pinned. - rerender(); + strip.setFocus('tok-1', 'list'); act(() => { jest.advanceTimersByTime(510); }); @@ -977,8 +937,7 @@ describe('ContinuousView scroll behavior', () => { try { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); act(() => { jest.useFakeTimers(); @@ -988,7 +947,7 @@ describe('ContinuousView scroll behavior', () => { // the initial real-timer mount is not deterministically advanceable). tok-1 shares GEN 1:1 // with tok-0, so there's no committed-active-segment flip: only the scroll effect's own hold // pins the group, exercising the same observer lifetime as the initial mount. - rerender(); + strip.setFocus('tok-1', 'request'); // Complete the fade-out (RECENTER_FADE_MS) so the instant snap + hold start. act(() => { jest.advanceTimersByTime(510); @@ -1026,17 +985,16 @@ describe('ContinuousView scroll behavior', () => { it('snaps the link slots (no transition) during an external jump so they do not slide after the fade-in', () => { const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { container, rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); act(() => { jest.useFakeTimers(); }); // External nav into the other verse: the active segment commits instantly behind the fade, so // the slots snap to their new widths rather than animating (which would slide the boxes). - rerender(); + strip.setFocus('tok-3', 'reseed'); - const slotWrapper = container.querySelector('[data-testid="link-slot-icon"]'); + const slotWrapper = strip.container.querySelector('[data-testid="link-slot-icon"]'); if (!(slotWrapper instanceof HTMLElement)) throw new Error('Expected a link-slot icon wrapper'); expect(slotWrapper.style.transitionDuration).toBe('0ms'); @@ -1046,30 +1004,8 @@ describe('ContinuousView scroll behavior', () => { }); }); - it('smooth-scrolls for internal nav once the parent echoes the ref back synchronously', async () => { - // The smooth-scroll path needs the displayed focus to agree with the prop and the strip to be - // visible, which only happens with a real (stateful) parent reflecting the ref change back, so - // simulate one here rather than a jest.fn() that never updates the prop. - const book = makeBook(); - const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); - function Parent() { - const [ref, setRef] = useState('tok-0'); - return ( - - ); - } - render(, withAnalysisStore); + it('smooth-scrolls for a move it made itself', async () => { + renderStrip(makeBook(), { focus: 'tok-0' }); // Wait for the initial fade-in (strip visible) before navigating; the smooth path is only taken // while the strip is already visible. await waitFor(() => @@ -1095,26 +1031,10 @@ describe('ContinuousView scroll behavior', () => { * whether the active-segment relayout has committed. */ function renderHideInactiveCrossing(): () => boolean { - const book = makeBook(); - const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); - function Parent() { - const [ref, setRef] = useState('tok-1'); - return ( - - ); - } - render(, withAnalysisStore); + renderStrip(makeBook(), { + focus: 'tok-1', + props: { viewOptions: { ...allFalseViewOptions, hideInactiveLinkButtons: true } }, + }); // Returns true when the tok-0/tok-1 link icon is rendered and its wrapper is visible. Suppressed // icons stay mounted but hidden via opacity:0, so query the wrapper's style, not spy calls. return () => { @@ -1212,30 +1132,7 @@ describe('ContinuousView scroll behavior', () => { // committed, its in-segment link icon stays mounted through the edit until the scroll settles. const book = makeBook(); const merged = resegmentBook(book, { removedVerseStarts: ['tok-2'], addedStarts: [] }); - const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); const mergedLookups = buildLookups(merged); - let applyBoundaryEdit: () => void = () => {}; - /** Stateful parent that starts on the verse book and swaps to the merged book on demand. */ - function Parent() { - const [ref, setRef] = useState('tok-1'); - const [edited, setEdited] = useState(false); - applyBoundaryEdit = () => setEdited(true); - const lookups = edited ? mergedLookups : { tokenSegmentMap, tokenDocOrder, wordTokenByRef }; - return ( - - ); - } /** Whether the tok-0/tok-1 link icon (in GEN 1:1) is mounted and visible. */ const inSegmentIconMounted = () => { const icon = document.querySelector( @@ -1245,7 +1142,10 @@ describe('ContinuousView scroll behavior', () => { }; jest.useFakeTimers(); try { - render(, withAnalysisStore); + const strip = renderStrip(book, { + focus: 'tok-1', + props: { viewOptions: { ...allFalseViewOptions, hideInactiveLinkButtons: true } }, + }); act(() => { jest.runOnlyPendingTimers(); }); @@ -1263,7 +1163,7 @@ describe('ContinuousView scroll behavior', () => { // unchanged; the reconcile defers rather than commit-and-relayout, so the old segment's // in-segment icon stays mounted. act(() => { - applyBoundaryEdit(); + strip.update({ book: merged, ...mergedLookups }); }); expect(inSegmentIconMounted()).toBe(true); } finally { @@ -1325,10 +1225,7 @@ describe('ContinuousView scroll behavior', () => { it('scrolls with the nearest-block, center-inline placement', () => { const book = makeBook(); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'tok-0' }); expect(scrollIntoViewMock).toHaveBeenCalledWith( expect.objectContaining({ block: 'nearest', inline: 'center' }), @@ -1339,24 +1236,15 @@ describe('ContinuousView scroll behavior', () => { // Inactive link slots hide via visibility:hidden (not max-width collapse), so toggling // hideInactiveLinkButtons doesn't shift layout; simplifyPhrases does, so it re-centers once. const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); scrollIntoViewMock.mockClear(); - rerender( - , - ); + strip.update({ viewOptions: { ...allFalseViewOptions, hideInactiveLinkButtons: true } }); expect(scrollIntoViewMock).not.toHaveBeenCalled(); - rerender( - , - ); + strip.update({ + viewOptions: { ...allFalseViewOptions, hideInactiveLinkButtons: true, simplifyPhrases: true }, + }); expect(scrollIntoViewMock).toHaveBeenCalledWith( expect.objectContaining({ behavior: 'auto', inline: 'center' }), ); @@ -1367,16 +1255,10 @@ describe('ContinuousView scroll behavior', () => { // Morpheme rows beneath tokens can widen phrase boxes, shifting the strip layout, so the // focused group must be snapped back to center when the toggle flips. const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'tok-0' }); scrollIntoViewMock.mockClear(); - rerender( - , - ); + strip.update({ viewOptions: { ...allFalseViewOptions, showMorphology: true } }); expect(scrollIntoViewMock).toHaveBeenCalledWith( expect.objectContaining({ behavior: 'auto', inline: 'center' }), ); @@ -1400,23 +1282,22 @@ describe('ContinuousView segmentation edits', () => { it('keeps the focused segment link buttons active when a merge changes the focused token segment id', () => { const book = makeBook(); - const props = { - ...requiredProps(book, { focusedTokenRef: 'tok-2' }), - viewOptions: { ...allFalseViewOptions, hideInactiveLinkButtons: true }, - }; - const { container, rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { + focus: 'tok-2', + props: { viewOptions: { ...allFalseViewOptions, hideInactiveLinkButtons: true } }, + }); // Focus sits in GEN 1:2, so the slot between its two tokens is active and visible. - expect(slotOpacity(container, 'tok-2', 'tok-3')).toBe('1'); + expect(slotOpacity(strip.container, 'tok-2', 'tok-3')).toBe('1'); // Merge GEN 1:2 into GEN 1:1. Token refs survive, so focus stays put, but the focused token's // segment id changes. const merged = resegmentBook(book, { removedVerseStarts: ['tok-2'], addedStarts: [] }); - rerender(); + strip.update({ book: merged, ...buildLookups(merged) }); // The committed active segment must follow the merge; a stale id would suppress every link // button until the next navigation. - expect(slotOpacity(container, 'tok-2', 'tok-3')).toBe('1'); + expect(slotOpacity(strip.container, 'tok-2', 'tok-3')).toBe('1'); }); }); @@ -1427,7 +1308,7 @@ describe('ContinuousView split marker', () => { * * @param altHeld - Whether Alt is held (defaults to held, so the marker appears). */ - function renderStrip(altHeld = true) { + function renderSplitMarker(altHeld = true) { const book = makeBook(); const dispatch = { merge: jest.fn(), split: jest.fn(), move: jest.fn() }; const segmentById = new Map(book.segments.map((seg) => [seg.id, seg])); @@ -1442,7 +1323,12 @@ describe('ContinuousView split marker', () => { render( - + + + , withAnalysisStore, @@ -1451,17 +1337,17 @@ describe('ContinuousView split marker', () => { } it('reveals a split marker on an intra-segment gap while Alt is held', () => { - renderStrip(true); + renderSplitMarker(true); expect(screen.getAllByTestId('boundary-split-marker').length).toBeGreaterThan(0); }); it('reveals no split marker while Alt is not held', () => { - renderStrip(false); + renderSplitMarker(false); expect(screen.queryByTestId('boundary-split-marker')).not.toBeInTheDocument(); }); it('dispatches a split on an Alt+click of the strip marker', () => { - const dispatch = renderStrip(true); + const dispatch = renderSplitMarker(true); // The gap between "In" (tok-0) and "the" (tok-1) inside GEN 1:1 splits before the second word. fireEvent.click(screen.getAllByTestId('boundary-split-marker')[0], { altKey: true }); expect(dispatch.split).toHaveBeenCalledWith('tok-1'); @@ -1482,7 +1368,7 @@ describe('ContinuousView RTL layout', () => { it('uses right-pointing arrow for Previous in RTL', () => { document.documentElement.dir = 'rtl'; const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); const prev = screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%', @@ -1493,7 +1379,7 @@ describe('ContinuousView RTL layout', () => { it('uses left-pointing arrow for Next in RTL', () => { document.documentElement.dir = 'rtl'; const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); const next = screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }); expect(next.textContent).toContain('←'); @@ -1502,7 +1388,7 @@ describe('ContinuousView RTL layout', () => { it('uses left-pointing arrow for Previous in LTR', () => { document.documentElement.dir = 'ltr'; const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); const prev = screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%', @@ -1514,20 +1400,14 @@ describe('ContinuousView RTL layout', () => { describe('ContinuousView phrase window', () => { it('renders the focused phrase from a large book', () => { const book = makeLargeBook(300); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'large-tok-150' }); expect(screen.getByText('word150')).toBeInTheDocument(); }); it('does not render tokens that fall outside the rendered window', () => { const book = makeLargeBook(300); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'large-tok-0' }); // tok-299 is well outside the rendered phrase window. expect(screen.queryByText('word299')).not.toBeInTheDocument(); @@ -1553,10 +1433,7 @@ describe('ContinuousView phrase window', () => { // take the whole arc with it and leave the visible fragment with no phrase cue at all. linkFarApartTokens(); const book = makeLargeBook(300); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'large-tok-150' }); expect(screen.getByText('word190')).toBeInTheDocument(); }); @@ -1564,10 +1441,7 @@ describe('ContinuousView phrase window', () => { it('widens no further than the phrase span it is covering', () => { linkFarApartTokens(); const book = makeLargeBook(300); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'large-tok-150' }); expect(screen.queryByText('word200')).not.toBeInTheDocument(); }); @@ -1576,8 +1450,7 @@ describe('ContinuousView phrase window', () => { // Widening start-ward mounts groups ahead of the focus at an unchanged scroll offset, the same // shift a resize causes — but the window size is unchanged, so a size-keyed correction misses it. const book = makeLargeBook(300); - const props = requiredProps(book, { focusedTokenRef: 'large-tok-150' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'large-tok-150' }); scrollIntoViewMock.mockClear(); addPhraseLinkWithNewIdentity({ @@ -1589,7 +1462,7 @@ describe('ContinuousView phrase window', () => { { tokenRef: 'large-tok-145', surfaceText: 'word145' }, ], }); - rerender(); + strip.update(); expect(screen.getByText('word110')).toBeInTheDocument(); expect(scrollIntoViewMock).toHaveBeenCalledWith(expect.objectContaining({ behavior: 'auto' })); @@ -1597,10 +1470,7 @@ describe('ContinuousView phrase window', () => { it('leaves an unlinked token at the same distance outside the window', () => { const book = makeLargeBook(300); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'large-tok-150' }); expect(screen.queryByText('word190')).not.toBeInTheDocument(); }); @@ -1614,10 +1484,7 @@ describe('ContinuousView phrase window', () => { try { const book = makeLargeBook(300); - render( - , - withAnalysisStore, - ); + renderStrip(book, { focus: 'large-tok-150' }); // jsdom lays nothing out, so the geometry the window measures is supplied: a viewport wide // enough to ask for more groups, over groups spaced a fixed pitch apart. @@ -1676,8 +1543,7 @@ describe('ContinuousView phrase window', () => { try { const book = makeLargeBook(300); // Focused close enough to the book start that the widened window's start clamps to 0. - const props = requiredProps(book, { focusedTokenRef: 'large-tok-12' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'large-tok-12' }); const viewport = screen.getByTestId('strip-scroll-viewport'); const stripRow = screen.getByTestId('token-strip'); @@ -1726,10 +1592,8 @@ describe('ContinuousView phrase window', () => { fireEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - const emitted = props.onFocusedTokenRefChange.mock.calls.at(-1)?.[0]; - const nextRef = typeof emitted === 'string' ? emitted : undefined; - expect(nextRef).toBe('large-tok-13'); - rerender(); + expect(strip.focusToken).toHaveBeenLastCalledWith('large-tok-13', 'strip'); + strip.update(); layOutGroups(); // Let the step's own glide settle, so the deferred-correction path is not what answers the @@ -1772,8 +1636,7 @@ describe('ContinuousView phrase window', () => { try { const book = makeLargeBook(300); - const props = requiredProps(book, { focusedTokenRef: 'large-tok-150' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'large-tok-150' }); // jsdom lays nothing out, so the geometry the window measures is supplied: a viewport wide // enough to ask for more groups, over groups spaced a fixed pitch apart. @@ -1809,15 +1672,13 @@ describe('ContinuousView phrase window', () => { }); layOutGroups(); - // Navigate one phrase forward while that hold is still alive, echoing the new ref back the - // way the parent does so the strip treats it as internal navigation. + // Navigate one phrase forward while that hold is still alive; the strip's own move is the + // path that glides. fireEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - const emitted = props.onFocusedTokenRefChange.mock.calls.at(-1)?.[0]; - const nextRef = typeof emitted === 'string' ? emitted : undefined; - expect(nextRef).toBe('large-tok-151'); - rerender(); + expect(strip.focusToken).toHaveBeenLastCalledWith('large-tok-151', 'strip'); + strip.update(); scrollIntoViewMock.mockClear(); // Report the window slide's own reflow, then run out the frames a restarted hold would use. @@ -1858,8 +1719,7 @@ describe('ContinuousView phrase window', () => { try { const book = makeLargeBook(300); - const props = requiredProps(book, { focusedTokenRef: 'large-tok-150' }); - const { rerender } = render(, withAnalysisStore); + const strip = renderStrip(book, { focus: 'large-tok-150' }); // jsdom lays nothing out, so the geometry the window measures is supplied: a viewport wide // enough to ask for more groups, over groups spaced a fixed pitch apart. @@ -1884,14 +1744,11 @@ describe('ContinuousView phrase window', () => { jest.useFakeTimers(); }); try { - // Echoing the emitted ref back the way the parent does is what makes the strip treat this - // as internal navigation, which is the only path that glides rather than snapping. + // A move the strip makes itself is the only path that glides rather than snapping. fireEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - const emitted = props.onFocusedTokenRefChange.mock.calls.at(-1)?.[0]; - const nextRef = typeof emitted === 'string' ? emitted : undefined; - rerender(); + strip.update(); scrollIntoViewMock.mockClear(); const windowObserver = resizeObserverInstances.find((o) => o.targets.includes(viewport)); @@ -1944,7 +1801,7 @@ describe('ContinuousView phrase grouping', () => { ], }); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); const phraseBoxes = document.querySelectorAll('[data-phrase-box="true"]'); // Two tokens grouped → one box; plus the two free tokens from segment 2 → 3 total. @@ -1964,7 +1821,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-2', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); const phraseBoxes = document.querySelectorAll('[data-phrase-id="phrase-1"]'); expect(phraseBoxes).toHaveLength(2); @@ -1987,7 +1844,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); // Hover the phrase group to set hoveredPhraseId='phrase-1'. const phraseGroupSpan = document.querySelector('[data-phrase-box="true"]')?.parentElement; @@ -2003,13 +1860,11 @@ describe('ContinuousView phrase grouping', () => { expect(screen.getByTestId('arc-split-btn')).toHaveAttribute('data-hovered-phrase-id', ''); }); - it('applies the internal focus transition when the parent reflects a click-driven ref change', async () => { - // A click Next stamps the ref internally-originated; when the parent echoes it back, the - // isInternal branch applies it immediately (no fade-out). The external branch would defer the - // display update behind a fade timeout, leaving 'In' (tok-0) focused right after the rerender. + it('applies a click-driven focus move immediately, with no fade', async () => { + // A move the strip made itself takes the internal branch, which applies it at once. The other + // branch would defer the display update behind a fade timeout, leaving 'In' (tok-0) focused. const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); + renderStrip(book, { focus: 'tok-0' }); // Sanity: tok-0's box ('In') is focused before the click. expect(screen.getByText('In').closest('[data-phrase-box="true"]')).toHaveAttribute( @@ -2020,8 +1875,6 @@ describe('ContinuousView phrase grouping', () => { await userEvent.click( screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ); - // Reflect the new ref back as a prop change; the click stamped it internal, so it applies at once. - rerender(); // The displayed focus moved synchronously to tok-1's box ('the') — the internal path. expect(screen.getByText('the').closest('[data-phrase-box="true"]')).toHaveAttribute( @@ -2047,31 +1900,13 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-2', phraseLink); phraseLinkMap.set('tok-3', phraseLink); const book = makeBook(); - const onFocusedTokenRefChange = jest.fn(); - const { rerender } = render( - , - withAnalysisStore, - ); + const strip = renderStrip(book, { focus: 'tok-0' }); // Switch to edit mode for phrase-1. - rerender( - , - ); - // The effect should call onFocusedTokenRefChange with the first token of the phrase. - expect(onFocusedTokenRefChange).toHaveBeenCalledWith('tok-2'); + strip.update({ + phraseMode: { kind: 'edit', phraseId: 'phrase-1', originalTokens: phraseLink.tokens }, + }); + expect(strip.focusToken).toHaveBeenCalledWith('tok-2', 'strip'); }); it('fires phrase group hover enter and leave without throwing', async () => { @@ -2087,7 +1922,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); // The PhraseGroup wrapper span contains the phrase box. const phraseBox = document.querySelector('[data-phrase-box="true"]'); @@ -2119,7 +1954,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); phraseLinkMap.set('tok-1', phraseLink); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); await userEvent.click(screen.getByTestId('arc-split-btn')); expect(deletePhrase).toHaveBeenCalledWith('phrase-1'); }); @@ -2133,7 +1968,7 @@ describe('ContinuousView phrase grouping', () => { mergePhrases: jest.fn(), }); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); await userEvent.click(screen.getByTestId('arc-split-btn')); expect(deletePhrase).not.toHaveBeenCalled(); }); @@ -2148,7 +1983,7 @@ describe('ContinuousView phrase grouping', () => { phraseLinkMap.set('tok-0', phraseLink); mockCandidateTokenRefs.current = new Set(['tok-0']); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); // The hovered candidate ref (tok-0) resolves to its phrase, forwarded to ArcOverlay. expect(screen.getByTestId('arc-split-btn')).toHaveAttribute( 'data-candidate-phrase-ids', @@ -2167,7 +2002,7 @@ describe('ContinuousView phrase grouping', () => { // No hovered candidate refs: the phrase exists, but nothing should resolve to it. mockCandidateTokenRefs.current = new Set(); const book = makeBook(); - render(, withAnalysisStore); + renderStrip(book); expect(screen.getByTestId('arc-split-btn')).toHaveAttribute('data-candidate-phrase-ids', ''); }); }); From 9727f7685348012498653aaec8846c039b3eb92e Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 24 Aug 2026 09:15:03 -0400 Subject: [PATCH 4/9] Trim the comments this branch added Same rules, applied to the prose this branch introduced: - Cut what only justified the change rather than explaining the code: that the precedence survives a hook reorder, that the store provider is split for testing, that hoisting the focus removes a re-render cost. - Cut what a neighbour documents better. The nav destructure restated `InterlinearNavContext`; `FocusOrigin`'s bullets restated each view's own fade/glide rules; `SegmentListView` explained gating that `useSegmentWindow` owns; a strip test restated `focusToken`'s cross-book guard. - Cut what the header already said. `FocusOrigin`'s lede spelled out the `list` bullet before the bullet did. - Split the run-on sentences: `write`'s dropped-write rule and the origin invariant it yields; the focus-request claim's four facts, two of which belong at the drop rather than at the claim. - Dropped `selectSegment`'s `@param` pair, one of which restated the signature and the other of which the summary now carries. - Stopped restating a constant's value ("half a second" for the fade) and narrating what the code used to take ("a prop the strip no longer takes"). Co-Authored-By: Claude Opus 5 (1M context) --- .../components/ContinuousView.test.tsx | 27 +--- src/__tests__/components/FocusStore.test.tsx | 19 ++- .../components/Interlinearizer.test.tsx | 8 +- src/components/ContinuousView.tsx | 31 ++--- src/components/FocusStore.tsx | 130 ++++++++---------- src/components/SegmentListView.tsx | 2 - src/hooks/useSegmentWindow.ts | 3 +- 7 files changed, 88 insertions(+), 132 deletions(-) diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index cafcfb03..87cd83e0 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -282,7 +282,6 @@ function buildLookups(book: Book): { return { tokenSegmentMap, tokenDocOrder, wordTokenByRef }; } -/** The strip props a test does not care about, with the lookup maps derived from `book`. */ type StripProps = ComponentProps; /** Minimal strip props, so a test states only what it actually varies. */ @@ -302,28 +301,18 @@ function requiredProps(book: Book): StripProps { /** What {@link renderStrip} hands back for driving and observing the mounted strip. */ type Strip = { - /** - * Every focus the strip wrote, as `(tokenRef, origin)` — the strip's side of the store, which - * would otherwise be invisible. - */ + /** Every focus the strip wrote, as `(tokenRef, origin)`. */ focusToken: jest.Mock; - /** - * Applies a focus from outside the strip, the way the resolution rules, the segment list or an - * outside request would. The origin is stated per call, since that is what the strip classifies - * on. - */ + /** Applies a focus from outside the strip, under the origin it should carry. */ setFocus: (tokenRef: string | undefined, origin: FocusOrigin) => void; /** Re-renders with `next` merged over the strip's props; call with nothing for a plain re-render. */ update: (next?: Partial) => void; - /** The rendered strip.container, for the tests that query the DOM directly. */ container: HTMLElement; }; /** - * Mounts the strip over a real focus store seeded with `focus`. Focus arrives and leaves exactly as - * it does in the app — through the store and the actions it publishes — with no view above the - * strip relaying it, so a test states the origin of every focus it drives and reads the strip's own - * moves off {@link Strip.focusToken}. + * Mounts the strip over a real focus store seeded with `focus`, so focus arrives and leaves through + * the store exactly as it does in the app. */ function renderStrip( book: Book, @@ -517,9 +506,8 @@ describe('ContinuousView initial render', () => { it('falls back to the live focus while the displayed ref names a token this book lacks', () => { // Through a book change the displayed ref lags by one fade, briefly naming a token absent from - // the mounted book. The window must follow the live focus rather than collapsing to phrase 0. - // Seeded with a foreign ref so the mounted book cannot resolve the displayed value, which is the - // state that fade leaves behind. + // the mounted book. Seeded with such a ref, so the window has to follow the live focus rather + // than collapse to phrase 0. const otherBook: Book = { id: 'MAT', bookRef: 'MAT', @@ -776,8 +764,7 @@ describe('ContinuousView arrow navigation', () => { it('steps from a focus it did not choose rather than from its own last target', async () => { // A step counts from where the strip is heading, so rapid presses accumulate. A focus the strip - // did not choose has to reset that count: otherwise the press after one lands a group away from - // what the reader is looking at. + // did not choose has to reset that count, or the press after one lands a group off. jest.useFakeTimers(); try { const book = makeBook(); diff --git a/src/__tests__/components/FocusStore.test.tsx b/src/__tests__/components/FocusStore.test.tsx index f5282e11..3e0f5785 100644 --- a/src/__tests__/components/FocusStore.test.tsx +++ b/src/__tests__/components/FocusStore.test.tsx @@ -55,10 +55,9 @@ function buildLookups(book: Book) { } /** - * Mounts a {@link FocusProvider} over a scroll-group stub whose reference the test controls, and - * exposes the focus and navigation surfaces plus a `setBook` / `setScrRef` pair for restaging the - * inputs the resolution rules classify on. A fresh reference object is required on each change so - * the nav provider adopts it. + * Mounts a {@link FocusProvider} over a scroll-group stub, exposing the focus and navigation + * surfaces plus a `setBook` / `setScrRef` pair for restaging the inputs the resolution rules + * classify on. */ function renderFocus(initialBook: Book, initialScrRef: SerializedVerseRef) { let book = initialBook; @@ -283,8 +282,8 @@ describe('FocusProvider seeding from a child', () => { } it('leaves a focus a child named on mount standing', () => { - // Child effects run before the provider's, so the resolution rules see the child's write. They - // must leave it alone: nothing about the book or the verse has changed since the mount. + // Child effects run before the provider's, so the resolution rules see the child's write and + // must leave it alone — neither the book nor the verse has moved since the mount. const noWordToken: Book = { id: 'GEN', bookRef: 'GEN', @@ -326,8 +325,8 @@ describe('FocusProvider focusToken', () => { }); it('does not echo a verse from a book the reference has already left', () => { - // Mid cross-book navigation the reference names the new book while the mounted book — and so - // this token — still belong to the previous one; echoing that verse would overwrite the target. + // The state a cross-book navigation passes through, where echoing this token's verse would + // overwrite the reference that named the new book. const harness = renderFocus(makeBook(), { book: 'MAT', chapterNum: 1, verseNum: 1 }); act(() => harness.read().actions.focusToken('GEN 1:2:0', 'strip')); @@ -449,8 +448,8 @@ describe('FocusProvider resolution rules', () => { }); it('claims a focus request over the verse reseed landing in the same commit', () => { - // A request moves focus and nothing else, so its caller navigates too; both land in one commit, - // which is exactly where the two rules would otherwise race. + // A request moves focus and nothing else, so its caller navigates too. Both land in one commit, + // which is where the two rules would otherwise race. const harness = renderFocus(makeBook(), GEN_1_1); act(() => { diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index fee1a06f..ce79f080 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -98,10 +98,7 @@ const mockDeletePhrase = jest.fn(); */ const mockPhraseLinkById = new Map(); -/** - * How many times the phrase-link-by-id map has been read. `Interlinearizer` reads it once per - * render, so this doubles as a render counter for the component under test. - */ +/** Read once per `Interlinearizer` render, so this doubles as a render counter. */ let phraseLinkByIdMapReads = 0; jest.mock('../../components/AnalysisStore', () => ({ @@ -1728,8 +1725,7 @@ describe('focus preservation across segmentation edits', () => { }); it('leaves Interlinearizer unrendered by a focus move inside the active verse', () => { - // The focus store exists so a move at arrow-step rate re-renders only the views that read it; - // an owner that re-rendered too would put the cost back where hoisting removed it. + // A move at arrow-step rate must re-render only the views that read focus. const scrRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 1 }; render(interlinearizerEl(GEN_TWO_TOKEN_V1_BOOK, scrRef)); const rendersBefore = phraseLinkByIdMapReads; diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index b3b30f37..b1c7f73e 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -136,8 +136,8 @@ export default function ContinuousView({ wordTokenByRef, viewOptions, }: ContinuousViewProps) { - // Focus drives every scroll, highlight and slot decision here, and its origin decides whether a - // change glides or fades. See FocusOrigin for what each origin asks of this view. + // Focus drives every scroll, highlight and slot decision here; its origin decides whether a + // change glides or fades. See FocusOrigin. const { tokenRef: focusedTokenRef, origin: focusOrigin } = useFocus(); const getFocus = useFocusGetter(); const { focusToken } = useFocusActions(); @@ -231,15 +231,15 @@ export default function ContinuousView({ const isInitialLoadInProgressRef = useRef(true); /** - * Whether the displayed-focus update just applied (or still pending behind the fade) came from a - * move this strip made. Carried from the focus-change effect to the scroll effect, which the fade - * timer can separate by half a second. + * Whether the displayed-focus update just applied — or still pending behind the fade — came from + * a move this strip made. Carried from the focus-change effect to the scroll effect, which the + * fade timer can separate. */ const lastDisplayUpdateWasInternalRef = useRef(false); /** * Ref mirror of the rendered focus index, read only as the fallback for a step whose live focus - * this book cannot place, so a step keeps one identity across focus moves. + * this book cannot place. A ref, so a step keeps one identity across focus moves. */ const focusPhraseIndexRef = useLatestRef(focusPhraseIndex); @@ -577,9 +577,8 @@ export default function ContinuousView({ * * Counts from the focus as of the press, taken from the store rather than from the rendered * index. That is what makes a second press before the re-render accumulate instead of repeating - * the first, and what keeps a step correct after anything else has moved the focused token's - * group — a focus set elsewhere, or a phrase-link edit that regrouped the strip without moving - * focus at all. + * the first, and what keeps a step right when a phrase-link edit has regrouped the strip without + * moving focus. * * @param delta - Number of phrases to move (positive = forward, negative = backward). */ @@ -634,10 +633,9 @@ export default function ContinuousView({ /** Splits a phrase arc at a token boundary and dispatches the resulting phrase-store writes. */ const handleArcSplit = useArcSplitHandler(tokenDocOrder); - // React to focus moves. For a move this strip made (arrow, phrase click, mode entry), apply the - // change immediately and smooth-scroll. For every other origin — segment-list click, Paratext - // verse selector, mode switch — fade the strip out, wait for the fade to complete, then snap the - // displayed focus into place so the scroll happens behind the curtain. + // React to focus moves. For a move this strip made, apply the change immediately and + // smooth-scroll. For every other origin, fade the strip out, wait for the fade to complete, then + // snap the displayed focus into place so the scroll happens behind the curtain. useEffect(() => { if (focusedTokenRef === displayFocusedTokenRef) return undefined; const isInternal = focusOrigin === 'strip'; @@ -653,9 +651,8 @@ export default function ContinuousView({ }, RECENTER_FADE_MS); return () => clearTimeout(timeout); // focusOrigin classifies the move that changed focusedTokenRef, so it is never itself a reason - // to re-run: listing it would re-fade for an origin change that moved no focus. Reading it - // without listing it is safe only because a write naming the standing focus is dropped, which - // is what keeps origin from moving while the token ref holds still — see FocusStore.write. + // to re-run. Reading it unlisted is safe because the origin cannot move while the token ref + // holds still — see FocusStore.write. // eslint-disable-next-line react-hooks/exhaustive-deps }, [focusedTokenRef, displayFocusedTokenRef]); @@ -665,7 +662,7 @@ export default function ContinuousView({ // // "Internal" here means the move carried this strip's own origin. The segment window asks a // different question — whether any view in the tree originated the nav — so the two classify the - // same event differently by design; FocusOrigin is where the two readings are set side by side. + // same event differently by design. See FocusOrigin. useEffect(() => { // Drop any hold still running: it pins the group the focus just left. Such a hold is armed by // something the focus does not control (a window resize, an active-segment flip) and keeps diff --git a/src/components/FocusStore.tsx b/src/components/FocusStore.tsx index 025df11f..e614db72 100644 --- a/src/components/FocusStore.tsx +++ b/src/components/FocusStore.tsx @@ -10,20 +10,17 @@ import { useInterlinearNav, verseKey } from './InterlinearNavContext'; /** * Where a focus change came from, recorded _at the call site_ rather than reconstructed by the - * consumers reacting to it. Every consumer maps origin to behavior itself, and the mappings - * deliberately disagree — a click in the segment list is `list`, which the list treats as its own - * doing and the strip treats as a jump it still has to travel: + * consumers reacting to it. Each consumer maps origin to behavior itself, and the mappings + * deliberately disagree: a move is already on screen for the view that made it and a jump for the + * other. * - * - `seed` — no user act: the initial resolution of the active verse, or the strip naming its first - * token when nothing resolved. The view is static or invisible, so the move is instant. - * - `strip` — the continuous strip emitted it (arrow step, phrase click, phrase-mode entry). The one - * origin the strip animates through rather than fading over, since it owns the scroll and the - * target is already on screen. - * - `list` — a click or focus inside the segment list. Already on screen for the list; a jump the - * strip has to fade for. - * - `reseed` — focus following the reference rather than driving it, when an external navigation - * lands somewhere the focused token cannot stay. - * - `request` — a focus asked for from outside the views and claimed by the book that resolves it. + * - `seed` — no user act: the initial resolution of the active verse, or a view naming its own first + * token when nothing resolved. + * - `strip` — the continuous strip's arrow step, phrase click, or phrase-mode entry. + * - `list` — a click or focus inside the segment list. + * - `reseed` — focus following the reference rather than driving it, when a navigation lands + * somewhere the focused token cannot stay. + * - `request` — a focus asked for from outside the views, claimed by the book that resolves it. */ export type FocusOrigin = 'seed' | 'strip' | 'list' | 'reseed' | 'request'; @@ -50,9 +47,9 @@ export interface FocusStore { subscribe: (onFocusChange: () => void) => () => void; /** * Sets the focused token and its origin. A write naming the token already focused is dropped, so - * a reseed or a claim that resolves to the standing focus wakes nobody — and so the origin never - * moves while the token ref holds still, which is what lets a reader treat the origin as the - * classification of the change it is already reacting to. + * a reseed or a claim resolving to the standing focus wakes nobody. + * + * The origin therefore never moves while the token ref holds still. */ write: (tokenRef: string | undefined, origin: FocusOrigin) => void; } @@ -60,26 +57,20 @@ export interface FocusStore { /** The write paths that own how focus moves; identities are stable for the provider's lifetime. */ export interface FocusActions { /** - * Focuses `tokenRef` and, when it lives in a different verse than the active one, navigates - * there. The single explicit focus-move operation behind strip arrow nav and phrase clicks: it - * sets the focused token and pushes the verse change as an _internal_ navigation (so the segment - * window tracks along without a recenter fade). A verse-0 segment (a chapter superscription) - * navigates like any other verse. + * Focuses `tokenRef` and, when it lives in a different verse than the active one, navigates there + * as an _internal_ navigation, so the segment window tracks along without a recenter fade. A + * verse-0 segment (a chapter superscription) navigates like any other verse. * - * Never navigates when the focused token's book differs from the active reference's book: during - * an external book change the reference can briefly name the new book while the mounted book (and - * this token) still belong to the previous one, and echoing that stale verse would overwrite the - * new reference. + * Never navigates when the token's book differs from the active reference's book. Mid cross-book + * navigation the reference names the new book while the mounted book still holds this token, and + * echoing that stale verse would overwrite the new reference. */ focusToken: (tokenRef: string, origin: FocusOrigin) => void; /** - * 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 + * Updates the active scripture reference and, when `tokenRef` names a clicked token rather than a + * whole segment, focuses it. Skips the write to PAPI when the clicked verse is already the + * current one, avoiding a gratuitous echo round-trip. A verse-0 segment (a chapter * superscription) writes like any other verse. - * - * @param ref - The verse coordinate that was selected. - * @param tokenRef - The token that was clicked; omitted when the whole segment was selected. */ selectSegment: (ref: ScriptureRef, tokenRef?: string) => void; } @@ -137,9 +128,8 @@ type FocusStoreProviderProps = Readonly<{ }>; /** - * Publishes an already-built store and action set to the subtree. Separate from - * {@link FocusProvider} so a view can be exercised against a store driven directly, without the book - * indexes and navigation surface the real provider resolves focus from. + * Publishes an already-built store and action set to the subtree, so a view can be mounted over a + * store driven directly rather than one resolved from a book and a reference. */ export function FocusStoreProvider({ store, actions, children }: FocusStoreProviderProps) { const value = useMemo(() => ({ store, actions }), [store, actions]); @@ -151,9 +141,8 @@ type FocusProviderProps = Readonly<{ /** Tokenized book the focused token must resolve within. */ book: Book; /** - * Current scripture reference. Resolved by the loader to a verse contained in some segment of - * `book` (when the chapter has segments), so the active segment behind a reseed is normally - * found. + * Current scripture reference, already resolved by the loader to a verse some segment of `book` + * contains whenever the chapter has segments. */ scrRef: SerializedVerseRef; /** Maps every segment id to its segment; resolves the focused token's own verse range. */ @@ -181,19 +170,16 @@ export function FocusProvider({ wordTokenByRef, children, }: FocusProviderProps) { - // `navigate` writes the reference (classifying internal vs external at the call site), and - // `consumeFocusRequest` / `focusRequestCount` collect a token focus asked for from outside the - // views. const { navigate, consumeFocusRequest, focusRequestCount } = useInterlinearNav(); /** - * Finds the book segment that owns the active verse: the first segment in document order whose - * verse range contains it. Containment (rather than an exact start-verse match) matters after - * boundary edits — a verse absorbed into a multi-verse segment, or named by a later portion of a - * split verse, still resolves to the segment that holds its text. The containment test also - * matches the book, so during a cross-book navigation (where the reference names the new book - * before its data loads, leaving `book` still the previous one) this finds nothing rather than - * resolving to the wrong book's verse. + * Finds the segment that owns the active verse: the first in document order whose verse range + * contains it. Containment rather than an exact start-verse match, so a verse absorbed into a + * multi-verse segment — or named by a later portion of a split verse — still resolves to the + * segment holding its text. + * + * Finds nothing while the reference names a book the mounted `book` is not, which is the state a + * cross-book navigation passes through before the new book's data arrives. */ const findActiveSegment = () => book.segments.find((seg) => segmentContainsVerse(seg, scrRef)); @@ -237,55 +223,49 @@ export function FocusProvider({ ); /** - * The inputs the resolution below classifies on, as of its last run. Compared rather than - * consumed from the dependency list because the rules need to know _which_ input moved: a book - * that no longer holds the focused token and a verse the focused segment no longer covers reseed - * on different tests. + * The inputs the resolution below classifies on, as of its last run. Compared rather than taken + * from the dependency list, because the rules test a moved book and a moved verse differently. */ const prevInputsRef = useRef({ book, verse: verseKey(scrRef) }); - // Resolve focus against the book and the active verse, in priority order. Ordering the rules - // inside one effect is what makes the precedence explicit: an outside request outranks both - // reseeds, and reordering hooks cannot change that. Runs after commit rather than during render - // so a claim is never made in a render React may discard. + // Resolve focus against the book and the active verse, in priority order: an outside request + // outranks both reseeds. Runs after commit rather than during render, so a claim is never made in + // a render React may discard. useEffect(() => { const prev = prevInputsRef.current; const verse = verseKey(scrRef); // Refreshed up front so no early return leaves an input stale for a later comparison. prevInputsRef.current = { book, verse }; - // Attempted on every run, not only when the count moves: the count is the only signal when a - // request names the verse already on screen, and the book the only one when it named a book - // that had yet to load. Claiming clears the request, so a run that finds nothing left is a - // no-op. A ref this book cannot resolve is dropped rather than held for a later attempt: one - // that outlived the load it was made for would fire on an unrelated navigation, long after the - // click that raised it. Logged because the drop is otherwise invisible. + // Attempted on every run rather than only when the count moves, since a request can name the + // verse already on screen or a book that had yet to load. Claiming clears it, so a run that + // finds nothing left is a no-op. const requested = consumeFocusRequest(book.bookRef); if (requested !== undefined) { if (wordTokenByRef.has(requested)) { store.write(requested, 'request'); return; } + // Dropped rather than held for a later attempt: a request outliving the load it was made for + // would fire on an unrelated navigation. Logged because the drop is otherwise invisible. logger.warn(`Interlinearizer: focus request "${requested}" matched no word token`); } const { tokenRef: current } = store.getFocus(); const resolvesInBook = current !== undefined && wordTokenByRef.has(current); - // A boundary edit (merge/split) produces a fresh book too, but token refs survive - // re-segmentation, so a still-resolving focus is kept rather than snapped back to the active - // verse's first word — and left to the verse rule below, which a re-tokenization arriving - // alongside a navigation still has to answer. + // Token refs survive re-segmentation, so a boundary edit keeps a still-resolving focus rather + // than snapping back to the active verse's first word. Kept focus falls through to the verse + // rule, which a re-tokenization arriving alongside a navigation still has to answer. if (book !== prev.book && !resolvesInBook) { store.write(firstWordTokenRefOf(findActiveSegment()), 'reseed'); return; } - // Skip when the focused token's *own* segment already contains the new verse — that means the - // change came from a token click or strip nav here, and reseeding would clobber the - // deliberately-focused token. Testing the focused token's own segment (not the active segment's - // id) is what lets a click on a non-first portion of a split verse stay put instead of being - // reseeded to the verse's first portion. + // Skip when the focused token's *own* segment already contains the new verse: the change came + // from a click or a strip step here, and reseeding would clobber the deliberate focus. Testing + // that segment rather than the active segment's id is what lets a click on a non-first portion + // of a split verse stay put. if (verse !== prev.verse) { const focusedSegId = current ? tokenSegmentMap.get(current) : undefined; const focusedSeg = focusedSegId ? segmentById.get(focusedSegId) : undefined; @@ -293,9 +273,9 @@ export function FocusProvider({ /* v8 ignore next -- the active segment is always found when the book includes the verse */ store.write(firstWordTokenRefOf(findActiveSegment()), 'reseed'); } - // findActiveSegment closes over the reactive inputs already listed; the lookup maps and - // consumeFocusRequest are read only as resolvers, and listing them would re-run the rules on - // a phrase edit that moved no focus. + // findActiveSegment closes over the inputs already listed. The lookup maps and + // consumeFocusRequest are read only as resolvers; listing them would re-run the rules on a + // phrase edit that moved no focus. // eslint-disable-next-line react-hooks/exhaustive-deps }, [book, scrRef.book, scrRef.chapterNum, scrRef.verseNum, focusRequestCount]); @@ -324,8 +304,8 @@ export function useFocus(): Focus { } /** - * Returns a stable getter for the focus as of the moment it is called, for event-time reads that - * must not subscribe the caller to focus moves. + * Returns a stable getter for the focus as of the call, for event-time reads that must not + * subscribe the caller to focus moves. * * @throws {Error} When called outside a {@link FocusProvider}. */ diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index 771a017e..b03c88ec 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -191,8 +191,6 @@ export default function SegmentListView({ tokenDocOrder, wordTokenByRef, }: SegmentListViewProps) { - // The list gates the focus highlight on its own recenter clock, so it needs the live focus but not - // the origin behind it. const { tokenRef: focusedTokenRef } = useFocus(); const { selectSegment } = useFocusActions(); diff --git a/src/hooks/useSegmentWindow.ts b/src/hooks/useSegmentWindow.ts index d71a9d5a..c63cb389 100644 --- a/src/hooks/useSegmentWindow.ts +++ b/src/hooks/useSegmentWindow.ts @@ -515,8 +515,7 @@ export default function useSegmentWindow({ // // "Internal" here means some view in the tree originated the nav — a wider question than the one // the strip asks of a focus move, which is whether it emitted that move itself. The two therefore - // classify the same event differently by design; FocusOrigin is where the two readings are set - // side by side. + // classify the same event differently by design. See FocusOrigin. // // A segments-identity change carrying a `segmentationVersion` bump is NOT a navigation: it is a // boundary edit (merge/split from the mounted controls). The window slice already re-renders the From 2d700bbe8c9d97e57183bca506479aafdcf955d9 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 24 Aug 2026 09:27:59 -0400 Subject: [PATCH 5/9] Pin that a request for another book survives a verse navigation The resolution rules attempt the focus-request claim on every run, so a navigation that moves only the verse attempts one too. A request naming a book that has yet to mount has to survive that attempt, or it would be lost to whatever navigation happened to precede the load it waits for. Only the count-bump half of that was covered. Follow the request through a verse navigation in the mounted book and then through the arrival of the book it names, so the claim it was waiting to make is still there. Co-Authored-By: Claude Opus 5 (1M context) --- src/__tests__/components/FocusStore.test.tsx | 23 ++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/__tests__/components/FocusStore.test.tsx b/src/__tests__/components/FocusStore.test.tsx index 3e0f5785..f0d3203b 100644 --- a/src/__tests__/components/FocusStore.test.tsx +++ b/src/__tests__/components/FocusStore.test.tsx @@ -493,4 +493,27 @@ describe('FocusProvider resolution rules', () => { expect(harness.read().tokenRef).toBe('GEN 1:1:0'); expect(jest.mocked(logger.warn)).not.toHaveBeenCalled(); }); + + it('keeps a request for another book claimable across a navigation in this one', () => { + // Every run attempts the claim, so a navigation that moves only the verse attempts one too. A + // request naming a book that has yet to mount has to survive that attempt, or it would be lost + // to whatever navigation happened to precede the load it is waiting for. + const matBook: Book = { + id: 'MAT', + bookRef: 'MAT', + textVersion: '1', + segments: [makeSegment('MAT 1:1', 'Alpha', [makeWordToken('MAT 1:1:0', 'Alpha')])], + }; + const harness = renderFocus(makeBook(), GEN_1_1); + act(() => harness.read().nav.requestFocusToken('MAT 1:1:0')); + + harness.setScrRef(GEN_1_2); + expect(harness.read()).toMatchObject({ tokenRef: 'GEN 1:2:0', origin: 'reseed' }); + + // The book the request named arrives; the claim it has been waiting for is still there to make. + harness.setBook(matBook); + + expect(harness.read()).toMatchObject({ tokenRef: 'MAT 1:1:0', origin: 'request' }); + expect(jest.mocked(logger.warn)).not.toHaveBeenCalled(); + }); }); From 2ec9db1c026a34539fa3351287bc954110f27ae0 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 24 Aug 2026 09:46:16 -0400 Subject: [PATCH 6/9] Disable the strip arrows until it adopts a focus it has to travel to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A step counts from the focus as of the press. While the strip is fading out for a jump it has not adopted that focus yet, so it is still painting the group focus left — and the arrows sit outside the fade wrapper, so they stay on screen and enabled through the whole window. A press there would step from a position the reader cannot see. Disable both arrows for that window. A glide adopts the focus in the same commit, so rapid presses still accumulate; the edge fade overlays keep tracking content rather than steppability, since they say what lies beyond the strip and not whether it can be stepped. Gated on the displayed focus lagging the live one rather than on strip visibility: the strip is also invisible for the frame before its first reveal, and the arrows have no reason to be dead there. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/ContinuousView.test.tsx | 26 +++++++++++++++++++ src/components/ContinuousView.tsx | 13 ++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index 87cd83e0..cf459979 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -681,6 +681,32 @@ describe('ContinuousView arrow disabled states', () => { screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }), ).toBeDisabled(); }); + + it('disables both arrows until the strip adopts a focus it has to travel to', () => { + // The arrows stay on screen through the fade, so without this a press would step from the + // incoming focus while the reader is still looking at the group it left. + jest.useFakeTimers(); + try { + const strip = renderStrip(makeBook(), { focus: 'tok-1' }); + const prev = () => + screen.getByRole('button', { name: '%interlinearizer_continuousView_previousToken%' }); + const next = () => + screen.getByRole('button', { name: '%interlinearizer_continuousView_nextToken%' }); + expect(next()).not.toBeDisabled(); + + strip.setFocus('tok-3', 'list'); + expect(prev()).toBeDisabled(); + expect(next()).toBeDisabled(); + + act(() => { + jest.advanceTimersByTime(RECENTER_FADE_MS); + }); + + expect(prev()).not.toBeDisabled(); + } finally { + jest.useRealTimers(); + } + }); }); describe('ContinuousView arrow navigation', () => { diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index b1c7f73e..438b0601 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -493,6 +493,15 @@ export default function ContinuousView({ const atStart = phraseGroups.length === 0 || focusPhraseIndex === 0; const atEnd = phraseGroups.length === 0 || focusPhraseIndex >= phraseGroups.length - 1; + + /** + * Whether a step is blocked outright, whatever the focus sits beside: the strip has yet to adopt + * the live focus, so it is still painting the group that focus left. The arrows live outside the + * fade wrapper and stay on screen through that window, and a step taken in it would count from a + * position the reader cannot see. A glide adopts the focus in the same commit, so rapid presses + * still accumulate. + */ + const isStepBlocked = focusedTokenRef !== displayFocusedTokenRef; const stripOpacityClass = isVisible ? 'tw:opacity-100' : 'tw:opacity-0'; /** Phrase groups mounted on each side of the focus, sized to the strip's visible width. */ @@ -1090,7 +1099,7 @@ export default function ContinuousView({ {/* Previous navigation arrow */}