Add tooltips to icon-only buttons

[alex-rawlings-yyc]

Every control that showed only an icon had its wording reachable by screen readers alone; each now names its action on hover. The mock Tooltip gained the provider requirement the real one enforces, which caught the view-options dropdown rendering outside every provider.

---

This change is 

Summary by CodeRabbit

• New Features

  • Added localized hover tooltips for phrase editing, token removal, linking, arc splitting, view options, and active-verse controls.
  • Added clearer accessible labels for adding tokens to phrases.
  • Tooltips now appear only when relevant and are hidden when actions are unavailable or panels are open.

• UI Improvements

  • Improved hover visibility and styling across light and dark themes.
  • Updated phrase layouts with better spacing around floating controls.
  • Improved focus and navigation consistency across continuous and segment views.

• Bug Fixes

  • Disabled controls can now provide helpful hover guidance without affecting keyboard accessibility.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 72f71753-0057-4c47-8ad5-661912c4cc89

📝 Walkthrough

Walkthrough

The change adds localized tooltips, enforces TooltipProvider usage, centralizes focus handling in FocusStore, propagates an add-token label through phrase-strip context, updates tests, and increases non-arc phrase top padding for the controls pill.

Changes

Tooltip, focus, and phrase-strip enhancements

Layer / File(s)	Summary
Tooltip provider infrastructure __mocks__/platform-bible-react.tsx, src/components/tooltip-delay.ts, src/__tests__/components/test-helpers.tsx, src/__tests__/components/*	Tooltip triggers forward props and require TooltipProvider. Shared delay and test helpers support provider-wrapped rendering.
Localized tooltip controls src/components/ArcOverlay.tsx, src/components/PhraseBox.tsx, src/components/TokenChip.tsx, src/components/TokenLinkIcon.tsx, src/components/SegmentListView.tsx, src/components/controls/ViewOptionsDropdown.tsx, src/__tests__/components/*	Controls render localized tooltip content when labels resolve. Tests cover unresolved labels, disabled states, accessible labels, and existing interactions.
Focus store coordination src/components/Interlinearizer.tsx, src/components/ContinuousView.tsx, src/components/SegmentListView.tsx, src/__tests__/components/Interlinearizer.test.tsx	Focus state, navigation, selection, scrolling, and transition handling use FocusStore through FocusProvider instead of parent-controlled props and callbacks.
Phrase context and control spacing contributions/localizedStrings.json, src/components/SegmentView.tsx, src/components/PhraseStripContext.tsx, src/hooks/usePhraseStripSetup.ts, src/components/ContinuousView.tsx, src/utils/phrase-arc.ts, src/__tests__/utils/phrase-arc.test.ts	The add-token localization template flows into phrase-strip context. Non-arc phrases reserve 28 px for the full controls pill, with coverage for arc and fallback cases.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Merge Risk: 🟡 Moderate · up to 5811c

At the current head, a same-group focus change can leave the continuous strip invisible, while some tooltips may still show an unresolved localization key and arc rendering may reserve excess space. These are bounded user-visible correctness issues, so the PR is not merge-ready until they are fixed or explicitly accepted.

Sequence Diagram(s)

sequenceDiagram
  participant Interlinearizer
  participant FocusProvider
  participant ContinuousView
  participant SegmentListView
  Interlinearizer->>FocusProvider: provide shared focus state
  FocusProvider->>ContinuousView: provide focus and navigation actions
  FocusProvider->>SegmentListView: provide focus and selection actions
  ContinuousView->>FocusProvider: update focused token
  SegmentListView->>FocusProvider: select focused segment

Loading

Suggested reviewers: imnasnainaec

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding tooltips to icon-only controls.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 24 files.
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.

✨ 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 219-tooltips-icon-only-buttons

---

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.}

[alex-rawlings-yyc]

@alex-rawlings-yyc resolved 1 discussion.
Reviewable status: 0 of 25 files reviewed, all discussions resolved.

[alex-rawlings-yyc]

@alex-rawlings-yyc made 5 comments.
Reviewable status: 12 of 25 files reviewed, 5 unresolved discussions (waiting on imnasnainaec).

---

src/components/ArcOverlay.tsx line 317 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

⛏️ I'm not sure but this might be overexplaining tw functionality.

Agreed — how tw-merge groups variants is Tailwind's to teach, not this file's. Cut to the constraint that actually has to hold, which is what a reader needs to know before touching the classes.

---

src/components/TokenChip.tsx line 391 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

⛏️ I'm not sure but this might be overexplaining tw functionality.

Same trim as the ArcOverlay one.

---

src/components/TokenLinkIcon.tsx line 289 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

⛏️ much of this comment appears to be pr-diff-justifying and lacking persistent value.

Agreed — the "the reason outranks the action" half was arguing for the choice rather than describing the code, which our comment rules call out directly ("the audience is the next reader of the code — never the reviewer of the current diff"). Kept the part that answers a real question for the next reader, namely which disabled states are deliberately silent.

---

src/components/TokenLinkIcon.tsx line 356 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

⛏️ putting a span between a tooltip and a disablable child is standard procedure and doesn't require a comment.

Agreed, dropped. It appeared twice in this file — the unlink trigger above had the same comment — so both are gone.

---

src/components/tooltip-delay.ts line 4 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

⛏️ "does not open on a different clock..." could be reworded away from the double-negative to a positive about what it does.

Reworded to the positive. Naming the two trees was also documenting consumers, which goes stale silently as trees are added or moved, so that went too.

[alex-rawlings-yyc]

@alex-rawlings-yyc made 1 comment.
Reviewable status: 8 of 25 files reviewed, 1 unresolved discussion (waiting on imnasnainaec).

---

src/utils/phrase-arc.ts line 10 at r3 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

⛏️ Unless this "HALF" is half of something that still exist, Devin suggests something functionally descriptive like CONTROLS_ARC_OVERHANG_PX or CONTROLS_ARC_RISE_PX.

Done — renamed to CONTROLS_ARC_OVERHANG_PX, and swept the same stale "half" framing out of the computeStripTopPadding and computeStripRowGap docs.

It wasn't just vague — 12 hasn't been half of anything since CONTROLS_HEIGHT_PX = 28 landed as its own constant (28/2 is 14).

Left the value at 12; changing it to CONTROLS_HEIGHT_PX / 2 is a row-spacing change, not a naming fix.

[alex-rawlings-yyc]

Both reproduce, and there was a third. Running your check across the suite fails 3 suites / 29 tests with exactly two distinct messages — %interlinearizer_boundaryControl_split% and %interlinearizer_analysisCatalog_noGloss%.

PhraseStripParts.tsx:233 (SplitMarker) and CatalogRowView.tsx:89 are fixed. The third is SegmentView.tsx:200 — BaselineSplitGap renders <TooltipContent>{splitLabel}</TooltipContent> raw off the same %interlinearizer_boundaryControl_split% key, which is why it hid under that message in the dedupe. Both split sites now take the resolvedOrEmpty + tooltipContentOrUndefined path their siblings use; aria-label keeps the raw value as before.

You were right that CatalogRowView needs a different fix. glossLabel is visible cell text, so resolvedOrEmpty alone would blank the gloss column — it now falls back to an em dash, which reads as "no gloss" in any language and is replaced by the real placeholder once the lookup lands. One knock-on: an existing test asserted the bare key as the visible cell text, i.e. it pinned the bug. I changed it to expect the em dash and added a second test for the resolved placeholder.

BaselineSplitGap's new guard also got the two tests it earned rather than a coverage ignore, since the unresolved-key branch is the behavior being fixed.

@alex-rawlings-yyc made 4 comments.
Reviewable status: 7 of 32 files reviewed, 3 unresolved discussions (waiting on imnasnainaec).

---

src/__tests__/components/ArcOverlay.test.tsx line 45 at r4 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

Idea you can take or leave:

function renderOverlay(overrides: Partial<Parameters<typeof ArcOverlay>[0]> = {}) {
  return render(withTooltipProvider(<ArcOverlay {...requiredProps()} {...overrides} />));
}

Done — the invariant is in the Tooltip mock, essentially as sketched.

One correction worth recording, because it changes the argument in your favor: I expected the invariant to break these 14 tests and it doesn't. All 7 suites pass with it in place. The reason is the point — these components resolve the key to '' before it reaches TooltipContent, so the invariant never sees them. It checks the same property from the opposite side, non-conflictingly. So it's additive rather than a replacement, and I've kept all 14 along with the positive tests above them.

I confirmed it's load-bearing rather than decorative: reverting one of the fixes below makes it throw, restoring it makes the suite green again.

[imnasnainaec]

@imnasnainaec reviewed 5 files and all commit messages, made 2 comments, and resolved 3 discussions.
Reviewable status: 12 of 32 files reviewed, 2 unresolved discussions (waiting on alex-rawlings-yyc).

---

__mocks__/platform-bible-react.tsx line 1033 at r5 (raw file):

  const text = tooltipContentText(tooltipText);
  if (UNRESOLVED_LOCALIZE_KEY.test(text)) {
    throw new Error(`Tooltip content carries an unresolved localize key: ${text}`);

Now that this throws, the per-component "shows no … unresolved key" tests can't fail through their own assertions. (A raw-label regression throws here during render, before any not.toHaveAttribute('title') runs, and a correct implementation never sets a title either way.) Let's delete them all.

---

src/__tests__/components/PhraseStripParts.test.tsx at r5 (raw file):
⛏️ Worth a test that SplitMarker's hover text reaches the trigger, mirroring names the split action on hover over the gap.