Fix arc alignment, punctuation routing, and link-button polish; extract test helpers and reorganize types

[imnasnainaec]

Devin: https://app.devin.ai/review/sillsdev/interlinearizer-extension/pull/100

Summary

Bug fixes and visual polish

• Arc alignment when hiding inactive link buttons: Inactive link-slot wrappers previously collapsed to max-width: 0, shifting phrase boxes horizontally and causing arcs to re-align whenever the setting was toggled. The wrapper now uses opacity: 0 with a reserved minimum height instead, keeping the slot's footprint constant so arcs stay put. The slotAnimationTick rAF loop in SegmentView (which compensated for the old layout shift) and the hideInactiveLinkButtons re-center effect in ContinuousView were removed as they are no longer needed.

• Punctuation sits below link icon: The link-slot flex container is flex-col so punctuation renders below the icon. The icon wrapper uses opacity: 0 (not visibility: hidden) and carries an explicit display: inline-flex + min-height so it always reserves the same vertical space, preventing punctuation from jumping when the icon renders nothing.

• Link icon a11y when suppressed: Pointer events are disabled and the element is hidden from the accessibility tree when the icon is not rendered.

• In-phrase punctuation duplication: buildRenderUnits mutated the punctuationBetween arrays on memoized TokenGroup objects in-place. Because buildRenderUnits re-runs on focus change while groupTokens is memoized at a coarser granularity, punctuation accumulated on every focus change. Fixed by replacing .push() with assignment so each run resets the gap rather than appending. A follow-up commit ensures the fix is fully idempotent.

• Punctuation routed to wrong gap in multi-token phrases: After processing a non-first word token, the pendingIntraGroup tracker was pointed at the gap before the current token rather than the gap after it. Subsequent punctuation was pushed into the wrong slot. Fixed by advancing the tracker to the next gap index. A guard clears the tracker when the current token is the last in its group, preventing an out-of-bounds access on post-phrase punctuation.

• Crash on punctuation after the last token of a phrase group: Fixed an index-out-of-bounds that occurred when punctuation followed the final token in a group.

• Guard mergePhrases against self-merge: Added a guard when targetPhraseId === absorbedPhraseId to prevent state corruption.

• Link button contrast: Active and inactive link buttons now use foreground/60 and foreground/20 respectively (previously muted-foreground at 100%/50%), producing a wider visual gap between actionable and suppressed slots.

• Arc contrast: Focused-phrase arc opacity reduced from 1.0 to 0.60; other-phrase arc opacity raised from 0.80 to 1.0 (the border variable is already low-contrast, so full opacity is still visually quiet). The gap between the two states is substantially narrower.

• Tooltip copy: Removed "yet" from the cross-segment link button tooltip ("not yet supported" → "not supported").

Refactoring and cleanup

• Type reorganization: Renamed type files to kebab-case, added @file JSDoc headers, and moved token-layout types to a dedicated module. ArcStrokeProps is no longer exported — no consumers exist outside phrase-arc.ts.

• Test directory layout: Moved controls and modals test files into subdirectories that match the source layout.

• Test helpers extracted (CLEANUP.md H1, H3, H5, H7, H8):

  • emptyAnalysis factory → src/types/emptyFactories.ts
  • makeWordToken fixture factory → src/__tests__/test-helpers.ts
  • makeStubProject factory → src/__tests__/test-helpers.ts
  • withAnalysisStore render wrapper → src/__tests__/components/test-helpers.tsx
  • defaultScrRef — removed two local redeclarations; both files now import the canonical export from test-helpers.ts

• Hook test helpers extracted (CLEANUP.md M15): mockProjectSettingReturn helper centralized, removing 15 inline tuple spellings across two hook test files.

Test plan

• Toggle "Hide link buttons outside active segment" — arcs should not move
• Verify punctuation between tokens of a multi-token phrase appears in the correct gaps
• Verify punctuation after a phrase appears in the inter-phrase slot, not inside the phrase
• Verify in-phrase punctuation does not duplicate when navigating between phrases
• Verify no crash when punctuation follows the last token of a phrase group
• Confirm merging a phrase with itself does not corrupt state
• Confirm link button active/inactive states are visually distinct
• Confirm focused-phrase arcs are less dominant without losing clarity
• Confirm suppressed link icons are not reachable via keyboard or screen reader
• Test in both light and dark mode

🤖 Generated with Claude Code

---

This change is

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR transitions link-icon suppression from a horizontal sliding-door effect to a vertical opacity fade, simplifies re-centering and arc-recalculation logic to remove layout-dependent animation state, updates visual styling for arcs and link icons, fixes punctuation routing in grouped tokens, and updates localization copy.

Changes

Link Icon Fade Transition and Arc Styling

