Repository navigation
Fix folder proxy icon drag - #3804
Conversation
|
@lederniermagicien is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
To use Codex here, create a Codex account and connect to github. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughOS-level window movability is enabled in AppDelegate and ContentView (isMovable = true) while keeping background dragging disabled. A new DetachedFolderDragIcon NSViewRepresentable hosts a borderless NSPanel with DraggableFolderNSView synchronized to the parent window. Drag-session movable-state bookkeeping and related restoration calls were removed and some debug logging was simplified. ChangesWindow Movability and Detached Folder Drag Icon
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 replaces the in-process window-drag-suppression mechanism (
Confidence Score: 5/5Safe to merge; the child-panel approach cleanly eliminates the drag-suppression state machine and actor isolation is correctly enforced throughout. The new NSPanel child-window correctly intercepts mouse events before the parent window sees them, deinit teardown is guarded by MainActor.assumeIsolated, the installParentWindowObservers guard now re-installs when the observed window changes, and canJoinAllSpaces is absent. The only finding is a dead-code fallback in mouseDragged that is unreachable in normal AppKit event delivery. Sources/DetachedFolderDragIcon.swift — new file carrying the core panel-lifecycle logic; worth a final read-through, but no blocking issues found. Important Files Changed
Sequence DiagramsequenceDiagram
participant SwiftUI as SwiftUI Layout
participant Host as DetachedFolderDragIconHostView
participant Panel as NSPanel (child window)
participant DragView as DraggableFolderNSView
participant Parent as Parent NSWindow
participant NC as NotificationCenter
SwiftUI->>Host: makeNSView / updateNSView
Host->>Host: syncDetachedIcon()
alt "childWindow == nil (first time)"
Host->>DragView: init(directory:)
Host->>Panel: NSPanel(borderless, nonactivating)
Panel->>DragView: "contentView = iconView"
Host->>Parent: addChildWindow(panel, ordered: .above)
Host->>NC: addObserver(didMove/didResize/didMiniaturize/didDeminiaturize)
end
Host->>Panel: orderFront(nil)
Host->>Host: syncDetachedIconFrame()
Host->>Panel: setFrame(screenRect)
NC-->>Host: didMove / didResize
Host->>Host: syncDetachedIconFrame()
Host->>Panel: setFrame(updatedRect)
Note over DragView: User drags icon
DragView->>DragView: mouseDown to pendingDragEvent
DragView->>DragView: mouseDragged (threshold met)
DragView->>DragView: beginDraggingSession(with: mouseDownEvent)
Note over Host: View removed from window
SwiftUI->>Host: viewDidMoveToWindow(nil)
Host->>Host: tearDownDetachedIcon()
Host->>NC: removeObserver (all)
Host->>Parent: removeChildWindow(panel)
Host->>Panel: orderOut(nil)
Reviews (7): Last reviewed commit: "Delay folder icon drag until movement" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/ContentView.swift`:
- Around line 14617-14774: The DetachedFolderDragIcon and
DetachedFolderDragIconHostView (and any small helper such as
DraggableFolderNSView references) should be moved out of ContentView.swift into
their own platform-bridge file (e.g. Sources/DetachedFolderDragIcon.swift):
create the new Swift file, copy the private struct DetachedFolderDragIcon and
the `@MainActor` final class DetachedFolderDragIconHostView (keeping their exact
symbols, access levels, MainActor annotation and methods like
makeDetachedIconWindow, installParentWindowObservers, syncDetachedIconFrame,
tearDownDetachedIcon), add necessary AppKit import(s), remove the duplicated
definitions from ContentView.swift so usages there continue to compile, and
ensure the new file is included in the target build settings; keep
DraggableFolderNSView reference unchanged (or move it too if it’s defined
nearby) so symbol resolution remains correct.
- Around line 2282-2283: DetachedFolderDragIcon currently replaces
DraggableFolderIcon but loses the tooltip and double-click-to-open-Finder
behavior; restore parity by re-applying the same UI modifiers/handlers
DraggableFolderIcon used. Wrap or modify the DetachedFolderDragIcon view
instance (the call in ContentView where DetachedFolderDragIcon(directory:
directory) is used) to add a .help(...) tooltip (e.g., the folder name/path) and
an onTapGesture(count: 2) handler that opens the folder in Finder (use
NSWorkspace.shared.activateFileViewerSelecting or call the existing
open-in-Finder helper if one exists), matching DraggableFolderIcon’s behavior.
Ensure the tooltip text and the double-click handler use the same directory
value as passed into DetachedFolderDragIcon.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: eed196e2-b930-42ee-a045-25256c12a923
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/cmuxApp.swift
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/DetachedFolderDragIcon.swift`:
- Line 242: The debug log in DetachedFolderDragIcon.swift leaks full directory
paths; update the cmuxDebugLog calls (e.g., the call that currently includes
directory in the message and the similar calls around the symbols at the other
two locations) to remove or redact the full path — log only non-sensitive
metadata such as a boolean presence, a safe identifier, or the directory's
lastPathComponent or a constant like "[REDACTED]" instead of the full directory
string; ensure the formatPoint(screenPoint), operation.rawValue, nowMovable and
windowOrigin remain unchanged in the message.
- Around line 258-263: In mouseDown(with event: NSEvent) on
DetachedFolderDragIcon, detect a Control-click
(event.modifierFlags.contains(.control)) at the top of the method and handle it
by invoking the contextual menu instead of continuing into drag logic—e.g., call
rightMouseDown(with: event) or NSMenu.popUpContextMenu(menuForEvent(event) ??
myContextMenu, with: event, for: self) and return; ensure this check occurs
before any drag-start or double-click handling so Control-click opens the
context menu reliably.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: cbd8c656-5945-456f-b7d5-73b984a51741
📒 Files selected for processing (3)
GhosttyTabs.xcodeproj/project.pbxprojSources/ContentView.swiftSources/DetachedFolderDragIcon.swift
|
All good guys (and girls)? @codex review |
@lederniermagicien I have started the AI code review. It will take a few minutes to complete. |
|
To use Codex here, create a Codex account and connect to github. |
1 similar comment
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Actions performedReview triggered.
|
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 `@Sources/DetachedFolderDragIcon.swift`:
- Around line 354-357: The openComputer(_:) implementation and its comment are
inconsistent: NSWorkspace.shared.open(URL(fileURLWithPath: "/", isDirectory:
true)) opens the root volume, not the Finder "Computer" container. Either update
the comment to reflect that it opens the root filesystem, or replace the body of
openComputer(_:) with code that runs an AppleScript to open the Computer view
(for example, execute an AppleScript via NSAppleScript or a shell osascript
invocation that tells Finder to open the computer container / show the computer
window). Modify the function openComputer(_:) accordingly and ensure the comment
matches the chosen behavior.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: bb2daf32-ca91-4157-b731-c96a66c8ec01
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/DetachedFolderDragIcon.swift
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was previously deployed
2 participants
Summary
Testing
CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag fix-swish-folder-drag --launchDemo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by cubic
Fixes folder proxy icon drags so they start a file drag, not a window drag. Keeps the titlebar layout stable and normal window movement, including with macOS and third‑party tiling tools.
NSPanelabove the titlebar so drags are file drags; delays drag start until slight cursor movement; adds double‑click to reveal in Finder, right/control‑click path menu, and a drag tooltip.isMovable = true) while background dragging stays off, including in minimal mode, to preserve UI interactions and tiling/window‑manager support..fullScreenAuxiliary,cmux.folderDragIcon), and replaces the old drag‑suppression view withDetachedFolderDragIcon.Written for commit e4bf4c7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes