Repository navigation
Fix terminal top-row click routing - #3720
Conversation
The terminal portal could defer top-row clicks to chrome when a tab-strip hit region overlapped the rendered terminal surface. Add a regression that models that overlap through the existing AppKit hit-test seam before changing routing policy. Constraint: Do not run XCUITests locally; this uses the unit-test hit-test seam instead. Confidence: high Scope-risk: narrow Directive: Keep this test focused on terminal-surface ownership rather than source-code shape. Tested: Not run locally by policy; intended to fail before the fix. Not-tested: Local xcodebuild-based test execution.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughGates tab-bar pass-through on whether a portal-hosted terminal surface is hit by adding ChangesTerminal Hit-Testing Pass-Through
Possibly Related PRs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 13 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes a hit-test routing bug (#3709) where the Bonsplit tab-strip pass-through was swallowing click events on the terminal's top row when tab-strip chrome regions geometrically overlapped the terminal surface. It gates pass-through on there being no visible
Confidence Score: 5/5Safe to merge; the change is narrowly scoped to one pass-through guard and has a targeted regression test covering the exact failure geometry. The fix adds a single, well-bounded helper that only fires after the existing BonsplitTabBarPassThrough decision already says pass through. No existing pass-through paths are widened, the subview walk mirrors the same reverse-z-order pattern used elsewhere in the file, and the ?? hostedView fallback correctly handles a transparent GhosttySurfaceScrollView.hitTest. The regression test exercises the precise 2-px overlap scenario that triggered the bug. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[hitTest called with point] --> B{isPointerEvent?}
B -- No --> Z[super.hitTest / nil]
B -- Yes --> C{shouldPassThroughToTitlebar?}
C -- Yes --> N1[return nil — titlebar wins]
C -- No --> D{shouldPassThroughToPaneTabBar?}
D --> E{BonsplitTabBarPassThrough decision.result?}
E -- No --> CONT[continue to sidebar/divider checks]
E -- Yes --> F{hostedTerminalHitView returns non-nil?}
F -- Yes --> G[return false — terminal wins, pass-through suppressed]
F -- No --> H[return true — tab strip wins, return nil]
G --> CONT
CONT --> I{shouldPassThroughToSidebarResizer?}
I -- Yes --> N3[return nil]
I -- No --> J{splitDividerCursorKind?}
J -- Yes --> N4[return nil]
J -- No --> K[super.hitTest → terminal subview]
Reviews (2): Last reviewed commit: "Keep terminal content above tab-strip hi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9a29679. Configure here.
The terminal portal now treats visible hosted terminal surfaces as the source of truth for terminal content hit ownership. Bonsplit tab-strip pass-through still handles real chrome, but it cannot swallow points that resolve inside a hosted terminal surface. Constraint: Top-row terminal clicks must reach Ghostty even when a chrome hit region overlaps terminal pixels. Rejected: Add a fixed Y offset below the tab strip | brittle across titlebar, font, and scale changes. Confidence: high Scope-risk: narrow Directive: Do not reintroduce tab-strip pass-through before checking hosted terminal ownership. Tested: Added unit regression in WindowTerminalHostViewTests; local xcodebuild execution skipped by policy. Not-tested: Local XCUITests; CI will run the project test matrix.
9a29679 to
c829a2e
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
@austinywang This is still an issue for me even with this fix. Should I open a new issue? |
|
This PR also didn't fix it for me |
|
Hey as an FYI, I noticed that the issue only still exists in "compact" mode. So a workaround for me was to turn "compact" mode off |
|
Still happening on the latest version ( |
This branch was successfully deployed
4 participants
Summary
Testing
Fixes #3709
Note
Medium Risk
Touches pointer hit-testing/routing in
WindowTerminalHostView, which can subtly affect mouse interactions across chrome/terminal boundaries. Scoped change with targeted regression coverage lowers the risk but UI event routing regressions are still possible.Overview
Fixes click routing when the Bonsplit tab strip’s pass-through region overlaps terminal pixels by only passing through to the pane tab bar if no visible
GhosttySurfaceScrollViewis hit.Adds a regression test ensuring a mouse-down on the terminal’s absolute top row is delivered to the hosted terminal even when a fake tab strip overlaps that area.
Reviewed by Cursor Bugbot for commit c829a2e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Keep the terminal’s top row clickable even when tab-strip hit regions overlap terminal pixels. Fixes #3709 by routing hits to the visible hosted terminal before any chrome pass-through.
Written for commit c829a2e. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests