fix(desktop): ReviewパネルのURLをSupersetブラウザペインで開く#35
Merged
Conversation
ReviewタブのCommentsでURLをクリックするとアプリ全体がそのURLを開いてしまう バグを修正。WorkspaceIdContextから直接workspaceIdを取得し、openInBrowserPane でSupersetのブラウザペインにURLを開くよう変更。 - comment.url、pr.url、ReactMarkdown内のリンク全てに対応 - useWorkspaceId()をコンテキストから直接取得（props経由のnullish値問題を回避）
📝 WalkthroughWalkthroughModified ReviewPanel component to route anchor clicks through Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx (1)
828-828: TheworkspaceIdprop may now be redundant.
ReviewPanelnow callsuseWorkspaceId()directly from context (line 64 in ReviewPanel.tsx) and usescontextWorkspaceId ?? workspaceIdfor resolution. SinceChangesViewis rendered within the sameWorkspaceIdContextprovider,contextWorkspaceIdwill always have the correct value, making the prop fallback unnecessary.Consider removing the prop if the context is always available here. However, keeping it provides a defensive fallback if the component is ever used outside the context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx` at line 828, Remove the now-redundant workspaceId prop: update the ChangesView usage to stop passing workspaceId and remove the workspaceId prop from the ChangesView component signature and its PropType/TypeScript interface; ensure any internal references to props.workspaceId are switched to using useWorkspaceId() (or rely on ReviewPanel's resolution which already uses useWorkspaceId()) and delete any fallback logic that used the passed-in workspaceId; verify related tests/types that referenced the prop are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx`:
- Around line 67-76: The click handler handleOpenUrl currently always calls
e.preventDefault()/e.stopPropagation(), which blocks native navigation when
resolvedWorkspaceId is falsy; change it so you first check resolvedWorkspaceId
and only call e.preventDefault()/e.stopPropagation() and
openInBrowserPane(resolvedWorkspaceId, url) when resolvedWorkspaceId is truthy,
otherwise allow the event to proceed (or explicitly open the URL with
window.open(url, '_blank') if you want a programmatic fallback); update the
useCallback body for handleOpenUrl accordingly so links remain functional when
resolvedWorkspaceId is unavailable.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx`:
- Line 828: Remove the now-redundant workspaceId prop: update the ChangesView
usage to stop passing workspaceId and remove the workspaceId prop from the
ChangesView component signature and its PropType/TypeScript interface; ensure
any internal references to props.workspaceId are switched to using
useWorkspaceId() (or rely on ReviewPanel's resolution which already uses
useWorkspaceId()) and delete any fallback logic that used the passed-in
workspaceId; verify related tests/types that referenced the prop are updated
accordingly.
🪄 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: d174a7a6-8890-4d99-a2ae-e64924910620
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx
- ReviewPanelのworkspaceId propを削除し、useWorkspaceId()で直接取得するよう整理 - shell-wrappers.test.tsのfishテストスナップショットを_superset_prompt_marker追加に合わせて更新
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx (1)
64-73: Theif (resolvedWorkspaceId)guard is unnecessary.Based on the
useWorkspaceId()implementation inWorkspaceIdContext.tsx, the hook throws an error when the context value is null rather than returning a falsy value. This meansresolvedWorkspaceIdis guaranteed to be a valid string whenever this component renders successfully.The guard at line 68 is dead code since the condition will always be true. You can simplify by removing it:
♻️ Suggested simplification
const handleOpenUrl = useCallback( (url: string, e: React.MouseEvent) => { e.preventDefault(); e.stopPropagation(); - if (resolvedWorkspaceId) { - openInBrowserPane(resolvedWorkspaceId, url); - } + openInBrowserPane(resolvedWorkspaceId, url); }, [resolvedWorkspaceId, openInBrowserPane], );Note: The previous review comment about links becoming non-functional when
resolvedWorkspaceIdis falsy does not apply here—that scenario cannot occur given the hook's behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx` around lines 64 - 73, The if-check guarding resolvedWorkspaceId inside the handleOpenUrl callback is dead code because useWorkspaceId() (from WorkspaceIdContext) throws if the context is missing and therefore resolvedWorkspaceId is always a valid string; remove the unnecessary conditional in the useCallback (function handleOpenUrl) and call openInBrowserPane(resolvedWorkspaceId, url) directly after preventing default/stopPropagation, keeping the same dependency array ([resolvedWorkspaceId, openInBrowserPane]) and behavior otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx`:
- Around line 64-73: The if-check guarding resolvedWorkspaceId inside the
handleOpenUrl callback is dead code because useWorkspaceId() (from
WorkspaceIdContext) throws if the context is missing and therefore
resolvedWorkspaceId is always a valid string; remove the unnecessary conditional
in the useCallback (function handleOpenUrl) and call
openInBrowserPane(resolvedWorkspaceId, url) directly after preventing
default/stopPropagation, keeping the same dependency array
([resolvedWorkspaceId, openInBrowserPane]) and behavior otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ddcfaa88-ffc2-42d0-8143-469d2ce2fb61
📒 Files selected for processing (2)
apps/desktop/src/main/lib/agent-setup/shell-wrappers.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx
This was referenced Apr 1, 2026
5 tasks
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
comment.url（コメントのGitHubリンク）、pr.url（PRタイトルリンク）、ReactMarkdown内のリンク全てを対象に修正useWorkspaceId()をコンテキストから直接取得し、addBrowserTabで新規ブラウザタブにURLを開くよう変更<a href>タグをElectronのナビゲーションが確実に発生しない<button>に変更Test plan