Fix navigation to code diff and diagram diff when visualizer panel is already active#2159
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe Visualizer now reveals its current webview before review navigation and before opening the popup view for ChangesVisualizer Focus Management
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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.
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (1)
887-888: 💤 Low valueRedundant
reveal()inopenWebViewservice can now be removed
VisualizerWebview.currentPanel?.getWebview()?.reveal()at line 887 is the intended early reveal. However, theopenWebViewservice at line 569 also callsreveal()whencurrentPanelalready exists, so the panel ends up being revealed twice on everyopenViewcall where the panel is already open. Both calls are harmless (idempotent), but the one insideopenWebViewis now redundant given this earlier unconditional call.♻️ Optional cleanup: remove the second reveal from `openWebView`
} else { - VisualizerWebview.currentPanel!.getWebview()?.reveal(); resolve(true); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-extension/src/stateMachine.ts` around lines 887 - 888, Remove the redundant reveal call inside the openWebView flow: since VisualizerWebview.currentPanel?.getWebview()?.reveal() is already invoked unconditionally before calling stateService.send, delete the additional .reveal() that openWebView performs when VisualizerWebview.currentPanel exists (refer to VisualizerWebview.currentPanel, getWebview, reveal and the openWebView service) so the panel is only revealed once on openView calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@workspaces/ballerina/ballerina-extension/src/stateMachine.ts`:
- Around line 887-888: Remove the redundant reveal call inside the openWebView
flow: since VisualizerWebview.currentPanel?.getWebview()?.reveal() is already
invoked unconditionally before calling stateService.send, delete the additional
.reveal() that openWebView performs when VisualizerWebview.currentPanel exists
(refer to VisualizerWebview.currentPanel, getWebview, reveal and the openWebView
service) so the panel is only revealed once on openView calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b197e387-94fa-449f-9bbd-e58948c093ec
📒 Files selected for processing (3)
workspaces/ballerina/ballerina-extension/src/features/ai/state/ApprovalViewManager.tsworkspaces/ballerina/ballerina-extension/src/stateMachine.tsworkspaces/ballerina/ballerina-extension/src/stateMachinePopup.ts
c8c9bdd to
0e019b8
Compare
There was a problem hiding this comment.
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
`@workspaces/ballerina/ballerina-extension/src/features/ai/state/ApprovalViewManager.ts`:
- Line 369: The Visualizer panel is being revealed without transferring keyboard
focus because navigateReviewMode uses
VisualizerWebview.currentPanel?.getWebview()?.reveal(undefined, true). Update
this call so it matches openPopupView and reveals the panel with focus allowed,
ensuring the Visualizer becomes the active target in Review Mode. Keep the
change localized to navigateReviewMode and the VisualizerWebview reveal
invocation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 91360c1b-c803-4f69-9561-75c5adcaf902
📒 Files selected for processing (2)
workspaces/ballerina/ballerina-extension/src/features/ai/state/ApprovalViewManager.tsworkspaces/ballerina/ballerina-extension/src/stateMachinePopup.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- workspaces/ballerina/ballerina-extension/src/stateMachinePopup.ts
0e019b8 to
a6ca554
Compare
Description
It resolves: wso2/product-integrator#1375
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit