From 000510e7835e6e5263174d4f5197a616300fbf27 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 19 May 2026 09:41:17 -0600 Subject: [PATCH 01/13] Add UI that enables user to gloss tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add gloss input to TokenChip (word tokens only): bordered chip now stacks surface text above an editable gloss field - Thread `glosses` (Record) and `onGlossChange(tokenId, value)` props down through TokenChip → PhraseBox → SegmentView → ContinuousView - Introduce a phrase-window around the focused index in ContinuousView to cap rendered PhraseBoxes and reduce overhead on large books - Replace SegmentView's `onClick` with `onSelect(ref, tokenId?)` so the clicked token id propagates up to Interlinearizer - Add `activePhraseIndex` prop to ContinuousView for external phrase jumps when a segment-view token is clicked in continuous-scroll mode - Update tests to cover gloss propagation, `activePhraseIndex` jumps, and both phrase-window boundary branches --- .../components/ContinuousView.test.tsx | 342 +++++++++++++++--- .../components/Interlinearizer.test.tsx | 253 ++++++++++++- src/__tests__/components/PhraseBox.test.tsx | 190 ++++++++-- src/__tests__/components/SegmentView.test.tsx | 147 ++++++-- src/__tests__/components/TokenChip.test.tsx | 55 ++- src/components/ContinuousView.tsx | 118 +++++- src/components/Interlinearizer.tsx | 157 +++++++- src/components/PhraseBox.tsx | 22 +- src/components/SegmentView.tsx | 94 +++-- src/components/TokenChip.tsx | 38 +- 10 files changed, 1238 insertions(+), 178 deletions(-) diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index 67d16d1d..42e4bf8a 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -4,20 +4,24 @@ import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import type { Book, Token } from 'interlinearizer'; +import type { Book, ScriptureRef, Token } from 'interlinearizer'; import ContinuousView from '../../components/ContinuousView'; jest.mock('../../components/PhraseBox', () => ({ __esModule: true, default: ({ + glosses, isFocused = false, index, onClick, + onGlossChange, tokens, }: { - isFocused?: boolean; + glosses: Record; + isFocused: boolean; index?: number; onClick?: (index?: number) => void; + onGlossChange: (tokenId: string, value: string) => void; tokens: Token[]; }) => ( ), })); @@ -262,10 +275,65 @@ function makeWordFreeBook(): Book { }; } +/** + * Builds a Book with `count` word tokens spread across one segment per token, each in GEN 1:N. Used + * to exercise the phrase-window windowing code paths (PHRASE_WINDOW_HALF = 100 on each side). + * + * @param count - Total number of word-token segments to create. + * @returns A Book with `count` single-token segments. + */ +function makeLargeBook(count: number): Book { + return { + id: 'GEN', + bookRef: 'GEN', + textVersion: '1', + segments: Array.from({ length: count }, (_, i) => ({ + id: `GEN 1:${i + 1}`, + startRef: { book: 'GEN', chapter: 1, verse: i + 1 }, + endRef: { book: 'GEN', chapter: 1, verse: i + 1 }, + baselineText: `word${i}`, + tokens: [ + { + id: `large-tok-${i}`, + surfaceText: `word${i}`, + writingSystem: 'en' as const, + type: 'word' as const, + charStart: 0, + charEnd: String(`word${i}`).length, + }, + ], + })), + }; +} + // --------------------------------------------------------------------------- const scrollIntoViewMock = jest.fn(); +/** + * Minimal required props for ContinuousView. Spread into render calls so tests only need to + * override what they actually care about. + * + * @returns An object containing all required ContinuousView props set to no-op stubs. + */ +function requiredProps(): { + activeVerse: ScriptureRef; + activePhraseIndex: undefined; + glosses: Record; + onFocusPhraseIndexChange: jest.Mock; + onGlossChange: jest.Mock; + onVerseChange: jest.Mock; +} { + return { + activeVerse: { book: 'GEN', chapter: 1, verse: 1 }, + activePhraseIndex: undefined, + glosses: {}, + onFocusPhraseIndexChange: jest.fn(), + onGlossChange: jest.fn(), + onVerseChange: jest.fn(), + }; +} + beforeAll(() => { // jsdom does not implement scrollIntoView. HTMLElement.prototype.scrollIntoView = scrollIntoViewMock; @@ -281,7 +349,7 @@ beforeEach(() => { describe('ContinuousView rendering', () => { it('renders all tokens from all segments as a flat list', () => { - render(); + render(); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('the')).toBeInTheDocument(); @@ -290,14 +358,14 @@ describe('ContinuousView rendering', () => { }); it('renders tokens from both chapters in a two-chapter book', () => { - render(); + render(); expect(screen.getByText('Alpha')).toBeInTheDocument(); expect(screen.getByText('Beta')).toBeInTheDocument(); }); it('does not render any verse label or segment separator', () => { - render(); + render(); // No verse numbers or colons that would indicate verse labels expect(screen.queryByText('1:1')).not.toBeInTheDocument(); @@ -307,7 +375,7 @@ describe('ContinuousView rendering', () => { }); it('renders a Previous token button and a Next token button', () => { - render(); + render(); expect(screen.getByRole('button', { name: 'Previous token' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Next token' })).toBeInTheDocument(); @@ -315,7 +383,7 @@ describe('ContinuousView rendering', () => { it('renders a non-word token via TokenChip within the strip', () => { // makeMixedBook: GEN 1:1 has a word token; GEN 1:2 has a punctuation token - render(); + render(); // Both the word chip ("In") and the punctuation chip (".") must appear expect(screen.getByText('In')).toBeInTheDocument(); @@ -323,14 +391,14 @@ describe('ContinuousView rendering', () => { }); it('renders without crashing when book has no word tokens (empty phraseEntries)', () => { - render(); + render(); // The punctuation token is rendered expect(screen.getByText('.')).toBeInTheDocument(); }); it('clicking an out-of-focus phrase box brings it into focus', async () => { - render(); + render(); const clickedPhraseBox = screen.getByText('beginning').closest('[data-phrase-box="true"]'); if (!clickedPhraseBox) throw new Error('Expected phrase box wrapper for token'); @@ -344,7 +412,7 @@ describe('ContinuousView rendering', () => { }); it('clicking the already-focused phrase box leaves it focused', async () => { - render(); + render(); // The first token is focused by default. const firstPhraseBox = screen.getByText('In').closest('[data-phrase-box="true"]'); @@ -366,26 +434,26 @@ describe('ContinuousView rendering', () => { describe('ContinuousView arrow disabled states', () => { it('disables the prev arrow on initial render (book start)', () => { - render(); + render(); expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); }); it('enables the next arrow on initial render when there are multiple tokens', () => { - render(); + render(); expect(screen.getByRole('button', { name: 'Next token' })).toBeEnabled(); }); it('disables both arrows when the book has exactly one token', () => { - render(); + render(); expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); expect(screen.getByRole('button', { name: 'Next token' })).toBeDisabled(); }); it('enables the prev arrow after clicking next once', async () => { - render(); + render(); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -393,7 +461,7 @@ describe('ContinuousView arrow disabled states', () => { }); it('disables the next arrow when advanced to the last token', async () => { - render(); + render(); const nextBtn = screen.getByRole('button', { name: 'Next token' }); // 4 tokens total: advance 3 times to reach index 3 (last) @@ -405,7 +473,7 @@ describe('ContinuousView arrow disabled states', () => { }); it('re-enables the next arrow after going prev from the last token', async () => { - render(); + render(); const nextBtn = screen.getByRole('button', { name: 'Next token' }); await userEvent.click(nextBtn); @@ -426,7 +494,7 @@ describe('ContinuousView arrow disabled states', () => { describe('ContinuousView fade overlays', () => { it('does not render prev fade at book start', () => { - const { container } = render(); + const { container } = render(); const gradients = container.querySelectorAll('[aria-hidden="true"]'); const prevFades = Array.from(gradients).filter((el) => @@ -436,7 +504,7 @@ describe('ContinuousView fade overlays', () => { }); it('renders next fade at book start (next side is enabled)', () => { - const { container } = render(); + const { container } = render(); const gradients = container.querySelectorAll('[aria-hidden="true"]'); const nextFades = Array.from(gradients).filter((el) => @@ -446,7 +514,7 @@ describe('ContinuousView fade overlays', () => { }); it('renders prev fade after moving away from book start', async () => { - const { container } = render(); + const { container } = render(); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -458,7 +526,7 @@ describe('ContinuousView fade overlays', () => { }); it('does not render next fade at book end', async () => { - const { container } = render(); + const { container } = render(); const nextBtn = screen.getByRole('button', { name: 'Next token' }); await userEvent.click(nextBtn); @@ -479,7 +547,7 @@ describe('ContinuousView fade overlays', () => { describe('ContinuousView cross-chapter traversal', () => { it('indexes tokens across chapter boundaries in segment order', () => { - render(); + render(); // Both chapter tokens should be present expect(screen.getByText('Alpha')).toBeInTheDocument(); @@ -487,7 +555,7 @@ describe('ContinuousView cross-chapter traversal', () => { }); it('can navigate across a chapter boundary with the next arrow', async () => { - render(); + render(); // Only one token per chapter, so clicking next once reaches chapter 2's token (index 1 = last) await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -505,7 +573,7 @@ describe('ContinuousView cross-chapter traversal', () => { describe('ContinuousView smooth-scroll intent', () => { it('calls scrollIntoView with smooth behaviour when next arrow is clicked', async () => { - render(); + render(); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -515,7 +583,7 @@ describe('ContinuousView smooth-scroll intent', () => { }); it('calls scrollIntoView with smooth behaviour when prev arrow is clicked', async () => { - render(); + render(); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); scrollIntoViewMock.mockClear(); @@ -528,7 +596,7 @@ describe('ContinuousView smooth-scroll intent', () => { }); it('does not call scrollIntoView when a disabled arrow is clicked', async () => { - render(); + render(); scrollIntoViewMock.mockClear(); // Prev arrow is disabled at start — clicking it should be a no-op @@ -553,7 +621,11 @@ describe('ContinuousView activeVerse verse-jump', () => { it('positions at focusIndex 0 when activeVerse matches the first segment', () => { render( - , + , ); // At index 0 the prev arrow should be disabled @@ -563,11 +635,19 @@ describe('ContinuousView activeVerse verse-jump', () => { it('jumps to the first token of the second segment when activeVerse points there', () => { // makeBook() has 4 tokens: index 0,1 in segment GEN 1:1 and index 2,3 in GEN 1:2 const { rerender } = render( - , + , ); rerender( - , + , ); // Advance past the fade-out delay so the pending focus jump fires. act(() => { @@ -582,6 +662,7 @@ describe('ContinuousView activeVerse verse-jump', () => { const { rerender } = render( , ); @@ -589,6 +670,7 @@ describe('ContinuousView activeVerse verse-jump', () => { rerender( , ); @@ -605,12 +687,20 @@ describe('ContinuousView activeVerse verse-jump', () => { // External jumps use behavior:'auto' (not 'smooth') to avoid double-animation with the // strip opacity fade that already plays during the jump. const { rerender } = render( - , + , ); scrollIntoViewMock.mockClear(); rerender( - , + , ); act(() => { jest.advanceTimersByTime(500); @@ -621,13 +711,18 @@ describe('ContinuousView activeVerse verse-jump', () => { it('does not call onVerseChange when activeVerse changes', () => { const { rerender } = render( - , + , ); const handleVerseChange = jest.fn(); rerender( , @@ -643,7 +738,11 @@ describe('ContinuousView activeVerse verse-jump', () => { // makeBook(): GEN 1:1 at index 0-1, GEN 1:2 at index 2-3. Mounting with verse 2 should // start the strip focused at index 2 immediately (lazy useState initializer, no effect wait). render( - , + , ); // Index 2 is not the start (prev enabled) and not the end (next enabled). @@ -652,7 +751,7 @@ describe('ContinuousView activeVerse verse-jump', () => { }); it('does not jump when activeVerse is undefined', () => { - render(); + render(); // Without activeVerse the strip stays at focusIndex 0 expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); @@ -660,7 +759,11 @@ describe('ContinuousView activeVerse verse-jump', () => { it('does not jump when activeVerse does not match any segment', () => { render( - , + , ); // No matching segment — strip stays at focusIndex 0 @@ -671,11 +774,19 @@ describe('ContinuousView activeVerse verse-jump', () => { // Start focused at GEN 1:1 (word token), then move activeVerse to GEN 1:2 (punctuation only). // getPhraseIndexForVerse should return undefined → no pending jump. const { rerender } = render( - , + , ); rerender( - , + , ); act(() => { jest.advanceTimersByTime(500); @@ -686,6 +797,101 @@ describe('ContinuousView activeVerse verse-jump', () => { }); }); +// --------------------------------------------------------------------------- +// activePhraseIndex direct jump +// --------------------------------------------------------------------------- + +describe('ContinuousView activePhraseIndex', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { + jest.useRealTimers(); + }); + + it('jumps to the specified phrase index after the fade delay when activePhraseIndex changes', () => { + const book = makeBook(); + const { rerender } = render( + , + ); + + const phraseBtns = () => + screen.getAllByRole('button').filter((b) => b.dataset.phraseBox === 'true'); + + expect(phraseBtns()[0]).toHaveAttribute('data-focus-state', 'focused'); + + rerender(); + act(() => { + jest.advanceTimersByTime(500); + }); + + expect(phraseBtns()[0]).toHaveAttribute('data-focus-state', 'default'); + expect(phraseBtns()[1]).toHaveAttribute('data-focus-state', 'focused'); + }); + + it('uses instant scrollIntoView behaviour after the fade completes', () => { + const book = makeBook(); + render(); + act(() => { + jest.advanceTimersByTime(500); + }); + + expect(globalThis.HTMLElement.prototype.scrollIntoView).toHaveBeenCalledWith( + expect.objectContaining({ behavior: 'auto' }), + ); + }); + + it('remains hidden while a second click overrides a pending jump mid-fade', () => { + // Regression: when a second activePhraseIndex arrives before the first fade-out timer fires, + // the RAF cleanup from the first jump must not reveal the strip prematurely. + const book = makeBook(); + const { rerender } = render( + , + ); + act(() => { + jest.advanceTimersByTime(500); + }); + + // First click — strip fades out; timer is pending. + rerender(); + + // Second click before the first 500 ms timer fires — overrides the pending jump. + rerender(); + + // Strip should still be hidden while the second fade-out timer is pending. + expect(screen.getByTestId('token-strip')).toHaveClass('tw:opacity-0'); + }); + + it('jumps to the correct phrase when a second click arrives before the first jump resolves', () => { + // Regression: second click before the first fade timer fires must end at the second target, not + // wherever the first jump would have landed. + const book = makeBook(); + const { rerender } = render( + , + ); + act(() => { + jest.advanceTimersByTime(500); + }); + + const phraseBtns = () => + screen.getAllByRole('button').filter((b) => b.dataset.phraseBox === 'true'); + + // First click to index 1. + rerender(); + + // Second click back to index 0 before the first timer fires. + rerender(); + + // Let the second fade-out timer fire. + act(() => { + jest.advanceTimersByTime(500); + }); + + expect(phraseBtns()[0]).toHaveAttribute('data-focus-state', 'focused'); + expect(phraseBtns()[1]).toHaveAttribute('data-focus-state', 'default'); + }); +}); + // --------------------------------------------------------------------------- // onVerseChange outbound propagation // --------------------------------------------------------------------------- @@ -694,7 +900,9 @@ describe('ContinuousView onVerseChange propagation', () => { it('calls onVerseChange when the next arrow crosses into a new verse', async () => { // makeBook(): segment GEN 1:1 has tokens at index 0,1; GEN 1:2 starts at index 2 const handleVerseChange = jest.fn(); - render(); + render( + , + ); // Advance twice to reach index 2 (first token of GEN 1:2) await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -708,6 +916,7 @@ describe('ContinuousView onVerseChange propagation', () => { render( , @@ -722,7 +931,9 @@ describe('ContinuousView onVerseChange propagation', () => { it('does not call onVerseChange for multiple arrow clicks within the same verse', async () => { const handleVerseChange = jest.fn(); - render(); + render( + , + ); handleVerseChange.mockClear(); // index 0 → index 1: both are in GEN 1:1, no verse change @@ -733,7 +944,13 @@ describe('ContinuousView onVerseChange propagation', () => { it('calls onVerseChange with the chapter-2 verse when crossing the chapter boundary', async () => { const handleVerseChange = jest.fn(); - render(); + render( + , + ); handleVerseChange.mockClear(); // ch1 has 1 token (index 0), ch2 starts at index 1 — one click crosses the boundary @@ -745,7 +962,7 @@ describe('ContinuousView onVerseChange propagation', () => { it('does not call onVerseChange when book changes and focus resets to the first phrase', async () => { const handleVerseChange = jest.fn(); const { rerender } = render( - , + , ); // Move focus away from index 0 so book-switch reset path is exercised. @@ -763,7 +980,9 @@ describe('ContinuousView onVerseChange propagation', () => { })), }; - rerender(); + rerender( + , + ); expect(handleVerseChange).not.toHaveBeenCalled(); }); @@ -783,16 +1002,51 @@ describe('ContinuousView RTL layout', () => { }); it('shows right-arrow (→) on the previous button in RTL mode', () => { - render(); + render(); const prevBtn = screen.getByRole('button', { name: 'Previous token' }); expect(prevBtn.querySelector('[aria-hidden="true"]')).toHaveTextContent('\u2192'); }); it('shows left-arrow (←) on the next button in RTL mode', () => { - render(); + render(); const nextBtn = screen.getByRole('button', { name: 'Next token' }); expect(nextBtn.querySelector('[aria-hidden="true"]')).toHaveTextContent('\u2190'); }); }); + +// --------------------------------------------------------------------------- +// Phrase window — windowing branches (PHRASE_WINDOW_HALF = 100 on each side) +// --------------------------------------------------------------------------- + +describe('ContinuousView phrase window', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { + jest.useRealTimers(); + }); + + it('activates both windowing branches when the focused phrase is deep inside a large book', () => { + // A book with 250 tokens: focusing phrase 125 means windowStart = 25 (> 0) and + // windowEnd = 225 (< 249), exercising both the windowStartTokenIndex and + // windowEndTokenIndex non-default branches. + const book = makeLargeBook(250); + render( + , + ); + act(() => { + jest.advanceTimersByTime(500); + }); + + // Phrase 125 is not at the start or end, so both arrows are enabled. + expect(screen.getByRole('button', { name: 'Previous token' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Next token' })).toBeEnabled(); + }); +}); diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 74a53c34..e6e5f591 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -3,7 +3,7 @@ /// import type { SerializedVerseRef } from '@sillsdev/scripture'; -import { render, screen } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; import type { Book, Segment } from 'interlinearizer'; import Interlinearizer from '../../components/Interlinearizer'; @@ -13,8 +13,9 @@ let capturedContinuousViewProps: Record = {}; type CapturedSegmentViewProps = { segment: Segment; displayMode?: string; + focusedTokenId?: string; isActive?: boolean; - onClick?: (ref: { book: string; chapter: number; verse: number }) => void; + onSelect?: (ref: { book: string; chapter: number; verse: number }, tokenId?: string) => void; }; let capturedSegmentViewPropsList: CapturedSegmentViewProps[] = []; @@ -22,17 +23,30 @@ jest.mock('../../components/ContinuousView', () => ({ __esModule: true, default: (props: Record) => { capturedContinuousViewProps = props; - return
; + return ( +
+ ); }, })); jest.mock('../../components/SegmentView', () => ({ __esModule: true, - SegmentView: ({ segment, ...rest }: CapturedSegmentViewProps) => { + SegmentView: ({ + segment, + ...rest + }: CapturedSegmentViewProps & { glosses?: Record }) => { capturedSegmentViewPropsList.push({ segment, ...rest }); return
; }, - default: ({ segment, ...rest }: CapturedSegmentViewProps) => { + default: ({ + segment, + ...rest + }: CapturedSegmentViewProps & { glosses?: Record }) => { capturedSegmentViewPropsList.push({ segment, ...rest }); return
; }, @@ -185,7 +199,7 @@ describe('Interlinearizer', () => { const mockSetScrRef = jest.fn(); renderInterlinearizer({ bookSegments: GEN_1_MULTI_BOOK.segments, setScrRef: mockSetScrRef }); - capturedSegmentViewPropsList[1].onClick?.({ book: 'GEN', chapter: 1, verse: 2 }); + capturedSegmentViewPropsList[1].onSelect?.({ book: 'GEN', chapter: 1, verse: 2 }); expect(mockSetScrRef).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 1, verseNum: 2 }); }); @@ -227,6 +241,76 @@ describe('Interlinearizer', () => { expect(allElements[0]).toBe(continuousView); }); + it('passes onSelect to SegmentView when continuousScroll is false', () => { + renderInterlinearizer({ continuousScroll: false }); + + expect(capturedSegmentViewPropsList[0].onSelect).toBeInstanceOf(Function); + }); + + it('passes onSelect to SegmentView when continuousScroll is true', () => { + renderInterlinearizer({ continuousScroll: true }); + + expect(capturedSegmentViewPropsList[0].onSelect).toBeInstanceOf(Function); + }); + + it('calls setScrRef with the segment ref when a token is clicked', () => { + const mockSetScrRef = jest.fn(); + renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + bookSegments: GEN_1_MULTI_BOOK.segments, + setScrRef: mockSetScrRef, + }); + + capturedSegmentViewPropsList[1].onSelect?.({ book: 'GEN', chapter: 1, verse: 2 }, 'GEN 1:2:0'); + + expect(mockSetScrRef).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 1, verseNum: 2 }); + }); + + it('passes activePhraseIndex to ContinuousView matching the clicked token', () => { + // Render in token-chip mode first so onSelect is available on SegmentView props. + const { rerender } = renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + bookSegments: GEN_1_MULTI_BOOK.segments, + continuousScroll: false, + }); + + // GEN 1:2 word token is phrase index 1 (after GEN 1:1's one word token at index 0). + const { onSelect } = capturedSegmentViewPropsList[1]; + if (typeof onSelect !== 'function') throw new Error('Expected onSelect to be a function'); + + act(() => { + onSelect({ book: 'GEN', chapter: 1, verse: 2 }, 'GEN 1:2:0'); + }); + + // Switch to continuous-scroll mode so ContinuousView is rendered and its props captured. + capturedSegmentViewPropsList = []; + rerender( + {}} + />, + ); + + expect(capturedContinuousViewProps.activePhraseIndex).toBe(1); + }); + + it('passes onGlossChange to ContinuousView and updates glosses state', () => { + renderInterlinearizer({ continuousScroll: true }); + + const { onGlossChange } = capturedContinuousViewProps; + if (typeof onGlossChange !== 'function') + throw new Error('Expected onGlossChange to be a function'); + + act(() => { + onGlossChange('token-1', 'hello'); + }); + + expect(capturedContinuousViewProps.glosses).toEqual({ 'token-1': 'hello' }); + }); + it('calls setScrRef when ContinuousView emits onVerseChange', () => { const mockSetScrRef = jest.fn(); renderInterlinearizer({ continuousScroll: true, setScrRef: mockSetScrRef }); @@ -241,4 +325,161 @@ describe('Interlinearizer', () => { expect(mockSetScrRef).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 2, verseNum: 3 }); }); + + it('updates continuousViewPhraseIndex when ContinuousView emits onFocusPhraseIndexChange', () => { + const { rerender } = renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + bookSegments: GEN_1_MULTI_BOOK.segments, + continuousScroll: true, + }); + + const { onFocusPhraseIndexChange } = capturedContinuousViewProps; + if (typeof onFocusPhraseIndexChange !== 'function') + throw new Error('Expected onFocusPhraseIndexChange to be a function'); + + act(() => { + onFocusPhraseIndexChange(1); + }); + + // Re-render in continuous mode — just verifying the callback does not throw and updates state. + capturedSegmentViewPropsList = []; + rerender( + {}} + />, + ); + + expect(capturedContinuousViewProps.activePhraseIndex).toBeUndefined(); + }); + + it('carries the strip phrase position into segment view when switching off continuousScroll', () => { + const { rerender } = renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + bookSegments: GEN_1_MULTI_BOOK.segments, + continuousScroll: true, + }); + + // Simulate ContinuousView reporting that phrase index 1 (GEN 1:2's token) is in view. + const { onFocusPhraseIndexChange } = capturedContinuousViewProps; + if (typeof onFocusPhraseIndexChange !== 'function') + throw new Error('Expected onFocusPhraseIndexChange to be a function'); + + act(() => { + onFocusPhraseIndexChange(1); + }); + + // Switch to segment view — Interlinearizer should carry over phrase index 1 as the focus. + capturedSegmentViewPropsList = []; + rerender( + {}} + />, + ); + + // The token at phrase index 1 is 'GEN 1:2:0'; it should now be the focusedTokenId. + const focused = capturedSegmentViewPropsList.find((p) => p.focusedTokenId === 'GEN 1:2:0'); + expect(focused).toBeDefined(); + }); + + it('falls back to the active-verse first word when switching off continuousScroll with no strip position', () => { + // Start in continuous mode without ContinuousView ever calling onFocusPhraseIndexChange. + const { rerender } = renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + bookSegments: GEN_1_MULTI_BOOK.segments, + continuousScroll: true, + scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 }, + }); + + // Switch to segment view without any strip position having been reported. + capturedSegmentViewPropsList = []; + rerender( + {}} + />, + ); + + // The fallback focuses the first word of GEN 1:1 ('GEN 1:1:0'). + const focused = capturedSegmentViewPropsList.find((p) => p.focusedTokenId === 'GEN 1:1:0'); + expect(focused).toBeDefined(); + }); + + it('preserves an existing focusedTokenId when switching off continuousScroll with no strip position', () => { + // Start in segment mode and focus a specific token. + const { rerender } = renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + bookSegments: GEN_1_MULTI_BOOK.segments, + continuousScroll: false, + }); + + // Click a token to set focusedTokenId to 'GEN 1:2:0'. + const { onSelect } = capturedSegmentViewPropsList[1]; + if (typeof onSelect !== 'function') throw new Error('Expected onSelect to be a function'); + act(() => { + onSelect({ book: 'GEN', chapter: 1, verse: 2 }, 'GEN 1:2:0'); + }); + + // Switch to continuous mode (without strip reporting any position). + capturedSegmentViewPropsList = []; + rerender( + {}} + />, + ); + + // Switch back to segment mode — existing focusedTokenId should be preserved. + capturedSegmentViewPropsList = []; + rerender( + {}} + />, + ); + + // 'GEN 1:2:0' was already focused, so the fallback must not overwrite it. + const stillFocused = capturedSegmentViewPropsList.find((p) => p.focusedTokenId === 'GEN 1:2:0'); + expect(stillFocused).toBeDefined(); + }); + + it('leaves focusedTokenId undefined when switching off continuousScroll with no strip position and no matching segment', () => { + // scrRef points to verse 99 which does not exist in GEN_1_MULTI_BOOK. + const { rerender } = renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + bookSegments: GEN_1_MULTI_BOOK.segments, + continuousScroll: true, + scrRef: { book: 'GEN', chapterNum: 1, verseNum: 99 }, + }); + + capturedSegmentViewPropsList = []; + rerender( + {}} + />, + ); + + // No segment matches verse 99 so focusedTokenId stays undefined for all views. + capturedSegmentViewPropsList.forEach((p) => expect(p.focusedTokenId).toBeUndefined()); + }); }); diff --git a/src/__tests__/components/PhraseBox.test.tsx b/src/__tests__/components/PhraseBox.test.tsx index 375fde55..b47b0047 100644 --- a/src/__tests__/components/PhraseBox.test.tsx +++ b/src/__tests__/components/PhraseBox.test.tsx @@ -9,8 +9,26 @@ import { PhraseBox } from '../../components/PhraseBox'; jest.mock('../../components/TokenChip', () => ({ __esModule: true, - default: ({ token }: { token: Token }) => ( - {token.surfaceText} + default: ({ + gloss, + onFocus, + onGlossChange, + token, + }: { + gloss?: string; + onFocus?: () => void; + onGlossChange?: (value: string) => void; + token: Token; + }) => ( + + {token.surfaceText} + onGlossChange?.(e.target.value)} + onFocus={onFocus} + value={gloss ?? ''} + /> + ), })); @@ -34,17 +52,48 @@ const TEST_TOKEN_2: Token = { charEnd: 11, }; -describe('PhraseBox', () => { - it('renders as a span when no onClick handler is provided', () => { - render(); +/** Shared props shape used by both helper functions. */ +type PhraseBoxTestProps = { + glosses: Record; + isFocused: boolean; + onClick?: (index?: number) => void; + onGlossChange: (tokenId: string, value: string) => void; + tokens: Token[]; +}; - const phraseBox = document.querySelector('[data-phrase-box="true"]'); - expect(phraseBox?.tagName).toBe('SPAN'); - }); +/** + * Minimal required props for PhraseBox. Spread into render calls so tests only need to override + * what they actually care about. + * + * @returns An object containing all required PhraseBox props set to no-op stubs. + */ +function requiredProps(): PhraseBoxTestProps { + return { + glosses: {}, + isFocused: false, + onClick: jest.fn(), + onGlossChange: jest.fn(), + tokens: [TEST_TOKEN], + }; +} + +/** + * Props with onClick omitted so the non-interactive span branch is rendered. + * + * @returns Props without an onClick handler. + */ +function nonInteractiveProps(): PhraseBoxTestProps { + return { + glosses: {}, + isFocused: false, + onGlossChange: jest.fn(), + tokens: [TEST_TOKEN], + }; +} - it('renders as a button when onClick handler is provided', () => { - const mockOnClick = jest.fn(); - render(); +describe('PhraseBox', () => { + it('renders as a button', () => { + render(); const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox?.tagName).toBe('BUTTON'); @@ -52,7 +101,7 @@ describe('PhraseBox', () => { }); it('renders tokens using TokenChip components', () => { - render(); + render(); expect(screen.getByTestId('token-token-1')).toBeInTheDocument(); expect(screen.getByTestId('token-token-2')).toBeInTheDocument(); @@ -60,7 +109,7 @@ describe('PhraseBox', () => { it('calls onClick when button is clicked', async () => { const mockOnClick = jest.fn(); - render(); + render(); const button = screen.getByRole('button'); await userEvent.click(button); @@ -68,67 +117,134 @@ describe('PhraseBox', () => { expect(mockOnClick).toHaveBeenCalledTimes(1); }); - it('applies focused styling when isFocused is true', () => { - render(); + it('applies focused border and background when isFocused is true', () => { + render(); const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox).toHaveAttribute('data-focus-state', 'focused'); expect(phraseBox).toHaveClass('tw:border-2'); + expect(phraseBox).toHaveClass('tw:border-white'); + expect(phraseBox).toHaveClass('tw:bg-muted/30'); }); - it('applies default styling when isFocused is false', () => { - render(); - - const phraseBox = document.querySelector('[data-phrase-box="true"]'); - expect(phraseBox).toHaveAttribute('data-focus-state', 'default'); - expect(phraseBox).not.toHaveClass('tw:border-2'); - }); - - it('applies default styling when isFocused is not provided', () => { - render(); + it('applies default border and background when isFocused is false', () => { + render(); const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox).toHaveAttribute('data-focus-state', 'default'); + expect(phraseBox).toHaveClass('tw:border'); + expect(phraseBox).toHaveClass('tw:border-border/40'); + expect(phraseBox).toHaveClass('tw:bg-muted/20'); }); it('button has correct focused styling and cursor', () => { - const mockOnClick = jest.fn(); - render(); + render(); const button = screen.getByRole('button'); expect(button).toHaveAttribute('data-focus-state', 'focused'); + expect(button).toHaveClass('tw:border-2'); expect(button).toHaveClass('tw:cursor-pointer'); expect(button).toHaveClass('tw:text-left'); }); it('button has hover styling', () => { - const mockOnClick = jest.fn(); - render(); + render(); const button = screen.getByRole('button'); expect(button).toHaveClass('tw:hover:bg-muted/30'); }); it('renders multiple tokens in order', () => { - render(); + render(); const tokens = document.querySelectorAll('[data-testid^="token-"]'); expect(tokens[0]).toHaveAttribute('data-testid', 'token-token-1'); expect(tokens[1]).toHaveAttribute('data-testid', 'token-token-2'); }); - it('applies base spacing classes to both button and span', () => { - const { rerender } = render(); + it('passes the gloss for each token from the glosses map', () => { + render( + , + ); + + expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveValue('hello'); + expect(screen.getByRole('textbox', { name: 'Gloss for World' })).toHaveValue('world'); + }); - const span = document.querySelector('[data-phrase-box="true"]'); - expect(span).toHaveClass('tw:px-1'); - expect(span).toHaveClass('tw:py-0.5'); + it('passes an undefined gloss when the token id is absent from the glosses map', () => { + render(); - const mockOnClick = jest.fn(); - rerender(); + expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveValue(''); + }); + + it('calls onGlossChange with the token id and new value when a gloss input changes', async () => { + const handleGlossChange = jest.fn(); + render( + , + ); + + await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for Hello' }), 'hi'); + + expect(handleGlossChange).toHaveBeenCalledTimes(2); + expect(handleGlossChange).toHaveBeenNthCalledWith(1, 'token-1', 'h'); + expect(handleGlossChange).toHaveBeenNthCalledWith(2, 'token-1', 'i'); + }); + + it('calls onClick with index when a gloss input receives focus', async () => { + const handleClick = jest.fn(); + render(); + + await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for Hello' })); + + expect(handleClick).toHaveBeenCalledWith(2); + }); + + it('button always has tabIndex -1 so tab focus goes only to gloss inputs', () => { + render(); + + expect(screen.getByRole('button')).toHaveAttribute('tabindex', '-1'); + }); + + it('applies base spacing classes to the button', () => { + render(); const button = screen.getByRole('button'); expect(button).toHaveClass('tw:px-1'); expect(button).toHaveClass('tw:py-0.5'); }); + + describe('non-interactive (no onClick)', () => { + it('renders as a span when onClick is not provided', () => { + render(); + + const phraseBox = document.querySelector('[data-phrase-box="true"]'); + expect(phraseBox?.tagName).toBe('SPAN'); + }); + + it('applies focused styling to the span when isFocused is true', () => { + render(); + + const phraseBox = document.querySelector('[data-phrase-box="true"]'); + expect(phraseBox).toHaveAttribute('data-focus-state', 'focused'); + expect(phraseBox).toHaveClass('tw:border-2'); + expect(phraseBox).toHaveClass('tw:border-white'); + }); + + it('applies default styling to the span when isFocused is false', () => { + render(); + + const phraseBox = document.querySelector('[data-phrase-box="true"]'); + expect(phraseBox).toHaveAttribute('data-focus-state', 'default'); + expect(phraseBox).toHaveClass('tw:border'); + expect(phraseBox).toHaveClass('tw:border-border/40'); + }); + }); }); diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index 653ed113..8f04795e 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -4,13 +4,42 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import type { Segment, Token } from 'interlinearizer'; +import type { ScriptureRef, Segment, Token } from 'interlinearizer'; import { SegmentView } from '../../components/SegmentView'; jest.mock('../../components/PhraseBox', () => ({ __esModule: true, - default: ({ tokens }: { tokens: Token[] }) => ( - {tokens.map((t) => t.surfaceText).join(' ')} + default: ({ + glosses, + isFocused = false, + onClick, + onGlossChange, + tokens, + }: { + glosses: Record; + isFocused: boolean; + onClick?: () => void; + onGlossChange: (tokenId: string, value: string) => void; + tokens: Token[]; + }) => ( + + {tokens.map((t) => ( + + {onClick ? ( + + ) : ( + {t.surfaceText} + )} + onGlossChange?.(t.id, e.target.value)} + value={glosses?.[t.id] ?? ''} + /> + + ))} + ), })); @@ -64,70 +93,142 @@ const PUNCT_SEGMENT: Segment = { ], }; +/** + * Minimal required props for SegmentView. Spread into render calls so tests only need to override + * what they actually care about. + * + * @returns An object containing all required SegmentView props set to no-op stubs. + */ +function requiredProps(): { + glosses: Record; + onGlossChange: (tokenId: string, value: string) => void; + onSelect: (ref: ScriptureRef, tokenId?: string) => void; + segment: Segment; +} { + return { + glosses: {}, + onGlossChange: jest.fn(), + onSelect: jest.fn(), + segment: WORD_SEGMENT, + }; +} + describe('SegmentView', () => { it('renders word token chips in token-chip mode (default)', () => { - render(); + render(); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('the')).toBeInTheDocument(); }); it('renders non-word (punctuation) tokens in token-chip mode', () => { - render(); + render(); expect(screen.getByText('.')).toBeInTheDocument(); }); it('renders baselineText in baseline-text mode', () => { - render(); + render(); expect(screen.getByText('In the beginning.')).toBeInTheDocument(); }); it('does not render individual tokens in baseline-text mode', () => { - render(); + render(); expect(screen.queryByText('In')).not.toBeInTheDocument(); expect(screen.queryByText('the')).not.toBeInTheDocument(); }); it('shows the verse number label', () => { - render(); + render(); expect(screen.getByText('1')).toBeInTheDocument(); }); it('sets aria-current="true" when isActive is true', () => { - render(); + const { container } = render(); - expect(screen.getByRole('button')).toHaveAttribute('aria-current', 'true'); + expect(container.firstChild).toHaveAttribute('aria-current', 'true'); }); it('does not set aria-current when isActive is false', () => { - render(); + const { container } = render(); - expect(screen.getByRole('button')).not.toHaveAttribute('aria-current'); + expect(container.firstChild).not.toHaveAttribute('aria-current'); }); it('does not set aria-current when isActive is omitted', () => { - render(); + const { container } = render(); + + expect(container.firstChild).not.toHaveAttribute('aria-current'); + }); + + it('does not set aria-current on the baseline-text button when isActive is false', () => { + const { container } = render( + , + ); + + expect(container.firstChild).not.toHaveAttribute('aria-current'); + }); + + it('sets aria-current="true" on the baseline-text button when isActive is true', () => { + const { container } = render( + , + ); + + expect(container.firstChild).toHaveAttribute('aria-current', 'true'); + }); + + it('calls onSelect when clicked in baseline-text mode', async () => { + const handleSelect = jest.fn(); + render( + , + ); - expect(screen.getByRole('button')).not.toHaveAttribute('aria-current'); + await userEvent.click(screen.getByTestId('segment-container')); + + expect(handleSelect).toHaveBeenCalledTimes(1); + expect(handleSelect).toHaveBeenCalledWith({ book: 'GEN', chapter: 1, verse: 1 }); }); - it('calls onClick when the button is clicked', async () => { - const handleClick = jest.fn(); - render(); + it('calls onSelect with the verse ref and token id when a word token is clicked', async () => { + const handleSelect = jest.fn(); + render(); + + await userEvent.click(screen.getByRole('button', { name: 'In' })); - await userEvent.click(screen.getByRole('button')); + expect(handleSelect).toHaveBeenCalledTimes(1); + expect(handleSelect).toHaveBeenCalledWith({ book: 'GEN', chapter: 1, verse: 1 }, 'tok-0'); + }); + + it('renders word tokens as interactive buttons when onSelect is provided', () => { + render(); - expect(handleClick).toHaveBeenCalledTimes(1); + expect(screen.getByRole('button', { name: 'In' })).toBeInTheDocument(); }); - it('does not throw when onClick is omitted and button is clicked', async () => { - render(); + it('passes glosses to word token inputs', () => { + render(); + + expect(screen.getByRole('textbox', { name: 'Gloss for In' })).toHaveValue('In'); + expect(screen.getByRole('textbox', { name: 'Gloss for the' })).toHaveValue('the'); + }); - await userEvent.click(screen.getByRole('button')); - // No assertion needed — test passes if no error is thrown + it('calls onGlossChange with the token id and new value when a gloss changes', async () => { + const handleGlossChange = jest.fn(); + render( + , + ); + + await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for In' }), 'In'); + + expect(handleGlossChange).toHaveBeenCalledTimes(2); + expect(handleGlossChange).toHaveBeenNthCalledWith(1, 'tok-0', 'I'); + expect(handleGlossChange).toHaveBeenNthCalledWith(2, 'tok-0', 'n'); }); }); diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index cf979ba5..b98bc5ca 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -3,6 +3,7 @@ /// import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import type { Token } from 'interlinearizer'; import { TokenChip } from '../../components/TokenChip'; @@ -37,8 +38,9 @@ describe('TokenChip', () => { it('applies a border class to word tokens', () => { render(); - const span = screen.getByText('hello'); - expect(span.className).toContain('tw:border'); + // The outer container holds the border; the inner span is just the surface text + const outer = screen.getByText('hello').closest('span')?.parentElement; + expect(outer?.className).toContain('tw:border'); }); it('does not apply a border class to punctuation tokens', () => { @@ -53,4 +55,53 @@ describe('TokenChip', () => { expect(wc.querySelector('span')).toBeInTheDocument(); expect(pc.querySelector('span')).toBeInTheDocument(); }); + + it('renders a gloss input for word tokens', () => { + render(); + expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toBeInTheDocument(); + }); + + it('does not render a gloss input for punctuation tokens', () => { + render(); + expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); + }); + + it('shows the current gloss value in the input', () => { + render(); + expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue('in'); + }); + + it('shows an empty input when no gloss is provided', () => { + render(); + expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue(''); + }); + + it('calls onGlossChange for each keystroke', async () => { + const handleChange = jest.fn(); + render(); + await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); + expect(handleChange).toHaveBeenCalledTimes(2); + expect(handleChange).toHaveBeenNthCalledWith(1, 'i'); + expect(handleChange).toHaveBeenNthCalledWith(2, 'in'); + }); + + it('does not throw when onGlossChange is omitted and the user types', async () => { + render(); + await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); + // No assertion needed — test passes if no error is thrown + }); + + it('calls onFocus when the input is focused', async () => { + const handleFocus = jest.fn(); + render(); + await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' })); + expect(handleFocus).toHaveBeenCalledTimes(1); + }); + + it('does not throw when onFocus is omitted', async () => { + render(); + await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' })); + await userEvent.tab(); + // No assertion needed — test passes if no error is thrown + }); }); diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index 2b7a2091..0772c4e3 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -11,6 +11,12 @@ const STRIP_FADE_EASING = 'cubic-bezier(0.65, 0, 0.35, 1)'; */ const STRIP_FADE_MS = 500; +/** + * Number of phrase slots rendered on each side of the focused phrase. Chosen large enough that no + * realistic viewport can ever render all tokens simultaneously. + */ +const PHRASE_WINDOW_HALF = 100; + /** * Renders all tokens from every segment in the given book as a single flat, horizontally scrollable * strip. Arrow buttons advance or retreat the view by one token at a time with smooth scrolling @@ -26,20 +32,34 @@ const STRIP_FADE_MS = 500; * navigation crosses a verse boundary `onVerseChange` is called with the new verse coordinate. * * @param props - Component props - * @param props.activeVerse - Optional verse coordinate; when it changes the strip scrolls to the - * first token of the matching segment + * @param props.activePhraseIndex - When set, the strip jumps to this phrase index; used to carry + * over a focused token when switching from segment view + * @param props.activeVerse - Verse coordinate; when it changes the strip scrolls to the first token + * of the matching segment * @param props.book - The full tokenized book whose tokens should be streamed + * @param props.glosses - Map from `Token.id` to current English gloss text + * @param props.onFocusPhraseIndexChange - Called whenever the focused phrase index changes so the + * parent can mirror the strip position + * @param props.onGlossChange - Called with the token id and new gloss value when a gloss is edited * @param props.onVerseChange - Called when arrow navigation moves the focus into a new verse * @returns A horizontal token strip with previous/next navigation arrows and edge-fade overlays */ export default function ContinuousView({ + activePhraseIndex, activeVerse, book, + glosses, + onFocusPhraseIndexChange, + onGlossChange, onVerseChange, }: Readonly<{ - activeVerse?: ScriptureRef; + activePhraseIndex: number | undefined; + activeVerse: ScriptureRef; book: Book; - onVerseChange?: (verse: ScriptureRef) => void; + glosses: Record; + onFocusPhraseIndexChange: (index: number) => void; + onGlossChange: (tokenId: string, value: string) => void; + onVerseChange: (verse: ScriptureRef) => void; }>) { const isRtl = document.documentElement.dir === 'rtl'; @@ -69,6 +89,11 @@ export default function ContinuousView({ .filter((entry) => entry.token.type === 'word'), [allTokens], ); + + /** + * Ref mirror of `phraseEntries`. Read inside effects and callbacks that need the latest list + * without declaring it as a dependency (which would cause spurious re-runs). + */ const phraseEntriesRef = useRef(phraseEntries); phraseEntriesRef.current = phraseEntries; @@ -125,9 +150,10 @@ export default function ContinuousView({ ); // Lazy-initialize to the target verse so on first render the strip is already positioned - // correctly before the initial-load fade-in fires. + // correctly before the initial-load fade-in fires. Prefer activePhraseIndex (e.g. a focused token + // carried over from segment view) so there is no flash to the verse-start position on mount. const [focusPhraseIndex, setFocusPhraseIndex] = useState(() => { - if (!activeVerse) return 0; + if (activePhraseIndex !== undefined) return activePhraseIndex; const seg = book.segments.find( (s) => @@ -146,15 +172,29 @@ export default function ContinuousView({ return phraseIndexByTokenIndex.get(tokenIdx) ?? 0; }); + /** + * The phrase index of the most recent external jump (prop-driven). Read inside the + * `focusPhraseIndex` effect to suppress the echo-back verse-change notification that would + * otherwise fire when the strip repositions itself in response to an incoming prop. + */ const jumpTargetRef = useRef(undefined); const [pendingExternalJumpPhraseIndex, setPendingExternalJumpPhraseIndex] = useState< number | undefined >(); const [isVisible, setIsVisible] = useState(false); + /** True while an externally triggered jump (prop change) is in progress; suppresses smooth scroll. */ const isExternalJumpInProgressRef = useRef(false); + /** True until the first scroll-into-view completes; suppresses smooth scroll on initial mount. */ const isInitialLoadInProgressRef = useRef(true); + /** + * True when the lazy `useState` initializer already positioned the strip at `activePhraseIndex`, + * so the first run of the `activePhraseIndex` effect should be skipped to avoid a redundant + * jump. + */ + const activePhraseIndexAppliedRef = useRef(activePhraseIndex !== undefined); + /** * Records the verse most recently reported via `onVerseChange`. When the parent echoes that verse * back as `activeVerse` we skip the jump — the change originated here, not externally. @@ -163,10 +203,24 @@ export default function ContinuousView({ */ const lastInternalVerseRef = useRef(activeVerse); - // Jump to the first token of the matching segment when the active verse changes. + // Jump to a specific phrase index when activePhraseIndex changes. useEffect(() => { - if (!activeVerse) return; + if (activePhraseIndex === undefined) return; + + // Skip the first run when the lazy initializer already positioned the strip here. + if (activePhraseIndexAppliedRef.current) { + activePhraseIndexAppliedRef.current = false; + return; + } + + jumpTargetRef.current = activePhraseIndex; + isExternalJumpInProgressRef.current = true; + setIsVisible(false); + setPendingExternalJumpPhraseIndex(activePhraseIndex); + }, [activePhraseIndex]); + // Jump to the first token of the matching segment when the active verse changes. + useEffect(() => { // Skip if this activeVerse update is an echo-back of a verse change we reported ourselves. const lastInternal = lastInternalVerseRef.current; if ( @@ -202,13 +256,17 @@ export default function ContinuousView({ }, [pendingExternalJumpPhraseIndex]); // Fire onVerseChange when arrow navigation crosses into a new verse. - // Initialise to the segment that owns the initial focusPhraseIndex so the initial render does not trigger the callback. + // Initialize to the segment that owns the initial focusPhraseIndex so the initial render does not trigger the callback. const firstVisibleSegId = phraseEntries.length > 0 ? tokenSegment[phraseEntries[0].tokenIndex]?.id : undefined; const initialFocusedPhrase = phraseEntries[focusPhraseIndex]; const initialSegId = initialFocusedPhrase ? tokenSegment[initialFocusedPhrase.tokenIndex]?.id : firstVisibleSegId; + /** + * Segment id of the last verse reported via `onVerseChange`. Compared against the current focused + * segment to avoid firing the callback redundantly when focus stays within the same verse. + */ const lastReportedSegIdRef = useRef(initialSegId); // Keep the reported-segment baseline in sync when switching to a different book. @@ -238,19 +296,40 @@ export default function ContinuousView({ verse: seg.startRef.verse, }; lastInternalVerseRef.current = verse; - onVerseChange?.(verse); + onVerseChange(verse); // onVerseChange and tokenSegmentRef are intentionally excluded — callers must stabilize the // reference (useCallback) and tokenSegmentRef is a ref so changes are always current. // eslint-disable-next-line react-hooks/exhaustive-deps }, [focusPhraseIndex]); - // One ref slot per phrase so we can call scrollIntoView on the focused one. + /** Ref mirror of `onFocusPhraseIndexChange` so the notification effect never needs it as a dep. */ + const onFocusPhraseIndexChangeRef = useRef(onFocusPhraseIndexChange); + onFocusPhraseIndexChangeRef.current = onFocusPhraseIndexChange; + useEffect(() => { + onFocusPhraseIndexChangeRef.current(focusPhraseIndex); + }, [focusPhraseIndex]); + + /** DOM ref array indexed by phrase index; used to scroll the focused chip into view. */ const phraseRefs = useRef<(HTMLSpanElement | null)[]>([]); const atStart = phraseEntries.length === 0 || focusPhraseIndex === 0; const atEnd = phraseEntries.length === 0 || focusPhraseIndex >= phraseEntries.length - 1; const stripOpacityClass = isVisible ? 'tw:opacity-100' : 'tw:opacity-0'; + /** The inclusive phrase-index bounds of the rendered window. */ + const windowStart = Math.max(0, focusPhraseIndex - PHRASE_WINDOW_HALF); + const windowEnd = Math.min(phraseEntries.length - 1, focusPhraseIndex + PHRASE_WINDOW_HALF); + + /** Token index of the first token in the rendered window. */ + const windowStartTokenIndex = + phraseEntries.length > 0 && windowStart > 0 ? phraseEntries[windowStart].tokenIndex : 0; + + /** Token index one past the last token in the rendered window. */ + const windowEndTokenIndex = + phraseEntries.length > 0 && windowEnd < phraseEntries.length - 1 + ? phraseEntries[windowEnd].tokenIndex + 1 + : allTokens.length; + /** * Advances the focused phrase by `delta` positions, clamping to valid bounds. * @@ -307,9 +386,10 @@ export default function ContinuousView({ return () => { cancelAnimationFrame(rafId); - // If the RAF was cancelled (another focus change fired before the first frame), - // still reveal the strip so it is not left invisible. - setIsVisible(true); + // Only reveal the strip on cleanup if no new external jump is about to take over. + // When a second click arrives before this RAF fires, isExternalJumpInProgressRef is already + // true for the new jump — revealing here would make the strip visible before it has scrolled. + if (!isExternalJumpInProgressRef.current) setIsVisible(true); }; }, [focusPhraseIndex]); @@ -346,14 +426,16 @@ export default function ContinuousView({ {/* Inner flex row */}
- {allTokens.map((token, tokenIndex) => { - if (token.type !== 'word') return ; + {allTokens.slice(windowStartTokenIndex, windowEndTokenIndex).map((token, i) => { + const tokenIndex = windowStartTokenIndex + i; + if (token.type !== 'word') return ; const phraseIndex = phraseIndexByTokenIndex.get(tokenIndex); const isFocusedPhrase = phraseIndex !== undefined && phraseIndex === focusPhraseIndex; @@ -365,9 +447,11 @@ export default function ContinuousView({ }} > diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index 91175cc4..33315ee3 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -1,6 +1,6 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; import type { Book, ScriptureRef, Segment } from 'interlinearizer'; -import { useCallback } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import ContinuousView from './ContinuousView'; import MemoizedSegmentView from './SegmentView'; @@ -14,8 +14,7 @@ import MemoizedSegmentView from './SegmentView'; * @param props.continuousScroll - Whether the continuous scroll view is shown * @param props.scrRef - Current scripture reference * @param props.setScrRef - Callback to update the scripture reference - * @returns The continuous-view strip (when enabled) above the scrollable segment list for the - * active chapter. + * @returns The full interlinearizer layout with optional continuous strip and segment list */ export default function Interlinearizer({ book, @@ -30,15 +29,139 @@ export default function Interlinearizer({ scrRef: SerializedVerseRef; setScrRef: (newScrRef: SerializedVerseRef) => void; }>) { + const [glosses, setGlosses] = useState>({}); + const [focusedTokenId, setFocusedTokenId] = useState(undefined); + + /** All word tokens in book order — index into this array is the phrase index. */ + const wordTokens = useMemo( + () => book.segments.flatMap((seg) => seg.tokens).filter((token) => token.type === 'word'), + [book.segments], + ); + + /** Maps each word token id to its phrase index across the full book. */ + const phraseIndexByTokenId = useMemo( + () => + wordTokens.reduce((map, token, idx) => { + map.set(token.id, idx); + return map; + }, new Map()), + [wordTokens], + ); + + const activePhraseIndex = + focusedTokenId === undefined ? undefined : phraseIndexByTokenId.get(focusedTokenId); + + /** The scrollable segment list div; targeted by `scrollActiveSegmentIntoView`. */ + const scrollContainerRef = useRef(undefined); + + /** + * Ref callback that stores the scroll container element so imperative scroll calls can target it. + * + * @param el - The mounted div, or `null` on unmount. + */ + const setScrollContainer = useCallback((el: HTMLDivElement | null) => { + scrollContainerRef.current = el ?? undefined; + }, []); + + /** + * Tracks the continuous strip's current phrase index as reported by ContinuousView, so we know + * exactly where it is when the user switches to segment view and so the active segment scrolls + * into view when the focused phrase changes. + */ + const [continuousViewPhraseIndex, setContinuousViewPhraseIndex] = useState( + undefined, + ); + + /** + * Ref mirror of `continuousViewPhraseIndex`. Read inside the mode-switch effect (which omits the + * state variable as a dep) so it always sees the latest value without causing a re-run. + */ + const continuousViewPhraseIndexRef = useRef(undefined); + + /** + * Keeps `continuousViewPhraseIndex` state in sync with the strip's current position so the + * segment list can scroll the right verse into view when the focused phrase changes. + * + * @param index - The phrase index now focused inside `ContinuousView`. + */ + const handleFocusPhraseIndexChange = useCallback((index: number) => { + continuousViewPhraseIndexRef.current = index; + setContinuousViewPhraseIndex(index); + }, []); + + /** + * Previous value of `continuousScroll`; lets the mode-switch effect detect the transition + * direction. + */ + const prevContinuousScrollRef = useRef(continuousScroll); + + // When switching from continuous to segment view, carry over the focused phrase from the strip. + // Only acts on the continuous→segment transition; leaving segment view does not overwrite focus. + useEffect(() => { + const wasOn = prevContinuousScrollRef.current; + prevContinuousScrollRef.current = continuousScroll; + + if (!wasOn || continuousScroll) return; + + // Transitioning continuous → segment: prefer the strip's last known position. + const idx = continuousViewPhraseIndexRef.current; + const token = idx === undefined ? undefined : wordTokens[idx]; + if (token) { + setFocusedTokenId(token.id); + return; + } + + // Fallback: only move to first word of the active segment if nothing is focused yet. + setFocusedTokenId((current) => { + if (current !== undefined) return current; + const activeSeg = bookSegments.find((seg) => seg.startRef.verse === scrRef.verseNum); + return activeSeg?.tokens.find((t) => t.type === 'word')?.id ?? current; + }); + // Only re-run when the mode switches; refs and stable arrays don't need to be deps. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [continuousScroll]); + + /** + * Scrolls the element marked `aria-current="true"` inside the segment scroll container into view + * at the top of the list, so the active verse is always visible after a mode switch or focus + * change. + */ + const scrollActiveSegmentIntoView = useCallback(() => { + const container = scrollContainerRef.current; + const active = container?.querySelector('[aria-current="true"]'); + /* v8 ignore next -- active is always found when a verse is rendered; guard for empty lists */ + active?.scrollIntoView({ behavior: 'auto', block: 'start' }); + }, []); + + // Scroll the active segment into view on mode switch and whenever the focused phrase changes + // while in continuous mode (ContinuousView is only mounted when continuousScroll is true, so + // continuousViewPhraseIndex only changes in that context). + useEffect(() => { + scrollActiveSegmentIntoView(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [continuousScroll, continuousViewPhraseIndex]); + + /** + * Merges an updated gloss value into the shared gloss map. + * + * @param tokenId - The id of the token whose gloss changed. + * @param value - The new gloss string entered by the user. + */ + const handleGlossChange = useCallback((tokenId: string, value: string) => { + setGlosses((prev) => ({ ...prev, [tokenId]: value })); + }, []); + /** - * Converts a `ScriptureRef` from `ContinuousView` into a `SerializedVerseRef` and forwards it to - * `setScrRef`. + * Updates the active scripture reference and, when a specific token was clicked, focuses that + * token. * - * @param v - The verse coordinate reported by the continuous view. + * @param ref - The verse coordinate that was selected. + * @param tokenId - The token that was clicked; omitted when the whole segment was selected. */ - const handleVerseChange = useCallback( - (v: ScriptureRef) => { - setScrRef({ book: v.book, chapterNum: v.chapter, verseNum: v.verse }); + const handleSegmentSelect = useCallback( + (ref: ScriptureRef, tokenId?: string) => { + setScrRef({ book: ref.book, chapterNum: ref.chapter, verseNum: ref.verse }); + if (tokenId) setFocusedTokenId(tokenId); }, [setScrRef], ); @@ -48,18 +171,25 @@ export default function Interlinearizer({ {continuousScroll && (
)} -
+
{bookSegments.length === 0 && (

No verse data for {scrRef.book} {scrRef.chapterNum}. @@ -74,7 +204,10 @@ export default function Interlinearizer({ segment={seg} isActive={seg.startRef.verse === scrRef.verseNum} displayMode={continuousScroll ? 'baseline-text' : 'token-chip'} - onClick={handleVerseChange} + focusedTokenId={continuousScroll ? undefined : focusedTokenId} + glosses={glosses} + onGlossChange={handleGlossChange} + onSelect={handleSegmentSelect} /> ))}

diff --git a/src/components/PhraseBox.tsx b/src/components/PhraseBox.tsx index e0dd946c..acc91c58 100644 --- a/src/components/PhraseBox.tsx +++ b/src/components/PhraseBox.tsx @@ -7,31 +7,44 @@ import MemoizedTokenChip from './TokenChip'; * Wraps one or more tokens in a phrase-level visual container. * * @param props - Component props - * @param props.index - Index passed back to `onClick` to identify which phrase was clicked + * @param props.glosses - Map from `Token.id` to current English gloss text for each token in this + * phrase. + * @param props.index - Index passed back to `onClick` to identify which phrase was interacted with * @param props.isFocused - Whether this phrase is the current navigation focus * @param props.onClick - Called with `index` when the phrase is clicked; omit to render a * non-interactive span + * @param props.onGlossChange - Called with the token id and new gloss value when a gloss is edited. * @param props.tokens - Tokens belonging to this phrase * @returns A bordered inline container */ export function PhraseBox({ + glosses, index, isFocused = false, onClick, + onGlossChange, tokens, }: Readonly<{ + glosses: Record; index?: number; - isFocused?: boolean; + isFocused: boolean; onClick?: (index?: number) => void; + onGlossChange: (tokenId: string, value: string) => void; tokens: Token[]; }>) { const baseClass = isFocused - ? 'tw:inline-flex tw:items-center tw:rounded tw:border-2 tw:border-border tw:bg-muted/30 tw:px-1 tw:py-0.5' + ? 'tw:inline-flex tw:items-center tw:rounded tw:border-2 tw:border-white tw:bg-muted/30 tw:px-1 tw:py-0.5' : 'tw:inline-flex tw:items-center tw:rounded tw:border tw:border-border/40 tw:bg-muted/20 tw:px-1 tw:py-0.5'; const innerContent = ( {tokens.map((token) => ( - + onClick?.(index)} + onGlossChange={(value) => onGlossChange(token.id, value)} + token={token} + /> ))} ); @@ -43,6 +56,7 @@ export function PhraseBox({ data-focus-state={isFocused ? 'focused' : 'default'} data-phrase-box="true" onClick={() => onClick?.(index)} + tabIndex={-1} type="button" > {innerContent} diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index be7d1d78..f45213e4 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -18,50 +18,90 @@ export type SegmentDisplayMode = 'token-chip' | 'baseline-text'; * * @param props - Component props * @param props.displayMode - Controls how segment content is rendered; defaults to `'token-chip'` + * @param props.focusedTokenId - When set, the matching word token's `PhraseBox` is rendered in the + * focused state; only meaningful in `token-chip` mode. + * @param props.glosses - Map from `Token.id` to current English gloss text for tokens in this + * segment. Pass an empty object when no glosses have been entered yet. * @param props.isActive - Whether this segment is the currently selected verse - * @param props.onClick - Callback invoked when the segment button is clicked + * @param props.onGlossChange - Called with the token id and new gloss value when a gloss is edited. + * @param props.onSelect - Called when the segment or one of its word tokens is interacted with. In + * `baseline-text` mode the whole segment is clickable and `tokenId` is omitted. In `token-chip` + * mode only word tokens trigger this callback and `tokenId` is always provided; omit to render + * word tokens as non-interactive spans. * @param props.segment - The segment to render - * @returns A button containing the segment's verse label and content + * @returns A button (baseline-text mode) or div (token-chip mode) containing a verse label and + * segment content */ export function SegmentView({ displayMode = 'token-chip', + focusedTokenId, + glosses, isActive, - onClick, + onGlossChange, + onSelect, segment, }: Readonly<{ displayMode?: SegmentDisplayMode; + focusedTokenId?: string; + glosses: Record; isActive?: boolean; - onClick?: (ref: ScriptureRef) => void; + onGlossChange: (tokenId: string, value: string) => void; + onSelect: (ref: ScriptureRef, tokenId?: string) => void; segment: Segment; }>) { const { book, chapter, verse } = segment.startRef; + const ref: ScriptureRef = { book, chapter, verse }; + const handleTokenClick = (tokenId: string) => onSelect(ref, tokenId); + + const sharedClassName = isActive + ? 'tw:w-full tw:rounded tw:border tw:border-border tw:bg-muted/50 tw:p-2' + : 'tw:w-full tw:rounded tw:p-2 tw:transition-colors tw:hover:bg-muted/30'; + + const verseLabel = ( + + {verse} + + ); + + if (displayMode === 'baseline-text') { + return ( + + ); + } return ( - + {verseLabel} + + {segment.tokens.map((token) => + token.type === 'word' ? ( + handleTokenClick(token.id)} + onGlossChange={onGlossChange} + tokens={[token]} + /> + ) : ( + + ), + )} + +
); } diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 430b1776..1e5fe780 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -2,17 +2,43 @@ import type { Token } from 'interlinearizer'; import { memo } from 'react'; /** - * Renders a single token as an inline chip. Word tokens get a bordered box; non-word tokens (e.g. - * punctuation) are rendered as muted inline text. + * Renders a single token as an inline chip. Word tokens get a bordered box with an editable gloss + * input below; non-word tokens (e.g. punctuation) are rendered as muted inline text with no gloss. * * @param props - Component props + * @param props.gloss - Current gloss text for this token (English). Absent when no gloss has been + * set. + * @param props.onFocus - Called when the gloss input receives focus; used by the parent to track + * which token is active. + * @param props.onGlossChange - Called with the new gloss value when the user edits the input. * @param props.token - The token to render - * @returns A styled inline span + * @returns A styled inline block */ -export function TokenChip({ token }: Readonly<{ token: Token }>) { +export function TokenChip({ + gloss, + onFocus, + onGlossChange, + token, +}: Readonly<{ + gloss?: string; + onFocus?: () => void; + onGlossChange?: (value: string) => void; + token: Token; +}>) { return token.type === 'word' ? ( - - {token.surfaceText} + + + {token.surfaceText} + + onGlossChange?.(e.target.value)} + onFocus={onFocus} + type="text" + /> ) : ( From ee1f731a7d716460231911eac11ab55a960f6ab3 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 19 May 2026 16:12:58 -0600 Subject: [PATCH 02/13] Minor adjustments --- src/__tests__/components/Interlinearizer.test.tsx | 7 ++++++- src/__tests__/components/TokenChip.test.tsx | 2 +- src/components/SegmentView.tsx | 6 ++++++ src/components/TokenChip.tsx | 2 +- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index e6e5f591..9bde861b 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -261,7 +261,12 @@ describe('Interlinearizer', () => { setScrRef: mockSetScrRef, }); - capturedSegmentViewPropsList[1].onSelect?.({ book: 'GEN', chapter: 1, verse: 2 }, 'GEN 1:2:0'); + act(() => { + capturedSegmentViewPropsList[1].onSelect?.( + { book: 'GEN', chapter: 1, verse: 2 }, + 'GEN 1:2:0', + ); + }); expect(mockSetScrRef).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 1, verseNum: 2 }); }); diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index b98bc5ca..60c2d4a4 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -82,7 +82,7 @@ describe('TokenChip', () => { await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); expect(handleChange).toHaveBeenCalledTimes(2); expect(handleChange).toHaveBeenNthCalledWith(1, 'i'); - expect(handleChange).toHaveBeenNthCalledWith(2, 'in'); + expect(handleChange).toHaveBeenNthCalledWith(2, 'n'); }); it('does not throw when onGlossChange is omitted and the user types', async () => { diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index f45213e4..e0ce0004 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -51,6 +51,12 @@ export function SegmentView({ }>) { const { book, chapter, verse } = segment.startRef; const ref: ScriptureRef = { book, chapter, verse }; + /** + * Forwards a token-chip click to the parent as a scripture reference + token id. + * + * @param tokenId - The id of the clicked token. + * @throws Propagates any error thrown by `onSelect`. + */ const handleTokenClick = (tokenId: string) => onSelect(ref, tokenId); const sharedClassName = isActive diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 1e5fe780..dedc8302 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -34,7 +34,7 @@ export function TokenChip({ aria-label={`Gloss for ${token.surfaceText}`} className="tw:mt-0.5 tw:rounded tw:border tw:border-border tw:bg-background tw:px-1 tw:text-center tw:text-sm tw:text-foreground tw:outline-none tw:focus:border-ring tw:focus:ring-1 tw:focus:ring-ring" style={{ fieldSizing: 'content', minWidth: '5ch' }} - defaultValue={gloss ?? ''} + value={gloss ?? ''} onChange={(e) => onGlossChange?.(e.target.value)} onFocus={onFocus} type="text" From f3a655b8cb3ab8eeb7f40e45a655a617c0486704 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 19 May 2026 17:38:42 -0600 Subject: [PATCH 03/13] Memoize `ref`, `handleTokenClick`, and `tokenArrays`; introduce `TokenChip` type guard to ensure that `input` always has a handler; update docs --- src/__tests__/components/PhraseBox.test.tsx | 14 ++--- src/__tests__/components/SegmentView.test.tsx | 6 +- src/__tests__/components/TokenChip.test.tsx | 58 ++++++++++--------- src/components/ContinuousView.tsx | 18 +++++- src/components/InterlinearizerLoader.tsx | 6 +- src/components/PhraseBox.tsx | 3 +- src/components/SegmentView.tsx | 47 +++++++++++---- src/components/TokenChip.tsx | 47 +++++++++------ 8 files changed, 126 insertions(+), 73 deletions(-) diff --git a/src/__tests__/components/PhraseBox.test.tsx b/src/__tests__/components/PhraseBox.test.tsx index b47b0047..aea82fd6 100644 --- a/src/__tests__/components/PhraseBox.test.tsx +++ b/src/__tests__/components/PhraseBox.test.tsx @@ -33,24 +33,24 @@ jest.mock('../../components/TokenChip', () => ({ })); /** Pre-built test token */ -const TEST_TOKEN: Token = { - ref: 'token-1', +const TEST_TOKEN = { + id: 'token-1', surfaceText: 'Hello', writingSystem: 'en', type: 'word', charStart: 0, charEnd: 5, -}; +} satisfies Token; /** Second test token */ -const TEST_TOKEN_2: Token = { - ref: 'token-2', +const TEST_TOKEN_2 = { + id: 'token-2', surfaceText: 'World', writingSystem: 'en', type: 'word', charStart: 6, charEnd: 11, -}; +} satisfies Token; /** Shared props shape used by both helper functions. */ type PhraseBoxTestProps = { @@ -58,7 +58,7 @@ type PhraseBoxTestProps = { isFocused: boolean; onClick?: (index?: number) => void; onGlossChange: (tokenId: string, value: string) => void; - tokens: Token[]; + tokens: (Token & { type: 'word' })[]; }; /** diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index 8f04795e..c0afa7dc 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -11,14 +11,16 @@ jest.mock('../../components/PhraseBox', () => ({ __esModule: true, default: ({ glosses, + index, isFocused = false, onClick, onGlossChange, tokens, }: { glosses: Record; + index?: number; isFocused: boolean; - onClick?: () => void; + onClick?: (index?: number) => void; onGlossChange: (tokenId: string, value: string) => void; tokens: Token[]; }) => ( @@ -26,7 +28,7 @@ jest.mock('../../components/PhraseBox', () => ({ {tokens.map((t) => ( {onClick ? ( - ) : ( diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 60c2d4a4..aa8f2e7f 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -7,27 +7,42 @@ import userEvent from '@testing-library/user-event'; import type { Token } from 'interlinearizer'; import { TokenChip } from '../../components/TokenChip'; -const WORD_TOKEN: Token = { - ref: 'GEN 1:1:0', +const WORD_TOKEN = { + id: 'GEN 1:1:0', surfaceText: 'hello', writingSystem: 'en', type: 'word', charStart: 0, charEnd: 5, -}; +} satisfies Token; -const PUNCT_TOKEN: Token = { - ref: 'GEN 1:1:5', +const PUNCT_TOKEN = { + id: 'GEN 1:1:5', surfaceText: '.', writingSystem: 'en', type: 'punctuation', charStart: 5, charEnd: 6, -}; +} satisfies Token; + +/** + * Minimal required props for a word-token `TokenChip`. Spread into render calls so tests only need + * to override what they actually care about. + * + * @returns An object with all required word-token props set to no-op stubs. + */ +function requiredWordProps() { + return { + token: WORD_TOKEN, + gloss: '', + onFocus: jest.fn(), + onGlossChange: jest.fn(), + } as const; +} describe('TokenChip', () => { it('renders the surface text for a word token', () => { - render(); + render(); expect(screen.getByText('hello')).toBeInTheDocument(); }); @@ -37,7 +52,7 @@ describe('TokenChip', () => { }); it('applies a border class to word tokens', () => { - render(); + render(); // The outer container holds the border; the inner span is just the surface text const outer = screen.getByText('hello').closest('span')?.parentElement; expect(outer?.className).toContain('tw:border'); @@ -50,14 +65,14 @@ describe('TokenChip', () => { }); it('renders word and punctuation tokens as inline spans', () => { - const { container: wc } = render(); + const { container: wc } = render(); const { container: pc } = render(); expect(wc.querySelector('span')).toBeInTheDocument(); expect(pc.querySelector('span')).toBeInTheDocument(); }); it('renders a gloss input for word tokens', () => { - render(); + render(); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toBeInTheDocument(); }); @@ -67,41 +82,28 @@ describe('TokenChip', () => { }); it('shows the current gloss value in the input', () => { - render(); + render(); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue('in'); }); - it('shows an empty input when no gloss is provided', () => { - render(); + it('shows an empty string in the input when gloss is empty', () => { + render(); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue(''); }); it('calls onGlossChange for each keystroke', async () => { const handleChange = jest.fn(); - render(); + render(); await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); expect(handleChange).toHaveBeenCalledTimes(2); expect(handleChange).toHaveBeenNthCalledWith(1, 'i'); expect(handleChange).toHaveBeenNthCalledWith(2, 'n'); }); - it('does not throw when onGlossChange is omitted and the user types', async () => { - render(); - await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); - // No assertion needed — test passes if no error is thrown - }); - it('calls onFocus when the input is focused', async () => { const handleFocus = jest.fn(); - render(); + render(); await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' })); expect(handleFocus).toHaveBeenCalledTimes(1); }); - - it('does not throw when onFocus is omitted', async () => { - render(); - await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' })); - await userEvent.tab(); - // No assertion needed — test passes if no error is thrown - }); }); diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index 0772c4e3..a9cb2ee1 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -3,8 +3,22 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import MemoizedPhraseBox from './PhraseBox'; import MemoizedTokenChip from './TokenChip'; -/** CSS easing for the strip opacity fade-in/out animation. */ +/** + * Narrows a `Token` to a word token. + * + * @param token - The token to test. + * @returns `true` when `token.type === 'word'`. + */ +function isWordToken(token: Token): token is Token & { type: 'word' } { + return token.type === 'word'; +} + +/** + * CSS easing for the strip opacity fade-in/out animation. Uses a sine-like curve for a natural feel + * at both ends of the transition. + */ const STRIP_FADE_EASING = 'cubic-bezier(0.65, 0, 0.35, 1)'; + /** * Duration of the strip fade animation in milliseconds. Must match the `setTimeout` in the * pending-jump effect. @@ -435,7 +449,7 @@ export default function ContinuousView({ > {allTokens.slice(windowStartTokenIndex, windowEndTokenIndex).map((token, i) => { const tokenIndex = windowStartTokenIndex + i; - if (token.type !== 'word') return ; + if (!isWordToken(token)) return ; const phraseIndex = phraseIndexByTokenIndex.get(tokenIndex); const isFocusedPhrase = phraseIndex !== undefined && phraseIndex === focusPhraseIndex; diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 55368cdc..6a5b1e29 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -24,10 +24,8 @@ const STRING_KEYS: `%${string}%`[] = ['%interlinearizer_continuousScrollToggle%' * @param props.projectId - PAPI project ID passed from the host * @param props.useWebViewScrollGroupScrRef - Hook that exposes the shared scroll-group scripture * reference and its setter - * @param props.useWebViewState - Hook for reading and writing values persisted in the WebView's - * saved state (survives tab restores) - * @returns The interlinearizer layout: tab toolbar, loading/error states or main view, and any - * currently open project modal. + * @returns The toolbar and either an error/loading state or the fully rendered + * {@link Interlinearizer} */ export default function InterlinearizerLoader({ projectId, diff --git a/src/components/PhraseBox.tsx b/src/components/PhraseBox.tsx index acc91c58..352c3bb3 100644 --- a/src/components/PhraseBox.tsx +++ b/src/components/PhraseBox.tsx @@ -30,7 +30,7 @@ export function PhraseBox({ isFocused: boolean; onClick?: (index?: number) => void; onGlossChange: (tokenId: string, value: string) => void; - tokens: Token[]; + tokens: (Token & { type: 'word' })[]; }>) { const baseClass = isFocused ? 'tw:inline-flex tw:items-center tw:rounded tw:border-2 tw:border-white tw:bg-muted/30 tw:px-1 tw:py-0.5' @@ -75,5 +75,6 @@ export function PhraseBox({ ); } +/** Memoized version of {@link PhraseBox}; use this for all render-stable phrase lists. */ const MemoizedPhraseBox = memo(PhraseBox); export default MemoizedPhraseBox; diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index e0ce0004..4e67d77c 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -1,8 +1,18 @@ -import type { ScriptureRef, Segment } from 'interlinearizer'; -import { memo } from 'react'; +import type { ScriptureRef, Segment, Token } from 'interlinearizer'; +import { memo, useCallback, useMemo } from 'react'; import MemoizedPhraseBox from './PhraseBox'; import MemoizedTokenChip from './TokenChip'; +/** + * Narrows a `Token` to a word token. + * + * @param token - The token to test. + * @returns `true` when `token.type === 'word'`. + */ +function isWordToken(token: Token): token is Token & { type: 'word' } { + return token.type === 'word'; +} + /** * The two display modes for {@link SegmentView}. * @@ -50,14 +60,29 @@ export function SegmentView({ segment: Segment; }>) { const { book, chapter, verse } = segment.startRef; - const ref: ScriptureRef = { book, chapter, verse }; + const ref: ScriptureRef = useMemo(() => ({ book, chapter, verse }), [book, chapter, verse]); + /** - * Forwards a token-chip click to the parent as a scripture reference + token id. + * Forwards a token-chip click (identified by its index in `segment.tokens`) to the parent as a + * scripture reference + token id. Stable across renders so `MemoizedPhraseBox` can memoize. * - * @param tokenId - The id of the clicked token. - * @throws Propagates any error thrown by `onSelect`. + * @param index - Index of the clicked token within `segment.tokens`. */ - const handleTokenClick = (tokenId: string) => onSelect(ref, tokenId); + const handleTokenClick = useCallback( + (index?: number) => { + if (index !== undefined) onSelect(ref, segment.tokens[index].id); + }, + [onSelect, ref, segment.tokens], + ); + + /** + * Stable single-token arrays for word tokens keyed by position, so `MemoizedPhraseBox` receives + * the same reference across renders. + */ + const tokenArrays = useMemo( + () => segment.tokens.map((token) => (isWordToken(token) ? [token] : [])), + [segment.tokens], + ); const sharedClassName = isActive ? 'tw:w-full tw:rounded tw:border tw:border-border tw:bg-muted/50 tw:p-2' @@ -92,15 +117,16 @@ export function SegmentView({ > {verseLabel} - {segment.tokens.map((token) => + {segment.tokens.map((token, index) => token.type === 'word' ? ( handleTokenClick(token.id)} + onClick={handleTokenClick} onGlossChange={onGlossChange} - tokens={[token]} + tokens={tokenArrays[index]} /> ) : ( @@ -111,5 +137,6 @@ export function SegmentView({ ); } +/** Memoized version of {@link SegmentView}; use this for all render-stable segment lists. */ const MemoizedSegmentView = memo(SegmentView); export default MemoizedSegmentView; diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index dedc8302..fd3806e8 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -1,30 +1,38 @@ import type { Token } from 'interlinearizer'; import { memo } from 'react'; +/** Props for a word token chip; requires gloss editing callbacks. */ +type WordProps = Readonly<{ + token: Token & { type: 'word' }; + gloss: string; + onFocus: () => void; + onGlossChange: (value: string) => void; +}>; + +/** Props for a punctuation token chip; editing props are excluded via `never`. */ +type PunctProps = Readonly<{ + token: Token; + gloss?: never; + onFocus?: never; + onGlossChange?: never; +}>; + /** * Renders a single token as an inline chip. Word tokens get a bordered box with an editable gloss - * input below; non-word tokens (e.g. punctuation) are rendered as muted inline text with no gloss. + * input below; punctuation tokens are rendered as muted inline text with no gloss. + * + * Props are a discriminated union on `token.type`: word tokens require `onGlossChange` and accept + * `gloss` and `onFocus`; punctuation tokens accept none of these. * * @param props - Component props - * @param props.gloss - Current gloss text for this token (English). Absent when no gloss has been - * set. - * @param props.onFocus - Called when the gloss input receives focus; used by the parent to track - * which token is active. - * @param props.onGlossChange - Called with the new gloss value when the user edits the input. - * @param props.token - The token to render + * @param props.token - The token to render; its `type` field discriminates the union. + * @param props.gloss - (word only) Current gloss text. Absent when no gloss has been set. + * @param props.onFocus - (word only) Called when the gloss input receives focus. + * @param props.onGlossChange - (word only) Called with the new gloss value when the user edits the + * input. * @returns A styled inline block */ -export function TokenChip({ - gloss, - onFocus, - onGlossChange, - token, -}: Readonly<{ - gloss?: string; - onFocus?: () => void; - onGlossChange?: (value: string) => void; - token: Token; -}>) { +export function TokenChip({ gloss, onFocus, onGlossChange, token }: WordProps | PunctProps) { return token.type === 'word' ? ( @@ -34,7 +42,7 @@ export function TokenChip({ aria-label={`Gloss for ${token.surfaceText}`} className="tw:mt-0.5 tw:rounded tw:border tw:border-border tw:bg-background tw:px-1 tw:text-center tw:text-sm tw:text-foreground tw:outline-none tw:focus:border-ring tw:focus:ring-1 tw:focus:ring-ring" style={{ fieldSizing: 'content', minWidth: '5ch' }} - value={gloss ?? ''} + value={gloss} onChange={(e) => onGlossChange?.(e.target.value)} onFocus={onFocus} type="text" @@ -47,5 +55,6 @@ export function TokenChip({ ); } +/** Memoized version of {@link TokenChip}; use this for all render-stable token lists. */ const MemoizedTokenChip = memo(TokenChip); export default MemoizedTokenChip; From ff32ce5ab97a761da1392c251853f33d4ea3aebb Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 20 May 2026 09:52:15 -0600 Subject: [PATCH 04/13] Clamp `activePhraseIndex`, memoize `allTokens` words as `tokenArrays`, add coalescing nullish fallback for token chip gloss, extract names prop types for components --- jest.config.ts | 1 + .../components/ContinuousView.test.tsx | 6 +++ src/__tests__/components/SegmentView.test.tsx | 6 +++ src/components/ContinuousView.tsx | 52 ++++++++++++++----- src/components/Interlinearizer.tsx | 17 +++--- src/components/PhraseBox.tsx | 23 ++++---- src/components/SegmentView.tsx | 26 ++++++---- src/components/component-types.ts | 10 ++++ 8 files changed, 101 insertions(+), 40 deletions(-) create mode 100644 src/components/component-types.ts diff --git a/jest.config.ts b/jest.config.ts index 670457f5..aa0b6040 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -38,6 +38,7 @@ const config: Config = { '!src/**/*.spec.{ts,tsx}', '!src/types/**', '!src/utils/interlinear-project-summary.ts', + '!src/components/component-types.ts', ], /** Directory for coverage output. */ diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index 42e4bf8a..7628a7e9 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -397,6 +397,12 @@ describe('ContinuousView rendering', () => { expect(screen.getByText('.')).toBeInTheDocument(); }); + it('renders without crashing when book has no word tokens and activePhraseIndex is set', () => { + render(); + + expect(screen.getByText('.')).toBeInTheDocument(); + }); + it('clicking an out-of-focus phrase box brings it into focus', async () => { render(); diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index c0afa7dc..fb804be2 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -102,13 +102,19 @@ const PUNCT_SEGMENT: Segment = { * @returns An object containing all required SegmentView props set to no-op stubs. */ function requiredProps(): { + displayMode: 'token-chip'; + focusedTokenId: string | undefined; glosses: Record; + isActive: boolean; onGlossChange: (tokenId: string, value: string) => void; onSelect: (ref: ScriptureRef, tokenId?: string) => void; segment: Segment; } { return { + displayMode: 'token-chip', + focusedTokenId: undefined, glosses: {}, + isActive: false, onGlossChange: jest.fn(), onSelect: jest.fn(), segment: WORD_SEGMENT, diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index a9cb2ee1..cff42706 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -1,5 +1,6 @@ import type { Book, ScriptureRef, Token } from 'interlinearizer'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import type { GlossHandlers } from './component-types'; import MemoizedPhraseBox from './PhraseBox'; import MemoizedTokenChip from './TokenChip'; @@ -13,6 +14,18 @@ function isWordToken(token: Token): token is Token & { type: 'word' } { return token.type === 'word'; } +/** + * Clamps `index` to `[0, len - 1]`, returning `0` when `len` is zero. + * + * @param index - The raw index to clamp. + * @param len - Length of the target array. + * @returns A safe index guaranteed to be within bounds. + */ +function clampIndex(index: number, len: number): number { + if (len === 0) return 0; + return Math.max(0, Math.min(index, len - 1)); +} + /** * CSS easing for the strip opacity fade-in/out animation. Uses a sine-like curve for a natural feel * at both ends of the transition. @@ -31,6 +44,17 @@ const STRIP_FADE_MS = 500; */ const PHRASE_WINDOW_HALF = 100; +/** Props for {@link ContinuousView}. */ +type ContinuousViewProps = Readonly< + GlossHandlers & { + activePhraseIndex: number | undefined; + activeVerse: ScriptureRef; + book: Book; + onFocusPhraseIndexChange: (index: number) => void; + onVerseChange: (verse: ScriptureRef) => void; + } +>; + /** * Renders all tokens from every segment in the given book as a single flat, horizontally scrollable * strip. Arrow buttons advance or retreat the view by one token at a time with smooth scrolling @@ -66,15 +90,7 @@ export default function ContinuousView({ onFocusPhraseIndexChange, onGlossChange, onVerseChange, -}: Readonly<{ - activePhraseIndex: number | undefined; - activeVerse: ScriptureRef; - book: Book; - glosses: Record; - onFocusPhraseIndexChange: (index: number) => void; - onGlossChange: (tokenId: string, value: string) => void; - onVerseChange: (verse: ScriptureRef) => void; -}>) { +}: ContinuousViewProps) { const isRtl = document.documentElement.dir === 'rtl'; const allTokens: Token[] = useMemo( @@ -104,6 +120,15 @@ export default function ContinuousView({ [allTokens], ); + /** + * Stable single-token arrays indexed by position in `allTokens`, so `MemoizedPhraseBox` receives + * the same array reference across renders and shallow memo comparison holds. + */ + const tokenArrays = useMemo( + () => allTokens.map((token) => (isWordToken(token) ? [token] : [])), + [allTokens], + ); + /** * Ref mirror of `phraseEntries`. Read inside effects and callbacks that need the latest list * without declaring it as a dependency (which would cause spurious re-runs). @@ -167,7 +192,7 @@ export default function ContinuousView({ // correctly before the initial-load fade-in fires. Prefer activePhraseIndex (e.g. a focused token // carried over from segment view) so there is no flash to the verse-start position on mount. const [focusPhraseIndex, setFocusPhraseIndex] = useState(() => { - if (activePhraseIndex !== undefined) return activePhraseIndex; + if (activePhraseIndex !== undefined) return clampIndex(activePhraseIndex, phraseEntries.length); const seg = book.segments.find( (s) => @@ -227,10 +252,11 @@ export default function ContinuousView({ return; } - jumpTargetRef.current = activePhraseIndex; + const clamped = clampIndex(activePhraseIndex, phraseEntriesRef.current.length); + jumpTargetRef.current = clamped; isExternalJumpInProgressRef.current = true; setIsVisible(false); - setPendingExternalJumpPhraseIndex(activePhraseIndex); + setPendingExternalJumpPhraseIndex(clamped); }, [activePhraseIndex]); // Jump to the first token of the matching segment when the active verse changes. @@ -466,7 +492,7 @@ export default function ContinuousView({ isFocused={isFocusedPhrase} onClick={handlePhraseSelect} onGlossChange={onGlossChange} - tokens={[token]} + tokens={tokenArrays[tokenIndex]} /> ); diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index 33315ee3..e0538adb 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -4,6 +4,15 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import ContinuousView from './ContinuousView'; import MemoizedSegmentView from './SegmentView'; +/** Props for {@link Interlinearizer}. */ +type InterlinearizerProps = Readonly<{ + book: Book; + bookSegments: Segment[]; + continuousScroll: boolean; + scrRef: SerializedVerseRef; + setScrRef: (newScrRef: SerializedVerseRef) => void; +}>; + /** * Main content area for the Interlinearizer. Renders an optional {@link ContinuousView} strip at the * top followed by a scrollable list of {@link MemoizedSegmentView}s for the current chapter. @@ -22,13 +31,7 @@ export default function Interlinearizer({ continuousScroll, scrRef, setScrRef, -}: Readonly<{ - book: Book; - bookSegments: Segment[]; - continuousScroll: boolean; - scrRef: SerializedVerseRef; - setScrRef: (newScrRef: SerializedVerseRef) => void; -}>) { +}: InterlinearizerProps) { const [glosses, setGlosses] = useState>({}); const [focusedTokenId, setFocusedTokenId] = useState(undefined); diff --git a/src/components/PhraseBox.tsx b/src/components/PhraseBox.tsx index 352c3bb3..acf1783a 100644 --- a/src/components/PhraseBox.tsx +++ b/src/components/PhraseBox.tsx @@ -1,8 +1,19 @@ /** @file Shared phrase-box wrapper used around word tokens. */ import type { Token } from 'interlinearizer'; import { memo } from 'react'; +import type { GlossHandlers } from './component-types'; import MemoizedTokenChip from './TokenChip'; +/** Props for {@link PhraseBox}. */ +type PhraseBoxProps = Readonly< + GlossHandlers & { + index?: number; + isFocused: boolean; + onClick?: (index?: number) => void; + tokens: (Token & { type: 'word' })[]; + } +>; + /** * Wraps one or more tokens in a phrase-level visual container. * @@ -17,6 +28,7 @@ import MemoizedTokenChip from './TokenChip'; * @param props.tokens - Tokens belonging to this phrase * @returns A bordered inline container */ + export function PhraseBox({ glosses, index, @@ -24,14 +36,7 @@ export function PhraseBox({ onClick, onGlossChange, tokens, -}: Readonly<{ - glosses: Record; - index?: number; - isFocused: boolean; - onClick?: (index?: number) => void; - onGlossChange: (tokenId: string, value: string) => void; - tokens: (Token & { type: 'word' })[]; -}>) { +}: PhraseBoxProps) { const baseClass = isFocused ? 'tw:inline-flex tw:items-center tw:rounded tw:border-2 tw:border-white tw:bg-muted/30 tw:px-1 tw:py-0.5' : 'tw:inline-flex tw:items-center tw:rounded tw:border tw:border-border/40 tw:bg-muted/20 tw:px-1 tw:py-0.5'; @@ -40,7 +45,7 @@ export function PhraseBox({ {tokens.map((token) => ( onClick?.(index)} onGlossChange={(value) => onGlossChange(token.id, value)} token={token} diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 4e67d77c..2250c1d2 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -1,5 +1,6 @@ import type { ScriptureRef, Segment, Token } from 'interlinearizer'; import { memo, useCallback, useMemo } from 'react'; +import type { GlossHandlers } from './component-types'; import MemoizedPhraseBox from './PhraseBox'; import MemoizedTokenChip from './TokenChip'; @@ -23,11 +24,22 @@ function isWordToken(token: Token): token is Token & { type: 'word' } { */ export type SegmentDisplayMode = 'token-chip' | 'baseline-text'; +/** Props for {@link SegmentView}. */ +type SegmentViewProps = Readonly< + GlossHandlers & { + displayMode: SegmentDisplayMode; + focusedTokenId: string | undefined; + isActive: boolean; + onSelect: (ref: ScriptureRef, tokenId?: string) => void; + segment: Segment; + } +>; + /** * Renders a single segment as either inline token chips or plain baseline text. * * @param props - Component props - * @param props.displayMode - Controls how segment content is rendered; defaults to `'token-chip'` + * @param props.displayMode - Controls how segment content is rendered * @param props.focusedTokenId - When set, the matching word token's `PhraseBox` is rendered in the * focused state; only meaningful in `token-chip` mode. * @param props.glosses - Map from `Token.id` to current English gloss text for tokens in this @@ -43,22 +55,14 @@ export type SegmentDisplayMode = 'token-chip' | 'baseline-text'; * segment content */ export function SegmentView({ - displayMode = 'token-chip', + displayMode, focusedTokenId, glosses, isActive, onGlossChange, onSelect, segment, -}: Readonly<{ - displayMode?: SegmentDisplayMode; - focusedTokenId?: string; - glosses: Record; - isActive?: boolean; - onGlossChange: (tokenId: string, value: string) => void; - onSelect: (ref: ScriptureRef, tokenId?: string) => void; - segment: Segment; -}>) { +}: SegmentViewProps) { const { book, chapter, verse } = segment.startRef; const ref: ScriptureRef = useMemo(() => ({ book, chapter, verse }), [book, chapter, verse]); diff --git a/src/components/component-types.ts b/src/components/component-types.ts new file mode 100644 index 00000000..610765eb --- /dev/null +++ b/src/components/component-types.ts @@ -0,0 +1,10 @@ +/** + * Props shared by components that display and edit token glosses. + * + * @field glosses - Map from `Token.id` to current gloss text. + * @field onGlossChange - Called with the token id and new value when a gloss is edited. + */ +export type GlossHandlers = { + glosses: Record; + onGlossChange: (tokenId: string, value: string) => void; +}; From 6a2b0b89d4e74e25bceed67144bbf3dec9af44cd Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 20 May 2026 12:56:18 -0600 Subject: [PATCH 05/13] Lift gloss state into GlossStoreProvider; add snap-to-active-verse button - Replace the `glosses`/`onGlossChange` prop-drilling pattern with a `GlossStoreProvider` context backed by `useSyncExternalStore`, so `TokenChip` reads and writes glosses directly without intermediary props passing through `PhraseBox`, `SegmentView`, `ContinuousView`, and `Interlinearizer`. - Add a sticky "Scroll to active verse" button (using the `LocateFixed` icon) that imperatively scrolls the `aria-current` segment into view. Rename `chapterSegments` to `bookSegments` throughout. --- __mocks__/lucide-react.tsx | 21 +- .../components/ContinuousView.test.tsx | 128 +++++---- src/__tests__/components/GlossStore.test.tsx | 188 +++++++++++++ .../components/Interlinearizer.test.tsx | 156 ++++++----- src/__tests__/components/PhraseBox.test.tsx | 256 ++++++++++-------- src/__tests__/components/SegmentView.test.tsx | 130 ++++----- src/__tests__/components/TokenChip.test.tsx | 131 +++++++-- .../hooks/useInterlinearizerBookData.test.ts | 2 +- src/components/ContinuousView.tsx | 49 ++-- src/components/GlossStore.tsx | 159 +++++++++++ src/components/Interlinearizer.tsx | 196 ++++++++------ src/components/InterlinearizerLoader.tsx | 2 +- src/components/PhraseBox.tsx | 71 ++--- src/components/SegmentView.tsx | 40 +-- src/components/TokenChip.tsx | 33 ++- src/components/component-types.ts | 15 +- src/parsers/papi/usjBookExtractor.ts | 5 +- 17 files changed, 1045 insertions(+), 537 deletions(-) create mode 100644 src/__tests__/components/GlossStore.test.tsx create mode 100644 src/components/GlossStore.tsx diff --git a/__mocks__/lucide-react.tsx b/__mocks__/lucide-react.tsx index dc182796..a4229d63 100644 --- a/__mocks__/lucide-react.tsx +++ b/__mocks__/lucide-react.tsx @@ -5,21 +5,12 @@ import type { ReactElement } from 'react'; /** - * Stub for the Trash2 icon; renders a bare SVG element so tests can locate the icon by test ID. + * Stub for the LocateFixed icon; renders a bare SVG element so tests can locate the icon by test + * ID. * - * @param props - SVG props forwarded from the component, including optional className and size. - * @returns A ReactElement SVG element used as a trash icon stub in tests. + * @param props - SVG props forwarded from the component, including optional className. + * @returns A ReactElement SVG element used as a locate-fixed icon stub in tests. */ -export function Trash2(props: Readonly<{ className?: string; size?: number }>): ReactElement { - return ; -} - -/** - * Stub for the Info icon; renders a bare SVG element so tests can locate the icon by test ID. - * - * @param props - SVG props forwarded from the component, including optional className and size. - * @returns A ReactElement SVG element used as an info icon stub in tests. - */ -export function Info(props: Readonly<{ className?: string; size?: number }>): ReactElement { - return ; +export function LocateFixed(props: Readonly<{ className?: string }>): ReactElement { + return ; } diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index 7628a7e9..5889ba34 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -5,25 +5,39 @@ import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { Book, ScriptureRef, Token } from 'interlinearizer'; +import type { ReactNode } from 'react'; import ContinuousView from '../../components/ContinuousView'; +import { GlossStoreProvider } from '../../components/GlossStore'; + +// --------------------------------------------------------------------------- +// GlossStore mock — pass-through provider so GlossStore.tsx stays out of scope +// --------------------------------------------------------------------------- + +jest.mock('../../components/GlossStore', () => ({ + __esModule: true, + GlossStoreProvider({ children }: Readonly<{ children: ReactNode }>) { + return children; + }, + useGloss: () => '', + useGlossDispatch: () => () => {}, +})); + +/** Render options that wrap every test render in a `GlossStoreProvider`. */ +const withGlossStore = { wrapper: GlossStoreProvider }; jest.mock('../../components/PhraseBox', () => ({ __esModule: true, default: ({ - glosses, isFocused = false, index, onClick, - onGlossChange, tokens, - }: { - glosses: Record; + }: Readonly<{ isFocused: boolean; index?: number; onClick?: (index?: number) => void; - onGlossChange: (tokenId: string, value: string) => void; tokens: Token[]; - }) => ( + }>) => ( ), @@ -319,17 +326,13 @@ const scrollIntoViewMock = jest.fn(); function requiredProps(): { activeVerse: ScriptureRef; activePhraseIndex: undefined; - glosses: Record; onFocusPhraseIndexChange: jest.Mock; - onGlossChange: jest.Mock; onVerseChange: jest.Mock; } { return { activeVerse: { book: 'GEN', chapter: 1, verse: 1 }, activePhraseIndex: undefined, - glosses: {}, onFocusPhraseIndexChange: jest.fn(), - onGlossChange: jest.fn(), onVerseChange: jest.fn(), }; } @@ -347,9 +350,9 @@ beforeEach(() => { // Rendering // --------------------------------------------------------------------------- -describe('ContinuousView rendering', () => { +describe('ContinuousView initial render', () => { it('renders all tokens from all segments as a flat list', () => { - render(); + render(, withGlossStore); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('the')).toBeInTheDocument(); @@ -357,15 +360,8 @@ describe('ContinuousView rendering', () => { expect(screen.getByText('God')).toBeInTheDocument(); }); - it('renders tokens from both chapters in a two-chapter book', () => { - render(); - - expect(screen.getByText('Alpha')).toBeInTheDocument(); - expect(screen.getByText('Beta')).toBeInTheDocument(); - }); - it('does not render any verse label or segment separator', () => { - render(); + render(, withGlossStore); // No verse numbers or colons that would indicate verse labels expect(screen.queryByText('1:1')).not.toBeInTheDocument(); @@ -375,7 +371,7 @@ describe('ContinuousView rendering', () => { }); it('renders a Previous token button and a Next token button', () => { - render(); + render(, withGlossStore); expect(screen.getByRole('button', { name: 'Previous token' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Next token' })).toBeInTheDocument(); @@ -383,7 +379,7 @@ describe('ContinuousView rendering', () => { it('renders a non-word token via TokenChip within the strip', () => { // makeMixedBook: GEN 1:1 has a word token; GEN 1:2 has a punctuation token - render(); + render(, withGlossStore); // Both the word chip ("In") and the punctuation chip (".") must appear expect(screen.getByText('In')).toBeInTheDocument(); @@ -391,20 +387,23 @@ describe('ContinuousView rendering', () => { }); it('renders without crashing when book has no word tokens (empty phraseEntries)', () => { - render(); + render(, withGlossStore); // The punctuation token is rendered expect(screen.getByText('.')).toBeInTheDocument(); }); it('renders without crashing when book has no word tokens and activePhraseIndex is set', () => { - render(); + render( + , + withGlossStore, + ); expect(screen.getByText('.')).toBeInTheDocument(); }); it('clicking an out-of-focus phrase box brings it into focus', async () => { - render(); + render(, withGlossStore); const clickedPhraseBox = screen.getByText('beginning').closest('[data-phrase-box="true"]'); if (!clickedPhraseBox) throw new Error('Expected phrase box wrapper for token'); @@ -418,7 +417,7 @@ describe('ContinuousView rendering', () => { }); it('clicking the already-focused phrase box leaves it focused', async () => { - render(); + render(, withGlossStore); // The first token is focused by default. const firstPhraseBox = screen.getByText('In').closest('[data-phrase-box="true"]'); @@ -440,26 +439,26 @@ describe('ContinuousView rendering', () => { describe('ContinuousView arrow disabled states', () => { it('disables the prev arrow on initial render (book start)', () => { - render(); + render(, withGlossStore); expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); }); it('enables the next arrow on initial render when there are multiple tokens', () => { - render(); + render(, withGlossStore); expect(screen.getByRole('button', { name: 'Next token' })).toBeEnabled(); }); it('disables both arrows when the book has exactly one token', () => { - render(); + render(, withGlossStore); expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); expect(screen.getByRole('button', { name: 'Next token' })).toBeDisabled(); }); it('enables the prev arrow after clicking next once', async () => { - render(); + render(, withGlossStore); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -467,7 +466,7 @@ describe('ContinuousView arrow disabled states', () => { }); it('disables the next arrow when advanced to the last token', async () => { - render(); + render(, withGlossStore); const nextBtn = screen.getByRole('button', { name: 'Next token' }); // 4 tokens total: advance 3 times to reach index 3 (last) @@ -479,7 +478,7 @@ describe('ContinuousView arrow disabled states', () => { }); it('re-enables the next arrow after going prev from the last token', async () => { - render(); + render(, withGlossStore); const nextBtn = screen.getByRole('button', { name: 'Next token' }); await userEvent.click(nextBtn); @@ -500,7 +499,10 @@ describe('ContinuousView arrow disabled states', () => { describe('ContinuousView fade overlays', () => { it('does not render prev fade at book start', () => { - const { container } = render(); + const { container } = render( + , + withGlossStore, + ); const gradients = container.querySelectorAll('[aria-hidden="true"]'); const prevFades = Array.from(gradients).filter((el) => @@ -510,7 +512,10 @@ describe('ContinuousView fade overlays', () => { }); it('renders next fade at book start (next side is enabled)', () => { - const { container } = render(); + const { container } = render( + , + withGlossStore, + ); const gradients = container.querySelectorAll('[aria-hidden="true"]'); const nextFades = Array.from(gradients).filter((el) => @@ -520,7 +525,10 @@ describe('ContinuousView fade overlays', () => { }); it('renders prev fade after moving away from book start', async () => { - const { container } = render(); + const { container } = render( + , + withGlossStore, + ); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -532,7 +540,10 @@ describe('ContinuousView fade overlays', () => { }); it('does not render next fade at book end', async () => { - const { container } = render(); + const { container } = render( + , + withGlossStore, + ); const nextBtn = screen.getByRole('button', { name: 'Next token' }); await userEvent.click(nextBtn); @@ -553,7 +564,7 @@ describe('ContinuousView fade overlays', () => { describe('ContinuousView cross-chapter traversal', () => { it('indexes tokens across chapter boundaries in segment order', () => { - render(); + render(, withGlossStore); // Both chapter tokens should be present expect(screen.getByText('Alpha')).toBeInTheDocument(); @@ -561,7 +572,7 @@ describe('ContinuousView cross-chapter traversal', () => { }); it('can navigate across a chapter boundary with the next arrow', async () => { - render(); + render(, withGlossStore); // Only one token per chapter, so clicking next once reaches chapter 2's token (index 1 = last) await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -579,7 +590,7 @@ describe('ContinuousView cross-chapter traversal', () => { describe('ContinuousView smooth-scroll intent', () => { it('calls scrollIntoView with smooth behaviour when next arrow is clicked', async () => { - render(); + render(, withGlossStore); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -589,7 +600,7 @@ describe('ContinuousView smooth-scroll intent', () => { }); it('calls scrollIntoView with smooth behaviour when prev arrow is clicked', async () => { - render(); + render(, withGlossStore); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); scrollIntoViewMock.mockClear(); @@ -602,7 +613,7 @@ describe('ContinuousView smooth-scroll intent', () => { }); it('does not call scrollIntoView when a disabled arrow is clicked', async () => { - render(); + render(, withGlossStore); scrollIntoViewMock.mockClear(); // Prev arrow is disabled at start — clicking it should be a no-op @@ -632,6 +643,7 @@ describe('ContinuousView activeVerse verse-jump', () => { {...requiredProps()} activeVerse={{ book: 'GEN', chapter: 1, verse: 1 }} />, + withGlossStore, ); // At index 0 the prev arrow should be disabled @@ -749,6 +761,7 @@ describe('ContinuousView activeVerse verse-jump', () => { {...requiredProps()} activeVerse={{ book: 'GEN', chapter: 1, verse: 2 }} />, + withGlossStore, ); // Index 2 is not the start (prev enabled) and not the end (next enabled). @@ -757,7 +770,7 @@ describe('ContinuousView activeVerse verse-jump', () => { }); it('does not jump when activeVerse is undefined', () => { - render(); + render(, withGlossStore); // Without activeVerse the strip stays at focusIndex 0 expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); @@ -770,6 +783,7 @@ describe('ContinuousView activeVerse verse-jump', () => { {...requiredProps()} activeVerse={{ book: 'GEN', chapter: 99, verse: 99 }} />, + withGlossStore, ); // No matching segment — strip stays at focusIndex 0 @@ -837,7 +851,10 @@ describe('ContinuousView activePhraseIndex', () => { it('uses instant scrollIntoView behaviour after the fade completes', () => { const book = makeBook(); - render(); + render( + , + withGlossStore, + ); act(() => { jest.advanceTimersByTime(500); }); @@ -908,6 +925,7 @@ describe('ContinuousView onVerseChange propagation', () => { const handleVerseChange = jest.fn(); render( , + withGlossStore, ); // Advance twice to reach index 2 (first token of GEN 1:2) @@ -926,6 +944,7 @@ describe('ContinuousView onVerseChange propagation', () => { activeVerse={{ book: 'GEN', chapter: 1, verse: 2 }} onVerseChange={handleVerseChange} />, + withGlossStore, ); handleVerseChange.mockClear(); @@ -939,6 +958,7 @@ describe('ContinuousView onVerseChange propagation', () => { const handleVerseChange = jest.fn(); render( , + withGlossStore, ); handleVerseChange.mockClear(); @@ -956,6 +976,7 @@ describe('ContinuousView onVerseChange propagation', () => { {...requiredProps()} onVerseChange={handleVerseChange} />, + withGlossStore, ); handleVerseChange.mockClear(); @@ -1008,14 +1029,14 @@ describe('ContinuousView RTL layout', () => { }); it('shows right-arrow (→) on the previous button in RTL mode', () => { - render(); + render(, withGlossStore); const prevBtn = screen.getByRole('button', { name: 'Previous token' }); expect(prevBtn.querySelector('[aria-hidden="true"]')).toHaveTextContent('\u2192'); }); it('shows left-arrow (←) on the next button in RTL mode', () => { - render(); + render(, withGlossStore); const nextBtn = screen.getByRole('button', { name: 'Next token' }); expect(nextBtn.querySelector('[aria-hidden="true"]')).toHaveTextContent('\u2190'); @@ -1046,6 +1067,7 @@ describe('ContinuousView phrase window', () => { activeVerse={{ book: 'GEN', chapter: 1, verse: 1 }} activePhraseIndex={125} />, + withGlossStore, ); act(() => { jest.advanceTimersByTime(500); diff --git a/src/__tests__/components/GlossStore.test.tsx b/src/__tests__/components/GlossStore.test.tsx new file mode 100644 index 00000000..9f61398e --- /dev/null +++ b/src/__tests__/components/GlossStore.test.tsx @@ -0,0 +1,188 @@ +/** @file Unit tests for components/GlossStore.tsx. */ +/// +/// + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { + GlossStoreProvider, + useAllGlosses, + useGloss, + useGlossDispatch, +} from '../../components/GlossStore'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Renders a component that displays the gloss for a single token, used to assert on `useGloss`. + * + * @param tokenId - Token id to subscribe to. + * @returns JSX element suitable for passing to `render`. + */ +function GlossReader({ tokenId }: Readonly<{ tokenId: string }>) { + const gloss = useGloss(tokenId); + return {gloss}; +} + +/** + * Renders a component that displays all glosses as JSON, used to assert on `useAllGlosses`. + * + * @returns JSX element suitable for passing to `render`. + */ +function AllGlossesReader() { + const glosses = useAllGlosses(); + return {JSON.stringify(glosses)}; +} + +/** + * Renders a component that calls `useGlossDispatch` without a provider, used to assert the hook + * throws outside a {@link GlossStoreProvider}. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function DispatchUser() { + useGlossDispatch(); + return undefined; +} + +/** + * Renders a button that calls `useGlossDispatch` to write a gloss, used to test dispatch. + * + * @param props.tokenId - Token id to write. + * @param props.value - Gloss value to write. + * @returns JSX element suitable for passing to `render`. + */ +function GlossWriter({ tokenId, value }: Readonly<{ tokenId: string; value: string }>) { + const dispatch = useGlossDispatch(); + return ( + + ); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('useGloss', () => { + it('returns an empty string for an unknown token', () => { + render( + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent(''); + }); + + it('returns the seeded value for a token in initialGlosses', () => { + render( + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent('hello'); + }); + + it('updates when the subscribed token changes', async () => { + render( + + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent(''); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(screen.getByTestId('gloss')).toHaveTextContent('world'); + }); + + it('does not update when a different token changes', async () => { + let renderCount = 0; + + function CountingGlossReader({ tokenId }: Readonly<{ tokenId: string }>) { + renderCount += 1; + const gloss = useGloss(tokenId); + return {gloss}; + } + + render( + + + + , + ); + const initialRenderCount = renderCount; + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(renderCount).toBe(initialRenderCount); + }); + + it('throws when called outside a GlossStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useGloss must be used inside a GlossStoreProvider', + ); + }); +}); + +describe('useAllGlosses', () => { + it('returns an empty object when no glosses have been set', () => { + render( + + + , + ); + expect(screen.getByTestId('all-glosses')).toHaveTextContent('{}'); + }); + + it('returns seeded glosses from initialGlosses', () => { + render( + + + , + ); + expect(screen.getByTestId('all-glosses')).toHaveTextContent(JSON.stringify({ 'tok-1': 'hi' })); + }); + + it('updates when any token changes', async () => { + render( + + + + , + ); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(screen.getByTestId('all-glosses')).toHaveTextContent( + JSON.stringify({ 'tok-1': 'world' }), + ); + }); + + it('throws when called outside a GlossStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useAllGlosses must be used inside a GlossStoreProvider', + ); + }); +}); + +describe('useGlossDispatch', () => { + it('calls the onGlossChange spy with tokenId and value', async () => { + const spy = jest.fn(); + render( + + + , + ); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith('tok-1', 'hi'); + }); + + it('throws when called outside a GlossStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useGlossDispatch must be used inside a GlossStoreProvider', + ); + }); +}); diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 9bde861b..bb74cd39 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -5,8 +5,14 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; import { act, render, screen } from '@testing-library/react'; import type { Book, Segment } from 'interlinearizer'; +import type { ReactNode } from 'react'; import Interlinearizer from '../../components/Interlinearizer'; +jest.mock('lucide-react', () => ({ + __esModule: true, + LocateFixed: () => , +})); + // Store captured props so tests can inspect what Interlinearizer passes down let capturedContinuousViewProps: Record = {}; @@ -19,6 +25,15 @@ type CapturedSegmentViewProps = { }; let capturedSegmentViewPropsList: CapturedSegmentViewProps[] = []; +jest.mock('../../components/GlossStore', () => ({ + __esModule: true, + GlossStoreProvider({ children }: Readonly<{ children: ReactNode }>) { + return children; + }, + useGloss: () => '', + useGlossDispatch: () => () => {}, +})); + jest.mock('../../components/ContinuousView', () => ({ __esModule: true, default: (props: Record) => { @@ -36,19 +51,25 @@ jest.mock('../../components/ContinuousView', () => ({ jest.mock('../../components/SegmentView', () => ({ __esModule: true, - SegmentView: ({ - segment, - ...rest - }: CapturedSegmentViewProps & { glosses?: Record }) => { - capturedSegmentViewPropsList.push({ segment, ...rest }); - return
; + SegmentView: ({ segment, isActive, ...rest }: CapturedSegmentViewProps) => { + capturedSegmentViewPropsList.push({ segment, isActive, ...rest }); + return ( +
+ ); }, - default: ({ - segment, - ...rest - }: CapturedSegmentViewProps & { glosses?: Record }) => { - capturedSegmentViewPropsList.push({ segment, ...rest }); - return
; + default: ({ segment, isActive, ...rest }: CapturedSegmentViewProps) => { + capturedSegmentViewPropsList.push({ segment, isActive, ...rest }); + return ( +
+ ); }, })); @@ -132,13 +153,13 @@ const GEN_1_MULTI_BOOK: Book = { */ function renderInterlinearizer({ book = GEN_1_1_BOOK, - bookSegments = GEN_1_1_BOOK.segments, + chapterSegments = GEN_1_1_BOOK.segments, continuousScroll = false, scrRef = defaultScrRef, setScrRef = () => {}, }: { book?: Book; - bookSegments?: Book['segments']; + chapterSegments?: Book['segments']; continuousScroll?: boolean; scrRef?: SerializedVerseRef; setScrRef?: (r: SerializedVerseRef) => void; @@ -146,7 +167,7 @@ function renderInterlinearizer({ return render( { + // jsdom does not implement scrollIntoView; stub it globally so components that call it don't throw. + Element.prototype.scrollIntoView = jest.fn(); +}); + describe('Interlinearizer', () => { beforeEach(() => { capturedContinuousViewProps = {}; @@ -167,13 +193,13 @@ describe('Interlinearizer', () => { }); it('shows a no-verse message when the tokenized book has no segments at all', () => { - renderInterlinearizer({ bookSegments: GEN_EMPTY_BOOK.segments }); + renderInterlinearizer({ chapterSegments: GEN_EMPTY_BOOK.segments }); expect(screen.getByText(/no verse data for gen 1\./i)).toBeInTheDocument(); }); it('renders a SegmentView for every segment in the current chapter', () => { - renderInterlinearizer({ bookSegments: GEN_1_MULTI_BOOK.segments }); + renderInterlinearizer({ chapterSegments: GEN_1_MULTI_BOOK.segments }); expect(screen.getAllByTestId('segment-view')).toHaveLength(2); expect(capturedSegmentViewPropsList[0].segment.id).toBe('GEN 1:1'); @@ -181,7 +207,7 @@ describe('Interlinearizer', () => { }); it('passes isActive=true only to the segment matching the current verse', () => { - renderInterlinearizer({ bookSegments: GEN_1_MULTI_BOOK.segments }); + renderInterlinearizer({ chapterSegments: GEN_1_MULTI_BOOK.segments }); // defaultScrRef is GEN 1:1 expect(capturedSegmentViewPropsList[0].isActive).toBe(true); @@ -190,28 +216,22 @@ describe('Interlinearizer', () => { it('renders all segments when navigating to a title reference (verse 0)', () => { const titleRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 0 }; - renderInterlinearizer({ bookSegments: GEN_1_MULTI_BOOK.segments, scrRef: titleRef }); + renderInterlinearizer({ chapterSegments: GEN_1_MULTI_BOOK.segments, scrRef: titleRef }); expect(screen.getAllByTestId('segment-view')).toHaveLength(2); }); - it('calls setScrRef with the segment ref when a verse box is clicked', () => { + it('calls setScrRef with the segment ref when a segment fires onSelect', () => { const mockSetScrRef = jest.fn(); - renderInterlinearizer({ bookSegments: GEN_1_MULTI_BOOK.segments, setScrRef: mockSetScrRef }); + renderInterlinearizer({ chapterSegments: GEN_1_MULTI_BOOK.segments, setScrRef: mockSetScrRef }); capturedSegmentViewPropsList[1].onSelect?.({ book: 'GEN', chapter: 1, verse: 2 }); expect(mockSetScrRef).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 1, verseNum: 2 }); }); - it('passes displayMode="baseline-text" to SegmentView when continuousScroll is true', () => { - renderInterlinearizer({ continuousScroll: true }); - - expect(capturedSegmentViewPropsList[0].displayMode).toBe('baseline-text'); - }); - it('passes displayMode="baseline-text" to all SegmentViews when continuousScroll is true', () => { - renderInterlinearizer({ bookSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: true }); + renderInterlinearizer({ chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: true }); capturedSegmentViewPropsList.forEach((p) => expect(p.displayMode).toBe('baseline-text')); }); @@ -230,7 +250,7 @@ describe('Interlinearizer', () => { it('renders ContinuousView above the chapter segment rows when both are present', () => { const { container } = renderInterlinearizer({ - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: true, }); @@ -241,15 +261,12 @@ describe('Interlinearizer', () => { expect(allElements[0]).toBe(continuousView); }); - it('passes onSelect to SegmentView when continuousScroll is false', () => { + it('passes onSelect to SegmentView regardless of continuousScroll mode', () => { renderInterlinearizer({ continuousScroll: false }); - expect(capturedSegmentViewPropsList[0].onSelect).toBeInstanceOf(Function); - }); - it('passes onSelect to SegmentView when continuousScroll is true', () => { + capturedSegmentViewPropsList = []; renderInterlinearizer({ continuousScroll: true }); - expect(capturedSegmentViewPropsList[0].onSelect).toBeInstanceOf(Function); }); @@ -257,7 +274,7 @@ describe('Interlinearizer', () => { const mockSetScrRef = jest.fn(); renderInterlinearizer({ book: GEN_1_MULTI_BOOK, - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, setScrRef: mockSetScrRef, }); @@ -275,7 +292,7 @@ describe('Interlinearizer', () => { // Render in token-chip mode first so onSelect is available on SegmentView props. const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: false, }); @@ -292,7 +309,7 @@ describe('Interlinearizer', () => { rerender( {}} @@ -302,20 +319,6 @@ describe('Interlinearizer', () => { expect(capturedContinuousViewProps.activePhraseIndex).toBe(1); }); - it('passes onGlossChange to ContinuousView and updates glosses state', () => { - renderInterlinearizer({ continuousScroll: true }); - - const { onGlossChange } = capturedContinuousViewProps; - if (typeof onGlossChange !== 'function') - throw new Error('Expected onGlossChange to be a function'); - - act(() => { - onGlossChange('token-1', 'hello'); - }); - - expect(capturedContinuousViewProps.glosses).toEqual({ 'token-1': 'hello' }); - }); - it('calls setScrRef when ContinuousView emits onVerseChange', () => { const mockSetScrRef = jest.fn(); renderInterlinearizer({ continuousScroll: true, setScrRef: mockSetScrRef }); @@ -331,10 +334,10 @@ describe('Interlinearizer', () => { expect(mockSetScrRef).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 2, verseNum: 3 }); }); - it('updates continuousViewPhraseIndex when ContinuousView emits onFocusPhraseIndexChange', () => { + it('does not update activePhraseIndex when ContinuousView emits onFocusPhraseIndexChange', () => { const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: true, }); @@ -351,7 +354,7 @@ describe('Interlinearizer', () => { rerender( {}} @@ -364,7 +367,7 @@ describe('Interlinearizer', () => { it('carries the strip phrase position into segment view when switching off continuousScroll', () => { const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: true, }); @@ -382,7 +385,7 @@ describe('Interlinearizer', () => { rerender( {}} @@ -398,7 +401,7 @@ describe('Interlinearizer', () => { // Start in continuous mode without ContinuousView ever calling onFocusPhraseIndexChange. const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: true, scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 }, }); @@ -408,7 +411,7 @@ describe('Interlinearizer', () => { rerender( {}} @@ -424,7 +427,7 @@ describe('Interlinearizer', () => { // Start in segment mode and focus a specific token. const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: false, }); @@ -440,7 +443,7 @@ describe('Interlinearizer', () => { rerender( {}} @@ -452,7 +455,7 @@ describe('Interlinearizer', () => { rerender( {}} @@ -464,11 +467,38 @@ describe('Interlinearizer', () => { expect(stillFocused).toBeDefined(); }); + it('renders the snap-to-active-verse button when segments are present', () => { + renderInterlinearizer({ chapterSegments: GEN_1_MULTI_BOOK.segments }); + + expect(screen.getByRole('button', { name: /scroll to active verse/i })).toBeInTheDocument(); + }); + + it('does not render the snap-to-active-verse button when there are no segments', () => { + renderInterlinearizer({ chapterSegments: GEN_EMPTY_BOOK.segments }); + + expect( + screen.queryByRole('button', { name: /scroll to active verse/i }), + ).not.toBeInTheDocument(); + }); + + it('snap button calls scrollIntoView on the active segment', () => { + renderInterlinearizer({ chapterSegments: GEN_1_1_BOOK.segments }); + + act(() => { + screen.getByRole('button', { name: /scroll to active verse/i }).click(); + }); + + expect(Element.prototype.scrollIntoView).toHaveBeenCalledWith({ + behavior: 'auto', + block: 'start', + }); + }); + it('leaves focusedTokenId undefined when switching off continuousScroll with no strip position and no matching segment', () => { // scrRef points to verse 99 which does not exist in GEN_1_MULTI_BOOK. const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, - bookSegments: GEN_1_MULTI_BOOK.segments, + chapterSegments: GEN_1_MULTI_BOOK.segments, continuousScroll: true, scrRef: { book: 'GEN', chapterNum: 1, verseNum: 99 }, }); @@ -477,7 +507,7 @@ describe('Interlinearizer', () => { rerender( {}} diff --git a/src/__tests__/components/PhraseBox.test.tsx b/src/__tests__/components/PhraseBox.test.tsx index aea82fd6..9ed21050 100644 --- a/src/__tests__/components/PhraseBox.test.tsx +++ b/src/__tests__/components/PhraseBox.test.tsx @@ -5,32 +5,77 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { Token } from 'interlinearizer'; +import type { ReactNode } from 'react'; +import { GlossStoreProvider } from '../../components/GlossStore'; import { PhraseBox } from '../../components/PhraseBox'; -jest.mock('../../components/TokenChip', () => ({ - __esModule: true, - default: ({ - gloss, - onFocus, - onGlossChange, - token, - }: { - gloss?: string; - onFocus?: () => void; - onGlossChange?: (value: string) => void; - token: Token; - }) => ( - - {token.surfaceText} - onGlossChange?.(e.target.value)} - onFocus={onFocus} - value={gloss ?? ''} - /> - - ), -})); +// --------------------------------------------------------------------------- +// GlossStore mock — reactive useState-based stub so GlossStore.tsx stays out of scope +// --------------------------------------------------------------------------- + +jest.mock('../../components/GlossStore', () => { + const { createContext, useCallback, useContext, useMemo, useState } = + jest.requireActual('react'); + + type GlossMap = Record; + type MockCtxValue = { + glosses: GlossMap; + dispatch: (tokenId: string, value: string) => void; + }; + const MockCtx = createContext({ glosses: {}, dispatch: () => {} }); + + return { + __esModule: true, + GlossStoreProvider({ + children, + initialGlosses, + onGlossChange, + }: Readonly<{ + children: ReactNode; + initialGlosses?: GlossMap; + onGlossChange?: (tokenId: string, value: string) => void; + }>) { + const [glosses, setGlosses] = useState(initialGlosses ?? {}); + const dispatch = useCallback( + (tokenId: string, value: string) => { + setGlosses((prev) => ({ ...prev, [tokenId]: value })); + onGlossChange?.(tokenId, value); + }, + [onGlossChange], + ); + const ctx = useMemo(() => ({ glosses, dispatch }), [glosses, dispatch]); + return {children}; + }, + useGloss(tokenId: string) { + return useContext(MockCtx).glosses[tokenId] ?? ''; + }, + useGlossDispatch() { + return useContext(MockCtx).dispatch; + }, + }; +}); + +jest.mock('../../components/TokenChip', () => { + const { useGloss, useGlossDispatch } = jest.requireMock< + typeof import('../../components/GlossStore') + >('../../components/GlossStore'); + function MockTokenChip({ onFocus, token }: Readonly<{ onFocus?: () => void; token: Token }>) { + const gloss = useGloss(token.id); + const dispatch = useGlossDispatch(); + return ( + + {token.surfaceText} + dispatch(token.id, e.target.value)} + onFocus={onFocus} + value={gloss} + /> + + ); + } + return { __esModule: true, default: MockTokenChip }; +}); /** Pre-built test token */ const TEST_TOKEN = { @@ -54,10 +99,9 @@ const TEST_TOKEN_2 = { /** Shared props shape used by both helper functions. */ type PhraseBoxTestProps = { - glosses: Record; + index: number | undefined; isFocused: boolean; - onClick?: (index?: number) => void; - onGlossChange: (tokenId: string, value: string) => void; + onClick: (index?: number) => void; tokens: (Token & { type: 'word' })[]; }; @@ -69,39 +113,32 @@ type PhraseBoxTestProps = { */ function requiredProps(): PhraseBoxTestProps { return { - glosses: {}, + index: undefined, isFocused: false, onClick: jest.fn(), - onGlossChange: jest.fn(), - tokens: [TEST_TOKEN], - }; -} - -/** - * Props with onClick omitted so the non-interactive span branch is rendered. - * - * @returns Props without an onClick handler. - */ -function nonInteractiveProps(): PhraseBoxTestProps { - return { - glosses: {}, - isFocused: false, - onGlossChange: jest.fn(), tokens: [TEST_TOKEN], }; } describe('PhraseBox', () => { it('renders as a button', () => { - render(); + render( + + + , + ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox?.tagName).toBe('BUTTON'); expect(phraseBox).toHaveAttribute('type', 'button'); }); - it('renders tokens using TokenChip components', () => { - render(); + it('renders one TokenChip per token in the tokens array', () => { + render( + + + , + ); expect(screen.getByTestId('token-token-1')).toBeInTheDocument(); expect(screen.getByTestId('token-token-2')).toBeInTheDocument(); @@ -109,7 +146,11 @@ describe('PhraseBox', () => { it('calls onClick when button is clicked', async () => { const mockOnClick = jest.fn(); - render(); + render( + + + , + ); const button = screen.getByRole('button'); await userEvent.click(button); @@ -118,7 +159,11 @@ describe('PhraseBox', () => { }); it('applies focused border and background when isFocused is true', () => { - render(); + render( + + + , + ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox).toHaveAttribute('data-focus-state', 'focused'); @@ -128,7 +173,11 @@ describe('PhraseBox', () => { }); it('applies default border and background when isFocused is false', () => { - render(); + render( + + + , + ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox).toHaveAttribute('data-focus-state', 'default'); @@ -137,70 +186,84 @@ describe('PhraseBox', () => { expect(phraseBox).toHaveClass('tw:bg-muted/20'); }); - it('button has correct focused styling and cursor', () => { - render(); + it('button always has cursor-pointer and text-left classes', () => { + render( + + + , + ); const button = screen.getByRole('button'); - expect(button).toHaveAttribute('data-focus-state', 'focused'); - expect(button).toHaveClass('tw:border-2'); expect(button).toHaveClass('tw:cursor-pointer'); expect(button).toHaveClass('tw:text-left'); }); - it('button has hover styling', () => { - render(); + it('button always has hover styling classes', () => { + render( + + + , + ); const button = screen.getByRole('button'); expect(button).toHaveClass('tw:hover:bg-muted/30'); }); - it('renders multiple tokens in order', () => { - render(); + it('renders tokens in the order they appear in the tokens array', () => { + render( + + + , + ); const tokens = document.querySelectorAll('[data-testid^="token-"]'); expect(tokens[0]).toHaveAttribute('data-testid', 'token-token-1'); expect(tokens[1]).toHaveAttribute('data-testid', 'token-token-2'); }); - it('passes the gloss for each token from the glosses map', () => { + it('passes the gloss for each token from the store', () => { render( - , + + + , ); expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveValue('hello'); expect(screen.getByRole('textbox', { name: 'Gloss for World' })).toHaveValue('world'); }); - it('passes an undefined gloss when the token id is absent from the glosses map', () => { - render(); + it('shows an empty string when the token id is absent from the store', () => { + render( + + + , + ); expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveValue(''); }); - it('calls onGlossChange with the token id and new value when a gloss input changes', async () => { - const handleGlossChange = jest.fn(); + it('updates the store when a gloss input changes', async () => { + const spy = jest.fn(); render( - , + + + , ); await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for Hello' }), 'hi'); - expect(handleGlossChange).toHaveBeenCalledTimes(2); - expect(handleGlossChange).toHaveBeenNthCalledWith(1, 'token-1', 'h'); - expect(handleGlossChange).toHaveBeenNthCalledWith(2, 'token-1', 'i'); + expect(spy).toHaveBeenCalledTimes(2); + expect(spy).toHaveBeenNthCalledWith(1, 'token-1', 'h'); + expect(spy).toHaveBeenNthCalledWith(2, 'token-1', 'hi'); }); it('calls onClick with index when a gloss input receives focus', async () => { const handleClick = jest.fn(); - render(); + render( + + + , + ); await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for Hello' })); @@ -208,43 +271,24 @@ describe('PhraseBox', () => { }); it('button always has tabIndex -1 so tab focus goes only to gloss inputs', () => { - render(); + render( + + + , + ); expect(screen.getByRole('button')).toHaveAttribute('tabindex', '-1'); }); - it('applies base spacing classes to the button', () => { - render(); + it('button always has padding and gap spacing classes', () => { + render( + + + , + ); const button = screen.getByRole('button'); expect(button).toHaveClass('tw:px-1'); expect(button).toHaveClass('tw:py-0.5'); }); - - describe('non-interactive (no onClick)', () => { - it('renders as a span when onClick is not provided', () => { - render(); - - const phraseBox = document.querySelector('[data-phrase-box="true"]'); - expect(phraseBox?.tagName).toBe('SPAN'); - }); - - it('applies focused styling to the span when isFocused is true', () => { - render(); - - const phraseBox = document.querySelector('[data-phrase-box="true"]'); - expect(phraseBox).toHaveAttribute('data-focus-state', 'focused'); - expect(phraseBox).toHaveClass('tw:border-2'); - expect(phraseBox).toHaveClass('tw:border-white'); - }); - - it('applies default styling to the span when isFocused is false', () => { - render(); - - const phraseBox = document.querySelector('[data-phrase-box="true"]'); - expect(phraseBox).toHaveAttribute('data-focus-state', 'default'); - expect(phraseBox).toHaveClass('tw:border'); - expect(phraseBox).toHaveClass('tw:border-border/40'); - }); - }); }); diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index fb804be2..39d6e020 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -5,25 +5,36 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { ScriptureRef, Segment, Token } from 'interlinearizer'; +import type { ReactNode } from 'react'; +import { GlossStoreProvider } from '../../components/GlossStore'; import { SegmentView } from '../../components/SegmentView'; +// --------------------------------------------------------------------------- +// GlossStore mock — pass-through provider so GlossStore.tsx stays out of scope +// --------------------------------------------------------------------------- + +jest.mock('../../components/GlossStore', () => ({ + __esModule: true, + GlossStoreProvider({ children }: Readonly<{ children: ReactNode }>) { + return children; + }, + useGloss: () => '', + useGlossDispatch: () => () => {}, +})); + jest.mock('../../components/PhraseBox', () => ({ __esModule: true, default: ({ - glosses, index, isFocused = false, onClick, - onGlossChange, tokens, - }: { - glosses: Record; + }: Readonly<{ index?: number; isFocused: boolean; onClick?: (index?: number) => void; - onGlossChange: (tokenId: string, value: string) => void; tokens: Token[]; - }) => ( + }>) => ( {tokens.map((t) => ( @@ -34,11 +45,6 @@ jest.mock('../../components/PhraseBox', () => ({ ) : ( {t.surfaceText} )} - onGlossChange?.(t.id, e.target.value)} - value={glosses?.[t.id] ?? ''} - /> ))} @@ -47,8 +53,8 @@ jest.mock('../../components/PhraseBox', () => ({ jest.mock('../../components/TokenChip', () => ({ __esModule: true, - default: ({ token }: { token: Token }) => {token.surfaceText}, - TokenChip: ({ token }: { token: Token }) => {token.surfaceText}, + default: ({ token }: Readonly<{ token: Token }>) => {token.surfaceText}, + TokenChip: ({ token }: Readonly<{ token: Token }>) => {token.surfaceText}, })); /** A word token segment. */ @@ -104,18 +110,14 @@ const PUNCT_SEGMENT: Segment = { function requiredProps(): { displayMode: 'token-chip'; focusedTokenId: string | undefined; - glosses: Record; isActive: boolean; - onGlossChange: (tokenId: string, value: string) => void; onSelect: (ref: ScriptureRef, tokenId?: string) => void; segment: Segment; } { return { displayMode: 'token-chip', focusedTokenId: undefined, - glosses: {}, isActive: false, - onGlossChange: jest.fn(), onSelect: jest.fn(), segment: WORD_SEGMENT, }; @@ -123,58 +125,72 @@ function requiredProps(): { describe('SegmentView', () => { it('renders word token chips in token-chip mode (default)', () => { - render(); + render( + + + , + ); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('the')).toBeInTheDocument(); }); it('renders non-word (punctuation) tokens in token-chip mode', () => { - render(); + render( + + + , + ); expect(screen.getByText('.')).toBeInTheDocument(); }); it('renders baselineText in baseline-text mode', () => { - render(); + render( + + + , + ); expect(screen.getByText('In the beginning.')).toBeInTheDocument(); }); it('does not render individual tokens in baseline-text mode', () => { - render(); + render( + + + , + ); expect(screen.queryByText('In')).not.toBeInTheDocument(); expect(screen.queryByText('the')).not.toBeInTheDocument(); }); it('shows the verse number label', () => { - render(); + render( + + + , + ); expect(screen.getByText('1')).toBeInTheDocument(); }); it('sets aria-current="true" when isActive is true', () => { - const { container } = render(); + const { container } = render( + + + , + ); expect(container.firstChild).toHaveAttribute('aria-current', 'true'); }); - it('does not set aria-current when isActive is false', () => { - const { container } = render(); - - expect(container.firstChild).not.toHaveAttribute('aria-current'); - }); - it('does not set aria-current when isActive is omitted', () => { - const { container } = render(); - - expect(container.firstChild).not.toHaveAttribute('aria-current'); - }); - - it('does not set aria-current on the baseline-text button when isActive is false', () => { const { container } = render( - , + + + , ); expect(container.firstChild).not.toHaveAttribute('aria-current'); @@ -182,7 +198,9 @@ describe('SegmentView', () => { it('sets aria-current="true" on the baseline-text button when isActive is true', () => { const { container } = render( - , + + + , ); expect(container.firstChild).toHaveAttribute('aria-current', 'true'); @@ -191,7 +209,9 @@ describe('SegmentView', () => { it('calls onSelect when clicked in baseline-text mode', async () => { const handleSelect = jest.fn(); render( - , + + + , ); await userEvent.click(screen.getByTestId('segment-container')); @@ -202,7 +222,11 @@ describe('SegmentView', () => { it('calls onSelect with the verse ref and token id when a word token is clicked', async () => { const handleSelect = jest.fn(); - render(); + render( + + + , + ); await userEvent.click(screen.getByRole('button', { name: 'In' })); @@ -211,32 +235,12 @@ describe('SegmentView', () => { }); it('renders word tokens as interactive buttons when onSelect is provided', () => { - render(); - - expect(screen.getByRole('button', { name: 'In' })).toBeInTheDocument(); - }); - - it('passes glosses to word token inputs', () => { - render(); - - expect(screen.getByRole('textbox', { name: 'Gloss for In' })).toHaveValue('In'); - expect(screen.getByRole('textbox', { name: 'Gloss for the' })).toHaveValue('the'); - }); - - it('calls onGlossChange with the token id and new value when a gloss changes', async () => { - const handleGlossChange = jest.fn(); render( - , + + + , ); - await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for In' }), 'In'); - - expect(handleGlossChange).toHaveBeenCalledTimes(2); - expect(handleGlossChange).toHaveBeenNthCalledWith(1, 'tok-0', 'I'); - expect(handleGlossChange).toHaveBeenNthCalledWith(2, 'tok-0', 'n'); + expect(screen.getByRole('button', { name: 'In' })).toBeInTheDocument(); }); }); diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index aa8f2e7f..a2db41db 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -5,8 +5,56 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { Token } from 'interlinearizer'; +import type { ReactNode } from 'react'; +import { GlossStoreProvider } from '../../components/GlossStore'; import { TokenChip } from '../../components/TokenChip'; +// --------------------------------------------------------------------------- +// GlossStore mock — reactive useState-based stub so GlossStore.tsx stays out of scope +// --------------------------------------------------------------------------- + +jest.mock('../../components/GlossStore', () => { + const { createContext, useCallback, useContext, useMemo, useState } = + jest.requireActual('react'); + + type GlossMap = Record; + type MockCtxValue = { + glosses: GlossMap; + dispatch: (tokenId: string, value: string) => void; + }; + const MockCtx = createContext({ glosses: {}, dispatch: () => {} }); + + return { + __esModule: true, + GlossStoreProvider({ + children, + initialGlosses, + onGlossChange, + }: Readonly<{ + children: ReactNode; + initialGlosses?: GlossMap; + onGlossChange?: (tokenId: string, value: string) => void; + }>) { + const [glosses, setGlosses] = useState(initialGlosses ?? {}); + const dispatch = useCallback( + (tokenId: string, value: string) => { + setGlosses((prev) => ({ ...prev, [tokenId]: value })); + onGlossChange?.(tokenId, value); + }, + [onGlossChange], + ); + const ctx = useMemo(() => ({ glosses, dispatch }), [glosses, dispatch]); + return {children}; + }, + useGloss(tokenId: string) { + return useContext(MockCtx).glosses[tokenId] ?? ''; + }, + useGlossDispatch() { + return useContext(MockCtx).dispatch; + }, + }; +}); + const WORD_TOKEN = { id: 'GEN 1:1:0', surfaceText: 'hello', @@ -34,75 +82,106 @@ const PUNCT_TOKEN = { function requiredWordProps() { return { token: WORD_TOKEN, - gloss: '', onFocus: jest.fn(), - onGlossChange: jest.fn(), } as const; } describe('TokenChip', () => { it('renders the surface text for a word token', () => { - render(); + render( + + + , + ); expect(screen.getByText('hello')).toBeInTheDocument(); }); it('renders the surface text for a punctuation token', () => { - render(); + render( + + + , + ); expect(screen.getByText('.')).toBeInTheDocument(); }); it('applies a border class to word tokens', () => { - render(); + render( + + + , + ); // The outer container holds the border; the inner span is just the surface text const outer = screen.getByText('hello').closest('span')?.parentElement; expect(outer?.className).toContain('tw:border'); }); it('does not apply a border class to punctuation tokens', () => { - render(); + render( + + + , + ); const span = screen.getByText('.'); expect(span.className).not.toContain('tw:border'); }); - it('renders word and punctuation tokens as inline spans', () => { - const { container: wc } = render(); - const { container: pc } = render(); - expect(wc.querySelector('span')).toBeInTheDocument(); - expect(pc.querySelector('span')).toBeInTheDocument(); - }); - it('renders a gloss input for word tokens', () => { - render(); + render( + + + , + ); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toBeInTheDocument(); }); it('does not render a gloss input for punctuation tokens', () => { - render(); + render( + + + , + ); expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); }); - it('shows the current gloss value in the input', () => { - render(); + it('shows the current gloss value from the store', () => { + render( + + + , + ); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue('in'); }); - it('shows an empty string in the input when gloss is empty', () => { - render(); + it('shows an empty string in the input when no gloss has been set', () => { + render( + + + , + ); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue(''); }); - it('calls onGlossChange for each keystroke', async () => { - const handleChange = jest.fn(); - render(); + it('calls the store onGlossChange spy with tokenId and value for each keystroke', async () => { + const spy = jest.fn(); + render( + + + , + ); await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); - expect(handleChange).toHaveBeenCalledTimes(2); - expect(handleChange).toHaveBeenNthCalledWith(1, 'i'); - expect(handleChange).toHaveBeenNthCalledWith(2, 'n'); + expect(spy).toHaveBeenCalledTimes(2); + expect(spy).toHaveBeenNthCalledWith(1, 'GEN 1:1:0', 'i'); + expect(spy).toHaveBeenNthCalledWith(2, 'GEN 1:1:0', 'in'); }); it('calls onFocus when the input is focused', async () => { const handleFocus = jest.fn(); - render(); + render( + + + , + ); await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' })); expect(handleFocus).toHaveBeenCalledTimes(1); }); diff --git a/src/__tests__/hooks/useInterlinearizerBookData.test.ts b/src/__tests__/hooks/useInterlinearizerBookData.test.ts index 1b8c9949..9c2540c1 100644 --- a/src/__tests__/hooks/useInterlinearizerBookData.test.ts +++ b/src/__tests__/hooks/useInterlinearizerBookData.test.ts @@ -111,7 +111,7 @@ describe('useInterlinearizerBookData', () => { setupDefaultProjectSettingMock(); }); - it('returns isLoading=true and no book or error while data is still loading', () => { + it('returns isLoading=true and no book when USJ data has not arrived', () => { jest.mocked(useProjectData).mockReturnValue({ BookUSJ: () => [undefined, jest.fn(), true] }); jest.mocked(extractBookFromUsj).mockReturnValue(TEST_RAW_BOOK); diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index cff42706..75cc4735 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -1,19 +1,9 @@ import type { Book, ScriptureRef, Token } from 'interlinearizer'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import type { GlossHandlers } from './component-types'; +import { isWordToken } from './component-types'; import MemoizedPhraseBox from './PhraseBox'; import MemoizedTokenChip from './TokenChip'; -/** - * Narrows a `Token` to a word token. - * - * @param token - The token to test. - * @returns `true` when `token.type === 'word'`. - */ -function isWordToken(token: Token): token is Token & { type: 'word' } { - return token.type === 'word'; -} - /** * Clamps `index` to `[0, len - 1]`, returning `0` when `len` is zero. * @@ -45,15 +35,13 @@ const STRIP_FADE_MS = 500; const PHRASE_WINDOW_HALF = 100; /** Props for {@link ContinuousView}. */ -type ContinuousViewProps = Readonly< - GlossHandlers & { - activePhraseIndex: number | undefined; - activeVerse: ScriptureRef; - book: Book; - onFocusPhraseIndexChange: (index: number) => void; - onVerseChange: (verse: ScriptureRef) => void; - } ->; +type ContinuousViewProps = Readonly<{ + activePhraseIndex: number | undefined; + activeVerse: ScriptureRef; + book: Book; + onFocusPhraseIndexChange: (index: number) => void; + onVerseChange: (verse: ScriptureRef) => void; +}>; /** * Renders all tokens from every segment in the given book as a single flat, horizontally scrollable @@ -75,10 +63,8 @@ type ContinuousViewProps = Readonly< * @param props.activeVerse - Verse coordinate; when it changes the strip scrolls to the first token * of the matching segment * @param props.book - The full tokenized book whose tokens should be streamed - * @param props.glosses - Map from `Token.id` to current English gloss text * @param props.onFocusPhraseIndexChange - Called whenever the focused phrase index changes so the * parent can mirror the strip position - * @param props.onGlossChange - Called with the token id and new gloss value when a gloss is edited * @param props.onVerseChange - Called when arrow navigation moves the focus into a new verse * @returns A horizontal token strip with previous/next navigation arrows and edge-fade overlays */ @@ -86,9 +72,7 @@ export default function ContinuousView({ activePhraseIndex, activeVerse, book, - glosses, onFocusPhraseIndexChange, - onGlossChange, onVerseChange, }: ContinuousViewProps) { const isRtl = document.documentElement.dir === 'rtl'; @@ -242,6 +226,14 @@ export default function ContinuousView({ */ const lastInternalVerseRef = useRef(activeVerse); + // These two effects (activePhraseIndex and activeVerse) could theoretically race: if both props + // changed in one render, the activeVerse effect would overwrite the activePhraseIndex jump, + // scrolling to verse-start rather than the exact token. This is safe because Interlinearizer + // only passes activePhraseIndex when continuousScroll is false (segment mode), where ContinuousView + // is unmounted. When continuousScroll is true, SegmentView renders in baseline-text mode and + // onSelect is called without a tokenId, so activePhraseIndex is never set from within continuous + // mode. Any future change that adds token-level clicks in continuous mode must revisit this. + // Jump to a specific phrase index when activePhraseIndex changes. useEffect(() => { if (activePhraseIndex === undefined) return; @@ -345,6 +337,9 @@ export default function ContinuousView({ /** Ref mirror of `onFocusPhraseIndexChange` so the notification effect never needs it as a dep. */ const onFocusPhraseIndexChangeRef = useRef(onFocusPhraseIndexChange); onFocusPhraseIndexChangeRef.current = onFocusPhraseIndexChange; + // Intentionally fires on mount with the lazy-initialized focusPhraseIndex. This notifies the + // parent of the initial strip position so the segment list scrolls the active verse into view + // on first render. The coupling is load-bearing — do not add an early-return guard here. useEffect(() => { onFocusPhraseIndexChangeRef.current(focusPhraseIndex); }, [focusPhraseIndex]); @@ -364,6 +359,10 @@ export default function ContinuousView({ const windowStartTokenIndex = phraseEntries.length > 0 && windowStart > 0 ? phraseEntries[windowStart].tokenIndex : 0; + // windowEndTokenIndex stops at the last word token in the window, so punctuation tokens that + // trail it (before the next word) are excluded from the rendered slice. Punctuation before the + // window's first word IS included (windowStartTokenIndex points at the word itself). This + // asymmetry is invisible with PHRASE_WINDOW_HALF=100 but would matter if the window shrinks. /** Token index one past the last token in the rendered window. */ const windowEndTokenIndex = phraseEntries.length > 0 && windowEnd < phraseEntries.length - 1 @@ -487,11 +486,9 @@ export default function ContinuousView({ }} > diff --git a/src/components/GlossStore.tsx b/src/components/GlossStore.tsx new file mode 100644 index 00000000..3d29d13e --- /dev/null +++ b/src/components/GlossStore.tsx @@ -0,0 +1,159 @@ +/** @file External gloss store with per-token subscriptions via `useSyncExternalStore`. */ +import { + createContext, + useCallback, + useContext, + useMemo, + useRef, + useSyncExternalStore, +} from 'react'; +import type { ReactNode } from 'react'; + +/** Shape of the value provided by {@link GlossStoreProvider}. */ +type GlossStoreContextValue = { + /** Registers a listener that fires whenever any gloss changes. Returns an unsubscribe function. */ + subscribe: (onStoreChange: () => void) => () => void; + /** Returns the current gloss for `tokenId`, or `''` when absent. */ + getGloss: (tokenId: string) => string; + /** Returns the entire current gloss map. Creates a new object reference on each call. */ + getAllGlosses: () => Record; + /** Writes a gloss value and notifies all subscribers. */ + onGlossChange: (tokenId: string, value: string) => void; +}; + +const GlossStoreCtx = createContext(undefined); + +/** Props for {@link GlossStoreProvider}. */ +type GlossStoreProviderProps = Readonly<{ + children: ReactNode; + /** + * Optional initial gloss values. Intended for test seeding only — not reactive; changes after + * mount are ignored. + */ + initialGlosses?: Record; + /** + * Optional spy called after each store write. Intended for test observability only — has no + * effect on store behaviour. + */ + onGlossChange?: (tokenId: string, value: string) => void; +}>; + +/** + * Provides an external gloss store to the subtree. Components inside can read per-token gloss + * values via {@link useGloss} and write them via {@link useGlossDispatch}. + * + * @param props - Component props + * @param props.children - Subtree that should have access to the gloss store + * @param props.initialGlosses - Seed values for tests; not reactive after mount + * @param props.onGlossChange - Spy called after each store write; for test observability only + * @returns A context provider wrapping the subtree + */ +export function GlossStoreProvider({ + children, + initialGlosses, + onGlossChange: spy, +}: GlossStoreProviderProps) { + const glossesRef = useRef>(initialGlosses ?? {}); + // Stable snapshot reference for useAllGlosses; only replaced on mutation so useSyncExternalStore + // can detect changes via reference equality without an infinite-loop. + const allGlossesSnapshotRef = useRef>(glossesRef.current); + const listenersRef = useRef(new Set<() => void>()); + + /** + * Registers `listener` to be called whenever any gloss changes. Returns an unsubscribe function + * that removes the listener from the set. + * + * @param listener - Zero-argument callback invoked after every store mutation. + * @returns An unsubscribe function that, when called, removes the listener. + */ + const subscribe = useCallback((listener: () => void) => { + listenersRef.current.add(listener); + return () => { + listenersRef.current.delete(listener); + }; + }, []); + + /** + * Returns the current gloss string for `tokenId`, or `''` when no gloss has been set. + * + * @param tokenId - The token whose gloss to retrieve. + * @returns The stored gloss, or `''` when absent. + */ + const getGloss = useCallback((tokenId: string) => glossesRef.current[tokenId] ?? '', []); + + /** + * Returns the current gloss snapshot object. The same reference is returned on every call until + * the next mutation, so `useSyncExternalStore` can detect changes via reference equality. + * + * @returns The current gloss map; a new object reference is produced on each mutation. + */ + const getAllGlosses = useCallback(() => allGlossesSnapshotRef.current, []); + + /** + * Writes `value` as the gloss for `tokenId`, replaces the snapshot reference, and notifies all + * subscribers. Also calls the optional `spy` prop for test observability. + * + * @param tokenId - The token whose gloss to update. + * @param value - The new gloss string. + */ + const onGlossChange = useCallback( + (tokenId: string, value: string) => { + glossesRef.current = { ...glossesRef.current, [tokenId]: value }; + allGlossesSnapshotRef.current = glossesRef.current; + listenersRef.current.forEach((l) => l()); + spy?.(tokenId, value); + }, + [spy], + ); + + const ctx = useMemo( + () => ({ subscribe, getGloss, getAllGlosses, onGlossChange }), + [subscribe, getGloss, getAllGlosses, onGlossChange], + ); + + return {children}; +} + +/** + * Returns the gloss string for the given token, re-rendering only when that token's gloss changes. + * + * @param tokenId - The token whose gloss to read. + * @returns The current gloss string, or `''` when no gloss has been set. + * @throws When called outside a {@link GlossStoreProvider}. + */ +export function useGloss(tokenId: string): string { + const ctx = useContext(GlossStoreCtx); + if (!ctx) throw new Error('useGloss must be used inside a GlossStoreProvider'); + + // Memoize snapshot by tokenId so useSyncExternalStore compares only the one string. + const getSnapshot = useMemo(() => () => ctx.getGloss(tokenId), [ctx, tokenId]); + + return useSyncExternalStore(ctx.subscribe, getSnapshot); +} + +/** + * Returns the entire gloss map, re-rendering on every gloss change. Intended for components that + * need all glosses (e.g. `ContinuousView` before it is migrated to per-token subscriptions). + * + * @returns A shallow copy of the current gloss record. + * @throws When called outside a {@link GlossStoreProvider}. + */ +export function useAllGlosses(): Record { + const ctx = useContext(GlossStoreCtx); + if (!ctx) throw new Error('useAllGlosses must be used inside a GlossStoreProvider'); + + return useSyncExternalStore(ctx.subscribe, ctx.getAllGlosses); +} + +/** + * Returns the stable `onGlossChange` callback from the nearest {@link GlossStoreProvider}. + * + * @returns A function `(tokenId, value) => void` that writes the gloss and notifies subscribers. + * @throws When called outside a {@link GlossStoreProvider}. + */ +export function useGlossDispatch(): (tokenId: string, value: string) => void { + const ctx = useContext(GlossStoreCtx); + if (!ctx) throw new Error('useGlossDispatch must be used inside a GlossStoreProvider'); + + return ctx.onGlossChange; +} diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index e0538adb..3da641e4 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -1,38 +1,34 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; import type { Book, ScriptureRef, Segment } from 'interlinearizer'; +import { LocateFixed } from 'lucide-react'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import ContinuousView from './ContinuousView'; +import { GlossStoreProvider } from './GlossStore'; import MemoizedSegmentView from './SegmentView'; /** Props for {@link Interlinearizer}. */ type InterlinearizerProps = Readonly<{ book: Book; - bookSegments: Segment[]; + chapterSegments: Segment[]; continuousScroll: boolean; scrRef: SerializedVerseRef; setScrRef: (newScrRef: SerializedVerseRef) => void; }>; /** - * Main content area for the Interlinearizer. Renders an optional {@link ContinuousView} strip at the - * top followed by a scrollable list of {@link MemoizedSegmentView}s for the current chapter. + * Inner component that renders the segment list and continuous view. Separated from + * {@link Interlinearizer} so it can consume the `GlossStoreProvider` context that wraps it. * - * @param props - Component props - * @param props.book - Book data used by the continuous view - * @param props.bookSegments - Segments to render as individual verse views - * @param props.continuousScroll - Whether the continuous scroll view is shown - * @param props.scrRef - Current scripture reference - * @param props.setScrRef - Callback to update the scripture reference - * @returns The full interlinearizer layout with optional continuous strip and segment list + * @param props - Same props as {@link Interlinearizer}. + * @returns The interlinearizer layout without the provider wrapper. */ -export default function Interlinearizer({ +function InterlinearizerInner({ book, - bookSegments, + chapterSegments, continuousScroll, scrRef, setScrRef, }: InterlinearizerProps) { - const [glosses, setGlosses] = useState>({}); const [focusedTokenId, setFocusedTokenId] = useState(undefined); /** All word tokens in book order — index into this array is the phrase index. */ @@ -51,45 +47,28 @@ export default function Interlinearizer({ [wordTokens], ); + // activePhraseIndex is intentionally not updated by ContinuousView arrow navigation — only token + // clicks via handleSegmentSelect change focusedTokenId. Arrow navigation updates + // continuousViewPhraseIndex instead (via handleFocusPhraseIndexChange), and the mode-switch + // effect reads continuousViewPhraseIndexRef to recover the strip position when returning to + // segment view. The stale activePhraseIndex during arrow navigation is safe because ContinuousView + // manages its own focusPhraseIndex state and the unchanged prop doesn't re-trigger its effect. const activePhraseIndex = focusedTokenId === undefined ? undefined : phraseIndexByTokenId.get(focusedTokenId); - /** The scrollable segment list div; targeted by `scrollActiveSegmentIntoView`. */ - const scrollContainerRef = useRef(undefined); - /** - * Ref callback that stores the scroll container element so imperative scroll calls can target it. - * - * @param el - The mounted div, or `null` on unmount. - */ - const setScrollContainer = useCallback((el: HTMLDivElement | null) => { - scrollContainerRef.current = el ?? undefined; - }, []); - - /** - * Tracks the continuous strip's current phrase index as reported by ContinuousView, so we know - * exactly where it is when the user switches to segment view and so the active segment scrolls - * into view when the focused phrase changes. - */ - const [continuousViewPhraseIndex, setContinuousViewPhraseIndex] = useState( - undefined, - ); - - /** - * Ref mirror of `continuousViewPhraseIndex`. Read inside the mode-switch effect (which omits the - * state variable as a dep) so it always sees the latest value without causing a re-run. + * Tracks the continuous strip's current phrase index as reported by ContinuousView. Read inside + * the mode-switch effect to recover the strip position when returning to segment view. */ const continuousViewPhraseIndexRef = useRef(undefined); /** - * Keeps `continuousViewPhraseIndex` state in sync with the strip's current position so the - * segment list can scroll the right verse into view when the focused phrase changes. + * Keeps `continuousViewPhraseIndexRef` in sync with the strip's current position. * * @param index - The phrase index now focused inside `ContinuousView`. */ const handleFocusPhraseIndexChange = useCallback((index: number) => { continuousViewPhraseIndexRef.current = index; - setContinuousViewPhraseIndex(index); }, []); /** @@ -98,6 +77,28 @@ export default function Interlinearizer({ */ const prevContinuousScrollRef = useRef(continuousScroll); + const scrollContainerRef = useRef(undefined); + + /** + * Ref callback that stores the scroll container element so imperative scroll calls can target it. + * + * @param el - The mounted div, or `null` on unmount. + */ + const setScrollContainer = useCallback((el: HTMLDivElement | null) => { + scrollContainerRef.current = el ?? undefined; + }, []); + + /** + * Scrolls the element marked `aria-current="true"` inside the scroll container into view at the + * top of the list. + */ + const snapToActive = useCallback(() => { + const container = scrollContainerRef.current; + const active = container?.querySelector('[aria-current="true"]'); + /* v8 ignore next -- active is always found when a verse is rendered; guard for empty lists */ + active?.scrollIntoView({ behavior: 'auto', block: 'start' }); + }, []); + // When switching from continuous to segment view, carry over the focused phrase from the strip. // Only acts on the continuous→segment transition; leaving segment view does not overwrite focus. useEffect(() => { @@ -117,42 +118,19 @@ export default function Interlinearizer({ // Fallback: only move to first word of the active segment if nothing is focused yet. setFocusedTokenId((current) => { if (current !== undefined) return current; - const activeSeg = bookSegments.find((seg) => seg.startRef.verse === scrRef.verseNum); + const activeSeg = chapterSegments.find((seg) => seg.startRef.verse === scrRef.verseNum); return activeSeg?.tokens.find((t) => t.type === 'word')?.id ?? current; }); // Only re-run when the mode switches; refs and stable arrays don't need to be deps. // eslint-disable-next-line react-hooks/exhaustive-deps }, [continuousScroll]); - /** - * Scrolls the element marked `aria-current="true"` inside the segment scroll container into view - * at the top of the list, so the active verse is always visible after a mode switch or focus - * change. - */ - const scrollActiveSegmentIntoView = useCallback(() => { - const container = scrollContainerRef.current; - const active = container?.querySelector('[aria-current="true"]'); - /* v8 ignore next -- active is always found when a verse is rendered; guard for empty lists */ - active?.scrollIntoView({ behavior: 'auto', block: 'start' }); - }, []); - - // Scroll the active segment into view on mode switch and whenever the focused phrase changes - // while in continuous mode (ContinuousView is only mounted when continuousScroll is true, so - // continuousViewPhraseIndex only changes in that context). + // Snap the segment list to the active verse when switching modes. useEffect(() => { - scrollActiveSegmentIntoView(); + snapToActive(); + // snapToActive is stable (useCallback with no changing deps), so this only re-runs on mode switch. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [continuousScroll, continuousViewPhraseIndex]); - - /** - * Merges an updated gloss value into the shared gloss map. - * - * @param tokenId - The id of the token whose gloss changed. - * @param value - The new gloss string entered by the user. - */ - const handleGlossChange = useCallback((tokenId: string, value: string) => { - setGlosses((prev) => ({ ...prev, [tokenId]: value })); - }, []); + }, [continuousScroll]); /** * Updates the active scripture reference and, when a specific token was clicked, focuses that @@ -169,6 +147,20 @@ export default function Interlinearizer({ [setScrRef], ); + /** + * Updates the active scripture reference when ContinuousView reports a verse change via arrow + * navigation. A separate wrapper from `handleSegmentSelect` because verse changes from the strip + * never carry a token id. + * + * @param ref - The new verse coordinate reported by the strip. + */ + const handleVerseChange = useCallback( + (ref: ScriptureRef) => { + handleSegmentSelect(ref); + }, + [handleSegmentSelect], + ); + return (
{continuousScroll && ( @@ -181,41 +173,71 @@ export default function Interlinearizer({ verse: scrRef.verseNum, }} book={book} - glosses={glosses} onFocusPhraseIndexChange={handleFocusPhraseIndexChange} - onGlossChange={handleGlossChange} - onVerseChange={handleSegmentSelect} + onVerseChange={handleVerseChange} />
)}
- {bookSegments.length === 0 && ( + {chapterSegments.length === 0 && (

No verse data for {scrRef.book} {scrRef.chapterNum}.

)} - {bookSegments.length > 0 && ( -
- {bookSegments.map((seg) => ( - - ))} -
+ {chapterSegments.length > 0 && ( + <> +
+ +
+ +
+ {chapterSegments.map((seg) => ( + + ))} +
+ )}
); } + +/** + * Main component for the Interlinearizer. Renders a sticky toolbar and continuous view at the top, + * followed by segmented views. Wraps the layout in a {@link GlossStoreProvider} so all descendant + * components can read and write gloss values without prop drilling. + * + * @param props - Component props + * @param props.book - Book data used by the continuous view + * @param props.chapterSegments - Segments to render as individual verse views + * @param props.continuousScroll - Whether the continuous scroll view is shown + * @param props.scrRef - Current scripture reference + * @param props.setScrRef - Callback to update the scripture reference + * @returns The full interlinearizer layout with optional continuous strip and segment list + */ +export default function Interlinearizer(props: InterlinearizerProps) { + return ( + + + + ); +} diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 6a5b1e29..37bb9421 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -180,7 +180,7 @@ export default function InterlinearizerLoader({ ) : ( void; - tokens: (Token & { type: 'word' })[]; - } ->; +type PhraseBoxProps = Readonly<{ + index: number | undefined; + isFocused: boolean; + onClick: (index?: number) => void; + tokens: (Token & { type: 'word' })[]; +}>; /** * Wraps one or more tokens in a phrase-level visual container. * * @param props - Component props - * @param props.glosses - Map from `Token.id` to current English gloss text for each token in this - * phrase. * @param props.index - Index passed back to `onClick` to identify which phrase was interacted with * @param props.isFocused - Whether this phrase is the current navigation focus * @param props.onClick - Called with `index` when the phrase is clicked; omit to render a * non-interactive span - * @param props.onGlossChange - Called with the token id and new gloss value when a gloss is edited. * @param props.tokens - Tokens belonging to this phrase * @returns A bordered inline container */ - -export function PhraseBox({ - glosses, - index, - isFocused = false, - onClick, - onGlossChange, - tokens, -}: PhraseBoxProps) { +export function PhraseBox({ index, isFocused = false, onClick, tokens }: PhraseBoxProps) { const baseClass = isFocused ? 'tw:inline-flex tw:items-center tw:rounded tw:border-2 tw:border-white tw:bg-muted/30 tw:px-1 tw:py-0.5' : 'tw:inline-flex tw:items-center tw:rounded tw:border tw:border-border/40 tw:bg-muted/20 tw:px-1 tw:py-0.5'; + + /** Forwards focus events on a child chip to the parent `onClick` handler with this phrase's index. */ + const handleFocus = useCallback(() => onClick?.(index), [onClick, index]); + /** + * Forwards click events on the phrase button to the parent `onClick` handler with this phrase's + * index. + */ + const handleClick = useCallback(() => onClick?.(index), [onClick, index]); + const innerContent = ( {tokens.map((token) => ( - onClick?.(index)} - onGlossChange={(value) => onGlossChange(token.id, value)} - token={token} - /> + ))} ); - if (onClick) { - return ( - - ); - } - return ( - {innerContent} - + ); } diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 2250c1d2..939deebb 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -1,19 +1,9 @@ -import type { ScriptureRef, Segment, Token } from 'interlinearizer'; +import type { ScriptureRef, Segment } from 'interlinearizer'; import { memo, useCallback, useMemo } from 'react'; -import type { GlossHandlers } from './component-types'; +import { isWordToken } from './component-types'; import MemoizedPhraseBox from './PhraseBox'; import MemoizedTokenChip from './TokenChip'; -/** - * Narrows a `Token` to a word token. - * - * @param token - The token to test. - * @returns `true` when `token.type === 'word'`. - */ -function isWordToken(token: Token): token is Token & { type: 'word' } { - return token.type === 'word'; -} - /** * The two display modes for {@link SegmentView}. * @@ -25,15 +15,13 @@ function isWordToken(token: Token): token is Token & { type: 'word' } { export type SegmentDisplayMode = 'token-chip' | 'baseline-text'; /** Props for {@link SegmentView}. */ -type SegmentViewProps = Readonly< - GlossHandlers & { - displayMode: SegmentDisplayMode; - focusedTokenId: string | undefined; - isActive: boolean; - onSelect: (ref: ScriptureRef, tokenId?: string) => void; - segment: Segment; - } ->; +type SegmentViewProps = Readonly<{ + displayMode: SegmentDisplayMode; + focusedTokenId: string | undefined; + isActive: boolean; + onSelect: (ref: ScriptureRef, tokenId?: string) => void; + segment: Segment; +}>; /** * Renders a single segment as either inline token chips or plain baseline text. @@ -42,10 +30,7 @@ type SegmentViewProps = Readonly< * @param props.displayMode - Controls how segment content is rendered * @param props.focusedTokenId - When set, the matching word token's `PhraseBox` is rendered in the * focused state; only meaningful in `token-chip` mode. - * @param props.glosses - Map from `Token.id` to current English gloss text for tokens in this - * segment. Pass an empty object when no glosses have been entered yet. * @param props.isActive - Whether this segment is the currently selected verse - * @param props.onGlossChange - Called with the token id and new gloss value when a gloss is edited. * @param props.onSelect - Called when the segment or one of its word tokens is interacted with. In * `baseline-text` mode the whole segment is clickable and `tokenId` is omitted. In `token-chip` * mode only word tokens trigger this callback and `tokenId` is always provided; omit to render @@ -57,9 +42,7 @@ type SegmentViewProps = Readonly< export function SegmentView({ displayMode, focusedTokenId, - glosses, isActive, - onGlossChange, onSelect, segment, }: SegmentViewProps) { @@ -113,6 +96,9 @@ export function SegmentView({ ); } + // Intentional: token-chip mode renders a div, not a button. In this mode individual word tokens + // (via PhraseBox gloss inputs) are the interactive elements, so the outer container does not need + // to be focusable. Keyboard access goes through the gloss inputs inside PhraseBox, not here. return (
) : ( diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index fd3806e8..6b5b47e7 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -1,38 +1,45 @@ import type { Token } from 'interlinearizer'; import { memo } from 'react'; +import { useGloss, useGlossDispatch } from './GlossStore'; -/** Props for a word token chip; requires gloss editing callbacks. */ +/** + * Props for a word token chip. Requires `onFocus` because gloss input focus must propagate to the + * parent `PhraseBox` for correct navigation state. + */ type WordProps = Readonly<{ + /** The word token to render. */ token: Token & { type: 'word' }; - gloss: string; + /** Called when the gloss input receives focus; used to notify the parent `PhraseBox`. */ onFocus: () => void; - onGlossChange: (value: string) => void; }>; -/** Props for a punctuation token chip; editing props are excluded via `never`. */ +/** + * Props for a punctuation token chip. `onFocus` is excluded via `never` to prevent callers from + * accidentally passing a focus handler that would be silently ignored. + */ type PunctProps = Readonly<{ + /** The punctuation token to render. */ token: Token; - gloss?: never; onFocus?: never; - onGlossChange?: never; }>; /** * Renders a single token as an inline chip. Word tokens get a bordered box with an editable gloss * input below; punctuation tokens are rendered as muted inline text with no gloss. * - * Props are a discriminated union on `token.type`: word tokens require `onGlossChange` and accept - * `gloss` and `onFocus`; punctuation tokens accept none of these. + * Props are a discriminated union on `token.type`: word tokens require `onFocus`; punctuation + * tokens accept none of these. Gloss value and dispatch are read from {@link GlossStoreProvider} + * context via {@link useGloss} and {@link useGlossDispatch}. * * @param props - Component props * @param props.token - The token to render; its `type` field discriminates the union. - * @param props.gloss - (word only) Current gloss text. Absent when no gloss has been set. * @param props.onFocus - (word only) Called when the gloss input receives focus. - * @param props.onGlossChange - (word only) Called with the new gloss value when the user edits the - * input. * @returns A styled inline block */ -export function TokenChip({ gloss, onFocus, onGlossChange, token }: WordProps | PunctProps) { +export function TokenChip({ onFocus, token }: WordProps | PunctProps) { + const gloss = useGloss(token.id); + const onGlossChange = useGlossDispatch(); + return token.type === 'word' ? ( @@ -43,7 +50,7 @@ export function TokenChip({ gloss, onFocus, onGlossChange, token }: WordProps | className="tw:mt-0.5 tw:rounded tw:border tw:border-border tw:bg-background tw:px-1 tw:text-center tw:text-sm tw:text-foreground tw:outline-none tw:focus:border-ring tw:focus:ring-1 tw:focus:ring-ring" style={{ fieldSizing: 'content', minWidth: '5ch' }} value={gloss} - onChange={(e) => onGlossChange?.(e.target.value)} + onChange={(e) => onGlossChange(token.id, e.target.value)} onFocus={onFocus} type="text" /> diff --git a/src/components/component-types.ts b/src/components/component-types.ts index 610765eb..3c59070b 100644 --- a/src/components/component-types.ts +++ b/src/components/component-types.ts @@ -1,10 +1,11 @@ +import type { Token } from 'interlinearizer'; + /** - * Props shared by components that display and edit token glosses. + * Narrows a `Token` to a word token. * - * @field glosses - Map from `Token.id` to current gloss text. - * @field onGlossChange - Called with the token id and new value when a gloss is edited. + * @param token - The token to test. + * @returns `true` when `token.type === 'word'`. */ -export type GlossHandlers = { - glosses: Record; - onGlossChange: (tokenId: string, value: string) => void; -}; +export function isWordToken(token: Token): token is Token & { type: 'word' } { + return token.type === 'word'; +} diff --git a/src/parsers/papi/usjBookExtractor.ts b/src/parsers/papi/usjBookExtractor.ts index 35b25aed..0340a8a8 100644 --- a/src/parsers/papi/usjBookExtractor.ts +++ b/src/parsers/papi/usjBookExtractor.ts @@ -181,7 +181,10 @@ function handleParaNode(node: UsjNode, state: TraversalState): void { if (node.content) traverse(node.content, state); } -/** Dispatch table mapping USJ node `type` strings to their traversal handlers. */ +/** + * Dispatch table mapping USJ node `type` strings to their traversal handler functions. Node types + * absent from this table (e.g. `char`) are handled generically by recursing into their `content`. + */ const NODE_HANDLERS: Partial void>> = { book: handleBookNode, chapter: handleChapterNode, From db628166a86b99c437e19ed6be0ef71b450848c5 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 20 May 2026 17:14:25 -0600 Subject: [PATCH 06/13] Split TokenChip into word and inert variants; convert PhraseBox to label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract `InertTokenChip` / `MemoizedInertTokenChip` for non-word tokens, removing the punctuation branch from `TokenChip` so it only handles words - Convert `PhraseBox` from a ` ) : ( @@ -51,12 +58,6 @@ jest.mock('../../components/PhraseBox', () => ({ ), })); -jest.mock('../../components/TokenChip', () => ({ - __esModule: true, - default: ({ token }: Readonly<{ token: Token }>) => {token.surfaceText}, - TokenChip: ({ token }: Readonly<{ token: Token }>) => {token.surfaceText}, -})); - /** A word token segment. */ const WORD_SEGMENT: Segment = { id: 'GEN 1:1', diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index a2db41db..16e8c265 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -7,7 +7,7 @@ import userEvent from '@testing-library/user-event'; import type { Token } from 'interlinearizer'; import type { ReactNode } from 'react'; import { GlossStoreProvider } from '../../components/GlossStore'; -import { TokenChip } from '../../components/TokenChip'; +import { InertTokenChip, TokenChip } from '../../components/TokenChip'; // --------------------------------------------------------------------------- // GlossStore mock — reactive useState-based stub so GlossStore.tsx stays out of scope @@ -64,90 +64,73 @@ const WORD_TOKEN = { charEnd: 5, } satisfies Token; -const PUNCT_TOKEN = { - id: 'GEN 1:1:5', - surfaceText: '.', - writingSystem: 'en', - type: 'punctuation', - charStart: 5, - charEnd: 6, -} satisfies Token; - /** - * Minimal required props for a word-token `TokenChip`. Spread into render calls so tests only need - * to override what they actually care about. + * Minimal required props for {@link TokenChip}. Spread into render calls so tests only need to + * override what they actually care about. * - * @returns An object with all required word-token props set to no-op stubs. + * @returns An object with all required props set to no-op stubs. */ -function requiredWordProps() { +function requiredProps() { return { token: WORD_TOKEN, onFocus: jest.fn(), } as const; } -describe('TokenChip', () => { - it('renders the surface text for a word token', () => { - render( - - - , - ); - expect(screen.getByText('hello')).toBeInTheDocument(); - }); +const PUNCT_TOKEN = { + id: 'GEN 1:1:p', + surfaceText: '.', + writingSystem: 'en', + type: 'punctuation', + charStart: 5, + charEnd: 6, +} satisfies Token; - it('renders the surface text for a punctuation token', () => { - render( - - - , - ); +describe('InertTokenChip', () => { + it('renders the surface text', () => { + render(); expect(screen.getByText('.')).toBeInTheDocument(); }); - it('applies a border class to word tokens', () => { - render( - - - , - ); - // The outer container holds the border; the inner span is just the surface text - const outer = screen.getByText('hello').closest('span')?.parentElement; - expect(outer?.className).toContain('tw:border'); + it('renders as an inline span', () => { + render(); + expect(screen.getByText('.').tagName).toBe('SPAN'); }); +}); - it('does not apply a border class to punctuation tokens', () => { +describe('TokenChip', () => { + it('renders the surface text', () => { render( - + , ); - const span = screen.getByText('.'); - expect(span.className).not.toContain('tw:border'); + expect(screen.getByText('hello')).toBeInTheDocument(); }); - it('renders a gloss input for word tokens', () => { + it('applies a border class to the outer container', () => { render( - + , ); - expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toBeInTheDocument(); + const outer = screen.getByText('hello').closest('span')?.parentElement; + expect(outer?.className).toContain('tw:border'); }); - it('does not render a gloss input for punctuation tokens', () => { + it('renders a gloss input', () => { render( - + , ); - expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); + expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toBeInTheDocument(); }); it('shows the current gloss value from the store', () => { render( - + , ); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue('in'); @@ -156,7 +139,7 @@ describe('TokenChip', () => { it('shows an empty string in the input when no gloss has been set', () => { render( - + , ); expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue(''); @@ -166,7 +149,7 @@ describe('TokenChip', () => { const spy = jest.fn(); render( - + , ); await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); @@ -179,7 +162,7 @@ describe('TokenChip', () => { const handleFocus = jest.fn(); render( - + , ); await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' })); diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index 75cc4735..84fc1ee7 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -2,7 +2,7 @@ import type { Book, ScriptureRef, Token } from 'interlinearizer'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { isWordToken } from './component-types'; import MemoizedPhraseBox from './PhraseBox'; -import MemoizedTokenChip from './TokenChip'; +import { MemoizedInertTokenChip } from './TokenChip'; /** * Clamps `index` to `[0, len - 1]`, returning `0` when `len` is zero. @@ -474,7 +474,7 @@ export default function ContinuousView({ > {allTokens.slice(windowStartTokenIndex, windowEndTokenIndex).map((token, i) => { const tokenIndex = windowStartTokenIndex + i; - if (!isWordToken(token)) return ; + if (!isWordToken(token)) return ; const phraseIndex = phraseIndexByTokenIndex.get(tokenIndex); const isFocusedPhrase = phraseIndex !== undefined && phraseIndex === focusPhraseIndex; @@ -488,7 +488,7 @@ export default function ContinuousView({ diff --git a/src/components/PhraseBox.tsx b/src/components/PhraseBox.tsx index af4a64bf..11017f16 100644 --- a/src/components/PhraseBox.tsx +++ b/src/components/PhraseBox.tsx @@ -7,7 +7,7 @@ import MemoizedTokenChip from './TokenChip'; type PhraseBoxProps = Readonly<{ index: number | undefined; isFocused: boolean; - onClick: (index?: number) => void; + onFocusPhrase: (index?: number) => void; tokens: (Token & { type: 'word' })[]; }>; @@ -15,45 +15,32 @@ type PhraseBoxProps = Readonly<{ * Wraps one or more tokens in a phrase-level visual container. * * @param props - Component props - * @param props.index - Index passed back to `onClick` to identify which phrase was interacted with + * @param props.index - Index passed back to `onFocusPhrase` to identify which phrase was focused * @param props.isFocused - Whether this phrase is the current navigation focus - * @param props.onClick - Called with `index` when the phrase is clicked; omit to render a - * non-interactive span + * @param props.onFocusPhrase - Called with `index` when any child gloss input receives focus * @param props.tokens - Tokens belonging to this phrase * @returns A bordered inline container */ -export function PhraseBox({ index, isFocused = false, onClick, tokens }: PhraseBoxProps) { +export function PhraseBox({ index, isFocused = false, onFocusPhrase, tokens }: PhraseBoxProps) { const baseClass = isFocused ? 'tw:inline-flex tw:items-center tw:rounded tw:border-2 tw:border-white tw:bg-muted/30 tw:px-1 tw:py-0.5' : 'tw:inline-flex tw:items-center tw:rounded tw:border tw:border-border/40 tw:bg-muted/20 tw:px-1 tw:py-0.5'; - /** Forwards focus events on a child chip to the parent `onClick` handler with this phrase's index. */ - const handleFocus = useCallback(() => onClick?.(index), [onClick, index]); - /** - * Forwards click events on the phrase button to the parent `onClick` handler with this phrase's - * index. - */ - const handleClick = useCallback(() => onClick?.(index), [onClick, index]); - - const innerContent = ( - - {tokens.map((token) => ( - - ))} - - ); + /** Notifies the parent when a child gloss input receives focus. */ + const handleFocus = useCallback(() => onFocusPhrase(index), [onFocusPhrase, index]); return ( - + + {tokens.map((token) => ( + + ))} + + ); } diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 939deebb..0a762980 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -2,7 +2,7 @@ import type { ScriptureRef, Segment } from 'interlinearizer'; import { memo, useCallback, useMemo } from 'react'; import { isWordToken } from './component-types'; import MemoizedPhraseBox from './PhraseBox'; -import MemoizedTokenChip from './TokenChip'; +import { MemoizedInertTokenChip } from './TokenChip'; /** * The two display modes for {@link SegmentView}. @@ -107,19 +107,18 @@ export function SegmentView({ > {verseLabel} - {segment.tokens.map((token, index) => - token.type === 'word' ? ( + {segment.tokens.map((token, index) => { + if (!isWordToken(token)) return ; + return ( - ) : ( - - ), - )} + ); + })}
); diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 6b5b47e7..43362f5b 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -3,46 +3,24 @@ import { memo } from 'react'; import { useGloss, useGlossDispatch } from './GlossStore'; /** - * Props for a word token chip. Requires `onFocus` because gloss input focus must propagate to the - * parent `PhraseBox` for correct navigation state. - */ -type WordProps = Readonly<{ - /** The word token to render. */ - token: Token & { type: 'word' }; - /** Called when the gloss input receives focus; used to notify the parent `PhraseBox`. */ - onFocus: () => void; -}>; - -/** - * Props for a punctuation token chip. `onFocus` is excluded via `never` to prevent callers from - * accidentally passing a focus handler that would be silently ignored. - */ -type PunctProps = Readonly<{ - /** The punctuation token to render. */ - token: Token; - onFocus?: never; -}>; - -/** - * Renders a single token as an inline chip. Word tokens get a bordered box with an editable gloss - * input below; punctuation tokens are rendered as muted inline text with no gloss. - * - * Props are a discriminated union on `token.type`: word tokens require `onFocus`; punctuation - * tokens accept none of these. Gloss value and dispatch are read from {@link GlossStoreProvider} - * context via {@link useGloss} and {@link useGlossDispatch}. + * Renders a single word token as an inline chip with an editable gloss input below the surface + * text. Gloss value and dispatch are read from {@link GlossStoreProvider} context via + * {@link useGloss} and {@link useGlossDispatch}. * * @param props - Component props - * @param props.token - The token to render; its `type` field discriminates the union. - * @param props.onFocus - (word only) Called when the gloss input receives focus. - * @returns A styled inline block + * @param props.token - The word token to render. + * @param props.onFocus - Called when the gloss input receives focus. + * @returns A styled label containing the surface text and a gloss input. */ -export function TokenChip({ onFocus, token }: WordProps | PunctProps) { +export function TokenChip({ + token, + onFocus, +}: Readonly<{ token: Token & { type: 'word' }; onFocus: () => void }>) { const gloss = useGloss(token.id); const onGlossChange = useGlossDispatch(); - - return token.type === 'word' ? ( - - + return ( + - ) : ( + + ); +} + +/** + * Renders a non-word token (e.g. punctuation) as muted inline monospace text with no gloss input. + * + * @param props - Component props + * @param props.token - The non-word token to render. + * @returns A muted inline span. + */ +export function InertTokenChip({ token }: Readonly<{ token: Token }>) { + return ( {token.surfaceText} @@ -65,3 +54,6 @@ export function TokenChip({ onFocus, token }: WordProps | PunctProps) { /** Memoized version of {@link TokenChip}; use this for all render-stable token lists. */ const MemoizedTokenChip = memo(TokenChip); export default MemoizedTokenChip; + +/** Memoized version of {@link InertTokenChip}; use this for all render-stable token lists. */ +export const MemoizedInertTokenChip = memo(InertTokenChip); From ffd572546bdadc9184a0d5b4503d62efdf61c724 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 20 May 2026 17:24:31 -0600 Subject: [PATCH 07/13] Minor adjustments --- src/__tests__/components/TokenChip.test.tsx | 4 ++-- src/components/SegmentView.tsx | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 16e8c265..f01c9a72 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -70,11 +70,11 @@ const WORD_TOKEN = { * * @returns An object with all required props set to no-op stubs. */ -function requiredProps() { +function requiredProps(): { token: Token & { type: 'word' }; onFocus: () => void } { return { token: WORD_TOKEN, onFocus: jest.fn(), - } as const; + }; } const PUNCT_TOKEN = { diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 0a762980..0bcdff46 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -31,10 +31,10 @@ type SegmentViewProps = Readonly<{ * @param props.focusedTokenId - When set, the matching word token's `PhraseBox` is rendered in the * focused state; only meaningful in `token-chip` mode. * @param props.isActive - Whether this segment is the currently selected verse - * @param props.onSelect - Called when the segment or one of its word tokens is interacted with. In - * `baseline-text` mode the whole segment is clickable and `tokenId` is omitted. In `token-chip` - * mode only word tokens trigger this callback and `tokenId` is always provided; omit to render - * word tokens as non-interactive spans. + * @param props.onSelect - Required callback invoked when the segment or one of its word tokens is + * interacted with. In `baseline-text` mode the whole segment is clickable and `tokenId` is + * omitted. In `token-chip` mode only word tokens trigger this callback and `tokenId` is always + * provided. * @param props.segment - The segment to render * @returns A button (baseline-text mode) or div (token-chip mode) containing a verse label and * segment content From 17a7ecdf372e86efd970f47f180d5cf8ddf3c2ee Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 21 May 2026 12:27:21 -0600 Subject: [PATCH 08/13] =?UTF-8?q?Replace=20GlossStore=20with=20AnalysisSto?= =?UTF-8?q?re;=20migrate=20token.id=20=E2=86=92=20token.ref?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `GlossStore` stored a flat string map keyed by `token.id`. `AnalysisStore` replaces it with a `TextAnalysis`-backed store: each gloss write creates a new approved `TokenAnalysis` + `TokenAnalysisLink`, the full analysis snapshot is accessible via `useAnalysis`, and `onSave` propagates changes to the caller for persistence. The store also accepts an `analysisLanguage` prop to support non-English glosses. - Token identity keys are migrated from token.id to token.ref throughout (`TokenChip`, `PhraseBox`, `SegmentView`, `ContinuousView`, `Interlinearizer`, and all corresponding tests). - Some unneeded tests removed and some adjustments to documents made --- __mocks__/lucide-react.tsx | 20 + .../components/AnalysisStore.test.tsx | 341 ++++++++++++++++++ .../components/ContinuousView.test.tsx | 86 ++--- src/__tests__/components/GlossStore.test.tsx | 188 ---------- .../components/Interlinearizer.test.tsx | 39 +- src/__tests__/components/PhraseBox.test.tsx | 129 ++++--- src/__tests__/components/SegmentView.test.tsx | 60 +-- src/__tests__/components/TokenChip.test.tsx | 80 ++-- .../hooks/useInterlinearizerBookData.test.ts | 27 +- src/components/AnalysisStore.tsx | 263 ++++++++++++++ src/components/ContinuousView.tsx | 11 +- src/components/GlossStore.tsx | 159 -------- src/components/Interlinearizer.tsx | 68 ++-- src/components/PhraseBox.tsx | 8 +- src/components/ProjectModals.tsx | 2 +- src/components/SegmentView.tsx | 31 +- src/components/TokenChip.tsx | 12 +- src/hooks/useInterlinearizerBookData.ts | 6 +- src/main.ts | 2 +- src/services/projectStorage.ts | 4 + src/types/interlinearizer.d.ts | 4 + 21 files changed, 943 insertions(+), 597 deletions(-) create mode 100644 src/__tests__/components/AnalysisStore.test.tsx delete mode 100644 src/__tests__/components/GlossStore.test.tsx create mode 100644 src/components/AnalysisStore.tsx delete mode 100644 src/components/GlossStore.tsx diff --git a/__mocks__/lucide-react.tsx b/__mocks__/lucide-react.tsx index a4229d63..e2b2bd69 100644 --- a/__mocks__/lucide-react.tsx +++ b/__mocks__/lucide-react.tsx @@ -14,3 +14,23 @@ import type { ReactElement } from 'react'; export function LocateFixed(props: Readonly<{ className?: string }>): ReactElement { return ; } + +/** + * Stub for the Info icon. + * + * @param props - SVG props forwarded from the component. + * @returns A ReactElement SVG element used as an info icon stub in tests. + */ +export function Info(props: Readonly<{ size?: number; className?: string }>): ReactElement { + return ; +} + +/** + * Stub for the Trash2 icon. + * + * @param props - SVG props forwarded from the component. + * @returns A ReactElement SVG element used as a trash icon stub in tests. + */ +export function Trash2(props: Readonly<{ size?: number; className?: string }>): ReactElement { + return ; +} diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx new file mode 100644 index 00000000..291deaac --- /dev/null +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -0,0 +1,341 @@ +/** @file Unit tests for components/AnalysisStore.tsx. */ +/// +/// + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import type { TextAnalysis, TokenAnalysis, TokenAnalysisLink } from 'interlinearizer'; +import { + AnalysisStoreProvider, + useAnalysis, + useGloss, + useGlossDispatch, +} from '../../components/AnalysisStore'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Builds a minimal `TextAnalysis` with a single approved `TokenAnalysis` for the given token. + * + * @param tokenRef - Token reference string. + * @param gloss - Gloss value for the `'en'` language key. + * @param surfaceText - Surface text of the token. + * @returns A `TextAnalysis` seeded with one approved token analysis. + */ +function makeAnalysisWithGloss( + tokenRef: string, + gloss: string, + surfaceText = 'word', +): TextAnalysis { + const ta: TokenAnalysis = { + id: `${tokenRef}-analysis`, + surfaceText, + gloss: { en: gloss }, + }; + const link: TokenAnalysisLink = { + analysisId: ta.id, + status: 'approved', + token: { tokenRef, surfaceText }, + }; + return { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [ta], + tokenAnalysisLinks: [link], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; +} + +/** + * Renders a component that displays the gloss for a single token, used to assert on `useGloss`. + * + * @param tokenRef - Token ref to subscribe to. + * @returns JSX element suitable for passing to `render`. + */ +function GlossReader({ tokenRef }: Readonly<{ tokenRef: string }>) { + const gloss = useGloss(tokenRef); + return {gloss}; +} + +/** + * Renders a component that displays the full analysis as JSON, used to assert on `useAnalysis`. + * + * @returns JSX element suitable for passing to `render`. + */ +function AnalysisReader() { + const analysis = useAnalysis(); + return {JSON.stringify(analysis)}; +} + +/** + * Renders a component that calls `useGlossDispatch` without a provider, used to assert the hook + * throws outside an {@link AnalysisStoreProvider}. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function DispatchUser() { + useGlossDispatch(); + return undefined; +} + +/** + * Renders a button that calls `useGlossDispatch` to write a gloss, used to test dispatch. + * + * @param props.tokenRef - Token ref to write. + * @param props.surfaceText - Surface text of the token. + * @param props.value - Gloss value to write. + * @returns JSX element suitable for passing to `render`. + */ +function GlossWriter({ + tokenRef, + surfaceText, + value, +}: Readonly<{ tokenRef: string; surfaceText: string; value: string }>) { + const dispatch = useGlossDispatch(); + return ( + + ); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('useGloss', () => { + it('returns an empty string for an unknown token', () => { + render( + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent(''); + }); + + it('returns the approved gloss from initialAnalysis', () => { + render( + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent('hello'); + }); + + it('returns empty string for a token with a non-approved link in initialAnalysis', () => { + const ta: TokenAnalysis = { id: 'ta-1', surfaceText: 'word', gloss: { en: 'hi' } }; + const link: TokenAnalysisLink = { + analysisId: 'ta-1', + status: 'suggested', + token: { tokenRef: 'tok-1', surfaceText: 'word' }, + }; + const analysis: TextAnalysis = { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [ta], + tokenAnalysisLinks: [link], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; + render( + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent(''); + }); + + it('updates when the subscribed token is glossed via dispatch', async () => { + render( + + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent(''); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(screen.getByTestId('gloss')).toHaveTextContent('world'); + }); + + it('does not re-render when a different token is glossed', async () => { + let renderCount = 0; + + function CountingGlossReader({ tokenRef }: Readonly<{ tokenRef: string }>) { + renderCount += 1; + const gloss = useGloss(tokenRef); + return {gloss}; + } + + render( + + + + , + ); + const initialRenderCount = renderCount; + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(renderCount).toBe(initialRenderCount); + }); + + it('uses the analysisLanguage prop to resolve the gloss', () => { + const ta: TokenAnalysis = { id: 'ta-1', surfaceText: 'mot', gloss: { fr: 'bonjour' } }; + const link: TokenAnalysisLink = { + analysisId: 'ta-1', + status: 'approved', + token: { tokenRef: 'tok-1', surfaceText: 'mot' }, + }; + const analysis: TextAnalysis = { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [ta], + tokenAnalysisLinks: [link], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; + render( + + + , + ); + expect(screen.getByTestId('gloss')).toHaveTextContent('bonjour'); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useGloss must be used inside an AnalysisStoreProvider', + ); + }); +}); + +describe('useAnalysis', () => { + it('returns an empty analysis when no initialAnalysis is provided', () => { + render( + + + , + ); + const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? ''); + expect(analysis.tokenAnalyses).toHaveLength(0); + expect(analysis.tokenAnalysisLinks).toHaveLength(0); + }); + + it('returns seeded analyses from initialAnalysis', () => { + const seed = makeAnalysisWithGloss('tok-1', 'hi'); + render( + + + , + ); + const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? ''); + expect(analysis.tokenAnalyses).toHaveLength(1); + expect(analysis.tokenAnalysisLinks).toHaveLength(1); + }); + + it('updates after a gloss write', async () => { + render( + + + + , + ); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? ''); + expect(analysis.tokenAnalyses).toHaveLength(1); + expect(analysis.tokenAnalyses[0].gloss).toStrictEqual({ en: 'world' }); + expect(analysis.tokenAnalysisLinks[0].status).toBe('approved'); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useAnalysis must be used inside an AnalysisStoreProvider', + ); + }); +}); + +describe('useGlossDispatch', () => { + it('creates a new approved TokenAnalysis on each write', async () => { + render( + + + + , + ); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? ''); + expect(analysis.tokenAnalyses).toHaveLength(2); + expect(analysis.tokenAnalysisLinks).toHaveLength(2); + analysis.tokenAnalysisLinks.forEach((link) => expect(link.status).toBe('approved')); + }); + + it('does not touch existing suggested analyses on write', async () => { + const suggested: TokenAnalysis = { + id: 'suggested-1', + surfaceText: 'word', + gloss: { en: 'old' }, + }; + const suggestedLink: TokenAnalysisLink = { + analysisId: 'suggested-1', + status: 'suggested', + token: { tokenRef: 'tok-1', surfaceText: 'word' }, + }; + const seed: TextAnalysis = { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [suggested], + tokenAnalysisLinks: [suggestedLink], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; + render( + + + + , + ); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? ''); + expect(analysis.tokenAnalyses).toHaveLength(2); + const suggestedEntry = analysis.tokenAnalysisLinks.find((l) => l.status === 'suggested'); + const approvedEntry = analysis.tokenAnalysisLinks.find((l) => l.status === 'approved'); + expect(suggestedEntry?.analysisId).toBe('suggested-1'); + expect(approvedEntry?.analysisId).not.toBe('suggested-1'); + }); + + it('calls the onGlossChange spy with tokenRef and value', async () => { + const spy = jest.fn(); + render( + + + , + ); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith('tok-1', 'hi'); + }); + + it('calls onSave with the updated TextAnalysis', async () => { + const onSave = jest.fn(); + render( + + + , + ); + await userEvent.click(screen.getByRole('button', { name: 'write' })); + expect(onSave).toHaveBeenCalledTimes(1); + const saved: TextAnalysis = onSave.mock.calls[0][0]; + expect(saved.tokenAnalyses[0].gloss).toStrictEqual({ en: 'hi' }); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useGlossDispatch must be used inside an AnalysisStoreProvider', + ); + }); +}); diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index e4dda957..386456ae 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -7,23 +7,23 @@ import userEvent from '@testing-library/user-event'; import type { Book, ScriptureRef, Token } from 'interlinearizer'; import type { ReactNode } from 'react'; import ContinuousView from '../../components/ContinuousView'; -import { GlossStoreProvider } from '../../components/GlossStore'; +import { AnalysisStoreProvider } from '../../components/AnalysisStore'; // --------------------------------------------------------------------------- -// GlossStore mock — pass-through provider so GlossStore.tsx stays out of scope +// AnalysisStore mock — pass-through provider so AnalysisStore.tsx stays out of scope // --------------------------------------------------------------------------- -jest.mock('../../components/GlossStore', () => ({ +jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, - GlossStoreProvider({ children }: Readonly<{ children: ReactNode }>) { + AnalysisStoreProvider({ children }: Readonly<{ children: ReactNode }>) { return children; }, useGloss: () => '', useGlossDispatch: () => () => {}, })); -/** Render options that wrap every test render in a `GlossStoreProvider`. */ -const withGlossStore = { wrapper: GlossStoreProvider }; +/** Render options that wrap every test render in a `AnalysisStoreProvider`. */ +const withAnalysisStore = { wrapper: AnalysisStoreProvider }; jest.mock('../../components/TokenChip', () => ({ __esModule: true, @@ -52,7 +52,7 @@ jest.mock('../../components/PhraseBox', () => ({ type="button" > {tokens.map((t) => ( - {t.surfaceText} + {t.surfaceText} ))} ), @@ -302,7 +302,7 @@ function makeLargeBook(count: number): Book { baselineText: `word${i}`, tokens: [ { - id: `large-tok-${i}`, + ref: `large-tok-${i}`, surfaceText: `word${i}`, writingSystem: 'en', type: 'word', @@ -353,7 +353,7 @@ beforeEach(() => { describe('ContinuousView initial render', () => { it('renders all tokens from all segments as a flat list', () => { - render(, withGlossStore); + render(, withAnalysisStore); expect(screen.getByText('In')).toBeInTheDocument(); expect(screen.getByText('the')).toBeInTheDocument(); @@ -362,7 +362,7 @@ describe('ContinuousView initial render', () => { }); it('does not render any verse label or segment separator', () => { - render(, withGlossStore); + render(, withAnalysisStore); // No verse numbers or colons that would indicate verse labels expect(screen.queryByText('1:1')).not.toBeInTheDocument(); @@ -372,7 +372,7 @@ describe('ContinuousView initial render', () => { }); it('renders a Previous token button and a Next token button', () => { - render(, withGlossStore); + render(, withAnalysisStore); expect(screen.getByRole('button', { name: 'Previous token' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Next token' })).toBeInTheDocument(); @@ -380,7 +380,7 @@ describe('ContinuousView initial render', () => { it('renders a non-word token via InertTokenChip within the strip', () => { // makeMixedBook: GEN 1:1 has a word token; GEN 1:2 has a punctuation token - render(, withGlossStore); + render(, withAnalysisStore); // Both the word chip ("In") and the inert chip (".") must appear expect(screen.getByText('In')).toBeInTheDocument(); @@ -388,7 +388,7 @@ describe('ContinuousView initial render', () => { }); it('renders without crashing when book has no word tokens (empty phraseEntries)', () => { - render(, withGlossStore); + render(, withAnalysisStore); // The punctuation token is rendered expect(screen.getByText('.')).toBeInTheDocument(); @@ -397,14 +397,14 @@ describe('ContinuousView initial render', () => { it('renders without crashing when book has no word tokens and activePhraseIndex is set', () => { render( , - withGlossStore, + withAnalysisStore, ); expect(screen.getByText('.')).toBeInTheDocument(); }); it('clicking an out-of-focus phrase box brings it into focus', async () => { - render(, withGlossStore); + render(, withAnalysisStore); const clickedPhraseBox = screen.getByText('beginning').closest('[data-phrase-box="true"]'); if (!clickedPhraseBox) throw new Error('Expected phrase box wrapper for token'); @@ -418,7 +418,7 @@ describe('ContinuousView initial render', () => { }); it('clicking the already-focused phrase box leaves it focused', async () => { - render(, withGlossStore); + render(, withAnalysisStore); // The first token is focused by default. const firstPhraseBox = screen.getByText('In').closest('[data-phrase-box="true"]'); @@ -440,26 +440,26 @@ describe('ContinuousView initial render', () => { describe('ContinuousView arrow disabled states', () => { it('disables the prev arrow on initial render (book start)', () => { - render(, withGlossStore); + render(, withAnalysisStore); expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); }); it('enables the next arrow on initial render when there are multiple tokens', () => { - render(, withGlossStore); + render(, withAnalysisStore); expect(screen.getByRole('button', { name: 'Next token' })).toBeEnabled(); }); it('disables both arrows when the book has exactly one token', () => { - render(, withGlossStore); + render(, withAnalysisStore); expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); expect(screen.getByRole('button', { name: 'Next token' })).toBeDisabled(); }); it('enables the prev arrow after clicking next once', async () => { - render(, withGlossStore); + render(, withAnalysisStore); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -467,7 +467,7 @@ describe('ContinuousView arrow disabled states', () => { }); it('disables the next arrow when advanced to the last token', async () => { - render(, withGlossStore); + render(, withAnalysisStore); const nextBtn = screen.getByRole('button', { name: 'Next token' }); // 4 tokens total: advance 3 times to reach index 3 (last) @@ -479,7 +479,7 @@ describe('ContinuousView arrow disabled states', () => { }); it('re-enables the next arrow after going prev from the last token', async () => { - render(, withGlossStore); + render(, withAnalysisStore); const nextBtn = screen.getByRole('button', { name: 'Next token' }); await userEvent.click(nextBtn); @@ -502,7 +502,7 @@ describe('ContinuousView fade overlays', () => { it('does not render prev fade at book start', () => { const { container } = render( , - withGlossStore, + withAnalysisStore, ); const gradients = container.querySelectorAll('[aria-hidden="true"]'); @@ -515,7 +515,7 @@ describe('ContinuousView fade overlays', () => { it('renders next fade at book start (next side is enabled)', () => { const { container } = render( , - withGlossStore, + withAnalysisStore, ); const gradients = container.querySelectorAll('[aria-hidden="true"]'); @@ -528,7 +528,7 @@ describe('ContinuousView fade overlays', () => { it('renders prev fade after moving away from book start', async () => { const { container } = render( , - withGlossStore, + withAnalysisStore, ); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -543,7 +543,7 @@ describe('ContinuousView fade overlays', () => { it('does not render next fade at book end', async () => { const { container } = render( , - withGlossStore, + withAnalysisStore, ); const nextBtn = screen.getByRole('button', { name: 'Next token' }); @@ -565,7 +565,7 @@ describe('ContinuousView fade overlays', () => { describe('ContinuousView cross-chapter traversal', () => { it('indexes tokens across chapter boundaries in segment order', () => { - render(, withGlossStore); + render(, withAnalysisStore); // Both chapter tokens should be present expect(screen.getByText('Alpha')).toBeInTheDocument(); @@ -573,7 +573,7 @@ describe('ContinuousView cross-chapter traversal', () => { }); it('can navigate across a chapter boundary with the next arrow', async () => { - render(, withGlossStore); + render(, withAnalysisStore); // Only one token per chapter, so clicking next once reaches chapter 2's token (index 1 = last) await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -591,7 +591,7 @@ describe('ContinuousView cross-chapter traversal', () => { describe('ContinuousView smooth-scroll intent', () => { it('calls scrollIntoView with smooth behaviour when next arrow is clicked', async () => { - render(, withGlossStore); + render(, withAnalysisStore); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); @@ -601,7 +601,7 @@ describe('ContinuousView smooth-scroll intent', () => { }); it('calls scrollIntoView with smooth behaviour when prev arrow is clicked', async () => { - render(, withGlossStore); + render(, withAnalysisStore); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); scrollIntoViewMock.mockClear(); @@ -614,7 +614,7 @@ describe('ContinuousView smooth-scroll intent', () => { }); it('does not call scrollIntoView when a disabled arrow is clicked', async () => { - render(, withGlossStore); + render(, withAnalysisStore); scrollIntoViewMock.mockClear(); // Prev arrow is disabled at start — clicking it should be a no-op @@ -644,7 +644,7 @@ describe('ContinuousView activeVerse verse-jump', () => { {...requiredProps()} activeVerse={{ book: 'GEN', chapter: 1, verse: 1 }} />, - withGlossStore, + withAnalysisStore, ); // At index 0 the prev arrow should be disabled @@ -762,7 +762,7 @@ describe('ContinuousView activeVerse verse-jump', () => { {...requiredProps()} activeVerse={{ book: 'GEN', chapter: 1, verse: 2 }} />, - withGlossStore, + withAnalysisStore, ); // Index 2 is not the start (prev enabled) and not the end (next enabled). @@ -771,7 +771,7 @@ describe('ContinuousView activeVerse verse-jump', () => { }); it('does not jump when activeVerse is undefined', () => { - render(, withGlossStore); + render(, withAnalysisStore); // Without activeVerse the strip stays at focusIndex 0 expect(screen.getByRole('button', { name: 'Previous token' })).toBeDisabled(); @@ -784,7 +784,7 @@ describe('ContinuousView activeVerse verse-jump', () => { {...requiredProps()} activeVerse={{ book: 'GEN', chapter: 99, verse: 99 }} />, - withGlossStore, + withAnalysisStore, ); // No matching segment — strip stays at focusIndex 0 @@ -854,7 +854,7 @@ describe('ContinuousView activePhraseIndex', () => { const book = makeBook(); render( , - withGlossStore, + withAnalysisStore, ); act(() => { jest.advanceTimersByTime(500); @@ -926,7 +926,7 @@ describe('ContinuousView onVerseChange propagation', () => { const handleVerseChange = jest.fn(); render( , - withGlossStore, + withAnalysisStore, ); // Advance twice to reach index 2 (first token of GEN 1:2) @@ -945,7 +945,7 @@ describe('ContinuousView onVerseChange propagation', () => { activeVerse={{ book: 'GEN', chapter: 1, verse: 2 }} onVerseChange={handleVerseChange} />, - withGlossStore, + withAnalysisStore, ); handleVerseChange.mockClear(); @@ -959,7 +959,7 @@ describe('ContinuousView onVerseChange propagation', () => { const handleVerseChange = jest.fn(); render( , - withGlossStore, + withAnalysisStore, ); handleVerseChange.mockClear(); @@ -977,7 +977,7 @@ describe('ContinuousView onVerseChange propagation', () => { {...requiredProps()} onVerseChange={handleVerseChange} />, - withGlossStore, + withAnalysisStore, ); handleVerseChange.mockClear(); @@ -1030,14 +1030,14 @@ describe('ContinuousView RTL layout', () => { }); it('shows right-arrow (→) on the previous button in RTL mode', () => { - render(, withGlossStore); + render(, withAnalysisStore); const prevBtn = screen.getByRole('button', { name: 'Previous token' }); expect(prevBtn.querySelector('[aria-hidden="true"]')).toHaveTextContent('\u2192'); }); it('shows left-arrow (←) on the next button in RTL mode', () => { - render(, withGlossStore); + render(, withAnalysisStore); const nextBtn = screen.getByRole('button', { name: 'Next token' }); expect(nextBtn.querySelector('[aria-hidden="true"]')).toHaveTextContent('\u2190'); @@ -1068,7 +1068,7 @@ describe('ContinuousView phrase window', () => { activeVerse={{ book: 'GEN', chapter: 1, verse: 1 }} activePhraseIndex={125} />, - withGlossStore, + withAnalysisStore, ); act(() => { jest.advanceTimersByTime(500); diff --git a/src/__tests__/components/GlossStore.test.tsx b/src/__tests__/components/GlossStore.test.tsx deleted file mode 100644 index 9f61398e..00000000 --- a/src/__tests__/components/GlossStore.test.tsx +++ /dev/null @@ -1,188 +0,0 @@ -/** @file Unit tests for components/GlossStore.tsx. */ -/// -/// - -import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { - GlossStoreProvider, - useAllGlosses, - useGloss, - useGlossDispatch, -} from '../../components/GlossStore'; - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -/** - * Renders a component that displays the gloss for a single token, used to assert on `useGloss`. - * - * @param tokenId - Token id to subscribe to. - * @returns JSX element suitable for passing to `render`. - */ -function GlossReader({ tokenId }: Readonly<{ tokenId: string }>) { - const gloss = useGloss(tokenId); - return {gloss}; -} - -/** - * Renders a component that displays all glosses as JSON, used to assert on `useAllGlosses`. - * - * @returns JSX element suitable for passing to `render`. - */ -function AllGlossesReader() { - const glosses = useAllGlosses(); - return {JSON.stringify(glosses)}; -} - -/** - * Renders a component that calls `useGlossDispatch` without a provider, used to assert the hook - * throws outside a {@link GlossStoreProvider}. - * - * @returns Nothing — only mounted to trigger the throw. - */ -function DispatchUser() { - useGlossDispatch(); - return undefined; -} - -/** - * Renders a button that calls `useGlossDispatch` to write a gloss, used to test dispatch. - * - * @param props.tokenId - Token id to write. - * @param props.value - Gloss value to write. - * @returns JSX element suitable for passing to `render`. - */ -function GlossWriter({ tokenId, value }: Readonly<{ tokenId: string; value: string }>) { - const dispatch = useGlossDispatch(); - return ( - - ); -} - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe('useGloss', () => { - it('returns an empty string for an unknown token', () => { - render( - - - , - ); - expect(screen.getByTestId('gloss')).toHaveTextContent(''); - }); - - it('returns the seeded value for a token in initialGlosses', () => { - render( - - - , - ); - expect(screen.getByTestId('gloss')).toHaveTextContent('hello'); - }); - - it('updates when the subscribed token changes', async () => { - render( - - - - , - ); - expect(screen.getByTestId('gloss')).toHaveTextContent(''); - await userEvent.click(screen.getByRole('button', { name: 'write' })); - expect(screen.getByTestId('gloss')).toHaveTextContent('world'); - }); - - it('does not update when a different token changes', async () => { - let renderCount = 0; - - function CountingGlossReader({ tokenId }: Readonly<{ tokenId: string }>) { - renderCount += 1; - const gloss = useGloss(tokenId); - return {gloss}; - } - - render( - - - - , - ); - const initialRenderCount = renderCount; - await userEvent.click(screen.getByRole('button', { name: 'write' })); - expect(renderCount).toBe(initialRenderCount); - }); - - it('throws when called outside a GlossStoreProvider', () => { - jest.spyOn(console, 'error').mockImplementation(() => {}); - expect(() => render()).toThrow( - 'useGloss must be used inside a GlossStoreProvider', - ); - }); -}); - -describe('useAllGlosses', () => { - it('returns an empty object when no glosses have been set', () => { - render( - - - , - ); - expect(screen.getByTestId('all-glosses')).toHaveTextContent('{}'); - }); - - it('returns seeded glosses from initialGlosses', () => { - render( - - - , - ); - expect(screen.getByTestId('all-glosses')).toHaveTextContent(JSON.stringify({ 'tok-1': 'hi' })); - }); - - it('updates when any token changes', async () => { - render( - - - - , - ); - await userEvent.click(screen.getByRole('button', { name: 'write' })); - expect(screen.getByTestId('all-glosses')).toHaveTextContent( - JSON.stringify({ 'tok-1': 'world' }), - ); - }); - - it('throws when called outside a GlossStoreProvider', () => { - jest.spyOn(console, 'error').mockImplementation(() => {}); - expect(() => render()).toThrow( - 'useAllGlosses must be used inside a GlossStoreProvider', - ); - }); -}); - -describe('useGlossDispatch', () => { - it('calls the onGlossChange spy with tokenId and value', async () => { - const spy = jest.fn(); - render( - - - , - ); - await userEvent.click(screen.getByRole('button', { name: 'write' })); - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith('tok-1', 'hi'); - }); - - it('throws when called outside a GlossStoreProvider', () => { - jest.spyOn(console, 'error').mockImplementation(() => {}); - expect(() => render()).toThrow( - 'useGlossDispatch must be used inside a GlossStoreProvider', - ); - }); -}); diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index bb74cd39..96f9af3f 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -19,15 +19,15 @@ let capturedContinuousViewProps: Record = {}; type CapturedSegmentViewProps = { segment: Segment; displayMode?: string; - focusedTokenId?: string; + focusedTokenRef?: string; isActive?: boolean; - onSelect?: (ref: { book: string; chapter: number; verse: number }, tokenId?: string) => void; + onSelect?: (ref: { book: string; chapter: number; verse: number }, tokenRef?: string) => void; }; let capturedSegmentViewPropsList: CapturedSegmentViewProps[] = []; -jest.mock('../../components/GlossStore', () => ({ +jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, - GlossStoreProvider({ children }: Readonly<{ children: ReactNode }>) { + AnalysisStoreProvider({ children }: Readonly<{ children: ReactNode }>) { return children; }, useGloss: () => '', @@ -261,15 +261,6 @@ describe('Interlinearizer', () => { expect(allElements[0]).toBe(continuousView); }); - it('passes onSelect to SegmentView regardless of continuousScroll mode', () => { - renderInterlinearizer({ continuousScroll: false }); - expect(capturedSegmentViewPropsList[0].onSelect).toBeInstanceOf(Function); - - capturedSegmentViewPropsList = []; - renderInterlinearizer({ continuousScroll: true }); - expect(capturedSegmentViewPropsList[0].onSelect).toBeInstanceOf(Function); - }); - it('calls setScrRef with the segment ref when a token is clicked', () => { const mockSetScrRef = jest.fn(); renderInterlinearizer({ @@ -392,8 +383,8 @@ describe('Interlinearizer', () => { />, ); - // The token at phrase index 1 is 'GEN 1:2:0'; it should now be the focusedTokenId. - const focused = capturedSegmentViewPropsList.find((p) => p.focusedTokenId === 'GEN 1:2:0'); + // The token at phrase index 1 is 'GEN 1:2:0'; it should now be the focusedTokenRef. + const focused = capturedSegmentViewPropsList.find((p) => p.focusedTokenRef === 'GEN 1:2:0'); expect(focused).toBeDefined(); }); @@ -419,11 +410,11 @@ describe('Interlinearizer', () => { ); // The fallback focuses the first word of GEN 1:1 ('GEN 1:1:0'). - const focused = capturedSegmentViewPropsList.find((p) => p.focusedTokenId === 'GEN 1:1:0'); + const focused = capturedSegmentViewPropsList.find((p) => p.focusedTokenRef === 'GEN 1:1:0'); expect(focused).toBeDefined(); }); - it('preserves an existing focusedTokenId when switching off continuousScroll with no strip position', () => { + it('preserves an existing focusedTokenRef when switching off continuousScroll with no strip position', () => { // Start in segment mode and focus a specific token. const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, @@ -431,7 +422,7 @@ describe('Interlinearizer', () => { continuousScroll: false, }); - // Click a token to set focusedTokenId to 'GEN 1:2:0'. + // Click a token to set focusedTokenRef to 'GEN 1:2:0'. const { onSelect } = capturedSegmentViewPropsList[1]; if (typeof onSelect !== 'function') throw new Error('Expected onSelect to be a function'); act(() => { @@ -450,7 +441,7 @@ describe('Interlinearizer', () => { />, ); - // Switch back to segment mode — existing focusedTokenId should be preserved. + // Switch back to segment mode — existing focusedTokenRef should be preserved. capturedSegmentViewPropsList = []; rerender( { ); // 'GEN 1:2:0' was already focused, so the fallback must not overwrite it. - const stillFocused = capturedSegmentViewPropsList.find((p) => p.focusedTokenId === 'GEN 1:2:0'); + const stillFocused = capturedSegmentViewPropsList.find( + (p) => p.focusedTokenRef === 'GEN 1:2:0', + ); expect(stillFocused).toBeDefined(); }); @@ -494,7 +487,7 @@ describe('Interlinearizer', () => { }); }); - it('leaves focusedTokenId undefined when switching off continuousScroll with no strip position and no matching segment', () => { + it('leaves focusedTokenRef undefined when switching off continuousScroll with no strip position and no matching segment', () => { // scrRef points to verse 99 which does not exist in GEN_1_MULTI_BOOK. const { rerender } = renderInterlinearizer({ book: GEN_1_MULTI_BOOK, @@ -514,7 +507,7 @@ describe('Interlinearizer', () => { />, ); - // No segment matches verse 99 so focusedTokenId stays undefined for all views. - capturedSegmentViewPropsList.forEach((p) => expect(p.focusedTokenId).toBeUndefined()); + // No segment matches verse 99 so focusedTokenRef stays undefined for all views. + capturedSegmentViewPropsList.forEach((p) => expect(p.focusedTokenRef).toBeUndefined()); }); }); diff --git a/src/__tests__/components/PhraseBox.test.tsx b/src/__tests__/components/PhraseBox.test.tsx index a2013668..3b841859 100644 --- a/src/__tests__/components/PhraseBox.test.tsx +++ b/src/__tests__/components/PhraseBox.test.tsx @@ -6,48 +6,58 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { Token } from 'interlinearizer'; import type { ReactNode } from 'react'; -import { GlossStoreProvider } from '../../components/GlossStore'; +import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { PhraseBox } from '../../components/PhraseBox'; // --------------------------------------------------------------------------- -// GlossStore mock — reactive useState-based stub so GlossStore.tsx stays out of scope +// AnalysisStore mock — reactive useState-based stub so AnalysisStore.tsx stays out of scope // --------------------------------------------------------------------------- -jest.mock('../../components/GlossStore', () => { +jest.mock('../../components/AnalysisStore', () => { const { createContext, useCallback, useContext, useMemo, useState } = jest.requireActual('react'); type GlossMap = Record; type MockCtxValue = { glosses: GlossMap; - dispatch: (tokenId: string, value: string) => void; + dispatch: (tokenRef: string, surfaceText: string, value: string) => void; }; const MockCtx = createContext({ glosses: {}, dispatch: () => {} }); return { __esModule: true, - GlossStoreProvider({ + AnalysisStoreProvider({ children, - initialGlosses, + initialAnalysis, onGlossChange, }: Readonly<{ children: ReactNode; - initialGlosses?: GlossMap; - onGlossChange?: (tokenId: string, value: string) => void; + initialAnalysis?: { + tokenAnalyses: { id: string; gloss?: GlossMap }[]; + tokenAnalysisLinks: { analysisId: string; status: string; token: { tokenRef: string } }[]; + }; + onGlossChange?: (tokenRef: string, value: string) => void; }>) { - const [glosses, setGlosses] = useState(initialGlosses ?? {}); + const byId = new Map((initialAnalysis?.tokenAnalyses ?? []).map((ta) => [ta.id, ta])); + const seed: GlossMap = (initialAnalysis?.tokenAnalysisLinks ?? []) + .filter((link) => link.status === 'approved') + .reduce((acc, link) => { + const gloss = byId.get(link.analysisId)?.gloss?.en; + return gloss === undefined ? acc : { ...acc, [link.token.tokenRef]: gloss }; + }, {}); + const [glosses, setGlosses] = useState(seed); const dispatch = useCallback( - (tokenId: string, value: string) => { - setGlosses((prev) => ({ ...prev, [tokenId]: value })); - onGlossChange?.(tokenId, value); + (tokenRef: string, _surfaceText: string, value: string) => { + setGlosses((prev) => ({ ...prev, [tokenRef]: value })); + onGlossChange?.(tokenRef, value); }, [onGlossChange], ); const ctx = useMemo(() => ({ glosses, dispatch }), [glosses, dispatch]); return {children}; }, - useGloss(tokenId: string) { - return useContext(MockCtx).glosses[tokenId] ?? ''; + useGloss(tokenRef: string) { + return useContext(MockCtx).glosses[tokenRef] ?? ''; }, useGlossDispatch() { return useContext(MockCtx).dispatch; @@ -57,17 +67,17 @@ jest.mock('../../components/GlossStore', () => { jest.mock('../../components/TokenChip', () => { const { useGloss, useGlossDispatch } = jest.requireMock< - typeof import('../../components/GlossStore') - >('../../components/GlossStore'); + typeof import('../../components/AnalysisStore') + >('../../components/AnalysisStore'); function MockTokenChip({ onFocus, token }: Readonly<{ onFocus?: () => void; token: Token }>) { - const gloss = useGloss(token.id); + const gloss = useGloss(token.ref); const dispatch = useGlossDispatch(); return ( - + {token.surfaceText} dispatch(token.id, e.target.value)} + onChange={(e) => dispatch(token.ref, token.surfaceText, e.target.value)} onFocus={onFocus} value={gloss} /> @@ -79,7 +89,7 @@ jest.mock('../../components/TokenChip', () => { /** Pre-built test token */ const TEST_TOKEN = { - id: 'token-1', + ref: 'token-1', surfaceText: 'Hello', writingSystem: 'en', type: 'word', @@ -89,7 +99,7 @@ const TEST_TOKEN = { /** Second test token */ const TEST_TOKEN_2 = { - id: 'token-2', + ref: 'token-2', surfaceText: 'World', writingSystem: 'en', type: 'word', @@ -123,9 +133,9 @@ function requiredProps(): PhraseBoxTestProps { describe('PhraseBox', () => { it('renders as a label', () => { render( - + - , + , ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); @@ -134,9 +144,9 @@ describe('PhraseBox', () => { it('renders one TokenChip per token in the tokens array', () => { render( - + - , + , ); expect(screen.getByTestId('token-token-1')).toBeInTheDocument(); @@ -145,9 +155,9 @@ describe('PhraseBox', () => { it('clicking the outer container focuses the first gloss input', async () => { render( - + - , + , ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); @@ -158,9 +168,9 @@ describe('PhraseBox', () => { it('applies focused border and background when isFocused is true', () => { render( - + - , + , ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); @@ -172,9 +182,9 @@ describe('PhraseBox', () => { it('applies default border and background when isFocused is false', () => { render( - + - , + , ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); @@ -186,9 +196,9 @@ describe('PhraseBox', () => { it('phrase box does not override cursor on gap areas', () => { render( - + - , + , ); const phraseBox = document.querySelector('[data-phrase-box="true"]'); @@ -197,9 +207,9 @@ describe('PhraseBox', () => { it('renders tokens in the order they appear in the tokens array', () => { render( - + - , + , ); const tokens = document.querySelectorAll('[data-testid^="token-"]'); @@ -208,10 +218,32 @@ describe('PhraseBox', () => { }); it('passes the gloss for each token from the store', () => { + const initialAnalysis = { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [ + { id: 'ta-1', surfaceText: 'Hello', gloss: { en: 'hello' } }, + { id: 'ta-2', surfaceText: 'World', gloss: { en: 'world' } }, + ], + tokenAnalysisLinks: [ + { + analysisId: 'ta-1', + status: 'approved' as const, + token: { tokenRef: 'token-1', surfaceText: 'Hello' }, + }, + { + analysisId: 'ta-2', + status: 'approved' as const, + token: { tokenRef: 'token-2', surfaceText: 'World' }, + }, + ], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; render( - + - , + , ); expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveValue('hello'); @@ -220,9 +252,9 @@ describe('PhraseBox', () => { it('shows an empty string when the token id is absent from the store', () => { render( - + - , + , ); expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveValue(''); @@ -231,9 +263,9 @@ describe('PhraseBox', () => { it('updates the store when a gloss input changes', async () => { const spy = jest.fn(); render( - + - , + , ); await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for Hello' }), 'hi'); @@ -246,9 +278,9 @@ describe('PhraseBox', () => { it('calls onFocusPhrase with index when a gloss input receives focus', async () => { const handleFocus = jest.fn(); render( - + - , + , ); await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for Hello' })); @@ -256,15 +288,4 @@ describe('PhraseBox', () => { expect(handleFocus).toHaveBeenCalledWith(2); }); - it('phrase box always has padding and gap spacing classes', () => { - render( - - - , - ); - - const phraseBox = document.querySelector('[data-phrase-box="true"]'); - expect(phraseBox).toHaveClass('tw:px-1'); - expect(phraseBox).toHaveClass('tw:py-0.5'); - }); }); diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index 06b70184..b780295d 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -6,16 +6,16 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { ScriptureRef, Segment, Token } from 'interlinearizer'; import type { ReactNode } from 'react'; -import { GlossStoreProvider } from '../../components/GlossStore'; +import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { SegmentView } from '../../components/SegmentView'; // --------------------------------------------------------------------------- -// GlossStore mock — pass-through provider so GlossStore.tsx stays out of scope +// AnalysisStore mock — pass-through provider so AnalysisStore.tsx stays out of scope // --------------------------------------------------------------------------- -jest.mock('../../components/GlossStore', () => ({ +jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, - GlossStoreProvider({ children }: Readonly<{ children: ReactNode }>) { + AnalysisStoreProvider({ children }: Readonly<{ children: ReactNode }>) { return children; }, useGloss: () => '', @@ -44,7 +44,7 @@ jest.mock('../../components/PhraseBox', () => ({ }>) => ( {tokens.map((t) => ( - + {onFocusPhrase ? (
@@ -247,7 +247,7 @@ function InterlinearizerInner({ * @param props.scrRef - Current scripture reference * @param props.setScrRef - Callback to update the scripture reference * @param props.initialAnalysis - Seed analysis data for the store; not reactive after mount - * @param props.analysisLanguage - BCP 47 tag for gloss read/write; defaults to `'en'` + * @param props.analysisLanguage - BCP 47 tag for gloss read/write * @param props.onSaveAnalysis - Called after each gloss write with the updated `TextAnalysis` * @returns The full interlinearizer layout with optional continuous strip and segment list */ diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 37bb9421..b92a255e 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -13,6 +13,7 @@ import Interlinearizer from './Interlinearizer'; import ProjectModals, { type ModalState } from './ProjectModals'; import ScriptureNavControls from './ScriptureNavControls'; +/** Localized string keys used by {@link InterlinearizerLoader}. */ const STRING_KEYS: `%${string}%`[] = ['%interlinearizer_continuousScrollToggle%']; /** @@ -39,6 +40,11 @@ export default function InterlinearizerLoader({ const [scrRef, setScrRef, scrollGroupId, setScrollGroupId] = useWebViewScrollGroupScrRef(); const [interfaceMode] = useSetting('platform.interfaceMode', 'simple'); + const [interfaceLanguages] = useSetting('platform.interfaceLanguage', ['und']); + /* v8 ignore next 3 -- useSetting never returns PlatformError for this key in practice */ + const analysisLanguage = isPlatformError(interfaceLanguages) + ? 'und' + : (interfaceLanguages[0] ?? 'und'); const { isLoading: isSettingLoading, @@ -184,6 +190,7 @@ export default function InterlinearizerLoader({ continuousScroll={continuousScroll} scrRef={scrRef} setScrRef={setScrRef} + analysisLanguage={analysisLanguage} /> )} diff --git a/src/hooks/useInterlinearizerBookData.ts b/src/hooks/useInterlinearizerBookData.ts index ed6be9f6..990a700e 100644 --- a/src/hooks/useInterlinearizerBookData.ts +++ b/src/hooks/useInterlinearizerBookData.ts @@ -69,7 +69,8 @@ export default function useInterlinearizerBookData({ useEffect(() => { if (!tokenizeError) return; - const ws = isPlatformError(writingSystem) ? 'und' : writingSystem || 'und'; /* v8 ignore next */ + /* v8 ignore next -- isPlatformError branch for writingSystem is unreachable through the mock setup */ + const ws = isPlatformError(writingSystem) ? 'und' : writingSystem || 'und'; logger.error('Failed to parse/tokenize USJ book', tokenizeError.raw, { message: tokenizeError.message, writingSystem: ws, From 8b9fa4f4679998700a412219048af1da633be8bf Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 22 May 2026 11:09:27 -0600 Subject: [PATCH 10/13] Doc/mock audit --- .../components/AnalysisStore.test.tsx | 9 ++ .../components/ContinuousView.test.tsx | 26 +++- .../components/Interlinearizer.test.tsx | 127 ++++++++++++------ .../components/InterlinearizerLoader.test.tsx | 39 ++---- src/__tests__/components/PhraseBox.test.tsx | 27 +++- .../components/ProjectModals.test.tsx | 10 +- src/__tests__/components/SegmentView.test.tsx | 16 +-- src/__tests__/components/TokenChip.test.tsx | 19 ++- src/__tests__/test-helpers.ts | 34 ++++- src/components/AnalysisStore.tsx | 9 +- src/components/Interlinearizer.tsx | 8 +- src/components/InterlinearizerLoader.tsx | 4 +- 12 files changed, 215 insertions(+), 113 deletions(-) diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index d033147a..e2c95578 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -166,6 +166,15 @@ describe('useGloss', () => { it('does not re-render when a different token is glossed', async () => { let renderCount = 0; + /** + * Renders the current gloss for a token while counting how many times it re-renders, so tests + * can assert that unrelated gloss changes do not cause extra renders. + * + * @param props - Component props. + * @param props.tokenRef - The token whose approved gloss to read via {@link useGloss}. + * @returns A span containing the gloss string. + * @throws When called outside an {@link AnalysisStoreProvider}. + */ function CountingGlossReader({ tokenRef }: Readonly<{ tokenRef: string }>) { renderCount += 1; const gloss = useGloss(tokenRef); diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index c2224316..e293e3eb 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -15,10 +15,28 @@ import { AnalysisStoreProvider } from '../../components/AnalysisStore'; jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, + /** + * Pass-through provider stub that renders children directly, keeping AnalysisStore.tsx out of + * scope. + * + * @param props - Component props. + * @param props.children - Child nodes to render. + * @returns The children unchanged. + */ AnalysisStoreProvider({ children }: Readonly<{ children: ReactNode; analysisLanguage: string }>) { return children; }, + /** + * Returns a fixed empty gloss string for any token. + * + * @returns An empty string. + */ useGloss: () => '', + /** + * Returns a no-op dispatch function. + * + * @returns A function that accepts any arguments and does nothing. + */ useGlossDispatch: () => () => {}, })); @@ -44,15 +62,15 @@ jest.mock('../../components/PhraseBox', () => ({ onFocusPhrase, tokens, }: Readonly<{ - index?: number; + index: number | undefined; isFocused: boolean; - onFocusPhrase?: (index?: number) => void; - tokens: Token[]; + onFocusPhrase: (index?: number) => void; + tokens: (Token & { type: 'word' })[]; }>) => ( - ) : ( - {t.surfaceText} - )} + ))} diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index d0d7ec8e..54be8574 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -4,7 +4,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import type { Token } from 'interlinearizer'; +import type { AssignmentStatus, Token } from 'interlinearizer'; import type { ReactNode } from 'react'; import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { InertTokenChip, TokenChip } from '../../components/TokenChip'; @@ -29,12 +29,17 @@ jest.mock('../../components/AnalysisStore', () => { AnalysisStoreProvider({ children, initialAnalysis, + analysisLanguage, onGlossChange, }: Readonly<{ children: ReactNode; initialAnalysis?: { tokenAnalyses: { id: string; gloss?: GlossMap }[]; - tokenAnalysisLinks: { analysisId: string; status: string; token: { tokenRef: string } }[]; + tokenAnalysisLinks: { + analysisId: string; + status: AssignmentStatus; + token: { tokenRef: string }; + }[]; }; analysisLanguage: string; onGlossChange?: (tokenRef: string, value: string) => void; @@ -43,7 +48,7 @@ jest.mock('../../components/AnalysisStore', () => { const seed: GlossMap = (initialAnalysis?.tokenAnalysisLinks ?? []) .filter((link) => link.status === 'approved') .reduce((acc, link) => { - const gloss = byId.get(link.analysisId)?.gloss?.en; + const gloss = byId.get(link.analysisId)?.gloss?.[analysisLanguage]; return gloss === undefined ? acc : { ...acc, [link.token.tokenRef]: gloss }; }, {}); const [glosses, setGlosses] = useState(seed); @@ -140,12 +145,16 @@ describe('TokenChip', () => { it('shows the current gloss value from the store', () => { const initialAnalysis = { - tokenAnalyses: [{ id: 'ta-1', surfaceText: 'hello', gloss: { en: 'in' } }], + tokenAnalyses: [{ id: 'ta-1', surfaceText: 'hello', gloss: { und: 'in' } }], tokenAnalysisLinks: [ { analysisId: 'ta-1', - status: 'approved' as const, + status: 'approved', token: { tokenRef: 'GEN 1:1:0', surfaceText: 'hello' }, + } satisfies { + analysisId: string; + status: AssignmentStatus; + token: { tokenRef: string; surfaceText: string }; }, ], segmentAnalyses: [], diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index bfabdb64..a39b4346 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -1,9 +1,11 @@ /** * @file Test helpers used to build type-safe mocks without type assertions. Provides a minimal - * ExecutionActivationContext that satisfies @papi/core types, and a `useWebViewState` hook stub - * for component tests. + * ExecutionActivationContext that satisfies @papi/core types, a `useWebViewState` hook stub for + * component tests, and shared Book fixtures. */ +import type { SerializedVerseRef } from '@sillsdev/scripture'; import type { ExecutionActivationContext } from '@papi/core'; +import type { Book } from 'interlinearizer'; import { UnsubscriberAsyncList } from 'platform-bible-utils'; /** Minimal execution token-shaped object for tests (structural match for ExecutionToken). */ @@ -56,6 +58,34 @@ export function makeWebViewState() { }; } +/** Genesis 1:1 serialized verse ref — shared across tests that need a default scroll position. */ +export const defaultScrRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 1 }; + +/** Pre-built Book with one GEN 1:1 segment and a single word token. */ +export const GEN_1_1_BOOK: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + segments: [ + { + id: 'GEN 1:1', + startRef: { book: 'GEN', chapter: 1, verse: 1 }, + endRef: { book: 'GEN', chapter: 1, verse: 1 }, + baselineText: 'In the beginning.', + tokens: [ + { + ref: 'GEN 1:1:0', + surfaceText: 'In', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 2, + }, + ], + }, + ], +}; + /** Minimal elevated privileges for tests (all properties optional per papi type). */ const mockElevatedPrivileges = { createProcess: undefined, diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index dd439346..074de1ab 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -167,10 +167,9 @@ export function AnalysisStoreProvider({ * gloss string (under `analysisLanguage`), replaces the analysis snapshot, notifies subscribers, * calls `onSave`, and calls the optional `spy` prop for test observability. * - * The new `TokenAnalysis` gets a UUID-like id built from `tokenRef` + current timestamp to ensure - * uniqueness within the project. Existing analyses for the token are left untouched — this - * follows the data model's "multiple competing analyses" design; the UI manages selection and - * deletion separately. + * The new `TokenAnalysis` gets a UUID (`crypto.randomUUID()`) as its id to ensure global + * uniqueness. Existing analyses for the token are left untouched — this follows the data model's + * "multiple competing analyses" design; the UI manages selection and deletion separately. * * @param tokenRef - The `Token.ref` of the token being glossed. * @param surfaceText - The surface text of the token (stored as `Analysis.surfaceText`). @@ -178,7 +177,7 @@ export function AnalysisStoreProvider({ */ const onGlossChange = useCallback( (tokenRef: string, surfaceText: string, value: string) => { - const id = `${tokenRef}:${Date.now()}`; + const id = crypto.randomUUID(); const newAnalysis: TokenAnalysis = { id, surfaceText, diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index eefd5fe2..ae676dd6 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -33,7 +33,13 @@ type InterlinearizerProps = Readonly<{ * Inner component that renders the segment list and continuous view. Separated from * {@link Interlinearizer} so it can consume the `AnalysisStoreProvider` context that wraps it. * - * @param props - Same props as {@link Interlinearizer}. + * @param props - Component props + * @param props.book - Tokenized book whose segments are rendered. + * @param props.chapterSegments - Segments belonging to the current chapter, filtered by the caller. + * @param props.continuousScroll - When true, the horizontal token strip is shown above the segment + * list. + * @param props.scrRef - Current scripture reference used to highlight the active verse. + * @param props.setScrRef - Called when the user navigates to a different verse. * @returns The interlinearizer layout without the provider wrapper. */ function InterlinearizerInner({ diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index b92a255e..a5220ff1 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -25,6 +25,8 @@ const STRING_KEYS: `%${string}%`[] = ['%interlinearizer_continuousScrollToggle%' * @param props.projectId - PAPI project ID passed from the host * @param props.useWebViewScrollGroupScrRef - Hook that exposes the shared scroll-group scripture * reference and its setter + * @param props.useWebViewState - Hook for reading and writing typed WebView-scoped state persisted + * by the PAPI host * @returns The toolbar and either an error/loading state or the fully rendered * {@link Interlinearizer} */ @@ -44,7 +46,7 @@ export default function InterlinearizerLoader({ /* v8 ignore next 3 -- useSetting never returns PlatformError for this key in practice */ const analysisLanguage = isPlatformError(interfaceLanguages) ? 'und' - : (interfaceLanguages[0] ?? 'und'); + : interfaceLanguages[0] || 'und'; const { isLoading: isSettingLoading, From 9de91fd8505153ae19fafe5b2af48aeaa969532b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 22 May 2026 11:36:01 -0600 Subject: [PATCH 11/13] Lazy build refs to prevent rebuilding of potentially large maps every render --- src/components/AnalysisStore.tsx | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index 074de1ab..939dd609 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -109,17 +109,22 @@ export function AnalysisStoreProvider({ const analysisRef = useRef(initialAnalysis ?? EMPTY_ANALYSIS); const listenersRef = useRef(new Set<() => void>()); + // These two indexes are built lazily via ??= so that passing an initializer expression to useRef + // (which evaluates on every render but is only used on the first mount) doesn't rebuild large Maps + // across a full-Bible analysis on every re-render. + /** Pre-built map of `TokenAnalysis.id` → `TokenAnalysis` for O(1) lookup by id. */ - const analysisByIdRef = useRef>( - new Map(analysisRef.current.tokenAnalyses.map((ta) => [ta.id, ta])), - ); + const analysisByIdRef = useRef | undefined>(undefined); + analysisByIdRef.current ??= new Map(analysisRef.current.tokenAnalyses.map((ta) => [ta.id, ta])); /** * Pre-built map of `tokenRef` → approved `TokenAnalysis.id` for the active language. Reset on * every mutation that changes the analysis. */ - const approvedAnalysisIdByTokenRef = useRef>( - buildApprovedGlossIndex(analysisRef.current, analysisByIdRef.current), + const approvedAnalysisIdByTokenRef = useRef | undefined>(undefined); + approvedAnalysisIdByTokenRef.current ??= buildApprovedGlossIndex( + analysisRef.current, + analysisByIdRef.current, ); /** @@ -145,9 +150,11 @@ export function AnalysisStoreProvider({ */ const getGloss = useCallback( (tokenRef: string) => { - const analysisId = approvedAnalysisIdByTokenRef.current.get(tokenRef); + // eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; TS can't see through the closure boundary + const analysisId = approvedAnalysisIdByTokenRef.current!.get(tokenRef); if (!analysisId) return ''; - const ta = analysisByIdRef.current.get(analysisId); + // eslint-disable-next-line no-type-assertion/no-type-assertion -- same: ??= guarantees non-null + const ta = analysisByIdRef.current!.get(analysisId); /* v8 ignore next -- optional chaining on ta?.gloss produces a branch V8 cannot reach through the mock */ return ta?.gloss?.[analysisLanguage] ?? ''; }, @@ -196,7 +203,8 @@ export function AnalysisStoreProvider({ }; analysisRef.current = next; - analysisByIdRef.current = new Map([...analysisByIdRef.current, [id, newAnalysis]]); + // eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; TS can't see through the closure boundary + analysisByIdRef.current = new Map([...analysisByIdRef.current!, [id, newAnalysis]]); approvedAnalysisIdByTokenRef.current = buildApprovedGlossIndex(next, analysisByIdRef.current); listenersRef.current.forEach((l) => l()); From c97dcdfa1fc8c47ac50517fc66cb027057ad9ac8 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 22 May 2026 15:19:47 -0600 Subject: [PATCH 12/13] Fire onGlossChange only when TokenChip input is blurred, adjust onGlossChange to keep stored analyses minimal for now --- .../components/AnalysisStore.test.tsx | 19 ++++- src/__tests__/components/TokenChip.test.tsx | 22 +++++- src/components/AnalysisStore.tsx | 73 +++++++++++++------ src/components/TokenChip.tsx | 19 +++-- 4 files changed, 99 insertions(+), 34 deletions(-) diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index e2c95578..4820a791 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -270,7 +270,7 @@ describe('useAnalysis', () => { }); describe('useGlossDispatch', () => { - it('creates a new approved TokenAnalysis on each write', async () => { + it('replaces the existing approved analysis on subsequent writes for the same token', async () => { render( @@ -280,9 +280,24 @@ describe('useGlossDispatch', () => { await userEvent.click(screen.getByRole('button', { name: 'write' })); await userEvent.click(screen.getByRole('button', { name: 'write' })); const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? ''); + expect(analysis.tokenAnalyses).toHaveLength(1); + expect(analysis.tokenAnalysisLinks).toHaveLength(1); + expect(analysis.tokenAnalysisLinks[0].status).toBe('approved'); + }); + + it('creates a new approved analysis when writing to a different token', async () => { + render( + + + + + , + ); + await userEvent.click(screen.getAllByRole('button', { name: 'write' })[0]); + await userEvent.click(screen.getAllByRole('button', { name: 'write' })[1]); + const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? ''); expect(analysis.tokenAnalyses).toHaveLength(2); expect(analysis.tokenAnalysisLinks).toHaveLength(2); - analysis.tokenAnalysisLinks.forEach((link) => expect(link.status).toBe('approved')); }); it('does not touch existing suggested analyses on write', async () => { diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 54be8574..4dd5a202 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -179,7 +179,7 @@ describe('TokenChip', () => { expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue(''); }); - it('calls the store onGlossChange spy with tokenId and value for each keystroke', async () => { + it('calls the store onGlossChange spy once on blur with the final value', async () => { const spy = jest.fn(); render( @@ -187,9 +187,23 @@ describe('TokenChip', () => { , ); await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in'); - expect(spy).toHaveBeenCalledTimes(2); - expect(spy).toHaveBeenNthCalledWith(1, 'GEN 1:1:0', 'i'); - expect(spy).toHaveBeenNthCalledWith(2, 'GEN 1:1:0', 'in'); + expect(spy).not.toHaveBeenCalled(); + await userEvent.tab(); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith('GEN 1:1:0', 'in'); + }); + + it('does not call the store onGlossChange spy when blurring without typing', async () => { + const spy = jest.fn(); + render( + + + , + ); + await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' })); + await userEvent.tab(); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith('GEN 1:1:0', ''); }); it('calls onFocus when the input is focused', async () => { diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index 939dd609..e4d98330 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -170,13 +170,11 @@ export function AnalysisStoreProvider({ const getAnalysis = useCallback(() => analysisRef.current, []); /** - * Creates a new approved `TokenAnalysis` + `TokenAnalysisLink` for `tokenRef` with the given - * gloss string (under `analysisLanguage`), replaces the analysis snapshot, notifies subscribers, - * calls `onSave`, and calls the optional `spy` prop for test observability. - * - * The new `TokenAnalysis` gets a UUID (`crypto.randomUUID()`) as its id to ensure global - * uniqueness. Existing analyses for the token are left untouched — this follows the data model's - * "multiple competing analyses" design; the UI manages selection and deletion separately. + * Writes an approved gloss for `tokenRef`. If an approved `TokenAnalysis` already exists for the + * token it is updated in-place; otherwise a new `TokenAnalysis` + `TokenAnalysisLink` are + * appended. Non-approved analyses for the token are left untouched. Replaces the analysis + * snapshot, notifies subscribers, calls `onSave`, and calls the optional `spy` prop for test + * observability. * * @param tokenRef - The `Token.ref` of the token being glossed. * @param surfaceText - The surface text of the token (stored as `Analysis.surfaceText`). @@ -184,28 +182,57 @@ export function AnalysisStoreProvider({ */ const onGlossChange = useCallback( (tokenRef: string, surfaceText: string, value: string) => { - const id = crypto.randomUUID(); - const newAnalysis: TokenAnalysis = { - id, - surfaceText, - gloss: { [analysisLanguage]: value }, - }; - const newLink: TokenAnalysisLink = { - analysisId: id, - status: 'approved', - token: { tokenRef, surfaceText }, - }; + // eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; TS can't see through the closure boundary + const existingApprovedId = approvedAnalysisIdByTokenRef.current!.get(tokenRef); + + let nextAnalyses: TokenAnalysis[]; + let nextLinks: TokenAnalysisLink[]; + let nextById: Map; + + if (existingApprovedId === undefined) { + const id = crypto.randomUUID(); + const newAnalysis: TokenAnalysis = { + id, + surfaceText, + gloss: { [analysisLanguage]: value }, + }; + const newLink: TokenAnalysisLink = { + analysisId: id, + status: 'approved', + token: { tokenRef, surfaceText }, + }; + nextAnalyses = [...analysisRef.current.tokenAnalyses, newAnalysis]; + nextLinks = [...analysisRef.current.tokenAnalysisLinks, newLink]; + // eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null + nextById = new Map([...analysisByIdRef.current!, [id, newAnalysis]]); + } else { + // Update the gloss on the existing approved analysis; preserve all other fields. + // eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; index only stores ids present in analysisByIdRef + const existing = analysisByIdRef.current!.get(existingApprovedId)!; + const updated: TokenAnalysis = { + ...existing, + surfaceText, + gloss: { ...existing.gloss, [analysisLanguage]: value }, + }; + nextAnalyses = analysisRef.current.tokenAnalyses.map( + /* v8 ignore next -- passthrough branch for non-matching tokens is structurally unreachable in tests */ + (ta) => (ta.id === existingApprovedId ? updated : ta), + ); + // Links are unchanged — the same link already points to existingApprovedId. + nextLinks = analysisRef.current.tokenAnalysisLinks; + // eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null + nextById = new Map([...analysisByIdRef.current!, [existingApprovedId, updated]]); + } const next: TextAnalysis = { ...analysisRef.current, - tokenAnalyses: [...analysisRef.current.tokenAnalyses, newAnalysis], - tokenAnalysisLinks: [...analysisRef.current.tokenAnalysisLinks, newLink], + tokenAnalyses: nextAnalyses, + tokenAnalysisLinks: nextLinks, }; analysisRef.current = next; - // eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; TS can't see through the closure boundary - analysisByIdRef.current = new Map([...analysisByIdRef.current!, [id, newAnalysis]]); - approvedAnalysisIdByTokenRef.current = buildApprovedGlossIndex(next, analysisByIdRef.current); + analysisByIdRef.current = nextById; + approvedAnalysisIdByTokenRef.current = buildApprovedGlossIndex(next, nextById); listenersRef.current.forEach((l) => l()); onSave?.(next); diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 36f2e7ec..8707d179 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -1,11 +1,12 @@ import type { Token } from 'interlinearizer'; -import { memo } from 'react'; +import { memo, useEffect, useState } from 'react'; import { useGloss, useGlossDispatch } from './AnalysisStore'; /** * Renders a single word token as an inline chip with an editable gloss input below the surface * text. Gloss value and dispatch are read from {@link AnalysisStoreProvider} context via - * {@link useGloss} and {@link useGlossDispatch}. + * {@link useGloss} and {@link useGlossDispatch}. The gloss is written to the store only on blur to + * avoid creating a new analysis entry on every keystroke. * * @param props - Component props * @param props.token - The word token to render. @@ -16,8 +17,15 @@ export function TokenChip({ token, onFocus, }: Readonly<{ token: Token & { type: 'word' }; onFocus: () => void }>) { - const gloss = useGloss(token.ref); + const committedGloss = useGloss(token.ref); const onGlossChange = useGlossDispatch(); + const [draft, setDraft] = useState(committedGloss); + + // Keep local draft in sync when the committed value changes externally (e.g. project switch). + useEffect(() => { + setDraft(committedGloss); + }, [committedGloss]); + return (