Repository navigation
Double-click custom titlebar to zoom or minimize - #2130
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
|
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.
|
📝 WalkthroughWalkthroughThis PR implements macOS standard double-click title bar functionality to toggle window zoom/minimize states. It adds event monitoring to the title bar and sidebar areas, resolves system preferences to determine the appropriate action, and integrates the behavior across multiple view components. Changes
Sequence DiagramsequenceDiagram
actor User
participant Monitor as TitlebarDoubleClickMonitorView
participant Handler as WindowDragHandleView
participant Prefs as System Preferences
participant Window as NSWindow
User->>Monitor: Double-click titlebar
Monitor->>Monitor: Detect left mouse down in bounds
Monitor->>Handler: performStandardTitlebarDoubleClick(window)
Handler->>Prefs: Read AppleActionOnDoubleClick
Handler->>Prefs: Read AppleMiniaturizeOnDoubleClick
Handler->>Handler: resolvedStandardTitlebarDoubleClickAction()
Handler-->>Monitor: Returns StandardTitlebarDoubleClickAction
alt Action is .zoom
Monitor->>Window: performZoom(nil)
else Action is .miniaturize
Monitor->>Window: miniaturize(nil)
else Action is .none
Monitor->>Monitor: No action taken
end
Monitor->>Monitor: Suppress further event handling if action resolved
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. Comment |
@austinywang 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. |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/ContentView.swift`:
- Line 2492: The TitlebarDoubleClickMonitorView is being added on top of regions
that already include WindowDragHandleView causing the double-click action to
fire twice; remove the duplicate dispatcher by ensuring each hit region uses
only one path—either remove the overlaying
.background(TitlebarDoubleClickMonitorView()) where WindowDragHandleView() is
present, or modify TitlebarDoubleClickMonitorView to detect and no-op when a
WindowDragHandleView is active (or add a boolean parameter to
TitlebarDoubleClickMonitorView to disable forwarding in those areas); update
calls to TitlebarDoubleClickMonitorView() and WindowDragHandleView() so only one
dispatcher is attached per titlebar region.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7f6d632-fb97-490e-a18d-ea8bb97c7b35
📒 Files selected for processing (4)
Sources/ContentView.swiftSources/WindowDragHandleView.swiftSources/WorkspaceContentView.swiftcmuxTests/GhosttyConfigTests.swift
| .frame(height: titlebarPadding) | ||
| .frame(maxWidth: .infinity) | ||
| .contentShape(Rectangle()) | ||
| .background(TitlebarDoubleClickMonitorView()) |
There was a problem hiding this comment.
Prevent duplicate titlebar double-click dispatch
On Line 2492 and Line 8614, TitlebarDoubleClickMonitorView() is layered onto regions that already use WindowDragHandleView(). Both paths invoke the standard titlebar double-click action, which can fire twice per gesture (notably causing zoom to toggle and immediately revert).
Suggested fix
- .background(TitlebarDoubleClickMonitorView())- .background(TitlebarDoubleClickMonitorView())If you still need monitor-based forwarding, keep a single dispatcher path (either monitor or drag-handle) for each hit region.
Also applies to: 8614-8614
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` at line 2492, The TitlebarDoubleClickMonitorView
is being added on top of regions that already include WindowDragHandleView
causing the double-click action to fire twice; remove the duplicate dispatcher
by ensuring each hit region uses only one path—either remove the overlaying
.background(TitlebarDoubleClickMonitorView()) where WindowDragHandleView() is
present, or modify TitlebarDoubleClickMonitorView to detect and no-op when a
WindowDragHandleView is active (or add a boolean parameter to
TitlebarDoubleClickMonitorView to disable forwarding in those areas); update
calls to TitlebarDoubleClickMonitorView() and WindowDragHandleView() so only one
dispatcher is attached per titlebar region.
Greptile SummaryThis PR adds standard macOS double-click-titlebar behaviour (zoom / minimize / no-op, driven by
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant NSApp
participant Monitor as TitlebarDoubleClickMonitorView<br/>(local event monitor)
participant DraggableView as DraggableView.mouseDown
participant Action as performStandardTitlebarDoubleClick
User->>NSApp: leftMouseDown (clickCount=2)
NSApp->>Monitor: local monitor fires (before dispatch)
alt click NOT in monitor view bounds
Monitor-->>NSApp: return event (pass through)
NSApp->>DraggableView: dispatch to hit-test winner
DraggableView->>Action: performStandardTitlebarDoubleClick
Action-->>DraggableView: .zoom / .miniaturize / .none / nil
DraggableView-->>NSApp: return (drag suppressed for non-nil)
else click IS in monitor view bounds
Monitor->>Action: performStandardTitlebarDoubleClick
Action->>NSApp: window.zoom(nil) or window.miniaturize(nil)
Action-->>Monitor: .zoom / .miniaturize / .none (non-nil)
Monitor-->>NSApp: return nil (event consumed)
Note over DraggableView: mouseDown never called
end
Reviews (1): Last reviewed commit: "Fix titlebar double-click zoom handling" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func testFallsBackToLegacyMiniaturizePreference() { | ||
| XCTAssertEqual( | ||
| resolvedStandardTitlebarDoubleClickAction(globalDefaults: [ | ||
| "AppleMiniaturizeOnDoubleClick": true, | ||
| ]), | ||
| .miniaturize | ||
| ) | ||
| } |
There was a problem hiding this comment.
AppleActionOnDoubleClick falls through to legacy key
The existing testFallsBackToLegacyMiniaturizePreference test only exercises the path where AppleActionOnDoubleClick is absent. There is a distinct untested path where the key is present but holds an unrecognised value (the default: break branch), and the code should then fall through to honour AppleMiniaturizeOnDoubleClick.
A minimal addition would be:
func testFallsBackToLegacyMiniaturizeWhenActionIsUnknown() {
XCTAssertEqual(
resolvedStandardTitlebarDoubleClickAction(globalDefaults: [
"AppleActionOnDoubleClick": "SomeFutureValue",
"AppleMiniaturizeOnDoubleClick": true,
]),
.miniaturize
)
}Without this test the default: break branch in resolvedStandardTitlebarDoubleClickAction has no coverage, so a future refactor could accidentally break that fallback silently.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/WindowDragHandleView.swift">
<violation number="1" location="Sources/WindowDragHandleView.swift:495">
P2: The `action == nil` check incorrectly evaluates to `false` when the action is `.none`, swallowing the event and preventing double-click-to-drag.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| guard view.bounds.contains(point) else { return event } | ||
|
|
||
| let action = performStandardTitlebarDoubleClick(window: window) | ||
| return action == nil ? event : nil |
There was a problem hiding this comment.
P2: The action == nil check incorrectly evaluates to false when the action is .none, swallowing the event and preventing double-click-to-drag.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/WindowDragHandleView.swift, line 495:
<comment>The `action == nil` check incorrectly evaluates to `false` when the action is `.none`, swallowing the event and preventing double-click-to-drag.</comment>
<file context>
@@ -440,3 +457,48 @@ struct WindowDragHandleView: NSViewRepresentable {
+ guard view.bounds.contains(point) else { return event }
+
+ let action = performStandardTitlebarDoubleClick(window: window)
+ return action == nil ? event : nil
+ }
+
</file context>
| return action == nil ? event : nil | |
| if let action, action != .none { return nil } | |
| return event |
This branch was successfully deployed
1 participant
Summary
AppleActionOnDoubleClick/ legacyAppleMiniaturizeOnDoubleClickpreference when deciding between zoom, minimize, or no actionFixes #2128.
Testing
./scripts/reload.sh --tag issue2128-doubleclickDemo Video
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by cubic
Double-clicking the custom titlebar now performs the standard macOS action (zoom, minimize, or none) based on the user’s setting, and works in minimal mode. Fixes #2128.
AppleActionOnDoubleClickand legacyAppleMiniaturizeOnDoubleClick, defaulting to zoom when unset.TitlebarDoubleClickMonitorViewand applied it to the drag handle and the minimal-mode top strip.Written for commit c60d452. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests