fix(vscode): open source files in different tab group from preview#2826
Conversation
When clicking a node in the preview panel, source files now open in a tab group separate from the preview and preserve focus on the diagram. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 1f8a5ea The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDerives editor placement from the diagram preview panel, adds utilities to open editors beside the preview, updates panel state and messenger/command flows to use the new utilities, and adjusts logging, RPC signatures, and a Projects screen data hook. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as DiagramPanel
participant S as Utils (findViewColumnForEditor / showEditorNextToPreview)
participant V as VSCode.Window/Editor
rect rgba(200,200,255,0.5)
U->>P: Trigger "open source" (locate / view-change)
end
rect rgba(200,255,200,0.5)
P->>S: provide panelViewColumn + location + options
S-->>P: computes target non-preview ViewColumn
end
rect rgba(255,200,200,0.5)
S->>V: vscode.window.showTextDocument(uri, { viewColumn: target, preserveFocus, selection? })
V-->>U: Source file opened (selection/reveal applied per options)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vscode/src/panel/useDiagramPanel.ts`:
- Line 261: panelViewColumn currently reads from a non-reactive state.panel so
the computed caches null and won't update when the panel is created or moved;
change state to expose a reactive ref for the panel or its viewColumn and make
panelViewColumn compute from that ref (e.g., create a ref like panelRef =
ref<...>(null) or panelViewColumnRef = ref<number | null>(null) and update it
when you create the panel and on the panel's view-column change event), then
replace computed(() => state.panel?.viewColumn ?? null) with computed(() =>
panelRef.value?.viewColumn ?? panelViewColumnRef.value ?? null) (and ensure the
panel creation code sets panelRef.value and listens to
panel.onDidChangeViewColumn to update panelViewColumnRef.value) so locate.ts /
activateMessenger.ts get fresh values via toValue(preview.panelViewColumn).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7271ce91-021e-4063-90e1-923cf28389f8
📒 Files selected for processing (5)
.changeset/fix-preview-source-navigation.mdpackages/vscode/src/commands/locate.tspackages/vscode/src/panel/activateMessenger.tspackages/vscode/src/panel/useDiagramPanel.tspackages/vscode/src/utils.ts
|
Good one, got annoyed by this quite a few times |
7229e52 to
9c5e3e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/vscode-preview/src/screens/Projects.tsx (1)
32-32: Use theViewId()factory function instead ofas ViewIdcast.Replace
'index' as ViewIdwithViewId('index')to avoid direct type assertion and maintain type safety. TheViewId()factory is available from@likec4/coreand is the proper typed helper for creating ViewId values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vscode-preview/src/screens/Projects.tsx` at line 32, Replace the unsafe type assertion in the call to ExtensionApi.navigateTo by using the ViewId factory: change the argument currently passed as 'index' as ViewId to ViewId('index') so the call becomes ExtensionApi.navigateTo(ViewId('index'), projectId); update the import to ensure ViewId is imported from `@likec4/core` if not already present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vscode-preview/src/screens/Projects.tsx`:
- Line 11: The Projects component currently relies on Suspense being removed and
therefore renders nothing when useQuery(queries.projectsOverview) returns both
data and error as undefined; restore an explicit loading state in the Projects
component by checking the query result (the variables data and error returned
from useQuery(queries.projectsOverview)) and render a loading placeholder
(spinner/text) while data === undefined && error === undefined; update the
render branch around the current conditional at the top of Projects.tsx (where
data, error, refetch are deconstructed) to return the loading UI, then continue
to handle error and data states as before so the screen is not blank during
initial load.
In `@packages/vscode/src/utils.ts`:
- Around line 79-82: The JSDoc for applySelection is incorrect: it says "Whether
to show the editor" but applySelection actually controls whether the selection
option is passed to showTextDocument (the editor is always shown). Update the
JSDoc for the applySelection property to clearly state that it toggles applying
the provided selection to the editor (i.e., whether to pass the selection to
showTextDocument), keep the `@default` true if intended, and ensure references to
selection and showTextDocument are mentioned so future readers understand the
semantics.
---
Nitpick comments:
In `@packages/vscode-preview/src/screens/Projects.tsx`:
- Line 32: Replace the unsafe type assertion in the call to
ExtensionApi.navigateTo by using the ViewId factory: change the argument
currently passed as 'index' as ViewId to ViewId('index') so the call becomes
ExtensionApi.navigateTo(ViewId('index'), projectId); update the import to ensure
ViewId is imported from `@likec4/core` if not already present.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f13f8407-2905-4344-b76b-86a401a3d26d
📒 Files selected for processing (10)
.changeset/fix-preview-source-navigation.mdpackages/language-server/src/model-change/ModelChanges.tspackages/vscode-preview/src/screens/Projects.tsxpackages/vscode/src/commands/locate.tspackages/vscode/src/panel/activateMessenger.tspackages/vscode/src/panel/useDiagramPanel.tspackages/vscode/src/useExtensionLogger.tspackages/vscode/src/useRpc.tspackages/vscode/src/utils.tspackages/vscode/tsdown.config.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/fix-preview-source-navigation.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/vscode/src/commands/locate.ts
- packages/vscode/src/panel/useDiagramPanel.ts
- packages/vscode/src/panel/activateMessenger.ts
|
|
||
| export function ProjectsScreen() { | ||
| const { data, error, refetch } = useSuspenseQuery(queries.projectsOverview) | ||
| const { data, error, refetch } = useQuery(queries.projectsOverview) |
There was a problem hiding this comment.
Restore an explicit loading state after dropping Suspense.
At Line 11 and Lines 27-35, the component now renders nothing while the query is pending (data and error are both undefined). This creates a blank screen during initial load.
Suggested patch
- const { data, error, refetch } = useQuery(queries.projectsOverview)
+ const { data, error, refetch, isPending } = useQuery(queries.projectsOverview)
@@
{error && <ErrorMessage error={error} onReset={refetch} />}
+ {isPending && <div>Loading projects…</div>}
{data &&
(
<LikeC4ProjectsOverviewAlso applies to: 27-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vscode-preview/src/screens/Projects.tsx` at line 11, The Projects
component currently relies on Suspense being removed and therefore renders
nothing when useQuery(queries.projectsOverview) returns both data and error as
undefined; restore an explicit loading state in the Projects component by
checking the query result (the variables data and error returned from
useQuery(queries.projectsOverview)) and render a loading placeholder
(spinner/text) while data === undefined && error === undefined; update the
render branch around the current conditional at the top of Projects.tsx (where
data, error, refetch are deconstructed) to return the loading UI, then continue
to handle error and data states as before so the screen is not blank during
initial load.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Denis Davydkov <denis@davydkov.com>
Summary
preserveFocus: true)findSourceViewColumn()utility that prefers an existing visible editor in another group, or picks the opposite column if none existsTest plan
🤖 Generated with Claude Code