Skip to content

Fix terminal drag hover overlay flicker - #1050

Merged
austinywang merged 2 commits into
mainfrom
issue-1045-drag-hover-terminal-flicker-v2
Mar 7, 2026
Merged

austinywang merged 2 commits into
mainfrom
issue-1045-drag-hover-terminal-flicker-v2

Conversation

@austinywang

@austinywang austinywang commented Mar 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • update Bonsplit to the repaired overlay commit that renders drag-hover indicators as true overlays
  • keep terminal pane geometry stable during drag hover so the terminal surface does not relayout or emit resize events
  • include the Bonsplit regression test that verifies hosted content geometry stays fixed while drop zones change

Testing

  • ./scripts/reload.sh --tag issue-1045-v2
  • GitHub Actions CI

Context

This is the clean re-land of issue #1045 after reverting the earlier bad submodule pointer. The underlying Bonsplit fix was re-applied on top of the restored SafeTooltip base and passed its own package CI before this PR was opened.


Summary by cubic

Fixes terminal drag-hover flicker by updating Bonsplit to render drag indicators as true overlays and keeping terminal geometry stable during hover. Also auto-rebinds the terminal portal when its host registry entry is missing to prevent rare detachment and flicker.

Written for commit e4d8002. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Improved terminal window binding so hosts rebind correctly when a portal entry is missing, reducing transient display/connectivity issues; added conditional debug logging for binding diagnostics.
  • Chores

    • Updated a vendored library dependency to the latest version.

@vercel

vercel Bot commented Mar 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 7, 2026 7:37pm

@coderabbitai

coderabbitai Bot commented Mar 7, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2d9f9665-6940-4fda-ab86-93437c99d53b

📥 Commits

Reviewing files that changed from the base of the PR and between 62125c1 and e4d8002.

📒 Files selected for processing (1)
  • Sources/GhosttyTerminalView.swift

📝 Walkthrough

Walkthrough

Adds a check for a missing portal entry in GhosttyTerminalView to trigger an earlier rebind via TerminalWindowPortalRegistry.bind, and updates the vendor/bonsplit submodule pointer to a newer commit.

Changes

Cohort / File(s) Summary
Submodule Update
vendor/bonsplit
Submodule pointer advanced from b1f4916a... to fa452db1... (version bump of bundled dependency).
Host/Portal binding logic
Sources/GhosttyTerminalView.swift
Adds portalEntryMissing check (uses TerminalWindowPortalRegistry.isHostedView) and forces an earlier rebind path; adds DEBUG log when rebind reason is portal entry missing.

Sequence Diagram(s)

sequenceDiagram
    participant Host as GhosttyTerminalView
    participant Registry as TerminalWindowPortalRegistry
    participant Hosted as HostedView/Surface

    Host->>Registry: isHostedView(hostedView)?
    Registry-->>Host: returns portalEntryMissing (true/false)
    alt portalEntryMissing == true
        Host->>Registry: bind(host, hostedView)  rgba(100,150,240,0.5)
        Registry-->>Hosted: establish portal entry  rgba(100,200,150,0.5)
        Registry-->>Host: confirm bound
    else portalEntryMissing == false
        Host->>Hosted: continue existing binding flow
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I checked the portal, found a gap so thin,
I nudged a bind and hopped right in.
A submodule hop, a tidy little feat,
New pointer, fresh code — my carrots are sweet! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix terminal drag hover overlay flicker' directly addresses the main change: resolving flicker issues in the terminal drag hover overlay behavior.
Description check ✅ Passed The PR description includes Summary, Testing, and Context sections with substantive detail. However, it lacks a Demo Video section and the Checklist is missing.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch issue-1045-drag-hover-terminal-flicker-v2

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 1 file

@greptile-apps

greptile-apps Bot commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR advances the vendor/bonsplit submodule pointer from b1f4916 (the restored SafeTooltip base that was re-established after reverting PR #1046) to fa452db, which is described as the repaired overlay commit. The single-file diff is a clean re-land of issue #1045 following the revert/re-apply workflow visible in the git history (e019293 → revert eaa7647 → this landing 62125c1).

  • Submodule update only — the diff is a one-line pointer change in vendor/bonsplit; all substantive code lives in the bonsplit package itself
  • Bonsplit is consumed as an XCLocalSwiftPackageReference in GhosttyTabs.xcodeproj, pointing at vendor/bonsplit; no Package.resolved entry is expected or required for local packages, and none was changed
  • TerminalPanelView.swift already carries .id(panel.id) to keep the NSViewRepresentable identity stable across bonsplit structural updates (comment: "Keep the NSViewRepresentable identity stable across bonsplit structural updates"), so the cmux-side geometry-stability measure pre-dates this PR
  • Re-land history is consistent: git submodule status confirms the working tree is at fa452db181f361514087558a29204bda7e38218f, matching the pointer written by this commit

Confidence Score: 4/5

  • Safe to merge; the change is a single submodule pointer bump with a well-documented revert-and-re-land history and confirmed bonsplit CI passage.
  • The diff is minimal (one submodule pointer line), the re-land workflow is traceable in git history, the Xcode project wiring for the local package is unchanged and correct, and Package.resolved correctly omits local packages. The only reason this is not a 5 is that the internal bonsplit changes (overlay rendering, geometry-stability fix, regression test) are not directly reviewable from this repository — reviewers must trust bonsplit's own CI and the PR description.
  • No files require special attention beyond the bonsplit submodule itself, which should be reviewed at the bonsplit repo level.

Important Files Changed

Filename Overview
vendor/bonsplit Submodule pointer bumped from b1f4916 (restored SafeTooltip base) to fa452db (overlay fix commit); the internal bonsplit changes are not directly reviewable here but the pointer is consistent with the checked-out submodule state and the PR description confirms bonsplit's own CI passed before landing.

Sequence Diagram

sequenceDiagram
    participant User
    participant cmux (SwiftUI)
    participant BonsplitLayout
    participant TerminalPanelView
    participant GhosttyTerminalView (NSView)

    User->>BonsplitLayout: Drag tab over pane
    Note over BonsplitLayout: OLD: drop-zone indicator shifted<br/>hosted content bounds → resize event fired
    BonsplitLayout-->>GhosttyTerminalView (NSView): Frame changed → terminal relayout + flicker

    Note over BonsplitLayout: NEW (fa452db): drop-zone rendered<br/>as true overlay, hosted content<br/>geometry unchanged
    User->>BonsplitLayout: Drag tab over pane
    BonsplitLayout->>TerminalPanelView: Geometry stable (no frame change)
    TerminalPanelView->>GhosttyTerminalView (NSView): No resize event emitted → no flicker
Loading

Last reviewed commit: 62125c1

@austinywang
austinywang merged commit 1fb5e19 into main Mar 7, 2026
15 of 16 checks passed
@austinywang
austinywang deleted the issue-1045-drag-hover-terminal-flicker-v2 branch March 7, 2026 20:23
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
…hover-terminal-flicker-v2

Fix terminal drag hover overlay flicker

This branch was successfully deployed

1 active deployment
Preview — e4d80021 Deployed Mar 7, 2026 by vercel[bot]
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