fix(desktop): refresh v2 terminal link tooltip editor + nudge plain clicks#3552
Merged
saddlepaddle merged 3 commits intomainfrom Apr 18, 2026
Merged
fix(desktop): refresh v2 terminal link tooltip editor + nudge plain clicks#3552saddlepaddle merged 3 commits intomainfrom
saddlepaddle merged 3 commits intomainfrom
Conversation
…lain clicks
- LinkHoverTooltip fetched the default editor in a mount-only useEffect, so
changing the default editor in settings left the modifier-shift label
("Open in Cursor", etc.) stale until the terminal pane unmounted. Refetch on
every hover-start instead.
- Plain (no-modifier) clicks on a detected file path in the v2 terminal were
silent, which made the modifier-key affordance undiscoverable. On a plain
file-link click, show a transient tooltip at the click position
("Hold ⌘ to open · ⌘⇧ for external", or Ctrl/Ctrl+Shift off-mac). Capped at
two shows per renderer session via a module-level counter, and suppressed
while the modifier-hover tooltip is already visible. Uses framer-motion's
AnimatePresence for fade in/out.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a transient click-hint system: a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TerminalPane
participant useLinkClickHint
participant LinkHoverTooltip
User->>TerminalPane: click file link (no Ctrl/Meta)
TerminalPane->>useLinkClickHint: showHint(clientX, clientY)
useLinkClickHint-->>TerminalPane: hint (updated)
TerminalPane->>LinkHoverTooltip: pass hint prop
LinkHoverTooltip->>LinkHoverTooltip: render animated modifier hint
Note right of useLinkClickHint: hint auto-clears after 2000ms
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR fixes two UX gaps in the v2 workspace terminal's link-click handling: it corrects a stale editor name in the modifier-hover tooltip (previously frozen at mount time, now refreshed on every hover-start via Key changes:
Confidence Score: 4/5Safe to merge — fixes a real stale-state bug and adds a well-scoped discoverability feature with no regressions to existing click/URL behavior. Logic is correct throughout: the per-hover getDefaultEditor fetch properly cancels in-flight requests, the AnimatePresence pattern is used correctly, the stable useCallback doesn't cause unintended effect re-runs, and the module-level counter is an intentional design choice matching the stated 'per renderer session' goal. Only P2 style suggestions remain. No files require special attention. The two P2 suggestions (deprecated navigator.platform and a clarifying comment on the module-level counter) are non-blocking.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkClickHint/useLinkClickHint.ts | New hook managing a per-session hint nudge with a module-level counter (intentional design), stable useCallback, and proper timeout cleanup on unmount. Logic is sound; the module-level counter is a deliberate trade-off documented in the PR. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkClickHint/index.ts | Barrel re-export for the new hook; no issues. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx | Refactored to fetch the default editor on every hover-start (fixes stale label bug), added AnimatePresence-animated hint overlay for plain-click nudge. The navigator.platform usage is deprecated but functional in Electron; effect dependency on hovering boolean is correct. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx | Integrates useLinkClickHint, calls showHint on plain file-link clicks, adds it to the useEffect dependency array (stable callback, no extra runs), and threads hint through to LinkHoverTooltip. |
Sequence Diagram
sequenceDiagram
participant User
participant TerminalPane
participant useLinkClickHint
participant LinkHoverTooltip
participant electronTrpcClient
Note over User,electronTrpcClient: Plain file-link click (no modifier)
User->>TerminalPane: click file link (no ⌘/Ctrl)
TerminalPane->>useLinkClickHint: showHint(clientX, clientY)
useLinkClickHint-->>useLinkClickHint: hintsRemaining > 0? decrement, setHint, setTimeout 2s
useLinkClickHint-->>LinkHoverTooltip: hint state updated
LinkHoverTooltip-->>User: fade-in "Hold ⌘ to open · ⌘⇧ for external"
Note over useLinkClickHint: after 2000ms
useLinkClickHint-->>LinkHoverTooltip: setHint(null)
LinkHoverTooltip-->>User: fade-out hint
Note over User,electronTrpcClient: Modifier-key hover (⌘ held)
User->>TerminalPane: hover file link with ⌘
TerminalPane->>LinkHoverTooltip: hoveredLink (modifier=true)
LinkHoverTooltip->>electronTrpcClient: getDefaultEditor.query()
electronTrpcClient-->>LinkHoverTooltip: ExternalApp (e.g. Cursor)
LinkHoverTooltip-->>User: show "Open in Cursor" tooltip (suppresses hint if visible)
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx
Line: 15-17
Comment:
**`navigator.platform` is deprecated**
`navigator.platform` has been deprecated in modern browser/Electron versions. The recommended alternative is `navigator.userAgentData?.platform` (with a fallback for non-supporting environments) or parsing `navigator.userAgent`. While this still works fine in current Electron, it may produce warnings in future versions.
```suggestion
const isMac =
typeof navigator !== "undefined" &&
(navigator.userAgentData
? navigator.userAgentData.platform.toLowerCase().includes("mac")
: navigator.userAgent.toLowerCase().includes("mac"));
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkClickHint/useLinkClickHint.ts
Line: 11
Comment:
**Module-level counter survives HMR reloads in dev**
`hintsRemaining` is initialized at module evaluation time and never resets as long as the module stays in memory. In development with hot module replacement, a file save that reloads only the component (not the module) won't reset the counter, so developers testing the nudge will see it fire fewer times than expected after the first two clicks. A comment noting this is intentional (or using `sessionStorage` to survive actual page reloads while still being resettable on dev reload) would help future contributors understand the choice.
This is non-critical for production users since HMR isn't active there.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(desktop): refresh v2 terminal link t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+15
to
+17
| const isMac = | ||
| typeof navigator !== "undefined" && | ||
| navigator.platform.toLowerCase().includes("mac"); |
There was a problem hiding this comment.
navigator.platform is deprecated
navigator.platform has been deprecated in modern browser/Electron versions. The recommended alternative is navigator.userAgentData?.platform (with a fallback for non-supporting environments) or parsing navigator.userAgent. While this still works fine in current Electron, it may produce warnings in future versions.
Suggested change
| const isMac = | |
| typeof navigator !== "undefined" && | |
| navigator.platform.toLowerCase().includes("mac"); | |
| const isMac = | |
| typeof navigator !== "undefined" && | |
| (navigator.userAgentData | |
| ? navigator.userAgentData.platform.toLowerCase().includes("mac") | |
| : navigator.userAgent.toLowerCase().includes("mac")); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx
Line: 15-17
Comment:
**`navigator.platform` is deprecated**
`navigator.platform` has been deprecated in modern browser/Electron versions. The recommended alternative is `navigator.userAgentData?.platform` (with a fallback for non-supporting environments) or parsing `navigator.userAgent`. While this still works fine in current Electron, it may produce warnings in future versions.
```suggestion
const isMac =
typeof navigator !== "undefined" &&
(navigator.userAgentData
? navigator.userAgentData.platform.toLowerCase().includes("mac")
: navigator.userAgent.toLowerCase().includes("mac"));
```
How can I resolve this? If you propose a fix, please make it concise.| const HINT_DURATION_MS = 2000; | ||
| const MAX_HINTS_PER_SESSION = 2; | ||
|
|
||
| let hintsRemaining = MAX_HINTS_PER_SESSION; |
There was a problem hiding this comment.
hintsRemaining is initialized at module evaluation time and never resets as long as the module stays in memory. In development with hot module replacement, a file save that reloads only the component (not the module) won't reset the counter, so developers testing the nudge will see it fire fewer times than expected after the first two clicks. A comment noting this is intentional (or using sessionStorage to survive actual page reloads while still being resettable on dev reload) would help future contributors understand the choice.
This is non-critical for production users since HMR isn't active there.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkClickHint/useLinkClickHint.ts
Line: 11
Comment:
**Module-level counter survives HMR reloads in dev**
`hintsRemaining` is initialized at module evaluation time and never resets as long as the module stays in memory. In development with hot module replacement, a file save that reloads only the component (not the module) won't reset the counter, so developers testing the nudge will see it fire fewer times than expected after the first two clicks. A comment noting this is intentional (or using `sessionStorage` to survive actual page reloads while still being resettable on dev reload) would help future contributors understand the choice.
This is non-critical for production users since HMR isn't active there.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx (1)
50-71:⚠️ Potential issue | 🟡 MinorRefetch on the hovered link identity, not just the boolean hover state.
Line 71 only reruns the default-editor query when
hoveringflips betweenfalseandtrue. IfhoveredLinkis replaced while it remains non-null, the tooltip can keep the previous editor label instead of refetching for that hover-start. Track the hovered link identity while preserving the guard against modifier-key-only updates.Proposed fix
- const hovering = hoveredLink !== null; + const hoveredInfo = hoveredLink?.info ?? null; useEffect(() => { - if (!hovering) return; + if (!hoveredInfo) return; let cancelled = false; electronTrpcClient.settings.getDefaultEditor .query() @@ return () => { cancelled = true; }; - }, [hovering]); + }, [hoveredInfo]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx around lines 50 - 71, The effect currently only depends on hovering, so it won't refetch when hoveredLink changes identity while remaining non-null; update the useEffect dependency array to include a stable identity from hoveredLink (e.g., hoveredLink?.href or hoveredLink?.id) in addition to hovering, and keep the early guard (if (!hovering) return) and cancellation logic; in other words, change useEffect([...]) to useEffect([hovering, hoveredLink?.href]) (or another unique property) so electronTrpcClient.settings.getDefaultEditor.query() is re-run for each distinct hovered link while still ignoring modifier-key-only updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx:
- Around line 50-71: The effect currently only depends on hovering, so it won't
refetch when hoveredLink changes identity while remaining non-null; update the
useEffect dependency array to include a stable identity from hoveredLink (e.g.,
hoveredLink?.href or hoveredLink?.id) in addition to hovering, and keep the
early guard (if (!hovering) return) and cancellation logic; in other words,
change useEffect([...]) to useEffect([hovering, hoveredLink?.href]) (or another
unique property) so electronTrpcClient.settings.getDefaultEditor.query() is
re-run for each distinct hovered link while still ignoring modifier-key-only
updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a66a3868-ecdf-4df9-b866-3dc27d21a6fd
📒 Files selected for processing (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkClickHint/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkClickHint/useLinkClickHint.ts
- "Open in editor" → "Open in pane" for the ⌘-click file case (native in-app file pane is what actually happens). - Shift variant always says "Open in external editor" instead of interpolating the configured editor name. openFileInEditor uses the global settings defaultEditor (non-editor apps like Finder can't be set there), so the interpolated name could disagree with a user's per-project preference — the generic label never lies. - Drops the getDefaultEditor fetch, defaultEditor state, and getAppOption/ getAppLabel plumbing that went with it.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx:27">
P2: Keep the shift-hover label tied to the current default editor instead of hard-coding a generic string.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| : "Open externally"; | ||
| } | ||
| return info.isDirectory ? "Reveal in sidebar" : "Open in editor"; | ||
| if (shift) return "Open in external editor"; |
Contributor
There was a problem hiding this comment.
P2: Keep the shift-hover label tied to the current default editor instead of hard-coding a generic string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx, line 27:
<comment>Keep the shift-hover label tied to the current default editor instead of hard-coding a generic string.</comment>
<file context>
@@ -24,52 +20,15 @@ interface LinkHoverTooltipProps {
- : "Open externally";
- }
- return info.isDirectory ? "Reveal in sidebar" : "Open in editor";
+ if (shift) return "Open in external editor";
+ return info.isDirectory ? "Reveal in sidebar" : "Open in pane";
}
</file context>
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx:25">
P2: Restore the non-shift URL tooltip label to match the browser action.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return defaultEditor | ||
| ? `Open in ${getAppLabel(defaultEditor)}` | ||
| : "Open externally"; | ||
| return shift ? "Open in external browser" : "Open in pane"; |
Contributor
There was a problem hiding this comment.
P2: Restore the non-shift URL tooltip label to match the browser action.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx, line 25:
<comment>Restore the non-shift URL tooltip label to match the browser action.</comment>
<file context>
@@ -22,7 +22,7 @@ interface LinkHoverTooltipProps {
function getLabel(info: LinkHoverInfo, shift: boolean): string {
if (info.kind === "url") {
- return shift ? "Open in external browser" : "Open in browser";
+ return shift ? "Open in external browser" : "Open in pane";
}
if (shift) return "Open in external editor";
</file context>
Suggested change
| return shift ? "Open in external browser" : "Open in pane"; | |
| return shift ? "Open in external browser" : "Open in browser"; |
This was referenced Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LinkHoverTooltipfetched the default editor in a mount-onlyuseEffect, so the modifier-shift label (Open in Cursor, etc.) stayed frozen at whatever the setting was when the terminal pane first mounted. Now refetches on every hover-start — the tooltip uses the imperativeelectronTrpcClient(v2 workspaces hijack theelectronTrpcReact-hooks context), so there's no react-query cache to invalidate; fresh fetch-on-hover is the right primitive.Hold ⌘ to open · ⌘⇧ for external(orCtrl/Ctrl+Shiftoff-mac) at the click position.AnimatePresencefor fade in/out.Test plan
Open in editor/Reveal in sidebar/Open in <editor name>.Summary by cubic
Fixes v2 terminal link tooltip labels and adds a small nudge for plain file‑link clicks. ⌘/Ctrl hover now shows clear, generic text (e.g., “Open in pane” for files and URLs, “Open in external editor” with Shift), and plain clicks briefly teach the modifier shortcuts.
Bug Fixes
New Features
framer-motion, capped at two per session, suppressed when the modifier-hover tooltip is visible; URL behavior unchanged.Written for commit 1196764. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes / UX