Layer / File(s)	Summary
Visual styling for arcs and link icons src/components/TokenLinkIcon.tsx, src/tailwind.css, src/utils/phrase-arc.ts, src/__tests__/utils/phrase-arc.test.ts	Focused arcs reduce opacity from 100% to 60%, dimmed arcs increase from 80% to 100%, and link-icon buttons shift from muted-foreground to foreground colors. Test constants and assertions updated to reflect the new opacity values.
Link icon fade transition implementation src/components/PhraseStripParts.tsx, src/components/PhraseStripContext.tsx, src/__tests__/components/PhraseStripParts.test.tsx	Link-icon wrapper transitions from maxWidth-based slide to opacity-only fade. Layout space is preserved via visibility: hidden. Documentation updated to clarify fade duration used for focus anchoring during segment flips. Tests updated to verify visibility state instead of maxWidth collapse.
Simplify re-centering and arc recalculation src/components/ContinuousView.tsx, src/components/SegmentView.tsx, src/__tests__/components/ContinuousView.test.tsx, src/__tests__/components/SegmentView.test.tsx	Remove hideInactiveLinkButtons from re-centering and arc-path dependencies. Re-center only when simplifyPhrases changes (single scrollIntoView call instead of rAF loop). Remove slot-animation-tick state from SegmentView since fade preserves layout space. Update arc-path dependency lists accordingly. Tests verify new behavior: hideInactiveLinkButtons does not trigger re-center; simplifyPhrases triggers exactly one scroll call; fade-transition duration is asserted after mount.
Punctuation routing fix for grouped tokens src/utils/token-layout.ts, src/__tests__/utils/token-layout.test.ts	Fix punctuation accumulation in buildRenderUnits when intra-group tokens are encountered: replace push-only logic with targeted reset/repopulate that either preserves or replaces the target gap, preventing cross-invocation accumulation on memoized token groups. New test asserts punctuation after the final group token routes to the inter-group slot, not the prior group's punctuationBetween.
Localization string update contributions/localizedStrings.json	Update cross-segment link button tooltip from "not yet supported" to "not supported".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• sillsdev/interlinearizer-extension#93: Modifies ContinuousView.tsx's scroll/focus update logic and re-centering behavior, overlapping at the same component code paths.

Suggested labels

🟥High

Suggested reviewers

• alex-rawlings-yyc
• jasonleenaylor

Poem

🐰 The link icons now fade, not slide—
Opacity whispers where maxWidth once tied.
Arcs glow less bright when dimmed from the view,
Re-centering flows simpler, with focus renewed.
Punctuation finds its slot, no more astray.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: it lists the three primary fixes (arc alignment, punctuation routing, link-button polish) and mentions the secondary refactoring (test helpers and type reorganization) that appear throughout the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch lt3

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/PhraseStripParts.tsx`:
- Around line 98-105: The link icon slot in PhraseStripParts is only visually
hidden via opacity but still receives pointer events; update the inline style
logic (the style object where display, minHeight, transitionDuration, opacity,
overflowAnchor are set) to also disable hit-testing when suppressed by adding
pointerEvents: suppressLinkIcon ? 'none' : undefined and mark it non-interactive
for assistive tech (e.g., set aria-hidden or ensure interactive child controls
are removed/disabled when suppressLinkIcon is true); keep the existing
transition behavior using skipLinkTransition and LINK_SLOT_TRANSITION_MS so
visual timing is unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 02f12a2b-f234-4310-8ab3-41a84a2ccefc

📥 Commits

Reviewing files that changed from the base of the PR and between e9f4e48 and 9895cb7.

📒 Files selected for processing (14)

• contributions/localizedStrings.json
• src/__tests__/components/ContinuousView.test.tsx
• src/__tests__/components/PhraseStripParts.test.tsx
• src/__tests__/components/SegmentView.test.tsx
• src/__tests__/utils/phrase-arc.test.ts
• src/__tests__/utils/token-layout.test.ts
• src/components/ContinuousView.tsx
• src/components/PhraseStripContext.tsx
• src/components/PhraseStripParts.tsx
• src/components/SegmentView.tsx
• src/components/TokenLinkIcon.tsx
• src/tailwind.css
• src/utils/phrase-arc.ts
• src/utils/token-layout.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Hidden link icons remain interactive while visually suppressed.

opacity: 0 alone hides the icon visually but does not remove hit-testing, so suppressed slot controls can still intercept pointer interaction.

Proposed fix

         <span
           className="tw:transition-opacity tw:ease-in-out"
           style={{
             display: 'inline-flex',
             minHeight: '1rem',
             transitionDuration: skipLinkTransition ? '0ms' : `${LINK_SLOT_TRANSITION_MS}ms`,
             opacity: suppressLinkIcon ? 0 : 1,
+            pointerEvents: suppressLinkIcon ? 'none' : 'auto',
             overflowAnchor: 'none',
           }}
         >

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	className="tw:transition-opacity tw:ease-in-out"
	style={{
	display: 'inline-flex',
	minHeight: '1rem',
	transitionDuration: skipLinkTransition ? '0ms' : `${LINK_SLOT_TRANSITION_MS}ms`,
	maxWidth: suppressLinkIcon ? '0' : '2rem',
	opacity: suppressLinkIcon ? 0 : 1,
	pointerEvents: suppressLinkIcon ? 'none' : undefined,
	overflowAnchor: 'none',
	}}
	className="tw:transition-opacity tw:ease-in-out"
	style={{
	display: 'inline-flex',
	minHeight: '1rem',
	transitionDuration: skipLinkTransition ? '0ms' : `${LINK_SLOT_TRANSITION_MS}ms`,
	opacity: suppressLinkIcon ? 0 : 1,
	pointerEvents: suppressLinkIcon ? 'none' : 'auto',
	overflowAnchor: 'none',
	}}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/PhraseStripParts.tsx` around lines 98 - 105, The link icon
slot in PhraseStripParts is only visually hidden via opacity but still receives
pointer events; update the inline style logic (the style object where display,
minHeight, transitionDuration, opacity, overflowAnchor are set) to also disable
hit-testing when suppressed by adding pointerEvents: suppressLinkIcon ? 'none' :
undefined and mark it non-interactive for assistive tech (e.g., set aria-hidden
or ensure interactive child controls are removed/disabled when suppressLinkIcon
is true); keep the existing transition behavior using skipLinkTransition and
LINK_SLOT_TRANSITION_MS so visual timing is unchanged.

[alex-rawlings-yyc]

Reviewable is unable to connect to GitHub right now (there appears to be a general GitHub API outage at the moment). Regardless, LGTM