Skip to content

Fix empty tab bar click handling - #116

Merged
lawrencecchen merged 3 commits into
mainfrom
feat-tabbar-click-zones
May 8, 2026
Merged

lawrencecchen merged 3 commits into
mainfrom
feat-tabbar-click-zones

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented May 8, 2026 •

Copy link
Copy Markdown

Summary

  • make empty tab-bar chrome create new tabs only on standard-mode double-click
  • keep minimal-mode empty tab-bar double-clicks on titlebar behavior
  • focus inactive standard panes on empty-chrome single-click
  • cover the empty chrome hit zone and click policy with Bonsplit tests

Testing

  • swift test --filter BonsplitTests/testTabBarDragZone, 7 tests passed
  • swift test, 111 tests passed
  • hosted Bonsplit PR checks passed

Checklist

  • Regression tests added
  • Review feedback addressed or resolved
  • CI green

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented May 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refines tab-bar interaction handling by introducing a HitRegion-based approach to distinguish trailing empty chrome from tab and split-button areas. DragNSView now conditionally captures hits based on the selected region and changes mouse event routing to defer single-click processing in standard mode, while tests verify minimal-mode chrome-like behavior and standard-mode double-click-only new-tab creation.

Changes

Trailing Empty Chrome Hit Region and Event Routing

Layer / File(s) Summary
HitRegion Data Contract
Sources/Bonsplit/Internal/Views/TabBarView.swift
HitRegion enum with entireBounds and trailingEmptyChrome(tabFrames:reservedTrailingWidth:) cases, and hitRegion property added to DragNSView for plumbing into makeNSView and updateNSView.
Hit-Test Predicate Logic
Sources/Bonsplit/Internal/Views/TabBarView.swift
DragNSView.hitTest delegates to shouldCaptureHit(at:) predicate; shouldCaptureHit gates trailingEmptyChrome hits to mouse-down/drag candidates and excludes points within padded tab-frame bounds.
Mouse Event Routing
Sources/Bonsplit/Internal/Views/TabBarView.swift
DragNSView.mouseDown skips single-click processing in standard mode, routes double-clicks to onDoubleClick callback, and preserves minimal-mode pending-window-drag flow.
TabBarView Overlay Integration
Sources/Bonsplit/Internal/Views/TabBarView.swift
TabBarView adds TabBarDragZoneView overlay with .trailingEmptyChrome hit region; reserved width computed from split-button render state; tab-frame exclusion built from tabFramesInBar.values.
Test Coverage
Tests/BonsplitTests/BonsplitTests.swift
Minimal-mode tests verify single/double/sequence clicks do not request new tabs or start window drag; standard-mode tests confirm single clicks are deferred and double clicks create exactly one tab; hit-region test ensures trailing-empty-chrome only captures truly empty space and does not intercept tabs or split-button.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • manaflow-ai/bonsplit#113: Centralizes split-button lane layout and hit/mask behavior, overlapping with the main PR's trailing action-lane hit-testing refactoring.
  • manaflow-ai/bonsplit#97: Modifies TabBarDragZoneView hit-testing and minimal-mode click/drag behavior, directly related to the refactored event routing and hit region logic.
  • manaflow-ai/bonsplit#31: Adds EmptyTabBarDoubleClickMonitorView to capture trailing empty tab-bar interactions for opening a new tab, addressing the same trailing-area interaction concern.

Poem

🐰 A rabbit hops through tabs so bright,
With empty chrome now hitting right,
No single-click shall slip through,
Double-clicks and regions true,
The trail is clear, the bounds are tight! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix empty tab bar click handling' clearly and concisely describes the main change in the PR, which involves fixing click behavior in empty areas of the tab bar across different modes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 feat-tabbar-click-zones

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.

@greptile-apps

greptile-apps Bot commented May 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes accidental tab creation caused by single clicks in the empty tab-bar chrome. It introduces a trailingEmptyChrome hit-region on a new outer TabBarDragZoneView overlay so that only the empty area after the last tab is captured, and it shifts standard-mode new-tab creation from a single click to a double-click while stripping minimal mode of its empty-chrome new-tab action entirely (double-click there now delegates to the system titlebar action).

  • Behavior change (standard mode): single click on empty trailing chrome is now consumed without action; a double-click is required to create a new tab. The onSingleClick: focusPaneFromTabBarChrome handler passed to the new overlay is never invoked in the standard-mode mouseDown path, leaving the event silently swallowed for unfocused panes.
  • Hit-region geometry: shouldCaptureHit computes the empty zone by excluding padded tab frames and the reserved split-button lane using NSApp.currentEvent?.type to distinguish mouse events from other hit-test callers; a hitTestEventTypeOverride escape hatch makes this testable.
  • Tests: four existing assertions are inverted to match the new click semantics and a new geometry test covers the trailingEmptyChrome boundary conditions.

Confidence Score: 3/5

The empty-chrome click rework is mostly correct, but standard-mode single clicks on unfocused panes are now silently swallowed — the outer overlay captures the event and returns without calling focusPaneFromTabBarChrome.

The outer overlay correctly narrows the hit region to the trailing empty chrome and the double-click new-tab path works as intended. The gap is in the standard-mode single-click branch: onSingleClick is passed to the new overlay but is never reachable in the !isMinimalMode code path, so a single tap on an unfocused pane's empty chrome does nothing — no focus transfer, no tab, no super call.

Sources/Bonsplit/Internal/Views/TabBarView.swift — specifically the !isMinimalMode branch of mouseDown in DragNSView and the shouldCaptureHit helper's reliance on NSApp.currentEvent.

Important Files Changed

Filename Overview
Sources/Bonsplit/Internal/Views/TabBarView.swift Adds a new outer TabBarDragZoneView overlay with a trailingEmptyChrome hit region; changes standard mode to require double-click for new tab and strips minimal mode of its new-tab-on-double-click path. Standard-mode mouseDown silently swallows single clicks without invoking onSingleClick, leaving unfocused panes unreachable by clicking the empty chrome.
Tests/BonsplitTests/BonsplitTests.swift Inverts four existing test assertions to match new click semantics, renames them accordingly, and adds testTabBarTrailingEmptyChromeCapturesOnlyEmptyArea covering the trailingEmptyChrome x-axis boundary conditions using hitTestEventTypeOverride.

Reviews (1): Last reviewed commit: "fix: stabilize empty tab bar click handl..." | Re-trigger Greptile

Comment thread Sources/Bonsplit/Internal/Views/TabBarView.swift
Comment thread Sources/Bonsplit/Internal/Views/TabBarView.swift
@lawrencecchen
lawrencecchen merged commit 586bafc into main May 8, 2026
5 checks passed
@lawrencecchen
lawrencecchen deleted the feat-tabbar-click-zones branch May 8, 2026 20:42
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