Repository navigation
Fix panel drag hover terminal flicker - #1213
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDrop-hover and drag handling were updated so drop-zone overlays are attached to parent containers (not hosted terminal views), overlay lifecycle and logging were extended, and terminal surface resize deferral and a pixel-size debug helper were added to avoid surface re-layout during active drags/overlays. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
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/GhosttyTerminalView.swift`:
- Around line 6730-6732: The predicate hasActiveDropZoneOverlay returns false as
soon as activeDropZone is cleared, but setDropZoneOverlay(zone: nil) only hides
the view and clears state after the hide animation completes, so keep the
predicate true until the hide completes: either delay clearing activeDropZone
until the completion handler in setDropZoneOverlay(zone: nil), or introduce a
transient flag (e.g. isHidingDropZoneOverlay) set when the hide animation starts
and cleared in its completion, and include that flag in hasActiveDropZoneOverlay
alongside activeDropZone and pendingDropZone; this ensures updateSurfaceSize()
retries won't think the overlay is gone while the hide animation is still
visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cfe74648-96c1-4193-878e-887cf2f83fe1
📒 Files selected for processing (2)
Sources/GhosttyTerminalView.swiftcmuxTests/CmuxWebViewKeyEquivalentTests.swift
| fileprivate var hasActiveDropZoneOverlay: Bool { | ||
| activeDropZone != nil || pendingDropZone != nil | ||
| } |
There was a problem hiding this comment.
Keep the overlay predicate true until the hide animation finishes.
activeDropZone is cleared at the start of setDropZoneOverlay(zone: nil), but the overlay view remains visible until the completion handler runs. If a deferred updateSurfaceSize() retry lands during that gap, Ghostty can resize under a still-visible hover overlay and reintroduce the flicker this patch is trying to remove.
💡 Suggested fix
fileprivate var hasActiveDropZoneOverlay: Bool {
- activeDropZone != nil || pendingDropZone != nil
+ activeDropZone != nil || pendingDropZone != nil || !dropZoneOverlayView.isHidden
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 6730 - 6732, The predicate
hasActiveDropZoneOverlay returns false as soon as activeDropZone is cleared, but
setDropZoneOverlay(zone: nil) only hides the view and clears state after the
hide animation completes, so keep the predicate true until the hide completes:
either delay clearing activeDropZone until the completion handler in
setDropZoneOverlay(zone: nil), or introduce a transient flag (e.g.
isHidingDropZoneOverlay) set when the hide animation starts and cleared in its
completion, and include that flag in hasActiveDropZoneOverlay alongside
activeDropZone and pendingDropZone; this ensures updateSurfaceSize() retries
won't think the overlay is gone while the hide animation is still visible.
There was a problem hiding this comment.
2 issues found across 2 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/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:3673">
P2: The new `dropOverlay` deferral path can spin a continuous main-queue retry loop while the overlay is active, causing avoidable CPU churn during drag hover.</violation>
<violation number="2" location="Sources/GhosttyTerminalView.swift:6731">
P2: Keep the drop-overlay deferral predicate true until the overlay view is actually hidden. As written, this can flip to false before the hide animation completes, allowing a surface resize under a still-visible overlay and causing flicker.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR fixes a terminal content flicker/reflow bug (#1209) that occurred when dragging a panel over a Ghostty terminal: the drop-zone hover overlay caused layout changes that triggered unwanted Ghostty surface resizes mid-drag. Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant DragSession
participant GhosttySurfaceScrollView
participant GhosttyNSView
participant TerminalSurface
DragSession->>GhosttySurfaceScrollView: setDropZoneOverlay(zone: .left)
Note over GhosttySurfaceScrollView: activeDropZone = .left<br/>hasActiveDropZoneOverlay = true
GhosttySurfaceScrollView->>GhosttyNSView: layout() [frame resize]
GhosttyNSView->>GhosttyNSView: updateSurfaceSize()
GhosttyNSView->>GhosttyNSView: activeSurfaceResizeDeferralReason()
GhosttyNSView-->>GhosttyNSView: "dropOverlay" (deferred)
GhosttyNSView->>GhosttyNSView: scheduleDeferredSurfaceSizeRetryIfNeeded()
Note over GhosttyNSView: pendingSurfaceSize = newSize<br/>resize blocked
DragSession->>GhosttySurfaceScrollView: setDropZoneOverlay(zone: nil)
Note over GhosttySurfaceScrollView: activeDropZone = nil<br/>hasActiveDropZoneOverlay = false
GhosttyNSView->>GhosttyNSView: deferred retry fires (DispatchQueue.main.async)
GhosttyNSView->>GhosttyNSView: updateSurfaceSize()
GhosttyNSView->>GhosttyNSView: activeSurfaceResizeDeferralReason() → nil
GhosttyNSView->>TerminalSurface: ghostty_surface_set_size(newWidth, newHeight)
Note over TerminalSurface: lastPixelWidth/Height updated
Last reviewed commit: 90ca636 |
…hover-flicker Fix panel drag hover terminal flicker
Summary
Closes #1209.
Summary by cubic
Stops terminal flicker during panel drag-hover by deferring terminal surface resize and rendering the hover overlay outside the hosted terminal so layout stays stable. Closes #1209.
Written for commit 379150a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests