From 2a368053fcadf28de01b4f06e6167674a859bfda Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 13:51:50 -0400 Subject: [PATCH 01/20] Reserve link-slot space when hidden to prevent arc re-alignment Inactive link slots now use visibility:hidden instead of max-width collapse, so toggling hideInactiveLinkButtons no longer shifts phrase box positions. Removes the slotAnimationTick rAF loop and the hideInactiveLinkButtons re-center effect that compensated for the old layout shift. Co-Authored-By: Claude Sonnet 4.6 --- .../components/ContinuousView.test.tsx | 81 +++++-------------- .../components/PhraseStripParts.test.tsx | 8 +- src/__tests__/components/SegmentView.test.tsx | 54 +------------ src/components/ContinuousView.tsx | 49 +++++------ src/components/PhraseStripParts.tsx | 11 ++- src/components/SegmentView.tsx | 50 +++--------- 6 files changed, 61 insertions(+), 192 deletions(-) diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index 54554e7..40e8b1d 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -831,15 +831,15 @@ describe('ContinuousView scroll behavior', () => { ); } render(, withAnalysisStore); - // Returns true when the tok-0/tok-1 link icon is rendered AND its sliding-door wrapper is - // open (not suppressed). After the animation change, icons stay mounted but collapse via - // maxWidth: '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 (not + // suppressed). Icons stay mounted but are hidden via visibility:'hidden' when suppressed, so + // we query the DOM wrapper's style rather than spy calls. return () => { const icon = document.querySelector( '[data-prev-ref="tok-0"][data-next-ref="tok-1"]', ); if (!icon) return false; - return icon.parentElement?.style.maxWidth !== '0'; + return icon.parentElement?.style.visibility !== 'hidden'; }; } @@ -983,64 +983,25 @@ describe('ContinuousView scroll behavior', () => { ); }); - it('re-centers the focused group each frame while a view-option toggle re-lays out the strip', () => { - // Toggling `hideInactiveLinkButtons` collapses/expands the out-of-segment link slots over - // LINK_SLOT_TRANSITION_MS, continuously shifting every box around the center. A single re-center - // would only fix the first frame; the focused group must be re-centered on every animation frame - // for the whole transition so it stays dead center, then the loop tears down once the transition - // completes. Fake timers drive both the rAF callbacks and performance.now() deterministically. - jest.useFakeTimers(); - try { - const book = makeBook(); - const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); - const { rerender } = render(, withAnalysisStore); - act(() => { - jest.runOnlyPendingTimers(); - }); - scrollIntoViewMock.mockClear(); - - // Toggling hideInactiveLinkButtons changes the strip layout, so the view re-centers. - rerender(); - - // First animation frame re-centers. - act(() => { - jest.advanceTimersByTime(50); - }); - expect(scrollIntoViewMock).toHaveBeenCalledWith( - expect.objectContaining({ behavior: 'auto', inline: 'center' }), - ); - - // Subsequent frames within the transition window re-center again. - scrollIntoViewMock.mockClear(); - act(() => { - jest.advanceTimersByTime(50); - }); - expect(scrollIntoViewMock).toHaveBeenCalledWith( - expect.objectContaining({ behavior: 'auto', inline: 'center' }), - ); + 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. + const book = makeBook(); + const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); + const { rerender } = render(, withAnalysisStore); + scrollIntoViewMock.mockClear(); - // Advance well past the transition window so the loop hits its deadline and stops scheduling. - act(() => { - jest.advanceTimersByTime(500); - }); - scrollIntoViewMock.mockClear(); - act(() => { - jest.advanceTimersByTime(500); - }); - expect(scrollIntoViewMock).not.toHaveBeenCalled(); + // Toggling hideInactiveLinkButtons should not cause any re-centering. + rerender(); + expect(scrollIntoViewMock).not.toHaveBeenCalled(); - // Toggling simplifyPhrases likewise starts a fresh re-center loop. - scrollIntoViewMock.mockClear(); - rerender(); - act(() => { - jest.advanceTimersByTime(50); - }); - expect(scrollIntoViewMock).toHaveBeenCalledWith( - expect.objectContaining({ behavior: 'auto', inline: 'center' }), - ); - } finally { - jest.useRealTimers(); - } + // Toggling simplifyPhrases re-centers exactly once (no rAF loop needed). + rerender(); + expect(scrollIntoViewMock).toHaveBeenCalledWith( + expect.objectContaining({ behavior: 'auto', inline: 'center' }), + ); + expect(scrollIntoViewMock).toHaveBeenCalledTimes(1); }); }); diff --git a/src/__tests__/components/PhraseStripParts.test.tsx b/src/__tests__/components/PhraseStripParts.test.tsx index bfac600..a885bc6 100644 --- a/src/__tests__/components/PhraseStripParts.test.tsx +++ b/src/__tests__/components/PhraseStripParts.test.tsx @@ -242,9 +242,9 @@ describe('PhraseSlot', () => { , ); - // Icon stays mounted for smooth sliding-door animation; the wrapper collapses it visually. + // Icon stays mounted but hidden in place (visibility:hidden preserves its space in the layout). const icon = screen.getByTestId('link-icon'); - expect(icon.parentElement?.style.maxWidth).toBe('0'); + expect(icon.parentElement?.style.visibility).toBe('hidden'); expect(icon.parentElement?.style.opacity).toBe('0'); }); @@ -288,9 +288,9 @@ describe('PhraseSlot', () => { , ); - // Icon stays mounted for smooth sliding-door animation; the wrapper collapses it visually. + // Icon stays mounted but hidden in place (visibility:hidden preserves its space in the layout). const icon = screen.getByTestId('link-icon'); - expect(icon.parentElement?.style.maxWidth).toBe('0'); + expect(icon.parentElement?.style.visibility).toBe('hidden'); expect(icon.parentElement?.style.opacity).toBe('0'); }); }); diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index 2a5e16b..bd0e0c6 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -3,7 +3,7 @@ /// import { useLocalizedStrings } from '@papi/frontend/react'; -import { act, render, screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { PhraseAnalysisLink, ScriptureRef, Segment, Token } from 'interlinearizer'; import type { ReactNode } from 'react'; @@ -641,66 +641,20 @@ describe('SegmentView', () => { expect(handleSelect).not.toHaveBeenCalled(); }); - it('animates the link-slot open/close transition after mount', () => { + it('enables the link-slot fade transition after mount', () => { const { container } = render( , ); - // The slot's sliding-door wrapper is the [data-link-slot]'s first element child. Once the mount - // effect has run (flushed by render/act), SegmentView stops suppressing the transition so a - // later active-segment flip slides the slot instead of snapping it — the reflow-during-click - // that otherwise mis-selects the phrase under the pointer. + // After mount, SegmentView stops suppressing the opacity transition so later toggles of + // isActive / hideInactiveLinkButtons fade the icon in/out instead of snapping. const slotWrapper = container.querySelector('[data-link-slot] > span'); if (!(slotWrapper instanceof HTMLElement)) throw new Error('Expected a link-slot wrapper span'); expect(slotWrapper.style.transitionDuration).toBe(`${LINK_SLOT_TRANSITION_MS}ms`); }); - it('re-measures arcs each frame while the link slots slide, then stops at the deadline', () => { - // Capture rAF callbacks and control the clock so we can step the slide loop deterministically. - const frames: FrameRequestCallback[] = []; - const rafSpy = jest - .spyOn(globalThis, 'requestAnimationFrame') - .mockImplementation((cb: FrameRequestCallback) => { - frames.push(cb); - return frames.length; - }); - jest.spyOn(globalThis, 'cancelAnimationFrame').mockImplementation(() => {}); - let now = 0; - jest.spyOn(performance, 'now').mockImplementation(() => now); - - const { rerender } = render( - - - , - ); - // Mount effect skips the loop; flipping isActive after mount starts it. - rerender( - - - , - ); - - const runNextFrame = () => { - const cb = frames.shift(); - if (!cb) throw new Error('Expected a scheduled animation frame'); - act(() => cb(now)); - }; - - // Before the deadline each frame schedules another (the bump + continue branch). - expect(frames.length).toBe(1); - runNextFrame(); - expect(frames.length).toBe(1); - - // Crossing the deadline runs the final frame without scheduling more (the stop branch). - now = LINK_SLOT_TRANSITION_MS + 1; - runNextFrame(); - expect(frames.length).toBe(0); - - rafSpy.mockRestore(); - }); - it('computes candidatePhraseIds from non-empty candidateTokenRefs', () => { const phraseLink: PhraseAnalysisLink = { analysisId: 'phrase-1', diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index c3fd040..ebf1fe6 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -531,15 +531,15 @@ export default function ContinuousView({ }; }, [focusPhraseIndex, commitPendingActiveSegment]); - // Keep the focused group pinned dead-center across the deferred inactive-link relayout. When + // Keep the focused group pinned dead-center after the deferred active-segment flip. When // `committedActiveSegmentId` flips (after an internal-nav scroll settles), inactive link icons - // slide open/closed over `LINK_SLOT_TRANSITION_MS`, continuously shifting every box around the - // center. A single re-center would only fix the first frame; the focused phrase would then drift - // as the slots keep animating. So we re-center every frame for the whole transition: a `rAF` loop - // re-anchors the focused group until the animation completes, canceling the shift at the center on - // every painted frame. The first run is skipped because the initial center is established by the - // scroll effect's instant jump. A `useLayoutEffect` seeds the loop so the very first re-center - // lands before paint (no initial flash), then `rAF` carries it through the animation. + // fade in/out over `LINK_SLOT_TRANSITION_MS`. Because they are hidden via `visibility: hidden` + // their layout space is preserved, so boxes do not shift — but any residual sub-pixel drift from + // the preceding smooth scroll is corrected by re-centering once before paint. The rAF loop holds + // the group centered for the full fade duration as a conservative guard against any future layout + // changes that could re-introduce drift. The first run is skipped because the initial center is + // established by the scroll effect's instant jump. A `useLayoutEffect` seeds the loop so the very + // first re-center lands before paint (no initial flash), then `rAF` carries it through the fade. const skipActiveSegmentRecenterRef = useRef(true); useLayoutEffect(() => { if (skipActiveSegmentRecenterRef.current) { @@ -566,33 +566,20 @@ export default function ContinuousView({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [committedActiveSegmentId]); - // Re-center the focused group when a view option toggles. Hiding/showing link buttons and phrase - // controls changes the strip's layout, so the previously-centered group drifts off-center; snap it - // back into view. Toggling `hideInactiveLinkButtons` collapses/expands the out-of-segment link - // slots via a `max-width`/`opacity` transition over `LINK_SLOT_TRANSITION_MS`, continuously - // shifting every box around the center for the whole animation. A single re-center would only fix - // the first frame; the focused phrase would then drift as the slots keep animating. So we re-center - // every frame until the transition completes, canceling the shift at the center on every painted - // frame — mirroring the active-segment-flip re-anchor loop above. + // Re-center the focused group when a view option toggles. Toggling `simplifyPhrases` changes + // the strip's layout, so the previously-centered group may drift off-center; snap it back into + // view. `hideInactiveLinkButtons` is excluded: inactive link slots now reserve their space even + // when hidden (visibility:hidden), so toggling it no longer shifts the layout. useEffect(() => { - /** Re-centers the focused group; called each `rAF` until the deadline. */ - const recenter = () => { - phraseRefs.current[focusPhraseIndex]?.scrollIntoView({ - behavior: 'auto', - block: 'nearest', - inline: 'center', - }); - }; - const deadline = performance.now() + LINK_SLOT_TRANSITION_MS; - let rafId = requestAnimationFrame(function recenterFrame() { - recenter(); - if (performance.now() < deadline) rafId = requestAnimationFrame(recenterFrame); + phraseRefs.current[focusPhraseIndex]?.scrollIntoView({ + behavior: 'auto', + block: 'nearest', + inline: 'center', }); - return () => cancelAnimationFrame(rafId); // focusPhraseIndex is intentionally excluded: it has its own scroll effect above. This effect // only re-centers in response to layout-affecting option toggles. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [hideInactiveLinkButtons, simplifyPhrases]); + }, [simplifyPhrases]); // When entering edit or confirm-unlink mode, smooth-scroll to the first group of the active // phrase by notifying the parent of the new focused token. Scroll then follows automatically @@ -722,7 +709,7 @@ export default function ContinuousView({ arcContainerRef, true, hasRealPhraseInRenderWindow, - [renderWindowGroups, phraseMode, committedActiveSegmentId, hideInactiveLinkButtons], + [renderWindowGroups, phraseMode, committedActiveSegmentId], ); /** diff --git a/src/components/PhraseStripParts.tsx b/src/components/PhraseStripParts.tsx index 73eb497..ecd81b3 100644 --- a/src/components/PhraseStripParts.tsx +++ b/src/components/PhraseStripParts.tsx @@ -13,9 +13,9 @@ import { } from '../utils/token-layout'; /** - * Duration, in milliseconds, of the link-slot sliding-door (`max-width` / `opacity`) transition. - * Exported so `ContinuousView` can re-center the focused phrase for exactly this long while the - * slots animate open/closed, keeping it pinned dead-center as the layout shifts around it. + * Duration, in milliseconds, of the link-slot opacity fade transition. Exported so `ContinuousView` + * can re-center the focused phrase for exactly this long after `committedActiveSegmentId` flips, + * keeping it anchored while the fade runs. */ export const LINK_SLOT_TRANSITION_MS = 200; @@ -95,12 +95,11 @@ export function PhraseSlot({ > {hasLinkableNeighbors && ( diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 7799f90..af38123 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -7,7 +7,7 @@ import { usePhraseDispatch, usePhraseLinkByIdMap, usePhraseLinkMap } from './Ana import type { PhraseMode } from '../types/phrase-mode'; import { PhraseStripProvider } from './PhraseStripContext'; import type { PhraseStripContextValue } from './PhraseStripContext'; -import { PhraseStrip, LINK_SLOT_TRANSITION_MS, type StripItem } from './PhraseStripParts'; +import { PhraseStrip, type StripItem } from './PhraseStripParts'; import { buildRenderUnits, groupTokens, @@ -201,46 +201,15 @@ export function SegmentView({ const tokenRowRef = useRef(null); /** - * `false` until just after the first paint, then `true`. Gates the link-slot open/close - * transition: the initial layout must snap to its final width before paint (animating from - * collapsed on mount would flash), but every later flip of `isActive` / `hideInactiveLinkButtons` - * should animate. Animating those flips is what fixes the mis-selection bug — when a click on an - * inactive segment flips it active, the inter-phrase slots slide open over - * `LINK_SLOT_TRANSITION_MS` instead of snapping, so the bubbled click still resolves against the - * pre-reflow layout and lands on the phrase the user aimed at rather than whatever instantly - * shifted under the pointer. + * `false` until just after the first paint, then `true`. Gates the link-slot fade transition: the + * initial visibility state must snap into place before paint (fading in on mount would flash), + * but every later flip of `isActive` / `hideInactiveLinkButtons` should animate. */ const [hasMounted, setHasMounted] = useState(false); useEffect(() => { setHasMounted(true); }, []); - /** - * Bumped on each animation frame while the link slots are sliding open/closed, then fed into the - * arc re-measure deps below. When `isActive` or `hideInactiveLinkButtons` flips, the inter-phrase - * slots animate their width over `LINK_SLOT_TRANSITION_MS`, continuously shifting the token - * boxes; a discontiguous phrase's arc endpoints move with them. The arc `ResizeObserver` only - * fires when the container's own box changes (e.g. a wrap), so a pure horizontal shift within a - * row would leave the arcs lagging the tokens for the whole transition. Re-measuring every frame - * keeps the arcs pinned to their runs throughout the slide — mirroring ContinuousView's re-center - * loop. - */ - const [slotAnimationTick, setSlotAnimationTick] = useState(0); - const skipSlotAnimationRef = useRef(true); - useEffect(() => { - // Skip the first run: the initial layout snaps (skipLinkTransition), so there's nothing sliding. - if (skipSlotAnimationRef.current) { - skipSlotAnimationRef.current = false; - return undefined; - } - const deadline = performance.now() + LINK_SLOT_TRANSITION_MS; - let rafId = requestAnimationFrame(function tick() { - setSlotAnimationTick((n) => n + 1); - if (performance.now() < deadline) rafId = requestAnimationFrame(tick); - }); - return () => cancelAnimationFrame(rafId); - }, [isActive, hideInactiveLinkButtons]); - /** * Ref to the outer `tw:relative tw:overflow-visible` div that is both the SVG parent and the arc * measurement container. Using this element (rather than the inner token-row span) aligns the @@ -419,11 +388,11 @@ export function SegmentView({ * background handler must not also fire and override that focus with the segment's first phrase — * which was most visible when clicking an out-of-segment phrase fragment. The `[data-link-slot]` * case is the inter-phrase link slot: when its link button is visible the button absorbs the - * click, but when `hideInactiveLinkButtons` collapses the button to zero width the slot becomes - * an empty clickable gap between phrases. Treating it as background was the bug where clicking - * near a phrase in an inactive segment (buttons hidden) snapped focus to the segment's first - * phrase; ignoring it leaves the click a no-op, matching the buttons-visible behavior. Everything - * else — padding, arc gutters, empty wrap space — focuses the first phrase. + * click, but when `hideInactiveLinkButtons` hides the button in place the slot becomes an empty + * clickable gap between phrases. Treating it as background was the bug where clicking near a + * phrase in an inactive segment (buttons hidden) snapped focus to the segment's first phrase; + * ignoring it leaves the click a no-op, matching the buttons-visible behavior. Everything else — + * padding, arc gutters, empty wrap space — focuses the first phrase. * * @param event - The click event on the segment container. */ @@ -457,7 +426,6 @@ export function SegmentView({ displayMode, isActive, hideInactiveLinkButtons, - slotAnimationTick, ]); if (displayMode === 'baseline-text') { From 380371a7be04d5a3f2632f96768f29b07f835af3 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 14:36:44 -0400 Subject: [PATCH 02/20] Fix punctuation jumping when link icon is hidden link-slot was flex-col, stacking the icon wrapper above the punctuation. Changing to flex-row keeps them side-by-side so punctuation position no longer depends on the icon wrapper's height. Co-Authored-By: Claude Sonnet 4.6 --- src/tailwind.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tailwind.css b/src/tailwind.css index 39ba111..7582703 100644 --- a/src/tailwind.css +++ b/src/tailwind.css @@ -52,7 +52,7 @@ } @utility link-slot { - @apply tw:inline-flex tw:flex-col tw:items-center; + @apply tw:inline-flex tw:flex-row tw:items-center; } @utility token-row { From 82e2b9f4fafe70f98f59b7cb2504457c7223a075 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 14:47:18 -0400 Subject: [PATCH 03/20] Fix in-phrase punctuation duplicating on every focus change buildRenderUnits was pushing into the shared TokenGroup.punctuationBetween arrays, which are owned by memoized objects that outlive a single render. Replace push() with assignment so each call resets the gap array instead of accumulating. Co-Authored-By: Claude Sonnet 4.6 --- src/utils/token-layout.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/token-layout.ts b/src/utils/token-layout.ts index cce3835..f3acb3a 100644 --- a/src/utils/token-layout.ts +++ b/src/utils/token-layout.ts @@ -255,9 +255,9 @@ export function buildRenderUnits(tokens: Token[], tokenGroups: TokenGroup[]): Re // This is a subsequent token of an already-open group. The gap index is tokenIndex - 1 // (the gap between tokens[tokenIndex-1] and tokens[tokenIndex]). const gapIndex = intraEntry.tokenIndex - 1; - // Flush any punctuation already buffered since the previous group token — it belongs to - // this intra-group gap, not to the upcoming inter-group slot. - intraEntry.group.punctuationBetween[gapIndex].push(...pendingPunctuation); + // Reset the gap array to a fresh one so repeated calls to buildRenderUnits (e.g. on focus + // change) do not accumulate into the memoized TokenGroup's shared arrays. + intraEntry.group.punctuationBetween[gapIndex] = [...pendingPunctuation]; pendingPunctuation = []; pendingIntraGroup = { group: intraEntry.group, gapIndex }; return; From 80b1d8cfb79a694958706a308aae1940bd366d0b Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 15:15:19 -0400 Subject: [PATCH 04/20] Fix crash on punctuation after the last token of a phrase group After the gapIndex fix, pendingIntraGroup pointed to a non-existent gap when the current token was the last in its group, causing a push() onto undefined when post-phrase punctuation arrived. Guard the assignment so the last token clears pendingIntraGroup instead; punctuation then falls into the inter-group LinkSlot as intended. Adds a regression test. Co-Authored-By: Claude Sonnet 4.6 --- src/__tests__/utils/token-layout.test.ts | 35 ++++++++++++++++++++++++ src/utils/token-layout.ts | 22 +++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/__tests__/utils/token-layout.test.ts b/src/__tests__/utils/token-layout.test.ts index ef520b8..a268764 100644 --- a/src/__tests__/utils/token-layout.test.ts +++ b/src/__tests__/utils/token-layout.test.ts @@ -324,4 +324,39 @@ describe('buildRenderUnits', () => { const allPunctuation = allSlots.flatMap((u) => (u.kind === 'slot' ? u.slot.punctuation : [])); expect(allPunctuation).toHaveLength(0); }); + + it('routes punctuation after the last token of a group into the following inter-group slot, not punctuationBetween', () => { + // Crash scenario: [A, B] (phrase group 1), punct, [C] (phrase group 2). + // After processing B (last token of group 1), pendingIntraGroup must be cleared so the + // punctuation is routed into the inter-group LinkSlot, not into an out-of-bounds + // punctuationBetween index. + const a = mkWord('tok-a'); + const b = mkWord('tok-b'); + const punct = mkPunct('p1', ','); + const c = mkWord('tok-c'); + const link1 = makePhraseLink('ph1', ['tok-a', 'tok-b']); + const link2 = makePhraseLink('ph2', ['tok-c']); + const groups = groupTokens( + [a, b, punct, c], + new Map([ + ['tok-a', link1], + ['tok-b', link1], + ['tok-c', link2], + ]), + ); + // Must not throw (was crashing with "Cannot read properties of undefined" before the fix). + const units = buildRenderUnits([a, b, punct, c], groups); + + // The punctuation must appear in the inter-group slot between group 1 and group 2. + const interGroupSlot = units.find( + (u) => u.kind === 'slot' && u.slot.prevGroup === groups[0] && u.slot.nextGroup === groups[1], + ); + expect(interGroupSlot?.kind).toBe('slot'); + if (interGroupSlot?.kind === 'slot') { + expect(interGroupSlot.slot.punctuation.map((t) => t.ref)).toEqual(['p1']); + } + + // The punctuation must NOT appear in group 1's punctuationBetween. + expect(groups[0].punctuationBetween.flat()).toHaveLength(0); + }); }); diff --git a/src/utils/token-layout.ts b/src/utils/token-layout.ts index f3acb3a..88c6f82 100644 --- a/src/utils/token-layout.ts +++ b/src/utils/token-layout.ts @@ -256,10 +256,26 @@ export function buildRenderUnits(tokens: Token[], tokenGroups: TokenGroup[]): Re // (the gap between tokens[tokenIndex-1] and tokens[tokenIndex]). const gapIndex = intraEntry.tokenIndex - 1; // Reset the gap array to a fresh one so repeated calls to buildRenderUnits (e.g. on focus - // change) do not accumulate into the memoized TokenGroup's shared arrays. - intraEntry.group.punctuationBetween[gapIndex] = [...pendingPunctuation]; + // change) do not accumulate into the memoized TokenGroup's shared arrays. If + // pendingIntraGroup is already tracking this exact gap, punctuation tokens were routed + // directly into it; preserve them by prepending rather than replacing. + if ( + pendingIntraGroup?.group === intraEntry.group && + pendingIntraGroup.gapIndex === gapIndex + ) { + intraEntry.group.punctuationBetween[gapIndex] = [ + ...pendingPunctuation, + ...intraEntry.group.punctuationBetween[gapIndex], + ]; + } else { + intraEntry.group.punctuationBetween[gapIndex] = [...pendingPunctuation]; + } pendingPunctuation = []; - pendingIntraGroup = { group: intraEntry.group, gapIndex }; + const nextGapIndex = intraEntry.tokenIndex; + pendingIntraGroup = + nextGapIndex < intraEntry.group.punctuationBetween.length + ? { group: intraEntry.group, gapIndex: nextGapIndex } + : undefined; return; } From 47f71cdae109b49073878045513269371335739a Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 15:17:49 -0400 Subject: [PATCH 05/20] Cut 'yet' --- contributions/localizedStrings.json | 2 +- src/components/PhraseStripContext.tsx | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index c15fdf8..84b01da 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -21,7 +21,7 @@ "%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_linkButton_crossSegmentDisabledTooltip%": "Cross-segment phrases are not yet supported. This link button is outside the current segment.", + "%interlinearizer_linkButton_crossSegmentDisabledTooltip%": "Cross-segment phrases are not supported. This link button is outside the current segment.", "%interlinearizer_modal_create_title%": "Create Interlinear Project", "%interlinearizer_modal_create_name_label%": "Name (optional)", diff --git a/src/components/PhraseStripContext.tsx b/src/components/PhraseStripContext.tsx index 6e59d07..d9bc5ad 100644 --- a/src/components/PhraseStripContext.tsx +++ b/src/components/PhraseStripContext.tsx @@ -65,10 +65,7 @@ export type PhraseStripContextValue = Readonly<{ * verse in both strips. */ activeSegmentId: string | undefined; - /** - * Tooltip shown on link buttons that are disabled because they are outside the currently focused - * segment. Explains that cross-segment phrases are not yet supported. - */ + /** Tooltip shown on disabled link buttons because they are outside the currently focused segment. */ crossSegmentLinkTooltip: string; /** * When `true`, the sliding-door transition on link-slot wrappers is suppressed (duration set to From 4e0f4514a62c69f772462ece03e30775dc1223d7 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 16:04:29 -0400 Subject: [PATCH 06/20] Tune link button and arc contrast Link buttons: use foreground/60 (active) and foreground/20 (inactive) instead of muted-foreground at 100%/50%, widening the visual gap between actionable and suppressed slots. Arcs: reduce focused-phrase stroke opacity from 1.0 to 0.75 so the current phrase's arcs are less dominant relative to other phrases. Co-Authored-By: Claude Sonnet 4.6 --- src/__tests__/utils/phrase-arc.test.ts | 8 ++++---- src/components/TokenLinkIcon.tsx | 2 +- src/tailwind.css | 4 ++-- src/utils/phrase-arc.ts | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/__tests__/utils/phrase-arc.test.ts b/src/__tests__/utils/phrase-arc.test.ts index 5fa6532..0d2527f 100644 --- a/src/__tests__/utils/phrase-arc.test.ts +++ b/src/__tests__/utils/phrase-arc.test.ts @@ -810,7 +810,7 @@ describe('computeAllArcPaths', () => { describe('getArcStrokeProps', () => { const dimmed = { stroke: 'var(--border)', strokeOpacity: 0.8, strokeWidth: 2 }; const hovered = { stroke: 'var(--foreground)', strokeOpacity: 0.55, strokeWidth: 2 }; - const highlighted = { stroke: 'var(--foreground)', strokeOpacity: 1, strokeWidth: 2 }; + const highlighted = { stroke: 'var(--foreground)', strokeOpacity: 0.75, strokeWidth: 2 }; const destructive = { stroke: 'var(--destructive)', strokeOpacity: 1, @@ -825,15 +825,15 @@ describe('getArcStrokeProps', () => { expect(getArcStrokeProps({ kind: 'view' }, 'p1', 'p1', undefined)).toEqual(hovered); }); - it('uses full white for the focused phrase arc in view mode', () => { + it('uses foreground at 75% opacity for the focused phrase arc in view mode', () => { expect(getArcStrokeProps({ kind: 'view' }, 'p1', undefined, 'p1')).toEqual(highlighted); }); - it('uses full white for the focused phrase even when it is also hovered', () => { + it('uses foreground at 75% opacity for the focused phrase even when it is also hovered', () => { expect(getArcStrokeProps({ kind: 'view' }, 'p1', 'p1', 'p1')).toEqual(highlighted); }); - it('whitens the edited phrase arc in edit mode regardless of hover', () => { + it('uses foreground at 75% opacity for the edited phrase arc in edit mode regardless of hover', () => { expect( getArcStrokeProps( { kind: 'edit', phraseId: 'p1', originalTokens: [] }, diff --git a/src/components/TokenLinkIcon.tsx b/src/components/TokenLinkIcon.tsx index 609d041..938f58a 100644 --- a/src/components/TokenLinkIcon.tsx +++ b/src/components/TokenLinkIcon.tsx @@ -314,7 +314,7 @@ export function TokenLinkIcon({ return (