improvement: add Preview button to AI apply confirmation dialog#756
Merged
breaking-brake merged 2 commits intomainbreaking-brake/cc-wf-studio:mainfrom May 4, 2026
improvement/ai-apply-dialog-overview-previewbreaking-brake/cc-wf-studio:improvement/ai-apply-dialog-overview-previewCopy head branch name to clipboard
Merged
improvement: add Preview button to AI apply confirmation dialog#756breaking-brake merged 2 commits intomainbreaking-brake/cc-wf-studio:mainfrom improvement/ai-apply-dialog-overview-previewbreaking-brake/cc-wf-studio:improvement/ai-apply-dialog-overview-previewCopy head branch name to clipboard
breaking-brake merged 2 commits intomainbreaking-brake/cc-wf-studio:mainfrom
improvement/ai-apply-dialog-overview-previewbreaking-brake/cc-wf-studio:improvement/ai-apply-dialog-overview-previewCopy head branch name to clipboard
Conversation
- Extract WorkflowOverview from OverviewMode for reuse - Add Preview button to DiffPreviewDialog footer - Show proposed workflow in z-index 10002 overlay - Document new DIALOG_PREVIEW_OVERLAY tier in CLAUDE.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a full‑size WorkflowOverview overlay accessible from DiffPreviewDialog, passes workflow data into the dialog, refactors OverviewMode into a thin wrapper, introduces WorkflowOverview as a reusable two‑pane viewer with splitter/focus behavior, adds i18n keys, and updates dialog z-index documentation to include a new preview overlay layer. ChangesWorkflow Overview in Diff Preview
Sequence DiagramsequenceDiagram
actor User
participant DiffPreviewDialog as DiffPreviewDialog
participant WorkflowOverview as WorkflowOverview Overlay
participant MermaidDiagram as MermaidDiagram
participant InstructionsPanel as InstructionsPanel
User->>DiffPreviewDialog: Click "Preview Overview" button
DiffPreviewDialog->>DiffPreviewDialog: set showOverview = true
DiffPreviewDialog->>WorkflowOverview: render with workflow prop
WorkflowOverview->>MermaidDiagram: initialize diagram
WorkflowOverview->>InstructionsPanel: load instructions pane
User->>MermaidDiagram: click node
MermaidDiagram->>WorkflowOverview: handleNodeClick(nodeId)
WorkflowOverview->>InstructionsPanel: scroll to node
User->>WorkflowOverview: drag splitter / press arrow keys
WorkflowOverview->>WorkflowOverview: update ratio, persist to localStorage
User->>WorkflowOverview: Click "Close Preview"
WorkflowOverview->>DiffPreviewDialog: set showOverview = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 407-414: The fenced z-index table block is untagged and triggers
markdownlint MD040; update the opening triple-backtick for that z-index block
(the fenced block showing "レイヤー z-index 用途" and the subsequent rows including
Base, Nested, Confirm, PreviewOverlay) to include a language tag (preferably
"text") so it becomes ```text and resolves the lint warning.
In `@src/webview/src/components/overview/WorkflowOverview.tsx`:
- Around line 255-257: In WorkflowOverview.tsx update the splitter's ARIA
semantics: change the element currently rendered with role="slider" and
aria-orientation="vertical" to use aria-orientation="horizontal" so
screen-reader semantics match the left/right (ArrowLeft/ArrowRight) keyboard
handling; verify the splitter element in the component that renders the slider
(the DOM node with role="slider") also exposes appropriate
aria-valuemin/aria-valuemax/aria-valuenow if present and that existing key
handlers remain aligned with horizontal orientation.
- Around line 83-95: The component can persist the old context's ratio into a
newly changed splitRatioStorageKey; to fix, add an effect that listens for
changes to splitRatioStorageKey and calls
setRatio(loadStoredRatio(splitRatioStorageKey)) so the component hydrates the
correct ratio before any write, and update the persisting effect (the useEffect
that writes localStorage.setItem using ratio and splitRatioStorageKey) to avoid
writing immediately on a key switch — e.g., track the previous key with a ref
(prevKeyRef) and skip the first persist when splitRatioStorageKey !==
prevKeyRef.current, then update prevKeyRef.current after hydration; reference:
ratio, splitRatioStorageKey, loadStoredRatio, setRatio, the persisting useEffect
and a new useEffect watching splitRatioStorageKey.
- Around line 106-126: The mousemove/mouseup listeners added in
handleSplitterMouseDown (which registers handleMove and handleUp and flips
isResizingRef) are not removed if the component unmounts mid-drag; add a cleanup
to remove those listeners on unmount and reset isResizingRef. Implement this by
returning a cleanup from a useEffect that references the same handler functions
(or stores the active handlers on refs) so
document.removeEventListener('mousemove', handleMove) and
document.removeEventListener('mouseup', handleUp) are called on unmount and set
isResizingRef.current = false; ensure references match the ones added in
handleSplitterMouseDown (handleMove, handleUp, isResizingRef, splitContainerRef,
setRatio).
🪄 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: 6a45db0e-9e0d-409c-9678-27d29454e013
📒 Files selected for processing (11)
CLAUDE.mdsrc/webview/src/App.tsxsrc/webview/src/components/dialogs/DiffPreviewDialog.tsxsrc/webview/src/components/overview/OverviewMode.tsxsrc/webview/src/components/overview/WorkflowOverview.tsxsrc/webview/src/i18n/translation-keys.tssrc/webview/src/i18n/translations/en.tssrc/webview/src/i18n/translations/ja.tssrc/webview/src/i18n/translations/ko.tssrc/webview/src/i18n/translations/zh-CN.tssrc/webview/src/i18n/translations/zh-TW.ts
Comment on lines
+83
to
+95
| const [ratio, setRatio] = useState<number>(() => loadStoredRatio(splitRatioStorageKey)); | ||
| const splitContainerRef = useRef<HTMLDivElement>(null); | ||
| const isResizingRef = useRef(false); | ||
| const instructionsRef = useRef<InstructionsPanelHandle>(null); | ||
| const [activeSanitizedNodeId, setActiveSanitizedNodeId] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| try { | ||
| localStorage.setItem(splitRatioStorageKey, ratio.toString()); | ||
| } catch { | ||
| // ignore quota errors | ||
| } | ||
| }, [ratio, splitRatioStorageKey]); |
There was a problem hiding this comment.
Reload ratio on splitRatioStorageKey changes before persisting
When splitRatioStorageKey changes, Line 91 can write the previous context’s ratio into the new key before this component hydrates from that key. That can break the intended per-context isolation.
Suggested patch
const [ratio, setRatio] = useState<number>(() => loadStoredRatio(splitRatioStorageKey));
+const skipNextPersistRef = useRef(false);
+useEffect(() => {
+ skipNextPersistRef.current = true;
+ setRatio(loadStoredRatio(splitRatioStorageKey));
+}, [splitRatioStorageKey]);
+
useEffect(() => {
+ if (skipNextPersistRef.current) {
+ skipNextPersistRef.current = false;
+ return;
+ }
try {
localStorage.setItem(splitRatioStorageKey, ratio.toString());
} catch {
// ignore quota errors
}
}, [ratio, splitRatioStorageKey]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [ratio, setRatio] = useState<number>(() => loadStoredRatio(splitRatioStorageKey)); | |
| const splitContainerRef = useRef<HTMLDivElement>(null); | |
| const isResizingRef = useRef(false); | |
| const instructionsRef = useRef<InstructionsPanelHandle>(null); | |
| const [activeSanitizedNodeId, setActiveSanitizedNodeId] = useState<string | null>(null); | |
| useEffect(() => { | |
| try { | |
| localStorage.setItem(splitRatioStorageKey, ratio.toString()); | |
| } catch { | |
| // ignore quota errors | |
| } | |
| }, [ratio, splitRatioStorageKey]); | |
| const [ratio, setRatio] = useState<number>(() => loadStoredRatio(splitRatioStorageKey)); | |
| const skipNextPersistRef = useRef(false); | |
| const splitContainerRef = useRef<HTMLDivElement>(null); | |
| const isResizingRef = useRef(false); | |
| const instructionsRef = useRef<InstructionsPanelHandle>(null); | |
| const [activeSanitizedNodeId, setActiveSanitizedNodeId] = useState<string | null>(null); | |
| useEffect(() => { | |
| skipNextPersistRef.current = true; | |
| setRatio(loadStoredRatio(splitRatioStorageKey)); | |
| }, [splitRatioStorageKey]); | |
| useEffect(() => { | |
| if (skipNextPersistRef.current) { | |
| skipNextPersistRef.current = false; | |
| return; | |
| } | |
| try { | |
| localStorage.setItem(splitRatioStorageKey, ratio.toString()); | |
| } catch { | |
| // ignore quota errors | |
| } | |
| }, [ratio, splitRatioStorageKey]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/webview/src/components/overview/WorkflowOverview.tsx` around lines 83 -
95, The component can persist the old context's ratio into a newly changed
splitRatioStorageKey; to fix, add an effect that listens for changes to
splitRatioStorageKey and calls setRatio(loadStoredRatio(splitRatioStorageKey))
so the component hydrates the correct ratio before any write, and update the
persisting effect (the useEffect that writes localStorage.setItem using ratio
and splitRatioStorageKey) to avoid writing immediately on a key switch — e.g.,
track the previous key with a ref (prevKeyRef) and skip the first persist when
splitRatioStorageKey !== prevKeyRef.current, then update prevKeyRef.current
after hydration; reference: ratio, splitRatioStorageKey, loadStoredRatio,
setRatio, the persisting useEffect and a new useEffect watching
splitRatioStorageKey.
Owner
Author
There was a problem hiding this comment.
Skipping for now: in current usage each call site mounts WorkflowOverview with a fixed splitRatioStorageKey (no in-place key swap), so the race cannot trigger. Worth revisiting if a future caller needs to swap keys on the same instance.
There was a problem hiding this comment.
@breaking-brake, that's a fair call — since all current mount sites use a stable key, the stale-write path is unreachable today. Acknowledged, no action needed now.
Would you like me to open a small follow-up GitHub issue to track the defensive guard (key-swap hydration) for if/when a dynamic splitRatioStorageKey caller is introduced?
🧠 Learnings used
Learnt from: breaking-brake
Repo: breaking-brake/cc-wf-studio PR: 695
File: src/webview/src/components/toolbar/CommentaryOptionsDropdown.tsx:8-39
Timestamp: 2026-03-31T09:22:07.702Z
Learning: In this repo’s webview code (src/webview/src/**/*.tsx), it’s acceptable to reference React types such as `React.CSSProperties` without importing `React`. `types/react` provides the global `React` namespace, so `React.CSSProperties` should resolve correctly even with the `react-jsx` JSX transform. Do not flag missing `import React` for this specific type-usage pattern as an error during future code reviews.
Learnt from: breaking-brake
Repo: breaking-brake/cc-wf-studio PR: 699
File: src/webview/src/components/UndoRedoControls.tsx:55-89
Timestamp: 2026-04-02T12:27:24.241Z
Learning: For toolbar-related components under src/webview/src/components/ that intentionally implement a hover-only popover using the shared usePopoverHover hook (e.g., Popover.Root with open={isHovered}), do not raise code review findings about missing keyboard/focus support for that popover implementation while the unified accessibility fix is still pending. This exemption applies specifically when the hover-only pattern is clearly wired through usePopoverHover and the popover’s open state is controlled by hover (isHovered).
- Tag z-index fenced block as text in CLAUDE.md (MD040) - Clean up document drag listeners on unmount - Fix splitter aria-orientation to horizontal Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
|
🎉 This PR is included in version 3.34.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This was referenced May 5, 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
Add a Preview button to the AI edit confirmation dialog (
DiffPreviewDialog) so users can inspect the proposed workflow in Overview format before accepting. As a prerequisite refactor, the Overview view is split into a reusableWorkflowOverviewcomponent so it can be hosted in multiple containers (full-screen mode, dialog overlay, and future call sites).What Changed
Lets users review AI-proposed workflows visually (Mermaid + per-node instructions) without leaving the confirmation flow.
Before
OverviewModewas tightly coupled to the canvas page (Git diff context, edit-mode switching), making it hard to reuse elsewhere.After
WorkflowOverview.OverviewModeis now a thin wrapper around the reusableWorkflowOverview, which acceptssplitRatioStorageKeyso each host keeps independent layout preferences (the preview uses its own key to avoid leaking into the main Overview).Changes
src/webview/src/components/overview/WorkflowOverview.tsx- New presentational component (extracted from OverviewMode); renders header + Mermaid + instructions split pane; acceptssplitRatioStorageKeyandhideHeader.src/webview/src/components/overview/OverviewMode.tsx- Reduced to a thin wrapper delegating toWorkflowOverview; preserves the existing public API.src/webview/src/components/dialogs/DiffPreviewDialog.tsx- Addworkflowprop, footer Preview button, nested Radix Dialog overlay (z-index 10002) hostingWorkflowOverview; visible close button at top-right with ESC / X / background-click behavior; resetsshowOverviewwhen the parent dialog closes.src/webview/src/App.tsx- PasspendingMcpApply.workflowintoDiffPreviewDialog.src/webview/src/i18n/translation-keys.ts- Adddialog.diffPreview.previewOverviewanddialog.diffPreview.closeOverviewkeys.src/webview/src/i18n/translations/{en,ja,ko,zh-CN,zh-TW}.ts- Localize the new keys (label kept short: "Preview" / "プレビュー" / "미리보기" / "预览" / "預覽").CLAUDE.md- Extend dialog z-index hierarchy from 3 tiers to 4 tiers; document newDIALOG_PREVIEW_OVERLAY = 10002tier and update the dialog inventory table.Testing
apply_workflow; Preview button appears; click opens full-size overlay with proposed workflow; ESC / X / background-click each return to the dialog without closing it; Accept/Reject continue to work.npm run check(Biome) passes.npm run build(extension + webview) passes.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Internationalization
Documentation