Skip to content

Fix standard tab-bar new-tab click handling - #101

Merged
austinywang merged 6 commits into
mainfrom
issue-3049-flaky-new-tab-click
Apr 21, 2026
Merged

austinywang merged 6 commits into
mainfrom
issue-3049-flaky-new-tab-click

Conversation

@austinywang

@austinywang austinywang commented Apr 21, 2026 •

Copy link
Copy Markdown

Publishes the bonsplit side of manaflow-ai/cmux issue 3049.\n\n- adds standard-mode tab-bar new-tab regression coverage\n- makes a click in the empty trailing tab-bar region request a new tab immediately\n- keeps follow-up double-click events from creating duplicate tabs\n\nThis PR is needed before the cmux parent repo can point at a clean bonsplit SHA on origin/main.

Summary by CodeRabbit

  • Bug Fixes

    • Expanded the tab bar drag zone hit-testing area
    • Improved single-click and double-click handling in the tab bar interface
    • Fixed window drag initiation behavior during click operations
    • Enhanced click response detection across minimal and standard modes
  • Tests

    • Added comprehensive test cases covering tab bar mouse interactions and behavior

Summary by cubic

Fixes tab-bar new-tab click handling to remove flakiness and accidental duplicates. Single clicks in the trailing empty space now create a tab, and double-clicks behave correctly in both standard and minimal modes while preserving window drag.

  • Bug Fixes
    • Standard mode: single click in trailing space opens a new tab; follow-up double-click is deduped.
    • Add a trailing overlay and widen it by 30pt to cover the inline dropZoneAfterTabs, keeping clickCount consistent across boundary shifts.
    • Stop advertising mouseDownCanMoveWindow; use custom mouseDragged → window.performDrag with a small movement threshold to prevent AppKit from stealing mouseUp.
    • Minimal mode: double-click reliably triggers new tab; window dragging still works when focused.
    • Add regression tests for standard/minimal modes, double-click dedupe, and drag behavior.

Written for commit 68659a2. Summary will update on new commits.

austinywang and others added 6 commits April 20, 2026 11:52
When the drag zone returns mouseDownCanMoveWindow=true, AppKit's own
window-drag tracking intercepts mouseUp on the first click. The second
click of a quick double-click then arrives as clickCount=1 instead of
clickCount=2, so the new-tab action never fires. Forcing the property
to false keeps the standard mouseDown/mouseUp/mouseDown sequence intact
while the custom mouseDragged -> window.performDrag flow still handles
actual window drags.

Same class of bug already documented on pane tab clicks (see
NonDraggableHostingView / SplitArrangedContainerView comments).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 21, 2026 •

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e6edfdd-fda6-4a7f-92a6-e806895c6071

📥 Commits

Reviewing files that changed from the base of the PR and between 433b8f5 and 68659a2.

📒 Files selected for processing (2)
  • Sources/Bonsplit/Internal/Views/TabBarView.swift
  • Tests/BonsplitTests/BonsplitTests.swift

📝 Walkthrough

Walkthrough

This PR modifies the tab bar drag zone's mouse event handling to prevent AppKit from interfering with click operations and to improve coordination between single-click, double-click, and window-drag behaviors. It restructures the mouseDown control flow, introduces pending drag-state coordination, updates overlay sizing, and adds comprehensive test coverage for the new behavior across minimal and standard modes.

Changes

Cohort / File(s) Summary
Tab Bar Mouse Handling
Sources/Bonsplit/Internal/Views/TabBarView.swift
Updated trailing overlay width from trailing to trailing + 30; changed mouseDownCanMoveWindow from conditional to hard false; restructured mouseDown to distinguish single vs. double-click paths with early returns for non-minimal mode; added pending-drag-state coordination across mouseDown/mouseDragged/mouseUp; introduced performTitlebarDoubleClickAction and startWindowDrag helpers.
Mouse Interaction Tests
Tests/BonsplitTests/BonsplitTests.swift
Added four new @MainActor test cases covering single/double-click behavior in minimal and standard modes; updated existing window-drag test to send explicit .leftMouseDragged event and assert mouseDownCanMoveWindow is false; refactored test helpers to generalize makeMouseEvent(type:in:at:clickCount) and delegate from makeLeftMouseDownEvent.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Click, click, hop! No AppKit theft,
Drag state dances, pending's left.
Single, double, each its way,
Mouse events coordinate the day! 🎯

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-3049-flaky-new-tab-click

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@austinywang
austinywang merged commit c1018c5 into main Apr 21, 2026
3 of 5 checks passed
@greptile-apps

