From 0ac7c3ea3044c12539e5cc22d71e702de2c852ff Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 14 Aug 2026 18:16:54 -0600 Subject: [PATCH 1/7] Cut the continuous view's cost per navigation step Size the strip's render window to the viewport instead of a fixed 100 groups each side, keep the per-group and per-slot memoization from being defeated by per-render callback identities, read each phrase box's geometry once per arc pass, and resolve token-chip labels once per strip rather than per chip. An arrow press drops from ~790ms to ~100ms on an 8th-gen i5. --- src/__tests__/components/MorphemeBox.test.tsx | 24 ++-- .../hooks/usePhraseWindowHalf.test.ts | 131 ++++++++++++++++++ src/__tests__/test-helpers.ts | 6 +- src/components/ContinuousView.tsx | 55 +++++--- src/components/MorphemeBox.tsx | 36 +++-- src/components/PhraseBox.tsx | 5 + src/components/PhraseStripContext.tsx | 42 ++++++ src/components/PhraseStripParts.tsx | 8 +- src/components/TokenChip.tsx | 43 +++--- src/hooks/useArcPaths.ts | 47 ++++--- src/hooks/usePhraseStripSetup.ts | 39 +++++- src/hooks/usePhraseWindowHalf.ts | 81 +++++++++++ src/utils/phrase-arc.ts | 38 ++--- 13 files changed, 449 insertions(+), 106 deletions(-) create mode 100644 src/__tests__/hooks/usePhraseWindowHalf.test.ts create mode 100644 src/hooks/usePhraseWindowHalf.ts diff --git a/src/__tests__/components/MorphemeBox.test.tsx b/src/__tests__/components/MorphemeBox.test.tsx index b44ce8d1..88c91138 100644 --- a/src/__tests__/components/MorphemeBox.test.tsx +++ b/src/__tests__/components/MorphemeBox.test.tsx @@ -1,25 +1,24 @@ /// /// -import { useLocalizedStrings } from '@papi/frontend/react'; import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { MorphemeAnalysis } from 'interlinearizer'; import * as AnalysisStore from '../../components/AnalysisStore'; import { MorphemeBox, MorphemeGlossInput } from '../../components/MorphemeBox'; +import { TOKEN_CHIP_LABEL_KEYS } from '../../components/PhraseStripContext'; import { makeWordToken } from '../test-helpers'; jest.mock('../../components/AnalysisStore'); -const LOCALIZED = { - '%interlinearizer_tokenChip_editMorphemes%': 'Edit morpheme breakdown for {token}', - '%interlinearizer_morphemeGloss_label%': 'Gloss for morpheme {form}', +// Resolved templates rather than the bare keys: the tests below target one morpheme's input among +// several, which only the substituted `{form}` distinguishes. +const LABELS = { + ...TOKEN_CHIP_LABEL_KEYS, + editMorphemes: 'Edit morpheme breakdown for {token}', + morphemeGloss: 'Gloss for morpheme {form}', }; -beforeEach(() => { - jest.mocked(useLocalizedStrings).mockReturnValue([LOCALIZED, false]); -}); - const WORD_TOKEN = makeWordToken('GEN 1:1:0', 'hello'); const MORPHEMES: MorphemeAnalysis[] = [ @@ -36,6 +35,7 @@ function renderBox(props: Partial[0]> = {}) { { { { it('renders an empty input when no gloss exists', () => { render( { it('renders the existing gloss value', () => { render( { render( { render( { const onFocus = jest.fn(); render( { render( + +import { act, renderHook } from '@testing-library/react'; +import usePhraseWindowHalf, { + MAX_PHRASE_WINDOW_HALF, + MIN_PHRASE_WINDOW_HALF, +} from '../../hooks/usePhraseWindowHalf'; + +/** Geometry a stubbed strip reports; jsdom lays nothing out, so every dimension is supplied. */ +type StripGeometry = { + /** `clientWidth` the clipping viewport reports. */ + viewportWidth: number; + /** `scrollWidth` the content row reports. */ + contentWidth: number; + /** How many group wrappers the content row holds. */ + groupCount: number; +}; + +/** + * Builds a viewport/content pair with the given geometry stubbed onto real elements, so the hook + * reads them through the same DOM properties it uses in a browser. + * + * @returns The two refs the hook takes, over elements attached to the document. + */ +function makeStrip({ viewportWidth, contentWidth, groupCount }: StripGeometry) { + const viewport = document.createElement('div'); + const content = document.createElement('div'); + viewport.appendChild(content); + document.body.appendChild(viewport); + Object.defineProperty(viewport, 'clientWidth', { configurable: true, value: viewportWidth }); + Object.defineProperty(content, 'scrollWidth', { configurable: true, value: contentWidth }); + for (let i = 0; i < groupCount; i += 1) { + const group = document.createElement('span'); + group.setAttribute('data-phrase-group', 'true'); + content.appendChild(group); + } + return { viewportRef: { current: viewport }, contentRef: { current: content } }; +} + +/** Renders the hook over a strip with the given geometry and returns its settled value. */ +function renderWindowHalf(geometry: StripGeometry): number { + const { viewportRef, contentRef } = makeStrip(geometry); + const { result } = renderHook(() => usePhraseWindowHalf(viewportRef, contentRef)); + return result.current; +} + +afterEach(() => { + document.body.innerHTML = ''; +}); + +describe('usePhraseWindowHalf', () => { + it('sizes the window from the viewport width and the measured per-group width', () => { + // The groups measure 100px each, so the viewport holds ten of them; the window is that count + // scaled by the viewports kept per side, rounded up to the step. + expect(renderWindowHalf({ viewportWidth: 1000, contentWidth: 2000, groupCount: 20 })).toBe(16); + }); + + it('rounds up to the step, so a small viewport change leaves the window alone', () => { + const narrower = renderWindowHalf({ viewportWidth: 980, contentWidth: 2000, groupCount: 20 }); + const wider = renderWindowHalf({ viewportWidth: 1000, contentWidth: 2000, groupCount: 20 }); + expect(narrower).toBe(wider); + }); + + it('clamps up to the minimum when the viewport holds only a few groups', () => { + expect(renderWindowHalf({ viewportWidth: 100, contentWidth: 2000, groupCount: 20 })).toBe( + MIN_PHRASE_WINDOW_HALF, + ); + }); + + it('clamps down to the maximum when the measurement asks for more', () => { + expect(renderWindowHalf({ viewportWidth: 100_000, contentWidth: 2000, groupCount: 20 })).toBe( + MAX_PHRASE_WINDOW_HALF, + ); + }); + + it('keeps the starting window while the strip measures zero wide', () => { + // Dividing by a zero content width would otherwise clamp straight to the maximum. + expect(renderWindowHalf({ viewportWidth: 1000, contentWidth: 0, groupCount: 20 })).toBe( + MIN_PHRASE_WINDOW_HALF, + ); + }); + + it('leaves the window unset while either element is unmounted', () => { + const { result } = renderHook(() => + // eslint-disable-next-line no-null/no-null + usePhraseWindowHalf({ current: null }, { current: null }), + ); + expect(result.current).toBe(MIN_PHRASE_WINDOW_HALF); + }); + + it('re-measures when the viewport resizes', () => { + let notifyResize: ResizeObserverCallback | undefined; + const originalResizeObserver = global.ResizeObserver; + global.ResizeObserver = class implements ResizeObserver { + constructor(callback: ResizeObserverCallback) { + notifyResize = callback; + } + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + observe() {} + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + unobserve() {} + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + disconnect() {} + }; + + try { + const { viewportRef, contentRef } = makeStrip({ + viewportWidth: 1000, + contentWidth: 2000, + groupCount: 20, + }); + const { result } = renderHook(() => usePhraseWindowHalf(viewportRef, contentRef)); + expect(result.current).toBe(16); + + Object.defineProperty(viewportRef.current, 'clientWidth', { + configurable: true, + value: 2000, + }); + act(() => { + notifyResize?.([], new global.ResizeObserver(() => {})); + }); + + expect(result.current).toBe(32); + } finally { + global.ResizeObserver = originalResizeObserver; + } + }); +}); diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index 6d715212..02fa2191 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -4,7 +4,10 @@ import type { Book, InterlinearProject, PhraseAnalysisLink, Segment, Token } fro import { UnsubscriberAsyncList } from 'platform-bible-utils'; import { tokenizeBook } from 'parsers/papi/bookTokenizer'; import type { RawBook } from 'parsers/papi/usjBookExtractor'; -import type { PhraseStripContextValue } from '../components/PhraseStripContext'; +import { + TOKEN_CHIP_LABEL_KEYS, + type PhraseStripContextValue, +} from '../components/PhraseStripContext'; import { emptyAnalysis } from '../types/empty-factories'; import { CURRENT_MODEL_VERSION } from '../types/model-version'; import type { InterlinearProjectSummary } from '../types/interlinear-project-summary'; @@ -89,6 +92,7 @@ export function makePhraseStripContext( boundaryMergeAltHint: '', boundarySplitLabel: '', glossPlaceholder: '', + tokenChipLabels: TOKEN_CHIP_LABEL_KEYS, skipLinkTransition: false, ...overrides, }; diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index efc7f86a..82fddd75 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -21,6 +21,7 @@ import { usePhraseStripContextValue, } from '../hooks/usePhraseStripSetup'; import useLatestRef from '../hooks/useLatestRef'; +import usePhraseWindowHalf from '../hooks/usePhraseWindowHalf'; import MemoizedArcOverlay from './ArcOverlay'; import { RECENTER_FADE_MS, RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; @@ -50,12 +51,6 @@ const SCROLL_SETTLE_FALLBACK_MS = 600; */ const HOLD_CENTERED_MAX_MS = 2_000; -/** - * Number of phrase slots rendered on each side of the focused phrase. Chosen large enough that no - * realistic viewport can ever render all tokens simultaneously. - */ -const PHRASE_WINDOW_HALF = 100; - /** * Localized string keys this view needs. Hoisted to module scope so the reference passed to * `useLocalizedStrings` is stable across renders. A fresh array literal each render makes the PAPI @@ -293,6 +288,26 @@ export default function ContinuousView({ /** DOM ref array indexed by group index; used to scroll the focused phrase box into view. */ const phraseRefs = useRef<(HTMLSpanElement | null)[]>([]); + /** Ref-setter callbacks for {@link phraseRefs}, keyed by the group index each one writes. */ + const groupRefSetters = useRef(new Map void>()); + + /** + * Returns the callback that records a group's wrapper element under `groupIndex`. Each index + * keeps one identity for as long as the strip lives, so handing the callback down cannot + * invalidate a memoized child on a render that changed nothing else about it. + */ + const getGroupRefSetter = useCallback((groupIndex: number) => { + const setters = groupRefSetters.current; + let setter = setters.get(groupIndex); + if (!setter) { + setter = (el: HTMLSpanElement | null) => { + phraseRefs.current[groupIndex] = el; + }; + setters.set(groupIndex, setter); + } + return setter; + }, []); + /** Ref to the token-strip row; the content row and mouse-leave target. */ // eslint-disable-next-line no-null/no-null const stripRowRef = useRef(null); @@ -492,9 +507,12 @@ export default function ContinuousView({ const atEnd = phraseGroups.length === 0 || focusPhraseIndex >= phraseGroups.length - 1; const stripOpacityClass = isVisible ? 'tw:opacity-100' : 'tw:opacity-0'; + /** Phrase groups mounted on each side of the focus, sized to the strip's visible width. */ + const phraseWindowHalf = usePhraseWindowHalf(scrollViewportRef, stripRowRef); + /** The inclusive group-index bounds of the rendered window. */ - const renderWindowStart = Math.max(0, focusPhraseIndex - PHRASE_WINDOW_HALF); - const renderWindowEnd = Math.min(phraseGroups.length - 1, focusPhraseIndex + PHRASE_WINDOW_HALF); + const renderWindowStart = Math.max(0, focusPhraseIndex - phraseWindowHalf); + const renderWindowEnd = Math.min(phraseGroups.length - 1, focusPhraseIndex + phraseWindowHalf); /** * The groups in the rendered window. Memoized on the bounds (and the source groups) so the array @@ -550,22 +568,29 @@ export default function ContinuousView({ /** Moves focus one phrase forward. */ const stepNext = useCallback(() => step(1), [step]); + /** Ref mirror of the focus so the select handler can compare against it without a dep on it. */ + const focusedTokenRefRef = useLatestRef(focusedTokenRef); + /** * Notifies the parent that the user selected the phrase whose first token is `ref`. The parent * echoes the new token ref back through `focusedTokenRef`; scroll + highlight follow - * automatically. + * automatically. Selecting the already-focused phrase is a no-op. + * + * Reads the current focus through a ref rather than closing over it, so the handler keeps one + * identity across focus moves and passing it down cannot invalidate a memoized child. * * @param ref - First-token ref (group key) of the selected phrase. */ const handlePhraseSelect = useCallback( (ref: string) => { const targetGroupIndex = groupIndexByTokenRef.get(ref); + const currentFocus = focusedTokenRefRef.current; const currentGroupIndex = - focusedTokenRef === undefined ? undefined : groupIndexByTokenRef.get(focusedTokenRef); + currentFocus === undefined ? undefined : groupIndexByTokenRef.get(currentFocus); if (targetGroupIndex !== undefined && targetGroupIndex === currentGroupIndex) return; emitInternalFocus(ref); }, - [focusedTokenRef, groupIndexByTokenRef, emitInternalFocus], + [focusedTokenRefRef, groupIndexByTokenRef, emitInternalFocus], ); /** Splits a phrase arc at a token boundary and dispatches the resulting phrase-store writes. */ @@ -938,12 +963,7 @@ export default function ContinuousView({ key: group.tokens[0].ref, group, isFocused: group.tokens.some((t) => t.ref === displayFocusedTokenRef), - // New closure per recomputation; React briefly nulls and reassigns each ref, but the - // cycle is synchronous and harmless. If renders become hot, move the assignment into - // MemoizedPhraseGroup (pass phraseRefs + groupIndex as props instead of a callback). - groupRef: (el: HTMLSpanElement | null) => { - phraseRefs.current[groupIndex] = el; - }, + groupRef: getGroupRefSetter(groupIndex), }; }), [ @@ -953,6 +973,7 @@ export default function ContinuousView({ focusedSideIsPrevByItem, displayFocusedTokenRef, verseStartLabelByTokenRef, + getGroupRefSetter, ], ); diff --git a/src/components/MorphemeBox.tsx b/src/components/MorphemeBox.tsx index b093dd28..02b2cd47 100644 --- a/src/components/MorphemeBox.tsx +++ b/src/components/MorphemeBox.tsx @@ -1,17 +1,9 @@ import type { MorphemeAnalysis, Token } from 'interlinearizer'; -import { useLocalizedStrings } from '@papi/frontend/react'; import { PopoverAnchor } from 'platform-bible-react'; import { formatReplacementString } from 'platform-bible-utils'; import { type MouseEvent, useEffect, useState } from 'react'; import { useMorphemeGlossDispatch, useReportGlossEditing } from './AnalysisStore'; - -const MORPHEME_GLOSS_STRING_KEYS = [ - '%interlinearizer_morphemeGloss_label%', -] as const satisfies `%${string}%`[]; - -const MORPHEME_BOX_STRING_KEYS = [ - '%interlinearizer_tokenChip_editMorphemes%', -] as const satisfies `%${string}%`[]; +import { TOKEN_CHIP_LABEL_KEYS, type TokenChipLabels } from './PhraseStripContext'; /** * Inline _display_ of an analyzed token's morpheme breakdown. The popover where forms are actually @@ -40,6 +32,7 @@ export function MorphemeBox({ popoverOpen, onEditBreakdown, onGlossFocus, + labels = TOKEN_CHIP_LABEL_KEYS, }: Readonly<{ /** The analyzed word token whose breakdown is shown. */ token: Token & { type: 'word' }; @@ -59,18 +52,19 @@ export function MorphemeBox({ * focusing one must move the view's focus just as focusing that input does. */ onGlossFocus: () => void; + /** + * Accessible labels for this box and its gloss inputs, resolved once per strip. Defaults to the + * unresolved keys, which is what they show until the strip's lookup lands. + */ + labels?: TokenChipLabels; }>) { - const [localizedStrings] = useLocalizedStrings(MORPHEME_BOX_STRING_KEYS); // Hovering anywhere in the box tints the whole forms row: clicking any cell opens the same // whole-breakdown editor, so the affordance is breakdown-wide, not per-morpheme. Tracking hover // on the container (rather than per cell) avoids a one-frame un-tint as the pointer crosses the // gap between adjacent form cells. const [isFormsHovered, setIsFormsHovered] = useState(false); - const editLabel = formatReplacementString( - localizedStrings['%interlinearizer_tokenChip_editMorphemes%'], - { token: token.surfaceText }, - ); + const editLabel = formatReplacementString(labels.editMorphemes, { token: token.surfaceText }); return ( @@ -139,6 +133,7 @@ export function MorphemeBox({ analysisLanguage={analysisLanguage} column={i + 1} disabled={disabled} + glossLabelTemplate={labels.morphemeGloss} morpheme={m} onFocus={onGlossFocus} tokenRef={token.ref} @@ -162,6 +157,7 @@ export function MorphemeGlossInput({ disabled, column, onFocus, + glossLabelTemplate = TOKEN_CHIP_LABEL_KEYS.morphemeGloss, }: Readonly<{ morpheme: MorphemeAnalysis; /** The token ref gloss writes are dispatched against. */ @@ -173,11 +169,16 @@ export function MorphemeGlossInput({ column: number; /** Called when the input receives focus, so the containing chip can report its token as focused. */ onFocus: () => void; + /** + * Accessible label for this input, with `{form}` still to be substituted for the morpheme's form. + * Resolved once per strip. Defaults to the unresolved key, which is what the input shows until + * the strip's lookup lands. + */ + glossLabelTemplate?: string; }>) { const committed = morpheme.gloss?.[analysisLanguage] ?? ''; const dispatchMorphemeGloss = useMorphemeGlossDispatch(); const [draft, setDraft] = useState(committed); - const [localizedStrings] = useLocalizedStrings(MORPHEME_GLOSS_STRING_KEYS); useEffect(() => { setDraft(committed); @@ -188,10 +189,7 @@ export function MorphemeGlossInput({ return ( ; + +/** + * The localize key each {@link TokenChipLabels} field resolves from. Doubles as the pre-resolution + * value of the bundle: the localization hook yields every key as its own value until its lookup + * lands, so a chip rendered before (or without) a strip's fetch shows exactly what it would show in + * that first frame. + */ +export const TOKEN_CHIP_LABEL_KEYS = { + glossLabel: '%interlinearizer_tokenChip_glossLabel%', + showSuggestions: '%interlinearizer_tokenChip_showSuggestions%', + defineMorphemes: '%interlinearizer_tokenChip_defineMorphemes%', + editMorphemes: '%interlinearizer_tokenChip_editMorphemes%', + morphemeGloss: '%interlinearizer_morphemeGloss_label%', + acceptSuggestion: '%interlinearizer_suggestion_accept%', + promoteSuggestion: '%interlinearizer_suggestion_promote%', +} as const satisfies Record; + /** * The stable, strip-wide context for one render of a token row: a single value is built per render * and provided around the row via {@link PhraseStripProvider}, reaching every phrase group and link @@ -102,6 +142,8 @@ export type PhraseStripContextValue = Readonly<{ * resolves (one strip-wide reflow at most, behind the initial fade). */ glossPlaceholder: string; + /** Labels every word token's chip formats for itself, resolved once per strip. */ + tokenChipLabels: TokenChipLabels; /** * When `true`, the sliding-door transition on link-slot wrappers is suppressed (duration set to * 0ms). Set during external navigation and initial mount so the layout snaps to its final state diff --git a/src/components/PhraseStripParts.tsx b/src/components/PhraseStripParts.tsx index 0fcb1cd3..b80b4dfd 100644 --- a/src/components/PhraseStripParts.tsx +++ b/src/components/PhraseStripParts.tsx @@ -388,6 +388,9 @@ export function PhraseSlot({ ); } +/** Memoized version of {@link PhraseSlot}; use in render-stable strips. */ +export const MemoizedPhraseSlot = memo(PhraseSlot); + // #endregion // #region PhraseGroup @@ -462,6 +465,9 @@ export const MemoizedPhraseGroup = memo(function PhraseGroup({ // The strip wrapper is `pointer-events-none` so its padding gaps let arc-split button clicks // through to the buttons beneath; re-enable events on the actual phrase content here. className="tw:pointer-events-auto" + // One element per group whatever the group renders as, so a count of these is a count of + // groups — unlike phrase boxes, of which a discontiguous phrase contributes several. + data-phrase-group="true" onMouseEnter={ allowHover ? () => { @@ -606,7 +612,7 @@ export function PhraseStrip({ return items.map((item) => { if (item.kind === 'slot') { return ( - void; @@ -100,8 +97,8 @@ export function TokenChip({ isSplitFree?: boolean; showMorphology?: boolean; glossPlaceholder?: string; + labels?: TokenChipLabels; }>) { - const [localizedStrings] = useLocalizedStrings(STRING_KEYS); const committedGloss = useGloss(token.ref); const onGlossChange = useGlossDispatch(); const morphemes = useMorphemes(token.ref); @@ -416,6 +413,7 @@ export function TokenChip({