diff --git a/__mocks__/lucide-react.tsx b/__mocks__/lucide-react.tsx index e123b235..5cf52279 100644 --- a/__mocks__/lucide-react.tsx +++ b/__mocks__/lucide-react.tsx @@ -56,7 +56,7 @@ export function Link2(props: Readonly<{ size?: number; className?: string }>): R } /** - * Stub for the Link2Off (unlink) icon used by the between-token unlink and arc-split buttons. + * Stub for the Link2Off (unlink) icon used by the arc-split button. * * @param props - SVG props forwarded from the component. * @returns A ReactElement SVG element used as an unlink icon stub in tests. @@ -65,6 +65,17 @@ export function Link2Off(props: Readonly<{ size?: number; className?: string }>) return ; } +/** + * Stub for the Unlink2 icon used by the between-token unlink button; a broken-chain glyph whose + * chain sits at the same vertical position as Link2 so their button centers line up. + * + * @param props - SVG props forwarded from the component. + * @returns A ReactElement SVG element used as an unlink icon stub in tests. + */ +export function Unlink2(props: Readonly<{ size?: number; className?: string }>): ReactElement { + return ; +} + /** * Stub for the Settings gear icon used by the view-options dropdown button. * @@ -84,3 +95,25 @@ export function Settings(props: Readonly<{ size?: number; className?: string }>) export function Plus(props: Readonly<{ size?: number; className?: string }>): ReactElement { return ; } +/** + * Stub for the Merge icon used by both merge controls (the row-gap merge in the segment list and the + * cross-segment merge in the continuous strip). A single Y-join glyph, deliberately a different + * shape from the split marker's arrows-apart glyph. + * + * @param props - SVG props forwarded from the component. + * @returns A ReactElement SVG element used as a merge icon stub in tests. + */ +export function Merge(props: Readonly<{ className?: string }>): ReactElement { + return ; +} + +/** + * Stub for the Split icon used by the Alt-gated split markers (token-chip and baseline-text). One + * stroke diverging into two — the mirror of the `Merge` glyph's join. + * + * @param props - SVG props forwarded from the component. + * @returns A ReactElement SVG element used as a split-marker icon stub in tests. + */ +export function Split(props: Readonly<{ size?: number; className?: string }>): ReactElement { + return ; +} diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 655effc7..f0420834 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -3,7 +3,7 @@ * without extra transform configuration. This stub provides the subset used by the extension. */ -import { forwardRef, useEffect, useRef } from 'react'; +import { Children, cloneElement, forwardRef, isValidElement, useEffect, useRef } from 'react'; import type { MouseEventHandler, ReactElement, ReactNode } from 'react'; export interface MenuItemContainingCommand { @@ -523,3 +523,72 @@ export function Label({ ); } + +/** + * Marker component identifying the tooltip's hover text within a {@link Tooltip}. The real component + * renders a portaled popover on hover; this stub carries no markup of its own — {@link Tooltip} + * reads its text children and projects them onto the trigger (see there) so the tooltip text is + * assertable on the trigger element without simulating hover. + * + * @param props - Component props. + * @param props.children - The tooltip text. + * @returns `null`; the text is surfaced by {@link Tooltip}, not rendered here. + */ +export function TooltipContent({ children: _children }: Readonly<{ children?: ReactNode }>): null { + return null; +} + +/** + * Stub tooltip trigger. With `asChild` (the only mode the extension uses) the real component merges + * its trigger onto the single child element rather than rendering a wrapper; this stub renders the + * child unchanged and lets {@link Tooltip} clone it to attach the tooltip text. `asChild` is assumed + * throughout, so no non-`asChild` fallback is modeled. + * + * @param props - Component props. + * @param props.children - The element the tooltip is anchored to. + * @returns The child unchanged. + */ +export function TooltipTrigger({ + children, +}: Readonly<{ children?: ReactNode; asChild?: boolean }>): ReactElement { + return <>{children}; +} + +/** + * Stub tooltip root. The real component shows {@link TooltipContent} in a portaled popover on hover; + * because native and Radix tooltips are both invisible in jsdom, this stub instead reads the + * `TooltipContent` text from its children and clones the `TooltipTrigger`'s child element with that + * text applied as a `title` attribute. This keeps the tooltip text assertable on the trigger + * element (mirroring where hover text lived before the migration) without simulating hover, while + * the real component supplies the modifier-key-immune tooltip in production. + * + * @param props - Component props. + * @param props.children - A {@link TooltipTrigger} and a {@link TooltipContent}, in either order. + * @returns The trigger's child element cloned with the tooltip text as its `title`. + */ +export function Tooltip({ children }: Readonly<{ children?: ReactNode }>): ReactNode { + let tooltipText: ReactNode; + let triggerChild: ReactNode; + Children.forEach(children, (child) => { + if (!isValidElement(child)) return; + if (child.type === TooltipContent) tooltipText = child.props.children; + if (child.type === TooltipTrigger) triggerChild = child.props.children; + }); + if (!isValidElement(triggerChild)) return <>{children}; + const title = typeof tooltipText === 'string' ? tooltipText : undefined; + return cloneElement(triggerChild, { title }); +} + +/** + * Stub tooltip provider that shares hover-delay config across nested tooltips. The stub renders its + * children unchanged; the delay has no effect in tests. + * + * @param props - Component props. + * @param props.children - The subtree whose tooltips share this provider. + * @returns The children unchanged. + */ +export function TooltipProvider({ + children, +}: Readonly<{ children?: ReactNode; delayDuration?: number }>): ReactElement { + return <>{children}; +} diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 58aecbc0..a068f4c1 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -20,19 +20,19 @@ "%interlinearizer_viewOption_continuousScroll%": "Continuous Scroll", "%interlinearizer_viewOption_hideInactiveLinkButtons%": "Hide out-of-segment link buttons", "%interlinearizer_viewOption_simplifyPhrases%": "Show phrase controls on focus only", - "%interlinearizer_viewOption_chapterLabelInVerse%": "Show chapter in verse label", "%interlinearizer_projectSettings_hideInactiveLinkButtons%": "Hide Out-of-Segment Link Buttons", "%interlinearizer_projectSettings_hideInactiveLinkButtonsDescription%": "Hide link buttons between phrases in segments that are not currently active", "%interlinearizer_projectSettings_simplifyPhrases%": "Show Phrase Controls on Focus Only", "%interlinearizer_projectSettings_simplifyPhrasesDescription%": "Hide interactive controls (split, unlink, remove-token) on phrases that are not currently focused, leaving only their style change on hover", - "%interlinearizer_projectSettings_chapterLabelInVerse%": "Show Chapter in Verse Label", - "%interlinearizer_projectSettings_chapterLabelInVerseDescription%": "Mark chapter boundaries by labeling the first verse of each chapter as chapter:verse instead of showing an inline chapter header above it", "%interlinearizer_viewOption_showMorphology%": "Show morphology", "%interlinearizer_projectSettings_showMorphology%": "Show Morphology", "%interlinearizer_projectSettings_showMorphologyDescription%": "Display morpheme breakdown and per-morpheme glosses beneath each word token", "%interlinearizer_viewOption_showFreeTranslation%": "Show free translation", "%interlinearizer_projectSettings_showFreeTranslation%": "Show Free Translation", "%interlinearizer_projectSettings_showFreeTranslationDescription%": "Display a free-translation input beneath each segment's tokens or baseline text", + "%interlinearizer_viewOption_showVerseGutter%": "Show verse gutter", + "%interlinearizer_projectSettings_showVerseGutter%": "Show Verse Gutter", + "%interlinearizer_projectSettings_showVerseGutterDescription%": "Show each segment's verse range in a left gutter column instead of inline verse superscripts", "%interlinearizer_viewOption_showSuggestions%": "Show suggestions", "%interlinearizer_glossInput_placeholder%": "gloss", "%interlinearizer_freeTranslationInput_placeholder%": "Free translation", @@ -43,7 +43,10 @@ "%interlinearizer_morphemeGloss_label%": "Gloss for morpheme {form}", "%interlinearizer_tokenChip_editMorphemes%": "Edit morpheme breakdown for {token}", "%interlinearizer_tokenChip_defineMorphemes%": "Define morpheme breakdown for {token}", - "%interlinearizer_linkButton_crossSegmentDisabledTooltip%": "Cross-segment phrases are not supported. This link button is outside the current segment.", + "%interlinearizer_linkButton_crossSegmentDisabledTooltip%": "Tokens in different segments can't be linked. Join the segments first to link across this boundary.", + "%interlinearizer_boundaryControl_merge%": "Join these two segments", + "%interlinearizer_boundaryControl_mergeAltHint%": "Join these two segments. Hold Alt (Option on Mac) and click between words to split.", + "%interlinearizer_boundaryControl_split%": "Split segment here", "%interlinearizer_modal_create_title%": "Create Interlinear Project", "%interlinearizer_modal_create_name_label%": "Name (optional)", diff --git a/contributions/projectSettings.json b/contributions/projectSettings.json index 3ff292e3..18f56b60 100644 --- a/contributions/projectSettings.json +++ b/contributions/projectSettings.json @@ -17,11 +17,6 @@ "description": "%interlinearizer_projectSettings_simplifyPhrasesDescription%", "default": false }, - "interlinearizer.chapterLabelInVerse": { - "label": "%interlinearizer_projectSettings_chapterLabelInVerse%", - "description": "%interlinearizer_projectSettings_chapterLabelInVerseDescription%", - "default": false - }, "interlinearizer.showMorphology": { "label": "%interlinearizer_projectSettings_showMorphology%", "description": "%interlinearizer_projectSettings_showMorphologyDescription%", @@ -31,6 +26,11 @@ "label": "%interlinearizer_projectSettings_showFreeTranslation%", "description": "%interlinearizer_projectSettings_showFreeTranslationDescription%", "default": false + }, + "interlinearizer.showVerseGutter": { + "label": "%interlinearizer_projectSettings_showVerseGutter%", + "description": "%interlinearizer_projectSettings_showVerseGutterDescription%", + "default": false } } } diff --git a/cspell.json b/cspell.json index 09ef9b56..c9dfac79 100644 --- a/cspell.json +++ b/cspell.json @@ -56,6 +56,7 @@ "recentering", "recenters", "relayout", + "resegment", "resnap", "sandboxed", "scriptio", diff --git a/src/__tests__/components/AltHeldContext.test.tsx b/src/__tests__/components/AltHeldContext.test.tsx new file mode 100644 index 00000000..9d3b8ea1 --- /dev/null +++ b/src/__tests__/components/AltHeldContext.test.tsx @@ -0,0 +1,32 @@ +/** @file Unit tests for components/AltHeldContext.tsx. */ +/// +/// + +import { render, screen } from '@testing-library/react'; +import { AltHeldProvider, useAltHeldValue } from '../../components/AltHeldContext'; + +/** + * Renders the current Alt-held value from context as a testable string. + * + * @returns A span containing the current Alt-held value stringified. + */ +function AltHeldProbe() { + const altHeld = useAltHeldValue(); + return {String(altHeld)}; +} + +describe('AltHeldContext', () => { + it('delivers the provided value to consumers', () => { + render( + + + , + ); + expect(screen.getByTestId('probe')).toHaveTextContent('true'); + }); + + it('defaults to false for a consumer rendered without a provider', () => { + render(); + expect(screen.getByTestId('probe')).toHaveTextContent('false'); + }); +}); diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index 926487be..ee211df8 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -16,6 +16,7 @@ import { useMorphemeDeleteDispatch, useMorphemeGlossDispatch, useMorphemes, + usePhraseLinkByIdGetter, usePhraseLinkByIdMap, usePhraseLinkForToken, usePhraseLinkMap, @@ -417,8 +418,7 @@ describe('useGlossDispatch', () => { await userEvent.click(screen.getByRole('button', { name: 'write' })); - // tok-1 reads the new 'b'; tok-2 keeps the original 'a' because the reducer forked the - // shared payload before editing. + // tok-1 reads the new 'b'; tok-2 keeps 'a' because editing forked the shared payload. expect(screen.getByTestId('gloss')).toHaveTextContent('b'); const saved: TextAnalysis = onSave.mock.calls[0][0]; expect(saved.tokenAnalyses).toHaveLength(2); @@ -684,6 +684,45 @@ describe('usePhraseLinkByIdMap', () => { }); }); +/** + * Renders a component that reads the phrase-link map through `usePhraseLinkByIdGetter` at render + * time and displays its size, used to assert the getter resolves current store state. + * + * @returns JSX element suitable for passing to `render`. + */ +function PhraseLinkByIdGetterReader() { + const getPhraseLinkById = usePhraseLinkByIdGetter(); + return {getPhraseLinkById().size}; +} + +/** + * Renders a component that calls `usePhraseLinkByIdGetter` without a provider, to assert it throws. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function PhraseLinkByIdGetterUser() { + usePhraseLinkByIdGetter(); + return undefined; +} + +describe('usePhraseLinkByIdGetter', () => { + it('returns a getter resolving the current phrase-link-by-id map', () => { + render( + + + , + ); + expect(screen.getByTestId('getter-map-size')).toHaveTextContent('1'); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'usePhraseLinkByIdGetter must be used inside an AnalysisStoreProvider', + ); + }); +}); + describe('usePhraseDispatch', () => { it('createPhrase adds a new phrase and calls onSave', async () => { const onSave = jest.fn(); @@ -755,8 +794,7 @@ describe('usePhraseDispatch', () => { await userEvent.click(screen.getByRole('button', { name: 'merge' })); - // A single dispatch means a single save — the intermediate state where tokens belonged to two - // phrases is never observed. + // A single dispatch means a single save — the intermediate two-phrase state is never observed. expect(onSave).toHaveBeenCalledTimes(1); const saved: TextAnalysis = onSave.mock.calls[0][0]; expect(saved.phraseAnalysisLinks[0].tokens.map((t) => t.tokenRef)).toStrictEqual([ @@ -1566,6 +1604,7 @@ describe('useReportGlossEditing', () => { * * @param props.tokenRef - Token ref to resolve. * @param props.surfaceText - The token's surface text. + * @param props.enabled - Whether the hook should derive; `false` short-circuits to `undefined`. * @param props.sink - Array each render appends its resolved value to (for render-count * assertions). * @returns JSX element suitable for passing to `render`. @@ -1573,13 +1612,15 @@ describe('useReportGlossEditing', () => { function ResolvedReader({ tokenRef, surfaceText, + enabled = true, sink, }: Readonly<{ tokenRef: string; surfaceText: string; + enabled?: boolean; sink?: (ResolvedTokenAnalysis | undefined)[]; }>) { - const resolved = useResolvedTokenAnalysis(tokenRef, surfaceText); + const resolved = useResolvedTokenAnalysis(tokenRef, surfaceText, enabled); sink?.push(resolved); return {resolved?.status ?? 'none'}; } @@ -1676,6 +1717,19 @@ describe('useResolvedTokenAnalysis', () => { expect(screen.getByTestId('resolved')).toHaveTextContent('none'); }); + it('short-circuits to undefined without deriving when disabled', () => { + // With enabled=false the hook must not resolve even a token that would otherwise be approved. + render( + + + , + ); + expect(screen.getByTestId('resolved')).toHaveTextContent('none'); + }); + it('does not re-render a suggested token when an unrelated token is glossed', async () => { const sink: (ResolvedTokenAnalysis | undefined)[] = []; render( @@ -1689,8 +1743,8 @@ describe('useResolvedTokenAnalysis', () => { ); const before = sink.length; - // Glossing 'cat' rebuilds the pool but leaves the 'logos' suggestion untouched; the custom - // equalityFn keeps the selected value referentially stable so the reader never re-renders. + // Glossing 'cat' rebuilds the pool but leaves the 'logos' suggestion referentially stable (via + // the custom equalityFn), so the reader never re-renders. await userEvent.click(screen.getByRole('button', { name: 'write' })); expect(sink.length).toBe(before); diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index dff035e8..c66d4fa7 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -7,8 +7,14 @@ import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event'; import type { Book, PhraseAnalysisLink, Token } from 'interlinearizer'; import { useState, type ReactNode } from 'react'; +import { resegmentBook } from 'parsers/papi/resegmentBook'; import type { PhraseDispatch } from '../../components/AnalysisStore'; +import { AltHeldProvider } from '../../components/AltHeldContext'; import ContinuousView from '../../components/ContinuousView'; +import { + SegmentationProvider, + type SegmentationContextValue, +} from '../../components/SegmentationStore'; import { isWordToken } from '../../types/type-guards'; import type { ViewOptions } from '../../types/view-options'; import { allFalseViewOptions, withAnalysisStore } from './test-helpers'; @@ -46,9 +52,8 @@ jest.mock('../../components/AnalysisStore', () => ({ usePhraseGlossDispatch: () => () => {}, })); -// The shared hover-preview state is covered in full by usePhraseHoverState.test.ts. Stub it here so -// ContinuousView's tests don't redundantly re-exercise the hook's internals; the view only forwards -// its handlers, which a no-op stub satisfies. +// Hover-preview state is covered by usePhraseHoverState.test.ts; the view only forwards its +// handlers, so a no-op stub suffices. const mockCandidateTokenRefs = { current: new Set() }; jest.mock('../../hooks/usePhraseHoverState', () => ({ __esModule: true, @@ -67,9 +72,9 @@ jest.mock('../../hooks/usePhraseHoverState', () => ({ jest.mock('../../components/TokenChip'); /** - * Spy invoked once per rendered link icon (mounted, whether suppressed or not). Rendering a span - * with data attributes encoding the token refs lets DOM queries check suppression state via the - * parent wrapper's style. Cleared in `beforeEach`. + * Spy invoked once per rendered link icon (mounted, whether suppressed or not). The rendered span + * encodes the token refs as data attributes so DOM queries can check suppression via the parent + * wrapper's style. Cleared in `beforeEach`. */ const tokenLinkIconSpy = jest.fn(); jest.mock('../../components/TokenLinkIcon', () => ({ @@ -179,6 +184,7 @@ function makeBook(overrides?: Partial): Book { charEnd: 6, }, ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], }, { id: 'GEN 1:2', @@ -203,6 +209,7 @@ function makeBook(overrides?: Partial): Book { charEnd: 13, }, ], + verseStarts: [{ charStart: 0, number: '2', chapter: 1 }], }, ], ...overrides, @@ -231,6 +238,7 @@ function makeTwoChapterBook(): Book { charEnd: 5, }, ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], }, { id: 'GEN 2:1', @@ -247,6 +255,7 @@ function makeTwoChapterBook(): Book { charEnd: 4, }, ], + verseStarts: [{ charStart: 0, number: '1', chapter: 2 }], }, ], }; @@ -274,6 +283,7 @@ function makeSingleTokenBook(): Book { charEnd: 4, }, ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], }, ], }; @@ -301,6 +311,7 @@ function makeMixedBook(): Book { charEnd: 2, }, ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], }, { id: 'GEN 1:2', @@ -317,6 +328,7 @@ function makeMixedBook(): Book { charEnd: 1, }, ], + verseStarts: [{ charStart: 0, number: '2', chapter: 1 }], }, ], }; @@ -344,6 +356,7 @@ function makeWordFreeBook(): Book { charEnd: 1, }, ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], }, ], }; @@ -370,6 +383,7 @@ function makeLargeBook(count: number): Book { charEnd: String(`word${i}`).length, }, ], + verseStarts: [{ charStart: 0, number: String(i + 1), chapter: 1 }], })), }; } @@ -445,6 +459,39 @@ function requiredProps( }; } +/** Every {@link TrackingResizeObserver} created since the last reset, newest last. */ +let resizeObserverInstances: TrackingResizeObserver[] = []; + +/** + * A ResizeObserver test double that records its callback and disconnect state and appends itself to + * {@link resizeObserverInstances}. Used by the hold-loop tests to fire a simulated late content + * reflow and to assert whether the active observer was disconnected. Module-scoped (rather than an + * inline class per test) so the file stays under `max-classes-per-file`. + */ +class TrackingResizeObserver implements ResizeObserver { + /** Whether {@link disconnect} has been called on this instance. */ + disconnected = false; + + /** @param callback - Stored so a test can fire it, simulating a late content reflow. */ + constructor(public callback: ResizeObserverCallback) { + resizeObserverInstances.push(this); + } + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + observe() {} + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + unobserve() {} + + /** + * Records the disconnect so tests can distinguish a still-connected observer from a torn-down + * one. + */ + disconnect() { + this.disconnected = true; + } +} + beforeAll(() => { // jsdom does not implement scrollIntoView. HTMLElement.prototype.scrollIntoView = scrollIntoViewMock; @@ -484,12 +531,65 @@ describe('ContinuousView initial render', () => { expect(screen.getByText('God')).toBeInTheDocument(); }); - it('does not render any verse label or segment separator', () => { + it('renders an inline verse-number superscript at each verse start', () => { + const book = makeBook(); + render(, withAnalysisStore); + + // Verses 1 and 2; the first opens chapter 1, so its label is chapter-qualified (`1:1`). + const sups = screen.getAllByTestId('verse-superscript'); + expect(sups.map((s) => s.textContent)).toEqual(['1:1', '2']); + }); + + it('renders no verse superscript at a mid-verse continuation start', () => { + // Verse 1 split across two segments; the second's verse start is flagged isContinuation. + const splitBook = makeBook({ + segments: [ + { + id: 'GEN 1:1', + startRef: { book: 'GEN', chapter: 1, verse: 1 }, + endRef: { book: 'GEN', chapter: 1, verse: 1, charIndex: 3 }, + baselineText: 'In', + tokens: [ + { + ref: 'tok-0', + surfaceText: 'In', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 2, + }, + ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], + }, + { + id: 'GEN 1:1:3', + startRef: { book: 'GEN', chapter: 1, verse: 1, charIndex: 3 }, + endRef: { book: 'GEN', chapter: 1, verse: 1 }, + baselineText: 'the', + tokens: [ + { + ref: 'tok-1', + surfaceText: 'the', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 3, + }, + ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1, isContinuation: true }], + }, + ], + }); + render(, withAnalysisStore); + + const sups = screen.getAllByTestId('verse-superscript'); + expect(sups.map((s) => s.textContent)).toEqual(['1:1']); + }); + + it('does not render an extension-generated segment separator', () => { const book = makeBook(); render(, withAnalysisStore); - expect(screen.queryByText('1:1')).not.toBeInTheDocument(); - expect(screen.queryByText('1:2')).not.toBeInTheDocument(); expect(screen.queryByText('GEN 1:1')).not.toBeInTheDocument(); }); @@ -544,17 +644,16 @@ describe('ContinuousView initial render', () => { }); it('falls back to focusedTokenRef when the lagging displayed ref is from another book', () => { - // During a book change displayFocusedTokenRef lags by one fade, so it briefly names a token from - // the previous book that no longer exists in the new book. The focus must follow the live - // focusedTokenRef (the new book's active verse) rather than collapsing to the book's first phrase. + // During a book change displayFocusedTokenRef lags by one fade, briefly naming a token absent + // from the new book; focus must follow the live focusedTokenRef, not collapse to phrase 0. const book = makeBook(); const { rerender } = render( , withAnalysisStore, ); - // Swap to a different book whose token refs share none of the previous book's. The displayed ref - // ('tok-2') is now absent; focusedTokenRef points at the new book's *second* phrase. + // A different book sharing no token refs: the displayed 'tok-2' is now absent, and + // focusedTokenRef points at the new book's second phrase. const otherBook: Book = { id: 'MAT', bookRef: 'MAT', @@ -575,6 +674,7 @@ describe('ContinuousView initial render', () => { charEnd: 5, }, ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], }, { id: 'MAT 1:2', @@ -591,6 +691,7 @@ describe('ContinuousView initial render', () => { charEnd: 4, }, ], + verseStarts: [{ charStart: 0, number: '2', chapter: 1 }], }, ], }; @@ -598,9 +699,7 @@ describe('ContinuousView initial render', () => { scrollIntoViewMock.mockClear(); rerender(); - // The scroll target is resolved through focusPhraseIndex, which falls back to focusedTokenRef - // ('mat-tok-1', the second phrase) rather than collapsing to phrase 0. So the element scrolled - // into view is the one containing "Beta", never "Alpha". + // The scroll lands on "Beta" (the focusedTokenRef phrase), never "Alpha" (phrase 0). const scrolledTexts = scrollIntoViewMock.mock.contexts.map((el) => el instanceof HTMLElement ? el.textContent : undefined, ); @@ -641,9 +740,8 @@ describe('ContinuousView focus changes', () => { }); it('does not notify the parent when clicking the group of an already-focused non-first token', async () => { - // Group tok-0 and tok-1 into one phrase box (keyed by tok-0), then focus tok-1 — the second - // token of the group, as a segment-view click on a middle token would. Clicking the box must - // stay a no-op even though its groupKey (tok-0) differs from focusedTokenRef (tok-1). + // tok-0/tok-1 grouped into one box (keyed by tok-0) with focus on tok-1: clicking the box stays + // a no-op even though its groupKey differs from focusedTokenRef. const phraseLink: PhraseAnalysisLink = { analysisId: 'phrase-1', status: 'approved', @@ -804,13 +902,11 @@ describe('ContinuousView arrow navigation', () => { }); it('steps from the externally-imposed focus, not the stale pending index, after an external change interrupts an in-flight internal nav', async () => { - // Sequence: an external nav (tok-3) starts its fade while tok-1 is still displayed; the user - // clicks Next during the fade (internal nav in flight — this parent never echoes it); then a - // second external change lands back on the still-displayed tok-1. Because that value equals the - // displayed ref, the focus-change effect early-returns without clearing the in-flight marker, - // so only the render-phase external-override detection resyncs the pending index. Without it, - // the next step would advance from the stale pending index (group 2 → tok-1) instead of the - // externally-imposed position (group 1 → tok-0). + // An external nav (tok-3) fades while tok-1 is displayed; the user clicks Next mid-fade (internal + // nav in flight, never echoed); a second external change lands back on the still-displayed tok-1. + // Since that equals the displayed ref, the focus-change effect early-returns without clearing the + // in-flight marker, so render-phase external-override detection must resync the pending index — + // otherwise the next step advances from the stale pending index instead of the imposed position. const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-1' }); const { rerender } = render(, withAnalysisStore); @@ -863,6 +959,159 @@ describe('ContinuousView scroll behavior', () => { expect(scrollIntoViewMock).toHaveBeenCalledWith(expect.objectContaining({ behavior: 'auto' })); }); + it('holds the group centered on animation frames after an external jump within the active segment', () => { + // An external jump within the already-active segment never flips committedActiveSegmentId, so the + // scroll effect's own hold loop must keep the group pinned while late layout (arc padding + // settling on the slid window) shifts the strip after the instant snap. + const book = makeBook(); + const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); + const { rerender } = render(, withAnalysisStore); + + act(() => { + jest.useFakeTimers(); + }); + try { + // tok-1 shares GEN 1:1 with tok-0, so the active segment is unchanged by this jump. + rerender(); + // Complete the fade-out (RECENTER_FADE_MS) so the displayed focus updates and the instant + // snap fires. + scrollIntoViewMock.mockClear(); + act(() => { + jest.advanceTimersByTime(510); + }); + expect(scrollIntoViewMock).toHaveBeenCalledWith( + expect.objectContaining({ behavior: 'auto', inline: 'center' }), + ); + + // Frames within the hold window keep re-centering after the snap. + scrollIntoViewMock.mockClear(); + act(() => { + jest.advanceTimersByTime(50); + }); + expect(scrollIntoViewMock).toHaveBeenCalledWith( + expect.objectContaining({ behavior: 'auto', inline: 'center' }), + ); + + // Past the deadline the loop stops scheduling further frames. + act(() => { + jest.advanceTimersByTime(500); + }); + scrollIntoViewMock.mockClear(); + act(() => { + jest.advanceTimersByTime(500); + }); + expect(scrollIntoViewMock).not.toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } + }); + + it('re-centers the focused group when the strip content reflows after the initial hold window', () => { + // Glosses/morpheme rows/arcs settle asynchronously over many frames; content widening to the + // left of the focus shifts the focused box sideways. The hold observes the content row and + // re-centers on each reflow, so a resize after the initial quiet window still snaps to center. + const originalResizeObserver = global.ResizeObserver; + resizeObserverInstances = []; + global.ResizeObserver = TrackingResizeObserver; + + try { + const book = makeBook(); + const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); + const { rerender } = render(, withAnalysisStore); + + act(() => { + jest.useFakeTimers(); + }); + try { + // tok-1 shares GEN 1:1 with tok-0, so the active segment is unchanged: only the scroll + // effect's own hold loop keeps the group pinned. + rerender(); + act(() => { + jest.advanceTimersByTime(510); + }); + + // Let the initial quiet window fully lapse. + act(() => { + jest.advanceTimersByTime(1000); + }); + scrollIntoViewMock.mockClear(); + + // A late content reflow fires the content-row observer; the hold must re-center in response. + act(() => { + const observer = resizeObserverInstances.at(-1); + if (!observer) throw new Error('Expected the hold to observe the content row'); + observer.callback([], { disconnect() {}, observe() {}, unobserve() {} }); + jest.advanceTimersByTime(16); + }); + + expect(scrollIntoViewMock).toHaveBeenCalledWith( + expect.objectContaining({ behavior: 'auto', inline: 'center' }), + ); + } finally { + jest.useRealTimers(); + } + } finally { + global.ResizeObserver = originalResizeObserver; + } + }); + + it('keeps the content-row observer connected after the quiet window so a late reflow still re-centers', () => { + // The dominant strip-width reflow (gloss-placeholder resolution / arc settle) can land after the + // hold's 200ms quiet window lapses. The observer must remain connected for the full hard-deadline + // window, restarting the re-center loop whenever a late reflow fires. + const originalResizeObserver = global.ResizeObserver; + resizeObserverInstances = []; + global.ResizeObserver = TrackingResizeObserver; + + try { + const book = makeBook(); + const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); + const { rerender } = render(, withAnalysisStore); + + act(() => { + jest.useFakeTimers(); + }); + try { + // Drive the hold loop via an external jump so it runs under fake-timer control (jsdom rAF on + // the initial real-timer mount is not deterministically advanceable). tok-1 shares GEN 1:1 + // with tok-0, so there's no committed-active-segment flip: only the scroll effect's own hold + // pins the group, exercising the same observer lifetime as the initial mount. + rerender(); + // Complete the fade-out (RECENTER_FADE_MS) so the instant snap + hold start. + act(() => { + jest.advanceTimersByTime(510); + }); + + // Let the quiet window (LINK_SLOT_TRANSITION_MS = 200) fully lapse so the tick loop goes + // idle — but stay well within HOLD_CENTERED_MAX_MS (2000). + act(() => { + jest.advanceTimersByTime(400); + }); + + // The active (latest) observer must still be connected to catch a reflow that lands this + // late. Superseded holds torn down during the fade/rerender are earlier instances; the last + // one is the live hold. + const activeObserver = resizeObserverInstances.at(-1); + if (!activeObserver) throw new Error('Expected the hold to create a content-row observer'); + expect(activeObserver.disconnected).toBe(false); + + // A late content reflow fires the still-connected observer; the hold re-centers in response. + scrollIntoViewMock.mockClear(); + act(() => { + activeObserver.callback([], { disconnect() {}, observe() {}, unobserve() {} }); + jest.advanceTimersByTime(16); + }); + expect(scrollIntoViewMock).toHaveBeenCalledWith( + expect.objectContaining({ behavior: 'auto', inline: 'center' }), + ); + } finally { + jest.useRealTimers(); + } + } finally { + global.ResizeObserver = originalResizeObserver; + } + }); + it('snaps the link slots (no transition) during an external jump so they do not slide after the fade-in', () => { const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); @@ -872,12 +1121,11 @@ describe('ContinuousView scroll behavior', () => { jest.useFakeTimers(); }); // External nav into the other verse: the active segment commits instantly behind the fade, so - // the slots must snap to their new widths rather than animating (which would slide the boxes for - // ~200ms after the strip fades back in). + // the slots snap to their new widths rather than animating (which would slide the boxes). rerender(); - const slotWrapper = container.querySelector('[data-link-slot] > span'); - if (!(slotWrapper instanceof HTMLElement)) throw new Error('Expected a link-slot wrapper span'); + const slotWrapper = container.querySelector('[data-testid="link-slot-icon"]'); + if (!(slotWrapper instanceof HTMLElement)) throw new Error('Expected a link-slot icon wrapper'); expect(slotWrapper.style.transitionDuration).toBe('0ms'); act(() => { @@ -887,10 +1135,9 @@ describe('ContinuousView scroll behavior', () => { }); it('smooth-scrolls for internal nav once the parent echoes the ref back synchronously', async () => { - // The smooth-scroll path requires the displayed focus to already agree with the prop and the - // strip to be visible when the scroll effect runs. That only happens when a real (stateful) - // parent reflects the internal ref change straight back, so simulate one here rather than - // driving the ref via a jest.fn() that never updates the prop. + // The smooth-scroll path needs the displayed focus to agree with the prop and the strip to be + // visible, which only happens with a real (stateful) parent reflecting the ref change back, so + // simulate one here rather than a jest.fn() that never updates the prop. const book = makeBook(); const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); function Parent() { @@ -911,8 +1158,8 @@ describe('ContinuousView scroll behavior', () => { ); } render(, withAnalysisStore); - // Wait for the initial-load requestAnimationFrame fade-in to complete (strip becomes visible) - // before navigating; the smooth path is only taken while the strip is already visible. + // Wait for the initial fade-in (strip visible) before navigating; the smooth path is only taken + // while the strip is already visible. await waitFor(() => expect(screen.getByTestId('strip-fade-wrapper').className).toContain('tw:opacity-100'), ); @@ -956,9 +1203,8 @@ describe('ContinuousView scroll behavior', () => { ); } render(, withAnalysisStore); - // Returns true when the tok-0/tok-1 link icon is rendered AND its wrapper is visible (not - // suppressed). Icons stay mounted but are hidden via opacity:0 when suppressed, so - // we query the DOM wrapper's style rather than spy calls. + // Returns true when the tok-0/tok-1 link icon is rendered and its wrapper is visible. Suppressed + // icons stay mounted but hidden via opacity:0, so query the wrapper's style, not spy calls. return () => { const icon = document.querySelector( '[data-prev-ref="tok-0"][data-next-ref="tok-1"]', @@ -969,10 +1215,9 @@ describe('ContinuousView scroll behavior', () => { } it('keeps the old segment’s link icon until the scroll settles, then drops it on scrollend', async () => { - // With hideInactiveLinkButtons on, crossing a boundary wants to add/remove icons — but doing so - // mid-scroll shifts every box and breaks the smooth glide. The view defers the active-segment - // switch until the scroll settles (signaled by the container's `scrollend`), so the old segment - // keeps its icon during the animation and only loses it once the scroll finishes. + // With hideInactiveLinkButtons on, adding/removing icons mid-scroll would shift every box and + // break the glide. The view defers the active-segment switch until the scroll settles (the + // container's `scrollend`), so the old segment keeps its icon through the animation. const inSegmentIconMounted = renderHideInactiveCrossing(); await waitFor(() => expect(screen.getByTestId('strip-fade-wrapper').className).toContain('tw:opacity-100'), @@ -985,9 +1230,8 @@ describe('ContinuousView scroll behavior', () => { fireEvent.click(screen.getByRole('button', { name: 'Next token' })); expect(inSegmentIconMounted()).toBe(true); - // The scroll settles → `scrollend` fires on the clipping viewport (the element that actually - // scrolls) → the active segment switches to GEN 1:2 and the GEN 1:1 icon disappears (its - // in-segment slot is now inactive and suppressed). + // On `scrollend` (fired on the clipping viewport that actually scrolls), the active segment + // switches to GEN 1:2 and the GEN 1:1 icon disappears (its in-segment slot is now suppressed). tokenLinkIconSpy.mockClear(); act(() => { screen.getByTestId('strip-scroll-viewport').dispatchEvent(new Event('scrollend')); @@ -1014,9 +1258,8 @@ describe('ContinuousView scroll behavior', () => { }); it('commits the deferred relayout via the fallback timeout when scrollend never fires', () => { - // Browsers without `scrollend` (or when the target was already centered, so no scroll happens) - // must still commit the deferred relayout. A backstop timeout covers that case. Fake timers are - // installed before render so every scheduled timer is captured, then advanced past the fallback. + // Browsers without `scrollend` (or when the target was already centered) still commit via a + // backstop timeout. Fake timers are installed before render so every timer is captured. jest.useFakeTimers(); try { const inSegmentIconMounted = renderHideInactiveCrossing(); @@ -1044,12 +1287,82 @@ describe('ContinuousView scroll behavior', () => { } }); + it('defers the reconcile to the scroll settle when a boundary edit lands mid-glide', () => { + // A boundary edit that changes the focused token's segment id mid-glide must not commit the + // active segment early (that would flip committedActiveSegmentId and truncate the glide into a + // jump); the reconcile defers to the scroll's settle. Probe: with the old segment still + // committed, its in-segment link icon stays mounted through the edit until the scroll settles. + const book = makeBook(); + const merged = resegmentBook(book, { removedVerseStarts: ['tok-2'], addedStarts: [] }); + const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); + const mergedLookups = buildLookups(merged); + let applyBoundaryEdit: () => void = () => {}; + /** + * Stateful parent that starts on the verse book and swaps to the merged book on demand. + * + * @returns The rendered `ContinuousView` element. + */ + function Parent() { + const [ref, setRef] = useState('tok-1'); + const [edited, setEdited] = useState(false); + applyBoundaryEdit = () => setEdited(true); + const lookups = edited ? mergedLookups : { tokenSegmentMap, tokenDocOrder, wordTokenByRef }; + return ( + + ); + } + /** + * Whether the tok-0/tok-1 link icon (in GEN 1:1) is mounted and visible. + * + * @returns `true` when that in-segment link icon is mounted and not hidden. + */ + const inSegmentIconMounted = () => { + const icon = document.querySelector( + '[data-prev-ref="tok-0"][data-next-ref="tok-1"]', + ); + return !!icon && icon.parentElement?.style.opacity !== '0'; + }; + jest.useFakeTimers(); + try { + render(, withAnalysisStore); + act(() => { + jest.runOnlyPendingTimers(); + }); + expect(inSegmentIconMounted()).toBe(true); + + // Step into GEN 1:2 (internal nav) — the scroll is now animating and its commit is pending. + act(() => { + fireEvent.click(screen.getByRole('button', { name: 'Next token' })); + }); + expect(inSegmentIconMounted()).toBe(true); + + // Boundary edit mid-glide: merge GEN 1:2 into GEN 1:1. Token refs survive so focus is + // unchanged; the reconcile defers rather than commit-and-relayout, so the old segment's + // in-segment icon stays mounted. + act(() => { + applyBoundaryEdit(); + }); + expect(inSegmentIconMounted()).toBe(true); + } finally { + jest.useRealTimers(); + } + }); + it('re-centers the focused group each frame while the inactive-link slots animate, then stops', () => { - // After the active segment commits (post-scrollend), the inactive-link slots slide open/closed - // over LINK_SLOT_TRANSITION_MS, continuously shifting every box around the center. The view - // re-centers the focused group on every animation frame for that whole window so it stays dead - // center, then tears the loop down once the transition completes. Fake timers drive both the - // rAF callbacks and performance.now() deterministically. + // After the active segment commits, the inactive-link slots slide open/closed over + // LINK_SLOT_TRANSITION_MS, shifting every box around the center. The view re-centers on every + // frame for that window, then tears the loop down once the transition completes. jest.useFakeTimers(); try { const inSegmentIconMounted = renderHideInactiveCrossing(); @@ -1109,15 +1422,13 @@ describe('ContinuousView scroll behavior', () => { }); it('re-centers once when simplifyPhrases toggles but not when hideInactiveLinkButtons toggles', () => { - // Inactive link slots are now hidden via visibility:hidden (not max-width collapse), so toggling - // hideInactiveLinkButtons no longer shifts the strip layout — no re-center needed. - // simplifyPhrases still affects layout, so it should trigger one re-center. + // Inactive link slots hide via visibility:hidden (not max-width collapse), so toggling + // hideInactiveLinkButtons doesn't shift layout; simplifyPhrases does, so it re-centers once. const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); const { rerender } = render(, withAnalysisStore); scrollIntoViewMock.mockClear(); - // Toggling hideInactiveLinkButtons should not cause any re-centering. rerender( { ); expect(scrollIntoViewMock).not.toHaveBeenCalled(); - // Toggling simplifyPhrases re-centers exactly once (no rAF loop needed). rerender( { }); }); +// --------------------------------------------------------------------------- +// Segmentation edits +// --------------------------------------------------------------------------- + +describe('ContinuousView segmentation edits', () => { + /** + * Reads the inline opacity of the link-slot wrapper between `prevRef` and `nextRef`, the style + * `PhraseSlot` uses to suppress link buttons outside the active segment. + * + * @param container - The render container to query. + * @param prevRef - Token ref on the start side of the slot. + * @param nextRef - Token ref on the end side of the slot. + * @returns The wrapper's inline `opacity` value. + */ + function slotOpacity(container: HTMLElement, prevRef: string, nextRef: string): string { + const icon = container.querySelector( + `[data-prev-ref="${prevRef}"][data-next-ref="${nextRef}"]`, + ); + const wrapper = icon?.parentElement; + if (!(wrapper instanceof HTMLElement)) throw new Error('Expected a link-slot wrapper span'); + return wrapper.style.opacity; + } + + it('keeps the focused segment link buttons active when a merge changes the focused token segment id', () => { + const book = makeBook(); + const props = requiredProps(book, { focusedTokenRef: 'tok-2' }); + props.viewOptions = { ...allFalseViewOptions, hideInactiveLinkButtons: true }; + const { container, rerender } = render(, withAnalysisStore); + + // Focus sits in GEN 1:2, so the slot between its two tokens is active and visible. + expect(slotOpacity(container, 'tok-2', 'tok-3')).toBe('1'); + + // Merge GEN 1:2 into GEN 1:1. Token refs survive, so focus stays put, but the focused token's + // segment id changes. + const merged = resegmentBook(book, { removedVerseStarts: ['tok-2'], addedStarts: [] }); + rerender(); + + // The committed active segment must follow the merge; a stale id would suppress every link + // button until the next navigation. + expect(slotOpacity(container, 'tok-2', 'tok-3')).toBe('1'); + }); +}); + +// --------------------------------------------------------------------------- +// Alt-gated split marker (shared PhraseStripParts, confirmed here in the strip) +// --------------------------------------------------------------------------- + +describe('ContinuousView split marker', () => { + /** + * Renders ContinuousView wrapped in the segmentation and Alt-held providers so the shared + * split-gap marker can be exercised in the horizontal strip. + * + * @param altHeld - Whether Alt is held (defaults to held, so the marker appears). + * @returns The dispatch spy for assertions. + */ + function renderStrip(altHeld = true) { + const book = makeBook(); + const dispatch = { merge: jest.fn(), split: jest.fn(), move: jest.fn() }; + const segmentById = new Map(book.segments.map((seg) => [seg.id, seg])); + const segmentOrder = new Map(book.segments.map((seg, i) => [seg.id, i])); + const value: SegmentationContextValue = { + dispatch, + segmentById, + segmentOrder, + formerBoundaries: new Map(), + straddledBoundaryRefs: new Set(), + }; + render( + + + + + , + withAnalysisStore, + ); + return dispatch; + } + + it('reveals a split marker on an intra-segment gap while Alt is held', () => { + renderStrip(true); + expect(screen.getAllByTestId('boundary-split-marker').length).toBeGreaterThan(0); + }); + + it('reveals no split marker while Alt is not held', () => { + renderStrip(false); + expect(screen.queryByTestId('boundary-split-marker')).not.toBeInTheDocument(); + }); + + it('dispatches a split on an Alt+click of the strip marker', () => { + const dispatch = renderStrip(true); + // The gap between "In" (tok-0) and "the" (tok-1) inside GEN 1:1 splits before the second word. + fireEvent.click(screen.getAllByTestId('boundary-split-marker')[0], { altKey: true }); + expect(dispatch.split).toHaveBeenCalledWith('tok-1'); + }); +}); + // --------------------------------------------------------------------------- // RTL layout // --------------------------------------------------------------------------- @@ -1278,9 +1684,8 @@ describe('ContinuousView phrase grouping', () => { }); it('clears the hovered phrase highlight when the pointer leaves the token strip', async () => { - // Group tok-0/tok-1 into one hoverable phrase so hovering it sets hoveredPhraseId, which - // ContinuousView forwards to ArcOverlay. Leaving the strip runs clearAllHoverState, which must - // reset hoveredPhraseId to undefined. + // tok-0/tok-1 grouped into one hoverable phrase so hovering it sets hoveredPhraseId (forwarded + // to ArcOverlay); leaving the strip must reset it to undefined. const phraseLink: PhraseAnalysisLink = { analysisId: 'phrase-1', status: 'approved', @@ -1309,12 +1714,9 @@ describe('ContinuousView phrase grouping', () => { }); it('applies the internal focus transition when the parent reflects a click-driven ref change', async () => { - // Simulate: ContinuousView clicks Next, sets internalFocusedTokenRefRef, calls - // onFocusedTokenRefChange. The parent then passes the new focusedTokenRef back. This exercises - // the isInternal=true branch of the focus-change effect, which applies the new ref *immediately* - // (setDisplayFocusedTokenRef) without fading the strip out. The external (non-internal) branch - // would instead defer the display update behind a fade timeout, so the focused box would still - // be 'In' (tok-0) right after the rerender. + // A click Next stamps the ref internally-originated; when the parent echoes it back, the + // isInternal branch applies it immediately (no fade-out). The external branch would defer the + // display update behind a fade timeout, leaving 'In' (tok-0) focused right after the rerender. const book = makeBook(); const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); const { rerender } = render(, withAnalysisStore); @@ -1326,12 +1728,10 @@ describe('ContinuousView phrase grouping', () => { ); await userEvent.click(screen.getByRole('button', { name: 'Next token' })); - // Now reflect the new ref back as a prop change (as a real parent would do). Because the click - // stamped tok-1 as internally-originated, the echo is recognized as internal and applied at once. + // Reflect the new ref back as a prop change; the click stamped it internal, so it applies at once. rerender(); - // The displayed focus moved synchronously to tok-1's box ('the') — the internal path, not the - // fade-then-snap external path (which would leave 'In' focused until the fade timeout fires). + // The displayed focus moved synchronously to tok-1's box ('the') — the internal path. expect(screen.getByText('the').closest('[data-phrase-box="true"]')).toHaveAttribute( 'data-focus-state', 'focused', @@ -1453,8 +1853,7 @@ describe('ContinuousView phrase grouping', () => { mockCandidateTokenRefs.current = new Set(['tok-0']); const book = makeBook(); render(, withAnalysisStore); - // useCandidatePhraseIds resolves the hovered candidate ref (tok-0) to the phrase that contains - // it, and ContinuousView forwards the set to ArcOverlay. The mock surfaces it as a data attr. + // The hovered candidate ref (tok-0) resolves to its phrase, forwarded to ArcOverlay. expect(screen.getByTestId('arc-split-btn')).toHaveAttribute( 'data-candidate-phrase-ids', 'phrase-1', diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 1502488d..9c3f2c08 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -2,16 +2,28 @@ /// /// +import { useLocalizedStrings } from '@papi/frontend/react'; import type { SerializedVerseRef } from '@sillsdev/scripture'; -import { act, render, screen } from '@testing-library/react'; -import type { Book, ScriptureRef, Segment, Token } from 'interlinearizer'; +import { act, fireEvent, render, screen } from '@testing-library/react'; +import type { Book, PhraseAnalysisLink, ScriptureRef, Segment, Token } from 'interlinearizer'; import type { ReactNode } from 'react'; import { useState } from 'react'; +import { resegmentBook } from 'parsers/papi/resegmentBook'; import Interlinearizer from '../../components/Interlinearizer'; import { InterlinearNavProvider } from '../../components/InterlinearNavContext'; +import { + useSegmentation, + type SegmentationContextValue, + type SegmentationDispatch, +} from '../../components/SegmentationStore'; import type { SegmentDisplayMode } from '../../components/SegmentView'; import { RECENTER_FADE_MS } from '../../components/recenter-fade'; -import { defaultScrRef, GEN_1_1_BOOK, type ScrollGroupTuple } from '../test-helpers'; +import { + defaultScrRef, + GEN_1_1_BOOK, + makePhraseLink, + type ScrollGroupTuple, +} from '../test-helpers'; import { allFalseViewOptions } from './test-helpers'; jest.mock('lucide-react', () => ({ @@ -22,6 +34,12 @@ jest.mock('lucide-react', () => ({ * @returns An SVG element with `data-testid="locate-fixed-icon"`. */ LocateFixed: () => , + /** + * Stub for the Merge icon used by the between-rows merge control. + * + * @returns An SVG element with `data-testid="merge-icon"`. + */ + Merge: () => , })); /** @@ -42,10 +60,20 @@ type CapturedContinuousViewProps = { }; let capturedContinuousViewProps: CapturedContinuousViewProps | undefined; +/** + * The segmentation context value seen by the (stubbed) ContinuousView — carries the force-break + * wrapped dispatch built by InterlinearizerInner, so tests can invoke its methods directly. + */ +let capturedSegmentation: SegmentationContextValue | undefined; + /** Props captured from SegmentView renders so tests can assert on what Interlinearizer passes down. */ type CapturedSegmentViewProps = { /** The segment the component is asked to render. */ segment: Segment; + /** Per-verse-start inline superscript labels (chapter-qualified at a chapter transition). */ + verseStartLabels?: readonly string[]; + /** The segment's verse/range gutter label. */ + gutterLabel?: string; /** Controls whether tokens are rendered as chips or as raw baseline text. */ displayMode: SegmentDisplayMode; /** The `Token.ref` string of the currently focused token, if any. */ @@ -64,6 +92,16 @@ let capturedSegmentViewPropsList: CapturedSegmentViewProps[] = []; /** Stable spy for `updatePhrase` — reset between tests via resetMocks. */ const mockUpdatePhrase = jest.fn(); +/** Stable spies for `createPhrase` / `deletePhrase`, asserted on by the force-break tests. */ +const mockCreatePhrase = jest.fn(); +const mockDeletePhrase = jest.fn(); + +/** + * Phrase-link-by-id map served by the mocked `usePhraseLinkByIdGetter`. Force-break tests seed it + * with phrases that straddle a boundary; cleared in the top-level `beforeEach`. + */ +const mockPhraseLinkById = new Map(); + jest.mock('../../components/AnalysisStore', () => ({ __esModule: true, /** @@ -95,18 +133,39 @@ jest.mock('../../components/AnalysisStore', () => ({ * @returns An empty `Map`. */ usePhraseLinkMap: () => new Map(), - usePhraseLinkByIdMap: () => new Map(), + /** + * Returns the test-owned phrase-link map so straddled-boundary tests can seed phrases the + * component's `straddledBoundaryRefs` memo sees. + * + * @returns The current phrase-link-by-id map. + */ + usePhraseLinkByIdMap: () => mockPhraseLinkById, + /** + * Returns a getter over the test-owned phrase-link map so force-break tests can seed straddling + * phrases. + * + * @returns A function returning the current map. + */ + usePhraseLinkByIdGetter: () => () => mockPhraseLinkById, usePhraseDispatch: () => ({ - createPhrase: () => {}, + createPhrase: (...args: Parameters) => mockCreatePhrase(...args), updatePhrase: (...args: Parameters) => mockUpdatePhrase(...args), - deletePhrase: () => {}, + deletePhrase: (...args: Parameters) => mockDeletePhrase(...args), }), })); jest.mock('../../components/ContinuousView', () => ({ __esModule: true, - default: (props: CapturedContinuousViewProps) => { + /** + * ContinuousView stub; captures its props and the segmentation context (the wrapped, + * force-breaking dispatch) so tests can invoke the dispatch directly. + * + * @param props - The props passed by Interlinearizer. + * @returns A minimal div carrying the focused token ref. + */ + default: function ContinuousViewStub(props: CapturedContinuousViewProps) { capturedContinuousViewProps = props; + capturedSegmentation = useSegmentation(); return (
); @@ -159,13 +218,13 @@ jest.mock('../../components/SegmentView', () => ({ * @param props.rest - Any additional props forwarded from the parent. * @returns A div with `data-testid="segment-view"` and the segment id. */ - default: ({ + default: function MockSegmentView({ segment, isActive, hoveredPhraseId, onHoverPhrase, ...rest - }: CapturedSegmentViewProps) => { + }: CapturedSegmentViewProps) { capturedSegmentViewPropsList.push({ segment, isActive, @@ -173,9 +232,13 @@ jest.mock('../../components/SegmentView', () => ({ onHoverPhrase, ...rest, }); + // Read lazily (not via an outer import) because jest.mock factories are hoisted. + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { useAltHeldValue } = require('../../components/AltHeldContext'); return (
@@ -209,6 +272,39 @@ jest.mock('../../components/modals/UnlinkPhraseConfirm', () => ({ default: () =>
, })); +/** + * A fixture book whose segments may omit `verseStarts` (filled in by + * {@link withDefaultVerseStarts}). + */ +type BookWithOptionalVerseStarts = Omit & { + segments: (Omit & Partial>)[]; +}; + +/** + * Fills in a default single `verseStarts` entry (offset 0, number from `startRef.verse`) on any + * hand-built fixture segment that lacks one, so the fixtures satisfy the `Segment` shape without + * every literal repeating the boilerplate. Segments that already carry `verseStarts` (e.g. produced + * by `resegmentBook`) are left untouched. + * + * @param book - The fixture book to normalize; its segments may omit `verseStarts`. + * @returns The book with every segment carrying at least one verse start. + */ +function withDefaultVerseStarts(book: BookWithOptionalVerseStarts): Book { + return { + ...book, + segments: book.segments.map((seg) => + seg.verseStarts + ? { ...seg, verseStarts: seg.verseStarts } + : { + ...seg, + verseStarts: [ + { charStart: 0, number: String(seg.startRef.verse), chapter: seg.startRef.chapter }, + ], + }, + ), + }; +} + /** Pre-built Book with no segments — used by the no-verse-data test. */ const GEN_EMPTY_BOOK: Book = { id: 'GEN', bookRef: 'GEN', textVersion: 'v1', segments: [] }; @@ -238,13 +334,14 @@ function makeLargeBook(count: number): Book { charEnd: 4, }, ], + verseStarts: [{ charStart: 0, number: String(v), chapter: 1 }], }); } return { id: 'GEN', bookRef: 'GEN', textVersion: 'v1', segments }; } /** Book with two segments in GEN 1 — used by chapter-display tests. */ -const GEN_1_MULTI_BOOK: Book = { +const GEN_1_MULTI_BOOK: Book = withDefaultVerseStarts({ id: 'GEN', bookRef: 'GEN', textVersion: 'v1', @@ -282,14 +379,66 @@ const GEN_1_MULTI_BOOK: Book = { ], }, ], -}; +}); + +/** + * GEN book with a token-less (empty) verse marker between two token-bearing verses. A merge into an + * empty predecessor cannot take effect (the empty verse forces its own segment boundary), so the + * list must offer no merge button for the segment after the empty one. + */ +const GEN_1_EMPTY_MIDDLE_BOOK: Book = withDefaultVerseStarts({ + 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: 'Alpha.', + tokens: [ + { + ref: 'GEN 1:1:0', + surfaceText: 'Alpha', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 5, + }, + ], + }, + { + id: 'GEN 1:2', + startRef: { book: 'GEN', chapter: 1, verse: 2 }, + endRef: { book: 'GEN', chapter: 1, verse: 2 }, + baselineText: '', + tokens: [], + verseStarts: [{ charStart: 0, number: '2', chapter: 1 }], + }, + { + id: 'GEN 1:3', + startRef: { book: 'GEN', chapter: 1, verse: 3 }, + endRef: { book: 'GEN', chapter: 1, verse: 3 }, + baselineText: 'Gamma.', + tokens: [ + { + ref: 'GEN 1:3:0', + surfaceText: 'Gamma', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 5, + }, + ], + }, + ], +}); /** - * Two-chapter GEN book: chapter 1 has verses 1-2, chapter 2 has verses 1-2. Used to exercise the - * focus-reseed guard when the host echoes a click back at chapter granularity (verse-0 / first - * verse), which a verse-exact guard would misread as the chapter's first segment. + * Two-chapter GEN book: chapter 1 has verses 1-2, chapter 2 has verses 1-2. Exercises the + * focus-reseed guard against a host click echoed back at chapter granularity. */ -const GEN_TWO_CHAPTER_BOOK: Book = { +const GEN_TWO_CHAPTER_BOOK: Book = withDefaultVerseStarts({ id: 'GEN', bookRef: 'GEN', textVersion: 'v1', @@ -311,10 +460,74 @@ const GEN_TWO_CHAPTER_BOOK: Book = { ], })), ), -}; +}); + +/** + * GEN book whose verse 1 has two word tokens (so it can be split into portions "1a"/"1b") and verse + * 2 one word. The default one-segment-per-verse form; {@link GEN_V1_SPLIT_BOOK} is its split + * counterpart. + */ +const GEN_SPLITTABLE_V1_BOOK: Book = withDefaultVerseStarts({ + 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 beginning.', + tokens: [ + { + ref: 'GEN 1:1:0', + surfaceText: 'In', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 2, + }, + { + ref: 'GEN 1:1:3', + surfaceText: 'beginning', + writingSystem: 'en', + type: 'word', + charStart: 3, + charEnd: 12, + }, + ], + }, + { + id: 'GEN 1:2', + startRef: { book: 'GEN', chapter: 1, verse: 2 }, + endRef: { book: 'GEN', chapter: 1, verse: 2 }, + baselineText: 'And the earth.', + tokens: [ + { + ref: 'GEN 1:2:0', + surfaceText: 'And', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 3, + }, + ], + }, + ], +}); + +/** + * {@link GEN_SPLITTABLE_V1_BOOK} with verse 1 split before its second word, so verse 1 becomes two + * portions ("1a" = segment `GEN 1:1` holding `GEN 1:1:0`; "1b" = segment `GEN 1:1:3` holding `GEN + * 1:1:3`). Both portions' verse ranges contain verse 1, so a verse-1 navigation resolves to the + * first portion. + */ +const GEN_V1_SPLIT_BOOK: Book = resegmentBook(withDefaultVerseStarts(GEN_SPLITTABLE_V1_BOOK), { + removedVerseStarts: [], + addedStarts: ['GEN 1:1:3'], +}); /** GEN book whose chapter 1 opens with a verse-0 superscription segment before verse 1. */ -const GEN_SUPERSCRIPTION_BOOK: Book = { +const GEN_SUPERSCRIPTION_BOOK: Book = withDefaultVerseStarts({ id: 'GEN', bookRef: 'GEN', textVersion: 'v1', @@ -352,7 +565,7 @@ const GEN_SUPERSCRIPTION_BOOK: Book = { ], }, ], -}; +}); /** * Wraps an `` element in an {@link InterlinearNavProvider} so the component's @@ -395,9 +608,11 @@ function renderInterlinearizer({ navigate = () => {}, hideInactiveLinkButtons = false, simplifyPhrases = false, - chapterLabelInVerse = false, showMorphology = false, showFreeTranslation = false, + showVerseGutter = false, + segmentationDispatch, + formerBoundaries, }: { book?: Book; continuousScroll?: boolean; @@ -405,15 +620,19 @@ function renderInterlinearizer({ navigate?: (r: SerializedVerseRef) => void; hideInactiveLinkButtons?: boolean; simplifyPhrases?: boolean; - chapterLabelInVerse?: boolean; showMorphology?: boolean; showFreeTranslation?: boolean; + showVerseGutter?: boolean; + segmentationDispatch?: SegmentationDispatch; + formerBoundaries?: ReadonlyMap; } = {}) { return render( withNav( , navigate, @@ -434,6 +653,17 @@ function renderInterlinearizer({ beforeEach(() => { // jsdom does not implement scrollIntoView; stub it globally so components that call it don't throw. Element.prototype.scrollIntoView = jest.fn(); + // The phrase-link map is a plain Map (not a jest mock), so resetMocks does not clear it. + mockPhraseLinkById.clear(); + capturedSegmentation = undefined; + // resetMocks clears the shared useLocalizedStrings implementation; re-establish the + // key-to-itself mapping the merge control's label relies on. + jest + .mocked(useLocalizedStrings) + .mockImplementation((keys: readonly string[]) => [ + keys.reduce>((acc, k) => ({ ...acc, [k]: k }), {}), + false, + ]); }); describe('Interlinearizer', () => { @@ -454,6 +684,45 @@ describe('Interlinearizer', () => { expect(screen.getByText(/no verse data for gen 1\./i)).toBeInTheDocument(); }); + it('passes per-verse-start superscript labels to the segment views', () => { + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + + // Verse 1 opens the book's first chapter, so its superscript is chapter-qualified; verse 2 is + // a bare number. + expect(capturedSegmentViewPropsList[0].verseStartLabels).toEqual(['1:1']); + expect(capturedSegmentViewPropsList[1].verseStartLabels).toEqual(['2']); + }); + + it('passes a superscript label for every absorbed verse start of a merged segment', () => { + const merged = resegmentBook(withDefaultVerseStarts(GEN_1_MULTI_BOOK), { + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: [], + }); + renderInterlinearizer({ book: merged }); + + expect(screen.getAllByTestId('segment-view')).toHaveLength(1); + // The merged segment absorbs verses 1 and 2; verse 1 opens the chapter so it is qualified. + expect(capturedSegmentViewPropsList[0].verseStartLabels).toEqual(['1:1', '2']); + }); + + it('passes each segment its bare verse gutter label', () => { + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + + expect(capturedSegmentViewPropsList[0].gutterLabel).toBe('1'); + expect(capturedSegmentViewPropsList[1].gutterLabel).toBe('2'); + }); + + it('passes a merged segment its covered-verse range gutter label', () => { + const merged = resegmentBook(withDefaultVerseStarts(GEN_1_MULTI_BOOK), { + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: [], + }); + renderInterlinearizer({ book: merged }); + + // The merged segment covers verses 1 and 2, so its gutter label is the range 1–2. + expect(capturedSegmentViewPropsList[0].gutterLabel).toBe('1–2'); + }); + it('renders a SegmentView for every segment in the current chapter', () => { renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); @@ -535,9 +804,8 @@ describe('Interlinearizer', () => { }); it('writes a verse-0 reference to the host when a verse-0 token is selected', () => { - // Selecting a superscription token navigates the host to verse 0 like any other verse; the - // 'internal' origin records a nav marker so the segment window skips the recenter fade for our - // own move. Default scrRef is GEN 1:1, so this is a real verse change. + // Selecting a superscription token navigates the host to verse 0 like any other verse. Default + // scrRef is GEN 1:1, so this is a real verse change. const mockNavigate = jest.fn(); renderInterlinearizer({ book: GEN_SUPERSCRIPTION_BOOK, navigate: mockNavigate }); @@ -662,9 +930,8 @@ describe('Interlinearizer', () => { it('does not echo scrRef when the focused token belongs to a different book than scrRef', () => { // During an external book change scrRef names the new book before its data loads, so the - // mounted book (and its focused token) still belong to the previous book. The echo-back effect - // must not fire that stale book's verse back as scrRef. Here the mounted book is GEN but scrRef - // names EXO, so a GEN focus move must not call navigate. + // mounted book (and its focused token) still belong to the previous book. Here the mounted book + // is GEN but scrRef names EXO, so a GEN focus move must not echo back as scrRef. const mockNavigate = jest.fn(); renderInterlinearizer({ book: GEN_1_MULTI_BOOK, @@ -838,10 +1105,9 @@ describe('Interlinearizer', () => { }); it('keeps the clicked token focused when the host echoes the click back as the clicked verse', () => { - // Active verse starts at GEN 1:1. Click a token in a later chapter/verse (GEN 2:2): focus is set - // to 'GEN 2:2:0'. The host echoes the navigation back as the actual clicked verse (GEN 2:2). The - // verse-exact reseed guard must see focus already in the active verse and leave the deliberately - // clicked token alone — never reseeding to the verse's (here, the only) first word from scratch. + // Active verse starts at GEN 1:1. Click a token in a later chapter/verse (GEN 2:2), setting + // focus to 'GEN 2:2:0'; the host then echoes the navigation back as the clicked verse (GEN 2:2). + // The reseed guard must see focus already in the active verse and leave the clicked token alone. const { rerender } = renderInterlinearizer({ book: GEN_TWO_CHAPTER_BOOK, continuousScroll: false, @@ -879,10 +1145,9 @@ describe('Interlinearizer', () => { }); it('reseeds focus to the first word of the active verse on an external within-chapter jump', () => { - // A genuine external jump within a long chapter (here GEN 2:1 → GEN 2:2) must move focus to the - // newly-named verse — a chapter-wide guard would wrongly strand focus on the old verse. Focus - // starts at the active verse's first word; after the jump it must point at the new verse's word. - // The segment view's focus highlight lags through the recenter fade, so advance past it. + // A genuine external jump within a chapter (GEN 2:1 → GEN 2:2) must move focus to the newly-named + // verse's first word. The segment view's focus highlight lags through the recenter fade, so + // advance past it. jest.useFakeTimers(); try { const { rerender } = renderInterlinearizer({ @@ -914,29 +1179,259 @@ describe('Interlinearizer', () => { } }); - it('renders an inline chapter header above the first verse of each chapter', () => { + it('keeps focus on a clicked non-first portion of a split verse', () => { + // Verse 1 is split into portions "1a" (GEN 1:1) and "1b" (GEN 1:1:3). Focus starts on verse 2. + // Clicking a token in "1b" sets focus to 'GEN 1:1:3' and navigates to verse 1; the host echoes + // verse 1 back, firing the reseed effect. Its guard must recognize the focused token already + // sits in a segment containing verse 1 and leave it alone, not reseed to portion "1a". + const { rerender } = render( + withNav( + {}} + viewOptions={{ ...allFalseViewOptions }} + />, + ), + ); + + const secondPortion = capturedSegmentViewPropsList.find((p) => p.segment.id === 'GEN 1:1:3'); + if (!secondPortion || typeof secondPortion.onSelect !== 'function') { + throw new Error('Expected an onSelect for the "1b" split portion (GEN 1:1:3)'); + } + act(() => { + secondPortion.onSelect?.({ book: 'GEN', chapter: 1, verse: 1 }, 'GEN 1:1:3'); + }); + + // Host delivers the echo of the clicked verse (verse 1). + capturedSegmentViewPropsList = []; + rerender( + withNav( + {}} + viewOptions={{ ...allFalseViewOptions }} + />, + ), + ); + + // Focus stays on the deliberately clicked "1b" token, not reseeded to "1a"'s first word. + const stillFocused = capturedSegmentViewPropsList.find( + (p) => p.focusedTokenRef === 'GEN 1:1:3', + ); + expect(stillFocused).toBeDefined(); + const reseededToFirstPortion = capturedSegmentViewPropsList.find( + (p) => p.focusedTokenRef === 'GEN 1:1:0', + ); + expect(reseededToFirstPortion).toBeUndefined(); + }); + + it('reseeds focus out of a split portion on an external jump to a verse it does not contain', () => { + // Focus sits in portion "1b" (GEN 1:1:3), which contains verse 1 but not verse 2. A genuine + // external jump to verse 2 lands outside the focused token's segment, so the containment guard + // lets the reseed run and moves focus to verse 2's first word. The segment view's focus highlight + // lags through the recenter fade, so advance past it. + jest.useFakeTimers(); + try { + const { rerender } = render( + withNav( + {}} + viewOptions={{ ...allFalseViewOptions }} + />, + ), + ); + + // Focus the second portion so focus sits off the verse's first word. + const secondPortion = capturedSegmentViewPropsList.find((p) => p.segment.id === 'GEN 1:1:3'); + if (!secondPortion || typeof secondPortion.onSelect !== 'function') { + throw new Error('Expected an onSelect for the "1b" split portion (GEN 1:1:3)'); + } + act(() => { + secondPortion.onSelect?.({ book: 'GEN', chapter: 1, verse: 1 }, 'GEN 1:1:3'); + }); + + // External jump to verse 2 — a verse the focused portion does not contain. + capturedSegmentViewPropsList = []; + rerender( + withNav( + {}} + viewOptions={{ ...allFalseViewOptions }} + />, + ), + ); + act(() => jest.advanceTimersByTime(RECENTER_FADE_MS)); + + const reseeded = capturedSegmentViewPropsList.find((p) => p.focusedTokenRef === 'GEN 1:2:0'); + expect(reseeded).toBeDefined(); + } finally { + jest.useRealTimers(); + } + }); + + /** + * Makes the segment `topSegmentId` the topmost one touching the container's top edge, so the + * pinned-chapter overlay resolves it deterministically in jsdom (which otherwise reports every + * rect as zero). Every `[data-segment-id]` before the target is placed fully above the top edge + * (negative bottom); the target and those after it sit at/below it. The container reports top 0. + * Restored automatically by `restoreMocks`. + * + * @param orderedSegmentIds - Segment ids in document order. + * @param topSegmentId - The segment id to place flush against the container top. + */ + function positionSegmentAtTop(orderedSegmentIds: string[], topSegmentId: string): void { + const targetIndex = orderedSegmentIds.indexOf(topSegmentId); + jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function getRect( + this: HTMLElement, + ): DOMRect { + const segmentId = this.getAttribute('data-segment-id'); + // Container (and any non-segment element) reports top 0. A segment before the target sits + // fully above the top edge; the target and later segments sit at or below it. + const index = segmentId ? orderedSegmentIds.indexOf(segmentId) : -1; + const top = index >= 0 && index < targetIndex ? -20 : 0; + return { + top, + bottom: top + 10, + left: 0, + right: 0, + width: 0, + height: 10, + x: 0, + y: top, + toJSON() {}, + }; + }); + } + + it('pins a book-and-chapter header for the chapter at the top of the list', () => { + positionSegmentAtTop(['GEN 1:1', 'GEN 1:2', 'GEN 2:1', 'GEN 2:2'], 'GEN 1:1'); renderInterlinearizer({ book: GEN_TWO_CHAPTER_BOOK, scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 }, continuousScroll: false, }); - // One header per chapter, rendered by the list (not inside SegmentView) at each boundary. - expect(screen.getByText('Chapter 1')).toBeInTheDocument(); - expect(screen.getByText('Chapter 2')).toBeInTheDocument(); - expect(screen.queryByText('Chapter 3')).not.toBeInTheDocument(); + // A single pinned overlay (not a header per chapter) shows the localized book name plus the + // chapter of the topmost visible segment. + expect(screen.getByText('Genesis 1')).toBeInTheDocument(); + expect(screen.queryByText('Genesis 2')).not.toBeInTheDocument(); }); - it('omits inline chapter headers when chapterLabelInVerse is set', () => { + it('pins the chapter of the top segment even when that chapter starts merged mid-segment', () => { + // Merge GEN 2:1 into GEN 1:2, so the segment containing chapter 2's first token starts in + // chapter 1. Scrolling so that merged segment is at the top pins its start chapter (1) — the + // overlay follows the topmost segment, and a merged chapter start reads as its host's chapter. + const merged = resegmentBook(withDefaultVerseStarts(GEN_TWO_CHAPTER_BOOK), { + removedVerseStarts: ['GEN 2:1:0'], + addedStarts: [], + }); + positionSegmentAtTop(['GEN 1:1', 'GEN 1:2', 'GEN 2:2'], 'GEN 1:1'); renderInterlinearizer({ - book: GEN_TWO_CHAPTER_BOOK, + book: merged, scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 }, continuousScroll: false, - chapterLabelInVerse: true, }); - expect(screen.queryByText('Chapter 1')).not.toBeInTheDocument(); - expect(screen.queryByText('Chapter 2')).not.toBeInTheDocument(); + expect(screen.getByText('Genesis 1')).toBeInTheDocument(); + }); + + it('pins the later chapter when a later-chapter segment is at the top of the list', () => { + positionSegmentAtTop(['GEN 1:1', 'GEN 1:2', 'GEN 2:1', 'GEN 2:2'], 'GEN 2:1'); + renderInterlinearizer({ + book: GEN_TWO_CHAPTER_BOOK, + scrRef: { book: 'GEN', chapterNum: 2, verseNum: 1 }, + continuousScroll: false, + }); + + expect(screen.getByText('Genesis 2')).toBeInTheDocument(); + expect(screen.queryByText('Genesis 1')).not.toBeInTheDocument(); + }); + + it('updates the pinned chapter on scroll, coalesced to one read per animation frame', () => { + jest.useFakeTimers(); + try { + const ids = ['GEN 1:1', 'GEN 1:2', 'GEN 2:1', 'GEN 2:2']; + // Mount with chapter 1 at the top. + positionSegmentAtTop(ids, 'GEN 1:1'); + const { container } = renderInterlinearizer({ + book: GEN_TWO_CHAPTER_BOOK, + scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 }, + continuousScroll: false, + }); + expect(screen.getByText('Genesis 1')).toBeInTheDocument(); + + // Scroll so a chapter-2 segment reaches the top, then fire two scroll events in the same frame. + // The rAF gate coalesces them into a single read, which settles the header on chapter 2. + positionSegmentAtTop(ids, 'GEN 2:1'); + const scrollContainer = container.querySelector('.tw\\:overflow-y-auto'); + if (!scrollContainer) throw new Error('scroll container not found'); + act(() => { + scrollContainer.dispatchEvent(new Event('scroll')); + scrollContainer.dispatchEvent(new Event('scroll')); + jest.runOnlyPendingTimers(); + }); + + expect(screen.getByText('Genesis 2')).toBeInTheDocument(); + expect(screen.queryByText('Genesis 1')).not.toBeInTheDocument(); + } finally { + jest.useRealTimers(); + } + }); + + it('cancels a scroll-scheduled animation frame when unmounted before it runs', () => { + jest.useFakeTimers(); + try { + const ids = ['GEN 1:1', 'GEN 1:2', 'GEN 2:1', 'GEN 2:2']; + positionSegmentAtTop(ids, 'GEN 1:1'); + const { container, unmount } = renderInterlinearizer({ + book: GEN_TWO_CHAPTER_BOOK, + scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 }, + continuousScroll: false, + }); + + // Fire a scroll to queue the coalescing rAF, then unmount before the frame runs. The effect + // cleanup must cancel that exact frame. Other cleanups may also cancel frames, so capture the + // handle the scroll schedules and match it specifically rather than asserting on any call. + const scrollContainer = container.querySelector('.tw\\:overflow-y-auto'); + if (!scrollContainer) throw new Error('scroll container not found'); + const scheduledHandles: number[] = []; + const rafSpy = jest.spyOn(globalThis, 'requestAnimationFrame').mockImplementation(() => { + const handle = scheduledHandles.length + 1; + scheduledHandles.push(handle); + return handle; + }); + const cancelSpy = jest.spyOn(globalThis, 'cancelAnimationFrame'); + act(() => { + scrollContainer.dispatchEvent(new Event('scroll')); + }); + expect(scheduledHandles).toHaveLength(1); + act(() => { + unmount(); + }); + + expect(cancelSpy).toHaveBeenCalledWith(scheduledHandles[0]); + rafSpy.mockRestore(); + } finally { + jest.useRealTimers(); + } }); it('renders the snap-to-active-verse button when segments are present', () => { @@ -1170,10 +1665,9 @@ describe('Interlinearizer', () => { throw new Error('Expected GEN 1:7 onSelect to be a function'); act(() => select({ book: 'GEN', chapter: 1, verse: 7 }, 'GEN 1:7:0')); - // Target the inner list wrapper (tw:gap-2) — the one that fades on external nav via isFaded — - // matching the positive-fade test above. The bare .tw:transition-opacity selector would return - // the outer mode-toggle wrapper instead, whose opacity is never driven here, masking a regressed - // (non-suppressed) recenter fade of the list. + // Target the inner list wrapper (tw:gap-2) — the one that fades on external nav via isFaded. + // The bare .tw:transition-opacity selector would return the outer mode-toggle wrapper instead, + // whose opacity is never driven here. expect(container.querySelector('.tw\\:gap-2.tw\\:transition-opacity')).toHaveStyle({ opacity: '1', }); @@ -1222,3 +1716,450 @@ describe('Interlinearizer', () => { } }); }); + +// --------------------------------------------------------------------------- +// Segmentation dispatch force-break wrapper + between-rows merge control +// --------------------------------------------------------------------------- + +describe('segmentation dispatch force-break', () => { + /** + * Builds a raw segmentation-dispatch spy; the wrapped dispatch the views receive must delegate + * every call to it. + * + * @returns A dispatch whose methods are jest spies. + */ + function makeRawDispatch(): SegmentationDispatch { + return { merge: jest.fn(), split: jest.fn(), move: jest.fn() }; + } + + /** + * Renders with continuous scroll on (so the stubbed ContinuousView captures the segmentation + * context) and returns the wrapped dispatch. + * + * @param raw - The raw dispatch handed to the component. + * @param book - The book fixture to render. + * @returns The wrapped dispatch captured from the segmentation context. + */ + function renderAndCaptureDispatch(raw: SegmentationDispatch, book: Book): SegmentationDispatch { + renderInterlinearizer({ book, continuousScroll: true, segmentationDispatch: raw }); + const dispatch = capturedSegmentation?.dispatch; + if (!dispatch) throw new Error('expected a captured segmentation dispatch'); + return dispatch; + } + + it('passes merge straight through without touching phrases', () => { + const raw = makeRawDispatch(); + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0'])); + const dispatch = renderAndCaptureDispatch(raw, GEN_1_MULTI_BOOK); + dispatch.merge('GEN 1:2:0'); + expect(raw.merge).toHaveBeenCalledWith('GEN 1:2:0'); + expect(mockUpdatePhrase).not.toHaveBeenCalled(); + expect(mockCreatePhrase).not.toHaveBeenCalled(); + expect(mockDeletePhrase).not.toHaveBeenCalled(); + }); + + it('force-breaks a two-token phrase straddling a split boundary (deletes it), then splits', () => { + const raw = makeRawDispatch(); + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0'])); + const dispatch = renderAndCaptureDispatch(raw, GEN_1_MULTI_BOOK); + dispatch.split('GEN 1:2:0'); + // Both halves of the straddled two-token phrase are single tokens, so the phrase is deleted. + expect(mockDeletePhrase).toHaveBeenCalledWith('p1'); + expect(raw.split).toHaveBeenCalledWith('GEN 1:2:0'); + // The break lands before the boundary write so no consumer observes a straddling phrase. + expect(mockDeletePhrase.mock.invocationCallOrder[0]).toBeLessThan( + jest.mocked(raw.split).mock.invocationCallOrder[0], + ); + }); + + it('force-breaks a longer straddling phrase into its two sides at the boundary', () => { + const raw = makeRawDispatch(); + mockPhraseLinkById.set( + 'p1', + makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0', 'GEN 1:3:0', 'GEN 1:4:0']), + ); + const dispatch = renderAndCaptureDispatch(raw, makeLargeBook(4)); + dispatch.split('GEN 1:3:0'); + expect(mockUpdatePhrase).toHaveBeenCalledWith('p1', [ + { tokenRef: 'GEN 1:1:0', surfaceText: 'GEN 1:1:0' }, + { tokenRef: 'GEN 1:2:0', surfaceText: 'GEN 1:2:0' }, + ]); + expect(mockCreatePhrase).toHaveBeenCalledWith([ + { tokenRef: 'GEN 1:3:0', surfaceText: 'GEN 1:3:0' }, + { tokenRef: 'GEN 1:4:0', surfaceText: 'GEN 1:4:0' }, + ]); + expect(raw.split).toHaveBeenCalledWith('GEN 1:3:0'); + }); + + it('leaves phrases alone when the split boundary cuts none of them', () => { + const raw = makeRawDispatch(); + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:3:0', 'GEN 1:4:0'])); + const dispatch = renderAndCaptureDispatch(raw, makeLargeBook(4)); + dispatch.split('GEN 1:3:0'); + expect(mockUpdatePhrase).not.toHaveBeenCalled(); + expect(mockCreatePhrase).not.toHaveBeenCalled(); + expect(mockDeletePhrase).not.toHaveBeenCalled(); + expect(raw.split).toHaveBeenCalledWith('GEN 1:3:0'); + }); + + it('force-breaks at the destination boundary of a move', () => { + const raw = makeRawDispatch(); + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0'])); + const dispatch = renderAndCaptureDispatch(raw, GEN_1_MULTI_BOOK); + dispatch.move('GEN 1:1:0', 'GEN 1:2:0'); + expect(mockDeletePhrase).toHaveBeenCalledWith('p1'); + expect(raw.move).toHaveBeenCalledWith('GEN 1:1:0', 'GEN 1:2:0'); + }); + + it('skips the force-break when the boundary ref is unknown to the book', () => { + const raw = makeRawDispatch(); + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0'])); + const dispatch = renderAndCaptureDispatch(raw, GEN_1_MULTI_BOOK); + dispatch.split('EXO 9:9:9'); + expect(mockDeletePhrase).not.toHaveBeenCalled(); + expect(raw.split).toHaveBeenCalledWith('EXO 9:9:9'); + }); +}); + +/** Presses (or releases) Alt so Alt-gated boundary controls reveal (or hide) their buttons. */ +function setAltHeld(held: boolean): void { + act(() => { + window.dispatchEvent(new KeyboardEvent(held ? 'keydown' : 'keyup', { altKey: held })); + }); +} + +describe('between-rows merge control', () => { + it('shows an always-visible, always-enabled merge button between rows even while Alt is not held', () => { + const raw: SegmentationDispatch = { merge: jest.fn(), split: jest.fn(), move: jest.fn() }; + renderInterlinearizer({ book: GEN_1_MULTI_BOOK, segmentationDispatch: raw }); + // Two rows -> exactly one gap between them, always carrying the rail and an enabled merge button + // (no Alt required). + expect(screen.queryByTestId('segment-row-gap')).not.toBeInTheDocument(); + const buttons = screen.getAllByTestId('segment-merge-btn'); + expect(buttons).toHaveLength(1); + expect(screen.getAllByTestId('segment-merge-indicator')).toHaveLength(1); + expect(buttons[0]).toBeEnabled(); + fireEvent.click(buttons[0]); + // Merging removes the boundary at the lower segment's first token. + expect(raw.merge).toHaveBeenCalledWith('GEN 1:2:0'); + }); + + it('keeps the merge button enabled while Alt is held', () => { + const raw: SegmentationDispatch = { merge: jest.fn(), split: jest.fn(), move: jest.fn() }; + renderInterlinearizer({ book: GEN_1_MULTI_BOOK, segmentationDispatch: raw }); + setAltHeld(true); + const button = screen.getByTestId('segment-merge-btn'); + expect(button).toBeEnabled(); + fireEvent.click(button); + expect(raw.merge).toHaveBeenCalledWith('GEN 1:2:0'); + }); + + it('renders no merge affordance when only one segment row is mounted', () => { + renderInterlinearizer({ book: GEN_1_1_BOOK }); + expect(screen.queryByTestId('segment-row-gap')).not.toBeInTheDocument(); + expect(screen.queryByTestId('segment-merge-indicator')).not.toBeInTheDocument(); + expect(screen.queryByTestId('segment-merge-btn')).not.toBeInTheDocument(); + }); + + it('offers no merge button for a segment whose predecessor is an empty verse (the merge would be a no-op)', () => { + // v1[Alpha] · v2[empty] · v3[Gamma]: merging v3 up cannot cross the empty v2 (it forces its own + // boundary), so the button — which would silently persist a dead boundary — must not appear. + renderInterlinearizer({ book: GEN_1_EMPTY_MIDDLE_BOOK }); + expect(screen.queryByTestId('segment-merge-btn')).not.toBeInTheDocument(); + expect(screen.queryByTestId('segment-merge-indicator')).not.toBeInTheDocument(); + }); + + it('adds the Alt-split hint to the merge tooltip while Alt is not held', () => { + // Alt up → split markers are hidden, so the always-enabled merge button advertises the Alt + // gesture that reveals them. + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + const button = screen.getByTestId('segment-merge-btn'); + expect(button).toHaveAttribute('aria-label', '%interlinearizer_boundaryControl_merge%'); + expect(button).toHaveAttribute('title', '%interlinearizer_boundaryControl_mergeAltHint%'); + }); + + it('labels the merge button with the plain merge string while Alt is held', () => { + // Alt held → the split markers are already visible, so the merge tooltip drops the hint. + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + setAltHeld(true); + const button = screen.getByTestId('segment-merge-btn'); + expect(button).toHaveAttribute('aria-label', '%interlinearizer_boundaryControl_merge%'); + expect(button).toHaveAttribute('title', '%interlinearizer_boundaryControl_merge%'); + }); + + it('renders no merge control while a phrase mode is active', () => { + // A merge mid-mode could re-segment the phrase the mode UI is operating on, so the between-rows + // control is omitted entirely (not merely disabled) throughout a phrase edit. + render( + withNav( + {}} + viewOptions={{ ...allFalseViewOptions }} + />, + ), + ); + expect(screen.queryByTestId('segment-merge-btn')).not.toBeInTheDocument(); + expect(screen.queryByTestId('segment-merge-indicator')).not.toBeInTheDocument(); + }); +}); + +describe('Alt-held wiring', () => { + it('feeds the Alt-held state through context to the views', () => { + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + // The context reads false before any Alt press. + expect(screen.getAllByTestId('segment-view')[0]).toHaveAttribute('data-alt-held', 'false'); + + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { altKey: true })); + }); + expect(screen.getAllByTestId('segment-view')[0]).toHaveAttribute('data-alt-held', 'true'); + + act(() => { + window.dispatchEvent(new KeyboardEvent('keyup', { altKey: false })); + }); + expect(screen.getAllByTestId('segment-view')[0]).toHaveAttribute('data-alt-held', 'false'); + }); +}); + +describe('focus preservation across segmentation edits', () => { + /** GEN book whose verse 1 has two word tokens and verse 2 one — lets focus sit off a verse start. */ + const GEN_TWO_TOKEN_V1_BOOK: Book = withDefaultVerseStarts({ + 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 beginning.', + tokens: [ + { + ref: 'GEN 1:1:0', + surfaceText: 'In', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 2, + }, + { + ref: 'GEN 1:1:3', + surfaceText: 'beginning', + writingSystem: 'en', + type: 'word', + charStart: 3, + charEnd: 12, + }, + ], + }, + { + id: 'GEN 1:2', + startRef: { book: 'GEN', chapter: 1, verse: 2 }, + endRef: { book: 'GEN', chapter: 1, verse: 2 }, + baselineText: 'And the earth.', + tokens: [ + { + ref: 'GEN 1:2:0', + surfaceText: 'And', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 3, + }, + ], + }, + ], + }); + + /** + * Builds the wrapped `` element used by these tests, so the initial render and + * the post-edit rerender share identical props apart from the book. + * + * @param book - The (re)segmented book to render. + * @param scrRef - The active scripture reference. + * @returns The element wrapped in a nav provider. + */ + function interlinearizerEl(book: Book, scrRef: SerializedVerseRef): ReactNode { + return withNav( + {}} + viewOptions={allFalseViewOptions} + />, + ); + } + + it('keeps the focused token when a merge removes the active verse segment start', () => { + const scrRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 2 }; + const { rerender } = render(interlinearizerEl(GEN_1_MULTI_BOOK, scrRef)); + expect(capturedContinuousViewProps?.focusedTokenRef).toBe('GEN 1:2:0'); + // Merge verse 2 into verse 1 — the exact transform the loader applies on a boundary edit. No + // segment starts at the active verse afterwards, but the focused token still exists. + const merged = resegmentBook(GEN_1_MULTI_BOOK, { + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: [], + }); + rerender(interlinearizerEl(merged, scrRef)); + expect(capturedContinuousViewProps?.focusedTokenRef).toBe('GEN 1:2:0'); + }); + + it('keeps a deliberately-focused token across a merge into the active verse', () => { + const scrRef: SerializedVerseRef = { book: 'GEN', chapterNum: 1, verseNum: 1 }; + const { rerender } = render(interlinearizerEl(GEN_TWO_TOKEN_V1_BOOK, scrRef)); + act(() => capturedContinuousViewProps?.onFocusedTokenRefChange('GEN 1:1:3')); + expect(capturedContinuousViewProps?.focusedTokenRef).toBe('GEN 1:1:3'); + // Merge verse 2 into verse 1; the active verse's segment survives (same start), so the + // deliberately-focused token must not be clobbered back to the verse's first word. + const merged = resegmentBook(GEN_TWO_TOKEN_V1_BOOK, { + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: [], + }); + rerender(interlinearizerEl(merged, scrRef)); + expect(capturedContinuousViewProps?.focusedTokenRef).toBe('GEN 1:1:3'); + }); +}); + +describe('former boundaries', () => { + it('provides the supplied formerBoundaries map to the views through the segmentation context', () => { + // Anchor word ref → removed default start ref (distinct when the verse opens with punctuation). + const formerBoundaries: ReadonlyMap = new Map([['GEN 1:2:1', 'GEN 1:2:0']]); + renderInterlinearizer({ + book: GEN_1_MULTI_BOOK, + continuousScroll: true, + formerBoundaries, + }); + expect(capturedSegmentation?.formerBoundaries).toBe(formerBoundaries); + }); + + it('defaults formerBoundaries to an empty map when the loader supplies none', () => { + renderInterlinearizer({ book: GEN_1_MULTI_BOOK, continuousScroll: true }); + expect(capturedSegmentation?.formerBoundaries?.size).toBe(0); + }); +}); + +describe('straddled boundary refs', () => { + /** + * Renders with continuous scroll on (so the stubbed ContinuousView captures the segmentation + * context) and returns the straddled-boundary set the component computed. Phrase links must be + * seeded into `mockPhraseLinkById` before calling. + * + * @param book - The book fixture to render. + * @returns The `straddledBoundaryRefs` set captured from the segmentation context. + */ + function renderAndCaptureStraddled(book: Book): ReadonlySet { + renderInterlinearizer({ book, continuousScroll: true }); + const straddled = capturedSegmentation?.straddledBoundaryRefs; + if (!straddled) throw new Error('expected a captured straddled-boundary set'); + return straddled; + } + + it('blocks the word refs strictly inside a contiguous three-word phrase', () => { + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0', 'GEN 1:3:0'])); + const straddled = renderAndCaptureStraddled(makeLargeBook(4)); + // A boundary before the 2nd or 3rd word would cut the phrase. + expect(straddled.has('GEN 1:2:0')).toBe(true); + expect(straddled.has('GEN 1:3:0')).toBe(true); + }); + + it('does not block the phrase-leading word ref or the word after the phrase', () => { + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0', 'GEN 1:3:0'])); + const straddled = renderAndCaptureStraddled(makeLargeBook(4)); + // A boundary before the phrase's first word or after its last word leaves it intact. + expect(straddled.has('GEN 1:1:0')).toBe(false); + expect(straddled.has('GEN 1:4:0')).toBe(false); + }); + + it('blocks the gap word ref inside a discontiguous phrase', () => { + // The phrase spans words 1 and 3; word 2 sits in the gap, but a boundary before it would still + // put the phrase's halves in different segments. + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:3:0'])); + const straddled = renderAndCaptureStraddled(makeLargeBook(4)); + expect(straddled.has('GEN 1:2:0')).toBe(true); + }); + + it('exposes an empty set when no phrase links exist', () => { + const straddled = renderAndCaptureStraddled(makeLargeBook(4)); + expect(straddled.size).toBe(0); + }); +}); + +describe('segmentationVersion pass-through', () => { + /** + * Builds the wrapped `` element for the pass-through tests, varying only the + * book and segmentation version between renders. + * + * @param book - The (re)segmented book to render. + * @param segmentationVersion - The boundary-edit counter forwarded to the segment window. + * @returns The element wrapped in a nav provider. + */ + function versionedEl(book: Book, segmentationVersion: number): ReactNode { + return withNav( + {}} + viewOptions={{ ...allFalseViewOptions }} + />, + ); + } + + it('redraws in place without a recenter fade when a boundary edit bumps segmentationVersion', () => { + const { container, rerender } = render(versionedEl(GEN_1_MULTI_BOOK, 0)); + const merged = resegmentBook(GEN_1_MULTI_BOOK, { + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: [], + }); + act(() => { + rerender(versionedEl(merged, 1)); + }); + // Were the prop dropped on the way to the segment window, the segments-identity change would be + // classified as a re-tokenization and fade the list out. + expect(container.querySelector('.tw\\:gap-2.tw\\:transition-opacity')).toHaveStyle({ + opacity: '1', + }); + }); + + it('fades the list for a segments change without a segmentationVersion bump', () => { + jest.useFakeTimers(); + try { + const { container, rerender } = render(versionedEl(GEN_1_MULTI_BOOK, 0)); + const merged = resegmentBook(GEN_1_MULTI_BOOK, { + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: [], + }); + act(() => { + rerender(versionedEl(merged, 0)); + }); + // Same segments change, same version: a re-tokenization, so the recenter fade runs. This + // guards the positive case above against passing vacuously in a setup where fades never fire. + expect(container.querySelector('.tw\\:gap-2.tw\\:transition-opacity')).toHaveStyle({ + opacity: '0', + }); + act(() => { + jest.advanceTimersByTime(RECENTER_FADE_MS); + }); + expect(container.querySelector('.tw\\:gap-2.tw\\:transition-opacity')).toHaveStyle({ + opacity: '1', + }); + } finally { + jest.useRealTimers(); + } + }); +}); diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 1c158e7f..aad28ae5 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -16,6 +16,7 @@ import useOptimisticBooleanSetting from '../../hooks/useOptimisticBooleanSetting import { emptyAnalysis, emptyDraft } from '../../types/empty-factories'; import type { PhraseMode } from '../../types/phrase-mode'; import type { ViewOptions } from '../../types/view-options'; +import type { SegmentationDispatch } from '../../components/SegmentationStore'; import { GEN_1_1_BOOK, makeScrollGroupHook, @@ -35,12 +36,12 @@ jest.mock('../../components/controls/ViewOptionsDropdown', () => ({ onHideInactiveLinkButtonsChange, simplifyPhrases, onSimplifyPhrasesChange, - chapterLabelInVerse, - onChapterLabelInVerseChange, showMorphology, onShowMorphologyChange, showFreeTranslation, onShowFreeTranslationChange, + showVerseGutter, + onShowVerseGutterChange, }: { continuousScroll: boolean; onContinuousScrollChange: (v: boolean) => void; @@ -48,12 +49,12 @@ jest.mock('../../components/controls/ViewOptionsDropdown', () => ({ onHideInactiveLinkButtonsChange: (v: boolean) => void; simplifyPhrases: boolean; onSimplifyPhrasesChange: (v: boolean) => void; - chapterLabelInVerse: boolean; - onChapterLabelInVerseChange: (v: boolean) => void; showMorphology: boolean; onShowMorphologyChange: (v: boolean) => void; showFreeTranslation: boolean; onShowFreeTranslationChange: (v: boolean) => void; + showVerseGutter: boolean; + onShowVerseGutterChange: (v: boolean) => void; }) => (
), })); @@ -158,6 +159,9 @@ type CapturedInterlinearizerProps = { phraseMode: PhraseMode; setPhraseMode: Dispatch>; viewOptions: ViewOptions; + segmentationDispatch: SegmentationDispatch; + formerBoundaries: ReadonlyMap; + segmentationVersion: number; }; let capturedInterlinearizerProps: CapturedInterlinearizerProps | undefined; let interlinearizerMountCount = 0; @@ -515,6 +519,7 @@ describe('InterlinearizerLoader', () => { endRef: { book: 'PSA', chapter: 3, verse: 0 }, baselineText: 'A Psalm by David.', tokens: [], + verseStarts: [{ charStart: 0, number: '0', chapter: 3 }], }, ], }; @@ -553,6 +558,7 @@ describe('InterlinearizerLoader', () => { endRef: { book: 'GEN', chapter: 3, verse: 1 }, baselineText: 'First verse.', tokens: [], + verseStarts: [{ charStart: 0, number: '1', chapter: 3 }], }, { id: 'GEN 3:2', @@ -560,6 +566,7 @@ describe('InterlinearizerLoader', () => { endRef: { book: 'GEN', chapter: 3, verse: 2 }, baselineText: 'Last verse of the chapter.', tokens: [], + verseStarts: [{ charStart: 0, number: '2', chapter: 3 }], }, { id: 'GEN 4:1', @@ -567,6 +574,7 @@ describe('InterlinearizerLoader', () => { endRef: { book: 'GEN', chapter: 4, verse: 1 }, baselineText: 'Next chapter.', tokens: [], + verseStarts: [{ charStart: 0, number: '1', chapter: 4 }], }, ], }; @@ -589,10 +597,100 @@ describe('InterlinearizerLoader', () => { }); }); - it('resolves a mid-segment verse to the owning segment start', async () => { - // A merged segment can span several verses; navigating to a verse inside the span has no - // segment starting at that verse, so the loader resolves to the nearest preceding segment - // start — the segment that contains the verse, never a later segment in the chapter. + it('resolves an over-shoot within a chapter opened by a cross-chapter segment', async () => { + // A cross-chapter segment covers 4:20 through 5:3 — its `startRef.chapter` is 4, but its verse + // starts include chapter 5's opening verses. When the host over-shoots past chapter 5's real end + // (verse 4 here, with only 5:1..5:3 present), the fallback must still find the nearest preceding + // verse start in chapter 5 — 5:3 — even though no segment's `startRef` sits in chapter 5. + const crossChapterBook: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + segments: [ + { + id: 'GEN 4:20', + startRef: { book: 'GEN', chapter: 4, verse: 20 }, + endRef: { book: 'GEN', chapter: 5, verse: 3 }, + baselineText: 'Chapter four tail folded into chapter five opening.', + tokens: [], + verseStarts: [ + { charStart: 0, number: '20', chapter: 4 }, + { charStart: 20, number: '1', chapter: 5 }, + { charStart: 30, number: '2', chapter: 5 }, + { charStart: 40, number: '3', chapter: 5 }, + ], + }, + ], + }; + mockBookData({ book: crossChapterBook }); + + await act(async () => { + renderLoader({ + useWebViewScrollGroupScrRef: makeScrollGroupHook({ + book: 'GEN', + chapterNum: 5, + verseNum: 4, + }), + }); + }); + + expect(capturedInterlinearizerProps?.scrRef).toEqual({ + book: 'GEN', + chapterNum: 5, + verseNum: 3, + }); + }); + + it('resolves a verse missing from the text to the nearest preceding segment start, never a later one', async () => { + // A verse absent from the book's content (e.g. bridged away in the source) is contained in no + // segment, so the loader resolves it to the nearest preceding segment start in the chapter — + // skipping the later segment that also sits in the chapter. + const gappedBook: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + segments: [ + { + id: 'GEN 3:1', + startRef: { book: 'GEN', chapter: 3, verse: 1 }, + endRef: { book: 'GEN', chapter: 3, verse: 1 }, + baselineText: 'First verse.', + tokens: [], + verseStarts: [{ charStart: 0, number: '1', chapter: 3 }], + }, + { + id: 'GEN 3:3', + startRef: { book: 'GEN', chapter: 3, verse: 3 }, + endRef: { book: 'GEN', chapter: 3, verse: 3 }, + baselineText: 'Verse after the gap.', + tokens: [], + verseStarts: [{ charStart: 0, number: '3', chapter: 3 }], + }, + ], + }; + mockBookData({ book: gappedBook }); + + await act(async () => { + renderLoader({ + useWebViewScrollGroupScrRef: makeScrollGroupHook({ + book: 'GEN', + chapterNum: 3, + verseNum: 2, + }), + }); + }); + + expect(capturedInterlinearizerProps?.scrRef).toEqual({ + book: 'GEN', + chapterNum: 3, + verseNum: 1, + }); + }); + + it('passes a mid-segment verse through unchanged', async () => { + // A merged segment can span several verses; a verse inside the span is contained in that + // segment even though no segment starts at it, so the loader passes the reference through + // unchanged and the views resolve it to the containing segment. const mergedSegmentBook: Book = { id: 'GEN', bookRef: 'GEN', @@ -604,6 +702,12 @@ describe('InterlinearizerLoader', () => { endRef: { book: 'GEN', chapter: 3, verse: 2 }, baselineText: 'Two verses merged into one segment.', tokens: [], + // A merged segment carries one verse start per absorbed verse, so containment resolves the + // interior verse 2 to it. + verseStarts: [ + { charStart: 0, number: '1', chapter: 3 }, + { charStart: 18, number: '2', chapter: 3 }, + ], }, { id: 'GEN 3:3', @@ -611,6 +715,7 @@ describe('InterlinearizerLoader', () => { endRef: { book: 'GEN', chapter: 3, verse: 3 }, baselineText: 'Verse after the merged segment.', tokens: [], + verseStarts: [{ charStart: 0, number: '3', chapter: 3 }], }, ], }; @@ -629,7 +734,7 @@ describe('InterlinearizerLoader', () => { expect(capturedInterlinearizerProps?.scrRef).toEqual({ book: 'GEN', chapterNum: 3, - verseNum: 1, + verseNum: 2, }); }); @@ -755,9 +860,9 @@ describe('InterlinearizerLoader', () => { expect(capturedInterlinearizerProps?.viewOptions.hideInactiveLinkButtons).toBe(false); expect(capturedInterlinearizerProps?.viewOptions.simplifyPhrases).toBe(false); - expect(capturedInterlinearizerProps?.viewOptions.chapterLabelInVerse).toBe(false); expect(capturedInterlinearizerProps?.viewOptions.showMorphology).toBe(false); expect(capturedInterlinearizerProps?.viewOptions.showFreeTranslation).toBe(false); + expect(capturedInterlinearizerProps?.viewOptions.showVerseGutter).toBe(false); }); it('wires ViewOptionsDropdown hide-inactive-link-buttons to onChange from useOptimisticBooleanSetting', async () => { @@ -780,34 +885,34 @@ describe('InterlinearizerLoader', () => { expect(onChangeByKey.get('interlinearizer.simplifyPhrases')).toHaveBeenCalledWith(true); }); - it('wires ViewOptionsDropdown chapter-label-in-verse to onChange from useOptimisticBooleanSetting', async () => { + it('wires ViewOptionsDropdown show-morphology to onChange from useOptimisticBooleanSetting', async () => { const onChangeByKey = mockOptimisticSetting(); await act(async () => { renderLoader(); }); - await userEvent.click(screen.getByTestId('chapter-label-in-verse-toggle')); - expect(onChangeByKey.get('interlinearizer.chapterLabelInVerse')).toHaveBeenCalledWith(true); + await userEvent.click(screen.getByTestId('show-morphology-toggle')); + expect(onChangeByKey.get('interlinearizer.showMorphology')).toHaveBeenCalledWith(true); }); - it('wires ViewOptionsDropdown show-morphology to onChange from useOptimisticBooleanSetting', async () => { + it('wires ViewOptionsDropdown show-free-translation to onChange from useOptimisticBooleanSetting', async () => { const onChangeByKey = mockOptimisticSetting(); await act(async () => { renderLoader(); }); - await userEvent.click(screen.getByTestId('show-morphology-toggle')); - expect(onChangeByKey.get('interlinearizer.showMorphology')).toHaveBeenCalledWith(true); + await userEvent.click(screen.getByTestId('show-free-translation-toggle')); + expect(onChangeByKey.get('interlinearizer.showFreeTranslation')).toHaveBeenCalledWith(true); }); - it('wires ViewOptionsDropdown show-free-translation to onChange from useOptimisticBooleanSetting', async () => { + it('wires ViewOptionsDropdown show-verse-gutter to onChange from useOptimisticBooleanSetting', async () => { const onChangeByKey = mockOptimisticSetting(); await act(async () => { renderLoader(); }); - await userEvent.click(screen.getByTestId('show-free-translation-toggle')); - expect(onChangeByKey.get('interlinearizer.showFreeTranslation')).toHaveBeenCalledWith(true); + await userEvent.click(screen.getByTestId('show-verse-gutter-toggle')); + expect(onChangeByKey.get('interlinearizer.showVerseGutter')).toHaveBeenCalledWith(true); }); it('passes continuousScroll=true to Interlinearizer when the setting is true', async () => { @@ -1143,6 +1248,386 @@ describe('InterlinearizerLoader', () => { }); }); + describe('segmentation dispatch', () => { + /** A two-verse book so boundary edits produce real, non-default deltas. */ + const TWO_VERSE_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: 'Alpha beta.', + tokens: [ + { + ref: 'GEN 1:1:0', + surfaceText: 'Alpha', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 5, + }, + { + ref: 'GEN 1:1:6', + surfaceText: 'beta', + writingSystem: 'en', + type: 'word', + charStart: 6, + charEnd: 10, + }, + ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], + }, + { + id: 'GEN 1:2', + startRef: { book: 'GEN', chapter: 1, verse: 2 }, + endRef: { book: 'GEN', chapter: 1, verse: 2 }, + baselineText: 'Gamma.', + tokens: [ + { + ref: 'GEN 1:2:0', + surfaceText: 'Gamma', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 5, + }, + ], + verseStarts: [{ charStart: 0, number: '2', chapter: 1 }], + }, + ], + }; + + /** + * Returns the segmentation delta from the most recent saveDraft call. + * + * @returns The persisted draft's `segmentation`, or `undefined` when not set / no call. + */ + function lastPersistedSegmentation(): DraftProject['segmentation'] { + const calls = mockSendCommand.mock.calls.filter(([c]) => c === 'interlinearizer.saveDraft'); + const last = calls[calls.length - 1]; + const json = last?.[2]; + return typeof json === 'string' ? JSON.parse(json).segmentation : undefined; + } + + /** + * Returns the segmentation dispatch captured from the rendered interlinearizer, failing the + * test if none was captured. + * + * @returns The captured `segmentationDispatch`. + * @throws If the interlinearizer did not render and capture a dispatch. + */ + function getSegmentationDispatch(): SegmentationDispatch { + const dispatch = capturedInterlinearizerProps?.segmentationDispatch; + if (!dispatch) throw new Error('expected a captured segmentationDispatch'); + return dispatch; + } + + it('persists split, merge, and move boundary edits made through the dispatch', async () => { + mockBookData({ book: TWO_VERSE_BOOK }); + await act(async () => { + renderLoader(); + }); + const dispatch = getSegmentationDispatch(); + + jest.useFakeTimers(); + // Split verse 1 before "beta" — a non-default delta is persisted. + act(() => dispatch.split('GEN 1:1:6')); + act(() => jest.advanceTimersByTime(300)); + expect(lastPersistedSegmentation()).toEqual({ + removedVerseStarts: [], + addedStarts: ['GEN 1:1:6'], + }); + + // Merge verse 2 into its predecessor — adds a removed verse start. + act(() => dispatch.merge('GEN 1:2:0')); + act(() => jest.advanceTimersByTime(300)); + expect(lastPersistedSegmentation()?.removedVerseStarts).toContain('GEN 1:2:0'); + + // Move the verse-2 boundary back onto "beta". "beta" (GEN 1:1:6) already begins a segment from + // the split above, so the move removes the (already-removed) verse-2 default start and re-adds + // the existing "beta" start: the normalized delta is unchanged — verse 1's split boundary and + // verse 2's merged boundary both persist. + act(() => dispatch.move('GEN 1:2:0', 'GEN 1:1:6')); + act(() => jest.advanceTimersByTime(300)); + jest.useRealTimers(); + expect(lastPersistedSegmentation()).toEqual({ + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: ['GEN 1:1:6'], + }); + }); + + it('re-renders the interlinearizer with the new segments in place after a boundary edit', async () => { + // The resegmented book is derived from the draft's ref-held segmentation, and the auto-save's + // `setDirty(true)` no-ops the re-render once the draft is dirty, so the new `book` prop reaches + // the interlinearizer only because `autosaveSegmentation` bumps a dedicated version. Assert the + // split reaches the rendered `book` (verse 1 becomes two segments) without a remount. + mockBookData({ book: TWO_VERSE_BOOK }); + await act(async () => { + renderLoader(); + }); + const dispatch = getSegmentationDispatch(); + expect(capturedInterlinearizerProps?.book.segments).toHaveLength(2); + const mountsBefore = interlinearizerMountCount; + + act(() => dispatch.split('GEN 1:1:6')); + + // Verse 1 is now two segments (before/after "beta"), so the book has three segments total, and + // the interlinearizer was updated in place rather than remounted. + expect(capturedInterlinearizerProps?.book.segments).toHaveLength(3); + expect(interlinearizerMountCount).toBe(mountsBefore); + }); + + it('sends the boundary delta on Save and clears the unsaved marker', async () => { + mockBookData({ book: TWO_VERSE_BOOK }); + let result: ReturnType | undefined; + await act(async () => { + result = renderLoader({ + useWebViewState: makeWebViewState({ activeProject: STUB_ACTIVE_PROJECT }), + }); + }); + const dispatch = getSegmentationDispatch(); + + // A boundary edit dirties the draft, so the tab marker lights up. + act(() => dispatch.merge('GEN 1:2:0')); + expect(result?.updateWebViewDefinition).toHaveBeenCalledWith({ title: 'Interlinearizer ●' }); + + result?.updateWebViewDefinition.mockClear(); + await userEvent.click(screen.getByTestId('tab-toolbar-save')); + + // Save sends the draft's boundary delta (not the "null" clear sentinel) alongside the + // analysis, so the project's boundaries match the analysis just written. + expect(mockSendCommand).toHaveBeenCalledWith( + 'interlinearizer.saveAnalysis', + 'proj-1', + JSON.stringify(emptyAnalysis()), + JSON.stringify({ removedVerseStarts: ['GEN 1:2:0'], addedStarts: [] }), + ); + // markSynced received the draft's exact analysis and segmentation references, so its + // identity guard matched and the unsaved marker cleared. + expect(result?.updateWebViewDefinition).toHaveBeenCalledWith({ title: 'Interlinearizer' }); + }); + + it('clears the segmentation field when an edit restores the default segmentation', async () => { + mockBookData({ book: TWO_VERSE_BOOK }); + await act(async () => { + renderLoader(); + }); + const dispatch = getSegmentationDispatch(); + + jest.useFakeTimers(); + // Merging the book's first token is a no-op, so the result is the default segmentation and the + // persisted field is cleared to undefined. + act(() => dispatch.merge('GEN 1:1:0')); + act(() => jest.advanceTimersByTime(300)); + jest.useRealTimers(); + expect(lastPersistedSegmentation()).toBeUndefined(); + }); + }); + + describe('former boundaries', () => { + /** + * A four-verse book covering every former-boundary shape: a word-initial verse (1:3), a verse + * that begins with punctuation so its first word token differs from its first token (1:2), a + * verse with no word token at all (1:4), and a token-less verse (1:5). + */ + const BOUNDARY_SHAPES_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: 'Alpha.', + tokens: [ + { + ref: 'GEN 1:1:0', + surfaceText: 'Alpha', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 5, + }, + ], + verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], + }, + { + id: 'GEN 1:2', + startRef: { book: 'GEN', chapter: 1, verse: 2 }, + endRef: { book: 'GEN', chapter: 1, verse: 2 }, + baselineText: '“Gamma.', + tokens: [ + { + ref: 'GEN 1:2:0', + surfaceText: '“', + writingSystem: 'en', + type: 'punctuation', + charStart: 0, + charEnd: 1, + }, + { + ref: 'GEN 1:2:1', + surfaceText: 'Gamma', + writingSystem: 'en', + type: 'word', + charStart: 1, + charEnd: 6, + }, + ], + verseStarts: [{ charStart: 0, number: '2', chapter: 1 }], + }, + { + id: 'GEN 1:3', + startRef: { book: 'GEN', chapter: 1, verse: 3 }, + endRef: { book: 'GEN', chapter: 1, verse: 3 }, + baselineText: 'Delta.', + tokens: [ + { + ref: 'GEN 1:3:0', + surfaceText: 'Delta', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 5, + }, + ], + verseStarts: [{ charStart: 0, number: '3', chapter: 1 }], + }, + { + id: 'GEN 1:4', + startRef: { book: 'GEN', chapter: 1, verse: 4 }, + endRef: { book: 'GEN', chapter: 1, verse: 4 }, + baselineText: '—', + tokens: [ + { + ref: 'GEN 1:4:0', + surfaceText: '—', + writingSystem: 'en', + type: 'punctuation', + charStart: 0, + charEnd: 1, + }, + ], + verseStarts: [{ charStart: 0, number: '4', chapter: 1 }], + }, + { + id: 'GEN 1:5', + startRef: { book: 'GEN', chapter: 1, verse: 5 }, + endRef: { book: 'GEN', chapter: 1, verse: 5 }, + baselineText: '', + tokens: [], + verseStarts: [{ charStart: 0, number: '5', chapter: 1 }], + }, + ], + }; + + /** + * Renders the loader on {@link BOUNDARY_SHAPES_BOOK} and returns the captured segmentation + * dispatch so a test can drive boundary edits. + * + * @returns The captured `segmentationDispatch`. + * @throws If the interlinearizer did not render and capture a dispatch. + */ + async function renderBoundaryBook(): Promise { + mockBookData({ book: BOUNDARY_SHAPES_BOOK }); + await act(async () => { + renderLoader(); + }); + const dispatch = capturedInterlinearizerProps?.segmentationDispatch; + if (!dispatch) throw new Error('expected a captured segmentationDispatch'); + return dispatch; + } + + it('passes an empty formerBoundaries map and segmentationVersion 0 before any boundary edit', async () => { + await renderBoundaryBook(); + + expect(capturedInterlinearizerProps?.formerBoundaries.size).toBe(0); + expect(capturedInterlinearizerProps?.segmentationVersion).toBe(0); + }); + + it('maps a merged word-initial verse start to itself and bumps segmentationVersion', async () => { + const dispatch = await renderBoundaryBook(); + + // Merge verse 3 into verse 2: the verse begins with a word token, so the split anchor (its + // first word token) and the removed default start are the same ref. + act(() => dispatch.merge('GEN 1:3:0')); + + expect(capturedInterlinearizerProps?.formerBoundaries.get('GEN 1:3:0')).toBe('GEN 1:3:0'); + expect(capturedInterlinearizerProps?.segmentationVersion).toBe(1); + }); + + it('keys a punct-initial merged verse by its first word token, mapped to the removed start', async () => { + const dispatch = await renderBoundaryBook(); + + // Merge verse 2 into verse 1: the verse opens with a quote mark, so the boundary slot's word + // anchor (the first word token) differs from the removed default start (the punct token). + act(() => dispatch.merge('GEN 1:2:0')); + + expect(capturedInterlinearizerProps?.formerBoundaries.get('GEN 1:2:1')).toBe('GEN 1:2:0'); + expect(capturedInterlinearizerProps?.formerBoundaries.has('GEN 1:2:0')).toBe(false); + }); + + it('skips a merged-away verse that has no word token', async () => { + const dispatch = await renderBoundaryBook(); + + // Merge verse 4 (punctuation only) into verse 3: with no word token there is no split anchor + // to key the former boundary by, so the verse contributes no map entry. + act(() => dispatch.merge('GEN 1:4:0')); + + expect(capturedInterlinearizerProps?.formerBoundaries.size).toBe(0); + }); + + it('does not map verses whose default start was not merged away', async () => { + const dispatch = await renderBoundaryBook(); + + // Only verse 3 is merged; the other verses keep their default boundaries and must stay out + // of the map so their slots do not render former-boundary ticks. + act(() => dispatch.merge('GEN 1:3:0')); + + expect(capturedInterlinearizerProps?.formerBoundaries.size).toBe(1); + expect(capturedInterlinearizerProps?.formerBoundaries.has('GEN 1:1:0')).toBe(false); + expect(capturedInterlinearizerProps?.formerBoundaries.has('GEN 1:2:1')).toBe(false); + }); + + it('shows the loading state when the book unloads while the draft holds removals', async () => { + // Render with a rebuildable element so the same loader instance can be re-invoked after the + // book-data mock changes (the mock mutates hook output, not React state). + mockBookData({ book: BOUNDARY_SHAPES_BOOK }); + const updateWebViewDefinition = jest.fn(() => true); + const scrollGroupHook = makeScrollGroupHook(); + const webViewState = makeWebViewState(); + const buildUi = () => ( + + ); + let view: ReturnType | undefined; + await act(async () => { + view = render(buildUi()); + }); + const dispatch = capturedInterlinearizerProps?.segmentationDispatch; + if (!dispatch) throw new Error('expected a captured segmentationDispatch'); + // Merge so the draft's delta has a removed verse start. + act(() => dispatch.merge('GEN 1:3:0')); + expect(capturedInterlinearizerProps?.formerBoundaries.size).toBe(1); + + // The book unloads (e.g. a cross-book swap): with no verse book to resolve refs against, the + // former boundaries cannot be derived and the loader falls back to the loading curtain. + mockBookData({ book: undefined, isLoading: true }); + view?.rerender(buildUi()); + + expect(screen.getByText('Loading…')).toBeInTheDocument(); + expect(screen.queryByTestId('interlinearizer')).not.toBeInTheDocument(); + }); + }); + describe('save command', () => { it('saves the draft analysis to the active project when Save is clicked with an active project', async () => { const draftAnalysis = emptyAnalysis(); @@ -1160,6 +1645,8 @@ describe('InterlinearizerLoader', () => { 'interlinearizer.saveAnalysis', 'proj-1', JSON.stringify(draftAnalysis), + // The draft has no custom boundaries, so Save sends "null" to clear any stored ones. + 'null', ); }); @@ -1539,10 +2026,9 @@ describe('InterlinearizerLoader', () => { }); expect(screen.getByTestId('interlinearizer')).toBeInTheDocument(); - // Cross-book jump to MAT while the loaded book is still GEN (the window before the USJ arrives / - // Interlinearizer remounts). Rather than leave the previous book's views mounted — where they - // would show through the fade as the swap happens — the loader shows the Loading curtain, so - // nothing of either book is visible until the new one mounts and fades in. + // Cross-book jump to MAT while the loaded book is still GEN (before the USJ arrives and + // Interlinearizer remounts). The loader shows the Loading curtain rather than the previous + // book's views, so nothing of either book is visible until the new one mounts and fades in. controls?.setRef({ book: 'MAT', chapterNum: 5, verseNum: 3 }); controls?.rerenderNow(); expect(screen.queryByTestId('interlinearizer')).not.toBeInTheDocument(); @@ -1572,8 +2058,7 @@ describe('InterlinearizerLoader', () => { expect(interlinearizerMountCount).toBe(1); // A book change must tear down the old instance and mount a fresh one keyed by the new book, so - // it never updates in place against carried-over (wrong-book) scroll/focus state — the shuffle - // that surfaced before the curtain settled. + // it never updates in place against carried-over (wrong-book) scroll/focus state. controls?.setRef({ book: 'MAT', chapterNum: 5, verseNum: 3 }); mockBookData({ book: { ...GEN_1_1_BOOK, id: 'MAT', bookRef: 'MAT' } }); controls?.rerenderNow(); diff --git a/src/__tests__/components/PhraseBox.test.tsx b/src/__tests__/components/PhraseBox.test.tsx index bd7fcafc..c71602a0 100644 --- a/src/__tests__/components/PhraseBox.test.tsx +++ b/src/__tests__/components/PhraseBox.test.tsx @@ -288,9 +288,8 @@ describe('PhraseBox', () => { const phraseBox = document.querySelector('[data-phrase-box="true"]'); await userEvent.click(phraseBox ?? document.body); - // The clicked box is already on screen; the browser's default scroll-focused-input-into-view - // would realign the segment list (the first input can sit on another wrapped row), so the - // forwarded focus must opt out of scrolling. + // The box is already on screen; the browser's default scroll-focused-input-into-view would + // realign the segment list, so the forwarded focus opts out of scrolling. expect(focusSpy).toHaveBeenCalledWith({ preventScroll: true }); }); @@ -304,11 +303,9 @@ describe('PhraseBox', () => { />, ); - // The token-row wrapper span is a descendant of the box container, not the container itself, so - // the old `target === currentTarget` guard ignored clicks on it. Such clicks must still focus the - // phrase (forwarding to the first gloss input, which fires onFocusPhrase) rather than doing - // nothing — otherwise the click fell through to the segment background and focused the wrong - // phrase. + // The token-row wrapper span is a descendant of the box, not the container itself. A click on it + // must still focus the phrase (forwarding to the first gloss input, which fires onFocusPhrase) + // rather than falling through to the segment background. const tokenRow = document.querySelector('[data-phrase-box="true"] .tw\\:phrase-token-row'); if (!tokenRow) throw new Error('Expected a nested token-row span inside the phrase box'); await userEvent.click(tokenRow); @@ -331,8 +328,8 @@ describe('PhraseBox', () => { const phraseBox = document.querySelector('[data-phrase-box="true"]'); await userEvent.click(phraseBox ?? document.body); - // Morpheme gloss inputs precede the token gloss input in DOM order; the box-click handler must - // skip them — only the token gloss input fires onFocus → onFocusPhrase. + // Morpheme gloss inputs precede the token gloss input in DOM order; the box-click handler skips + // them, so only the token gloss input fires onFocus → onFocusPhrase. expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveFocus(); expect(onFocusPhrase).toHaveBeenCalledWith('test-group'); }); @@ -365,6 +362,22 @@ describe('PhraseBox', () => { expect(phraseBox).toHaveClass('tw:phrase-dimmed'); }); + it('applies the candidate outline when isCandidate is true', () => { + renderBox(); + + const phraseBox = document.querySelector('[data-phrase-box="true"]'); + expect(phraseBox).toHaveClass('tw:phrase-candidate'); + }); + + it('gives the candidate outline precedence over the focused style', () => { + renderBox(); + + // While an operation preview is hovered, what it affects matters more than what is focused. + const phraseBox = document.querySelector('[data-phrase-box="true"]'); + expect(phraseBox).toHaveClass('tw:phrase-candidate'); + expect(phraseBox).not.toHaveClass('tw:phrase-focused'); + }); + it('reddens only the chips whose refs are in splitFreeTokenRefs, leaving the box border neutral', () => { renderBox( { />, ); - // Only one of the two tokens would become free, so the box border stays neutral and just the - // affected chip is flagged. + // Only one token would become free, so the box border stays neutral and just that chip is flagged. const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox).not.toHaveClass('tw:phrase-destructive'); expect(screen.getByTestId('token-token-1')).toHaveAttribute('data-split-free', 'false'); @@ -391,8 +403,8 @@ describe('PhraseBox', () => { />, ); - // A 2-token phrase splits into two free tokens; each is shown on its own chip, never as a - // whole-box border (that would draw a single border around both rather than per token). + // A 2-token phrase splits into two free tokens, each flagged on its own chip, never as a + // whole-box border. const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox).not.toHaveClass('tw:phrase-destructive'); expect(screen.getByTestId('token-token-1')).toHaveAttribute('data-split-free', 'true'); @@ -408,8 +420,8 @@ describe('PhraseBox', () => { />, ); - // A single-token fragment (e.g. one run of a discontiguous phrase) reddens at the box level; - // per-chip flagging is suppressed so the border isn't drawn twice. + // A single-token fragment reddens at the box level; per-chip flagging is suppressed so the + // border isn't drawn twice. const phraseBox = document.querySelector('[data-phrase-box="true"]'); expect(phraseBox).toHaveClass('tw:phrase-destructive'); expect(screen.getByTestId('token-token-1')).toHaveAttribute('data-split-free', 'false'); @@ -671,8 +683,8 @@ describe('PhraseBox', () => { }); it('does not remove the last remaining token of the edited phrase (would empty it)', async () => { - // A single-token phrase: removing its only token would leave zero tokens — the early-return - // guard keeps the phrase alive so the user can add more tokens before committing. + // A single-token phrase: removing its only token would empty it, so the guard keeps the phrase + // alive for the user to add more tokens before committing. const singleTokenLink: PhraseAnalysisLink = { analysisId: 'phrase-1', status: 'approved', @@ -742,9 +754,9 @@ describe('PhraseBox', () => { }); it('splits a discontiguous phrase at the last intra-box boundary in document order even when the stored token list is scrambled', async () => { - // Phrase displayed as [A,C,D,E] (A discontiguous, [C,D,E] a contiguous run) but STORED out of - // document order — the bug that frees the wrong tokens. The split must use document order, so - // clicking the last intra-box boundary (D|E) frees E and keeps [A,C,D]. + // Phrase displayed as [A,C,D,E] (A discontiguous, [C,D,E] a contiguous run) but stored out of + // document order. The split uses document order, so clicking the last intra-box boundary (D|E) + // frees E and keeps [A,C,D]. const phraseLink: PhraseAnalysisLink = { analysisId: 'phrase-x', status: 'approved', @@ -781,10 +793,10 @@ describe('PhraseBox', () => { ); const unlinkBtns = screen.getAllByTestId('token-unlink-btn'); - // Click the LAST intra-box button (boundary between D and E in document order). + // Click the last intra-box button (the D|E boundary in document order). await userEvent.click(unlinkBtns[unlinkBtns.length - 1]); - // Expect: phrase shrinks to [A,C,D] (document order), E freed — not the scrambled stored order. + // Phrase shrinks to [A,C,D] in document order, E freed. expect(updatePhraseSpy).toHaveBeenCalledWith('phrase-x', [ { tokenRef: 'A', surfaceText: 'A' }, { tokenRef: 'C', surfaceText: 'C' }, @@ -794,8 +806,8 @@ describe('PhraseBox', () => { }); it('hovering an intra-phrase unlink button reports the would-be-free tokens to onHoverSplitFreeTokens', async () => { - // Splitting a two-token phrase leaves both halves length-1, so both tokens would become free. - // The intra-phrase icon must forward that preview up so the parent can redden the chips. + // Splitting a two-token phrase leaves both halves length-1, so both tokens would become free; + // the icon forwards that preview up so the parent can redden the chips. const phraseLink: PhraseAnalysisLink = { analysisId: 'phrase-x', status: 'approved', @@ -829,10 +841,9 @@ describe('PhraseBox', () => { }); it('clicking an inline unlink button does not pop out any other token (no label click-forwarding)', async () => { - // The phrase box used to be a