greptile-apps Bot commented Apr 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes new-tab click handling in the standard-mode tab bar by (1) hardcoding mouseDownCanMoveWindow to false to prevent AppKit from stealing mouseUp and corrupting double-click tracking, (2) extending the trailing overlay by 30pt to unify the inline and overlay drag zones under a single DragNSView, and (3) firing the new-tab action on the very first click in standard mode while deduplicating any follow-up clickCount ≥ 2 events. New regression tests cover the single-click and double-click deduplication paths; the window-drag test is updated to exercise mouseDragged and assert the new mouseDownCanMoveWindow = false invariant.

Confidence Score: 5/5

Safe to merge; all findings are P2 and do not block the primary new-tab fix.

The core logic is sound and well-commented. All changes are covered by new or updated unit tests. The one flagged concern (unfocused-pane standard-mode behavior) is a pre-existing edge case with no new regression introduced by this PR, making this a clarification/test-coverage note rather than a blocker.

No files require special attention beyond the P2 clarification on TabBarView.swift.

Important Files Changed

Filename Overview
Sources/Bonsplit/Internal/Views/TabBarView.swift Hardens trailing-area new-tab handling: overlay width extended by 30pt to unify inline/overlay drag zones, mouseDownCanMoveWindow hardcoded to false to fix AppKit double-click tracking, standard mode now fires new-tab on single click with deduplication for follow-up clicks. One unaddressed edge case: unfocused-pane standard-mode path skips onSingleClick.
Tests/BonsplitTests/BonsplitTests.swift Adds regression tests for standard-mode single-click new-tab creation and double-click deduplication; extends existing minimal-mode drag test to simulate mouseDragged and updates the mouseDownCanMoveWindow assertion.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[mouseDown event received] --> B{isMinimalMode?}

    B -- No: standard mode --> C[clearPendingWindowDrag]
    C --> D{clickCount == 1?}
    D -- Yes --> E[call onDoubleClick new-tab]
    E --> F{returned true?}
    F -- Yes --> G[return — tab created]
    F -- No --> H[super.mouseDown]
    D -- No clickCount ≥ 2 --> I[return — deduplicated]

    B -- Yes: minimal mode --> J{clickCount ≥ 2?}
    J -- Yes --> K[clearPendingWindowDrag]
    K --> L{onDoubleClick?}
    L -- true --> M[return — tab created]
    L -- false/nil --> N[performTitlebarDoubleClickAction]
    J -- No --> O{isFocusedPane?}
    O -- No --> P{onSingleClick?}
    P -- true --> Q[clearPendingWindowDrag / return]
    P -- false/nil --> R[pendingWindowDragEvent = event]
    O -- Yes --> R

    R --> S[mouseDragged: distance ≥ 4px?]
    S -- Yes --> T[startWindowDrag via window.performDrag]
    S -- No --> U[wait for more drag events]
Loading

Reviews (1): Last reviewed commit: "Fix standard tab-bar new-tab click handl..." | Re-trigger Greptile

Comment on lines +1141 to +1157
if !isMinimalMode {
clearPendingWindowDrag()
if event.clickCount == 1 {
if onDoubleClick?() == true {
#if DEBUG
dlog("tab.bar.dragZone.singleClick action=newTab")
#endif
return
}
super.mouseDown(with: event)
return
}
// clickCount >= 2: same gesture as a click we already acted on.
#if DEBUG
dlog("tab.bar.dragZone.click skipped reason=dedupeStandardMode clickCount=\(event.clickCount)")
#endif
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 onSingleClick (focus-pane) bypassed in standard mode for unfocused panes

In standard mode the code immediately calls onDoubleClick (new-tab) on the first click regardless of isFocusedPane, skipping the onSingleClick → focusPaneFromTabBarChrome path that minimal mode uses. If a split layout has two panes in standard mode and the user clicks the empty trailing area of the unfocused pane, a tab is created in that pane without first focusing it. No test covers isMinimalMode = false, isFocusedPane = false, and no comment explains whether this is deliberate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant