Merge pull request #313 from manaflow-ai/feat-terminal-find-overlay-portal
Fix terminal Cmd+F overlay visibility regression and add layering guardrails
This commit is contained in:
Lawrence Chen
GitHub
commit
146de6030e
4 changed files with 181 additions and 31 deletions
|
|
@ -93,6 +93,7 @@ tail -f "$(cat /tmp/cmux-last-debug-log-path 2>/dev/null || echo /tmp/cmux-debug
|
|||
|
||||
- **Custom UTTypes** for drag-and-drop must be declared in `Resources/Info.plist` under `UTExportedTypeDeclarations` (e.g. `com.splittabbar.tabtransfer`, `com.cmux.sidebar-tab-reorder`).
|
||||
- Do not add an app-level display link or manual `ghostty_surface_draw` loop; rely on Ghostty wakeups/renderer to avoid typing lag.
|
||||
- **Terminal find layering contract:** `SurfaceSearchOverlay` must be mounted from `GhosttySurfaceScrollView` in `Sources/GhosttyTerminalView.swift` (AppKit portal layer), not from SwiftUI panel containers such as `Sources/Panels/TerminalPanelView.swift`. Portal-hosted terminal views can sit above SwiftUI during split/workspace churn.
|
||||
- **Submodule safety:** When modifying a submodule (ghostty, vendor/bonsplit, etc.), always push the submodule commit to its remote `main` branch BEFORE committing the updated pointer in the parent repo. Never commit on a detached HEAD or temporary branch — the commit will be orphaned and lost. Verify with: `cd <submodule> && git merge-base --is-ancestor HEAD origin/main`.
|
||||
|
||||
## Socket command threading policy
|
||||
|
|
|
|||
|
|
@ -3025,6 +3025,7 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
private let notificationRingLayer: CAShapeLayer
|
||||
private let flashOverlayView: GhosttyFlashOverlayView
|
||||
private let flashLayer: CAShapeLayer
|
||||
private var searchOverlayHostingView: NSHostingView<SurfaceSearchOverlay>?
|
||||
private var observers: [NSObjectProtocol] = []
|
||||
private var windowObservers: [NSObjectProtocol] = []
|
||||
private var isLiveScrolling = false
|
||||
|
|
@ -3408,6 +3409,59 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
CATransaction.commit()
|
||||
}
|
||||
|
||||
func setSearchOverlay(searchState: TerminalSurface.SearchState?) {
|
||||
if !Thread.isMainThread {
|
||||
DispatchQueue.main.async { [weak self] in
|
||||
self?.setSearchOverlay(searchState: searchState)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// Layering contract: keep terminal Cmd+F UI inside this portal-hosted AppKit view.
|
||||
// SwiftUI panel-level overlays can fall behind portal-hosted terminal surfaces.
|
||||
guard let terminalSurface = surfaceView.terminalSurface,
|
||||
let searchState else {
|
||||
searchOverlayHostingView?.removeFromSuperview()
|
||||
searchOverlayHostingView = nil
|
||||
return
|
||||
}
|
||||
|
||||
let rootView = SurfaceSearchOverlay(
|
||||
surface: terminalSurface,
|
||||
searchState: searchState,
|
||||
onClose: { [weak self] in
|
||||
self?.surfaceView.terminalSurface?.searchState = nil
|
||||
self?.moveFocus()
|
||||
}
|
||||
)
|
||||
|
||||
if let overlay = searchOverlayHostingView {
|
||||
overlay.rootView = rootView
|
||||
if overlay.superview !== self {
|
||||
overlay.removeFromSuperview()
|
||||
addSubview(overlay)
|
||||
NSLayoutConstraint.activate([
|
||||
overlay.topAnchor.constraint(equalTo: topAnchor),
|
||||
overlay.bottomAnchor.constraint(equalTo: bottomAnchor),
|
||||
overlay.leadingAnchor.constraint(equalTo: leadingAnchor),
|
||||
overlay.trailingAnchor.constraint(equalTo: trailingAnchor),
|
||||
])
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
let overlay = NSHostingView(rootView: rootView)
|
||||
overlay.translatesAutoresizingMaskIntoConstraints = false
|
||||
addSubview(overlay)
|
||||
NSLayoutConstraint.activate([
|
||||
overlay.topAnchor.constraint(equalTo: topAnchor),
|
||||
overlay.bottomAnchor.constraint(equalTo: bottomAnchor),
|
||||
overlay.leadingAnchor.constraint(equalTo: leadingAnchor),
|
||||
overlay.trailingAnchor.constraint(equalTo: trailingAnchor),
|
||||
])
|
||||
searchOverlayHostingView = overlay
|
||||
}
|
||||
|
||||
private func dropZoneOverlayFrame(for zone: DropZone, in size: CGSize) -> CGRect {
|
||||
let padding: CGFloat = 4
|
||||
switch zone {
|
||||
|
|
@ -3688,6 +3742,11 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
)
|
||||
}
|
||||
|
||||
func debugHasSearchOverlay() -> Bool {
|
||||
guard let overlay = searchOverlayHostingView else { return false }
|
||||
return overlay.superview === self && !overlay.isHidden
|
||||
}
|
||||
|
||||
#endif
|
||||
|
||||
/// Handle file/URL drops, forwarding to the terminal as shell-escaped paths.
|
||||
|
|
@ -4359,6 +4418,7 @@ struct GhosttyTerminalView: NSViewRepresentable {
|
|||
var showsUnreadNotificationRing: Bool = false
|
||||
var inactiveOverlayColor: NSColor = .clear
|
||||
var inactiveOverlayOpacity: Double = 0
|
||||
var searchState: TerminalSurface.SearchState? = nil
|
||||
var reattachToken: UInt64 = 0
|
||||
var onFocus: ((UUID) -> Void)? = nil
|
||||
var onTriggerFlash: (() -> Void)? = nil
|
||||
|
|
@ -4460,6 +4520,7 @@ struct GhosttyTerminalView: NSViewRepresentable {
|
|||
visible: showsInactiveOverlay
|
||||
)
|
||||
hostedView.setNotificationRing(visible: showsUnreadNotificationRing)
|
||||
hostedView.setSearchOverlay(searchState: searchState)
|
||||
hostedView.setFocusHandler { onFocus?(terminalSurface.id) }
|
||||
hostedView.setTriggerFlashHandler(onTriggerFlash)
|
||||
let forwardedDropZone = isVisibleInUI ? paneDropZone : nil
|
||||
|
|
|
|||
|
|
@ -15,37 +15,26 @@ struct TerminalPanelView: View {
|
|||
let onTriggerFlash: () -> Void
|
||||
|
||||
var body: some View {
|
||||
ZStack(alignment: .topLeading) {
|
||||
GhosttyTerminalView(
|
||||
terminalSurface: panel.surface,
|
||||
isActive: isFocused,
|
||||
isVisibleInUI: isVisibleInUI,
|
||||
portalZPriority: portalPriority,
|
||||
showsInactiveOverlay: isSplit && !isFocused,
|
||||
showsUnreadNotificationRing: hasUnreadNotification,
|
||||
inactiveOverlayColor: appearance.unfocusedOverlayNSColor,
|
||||
inactiveOverlayOpacity: appearance.unfocusedOverlayOpacity,
|
||||
reattachToken: panel.viewReattachToken,
|
||||
onFocus: { _ in onFocus() },
|
||||
onTriggerFlash: onTriggerFlash
|
||||
)
|
||||
// Keep the NSViewRepresentable identity stable across bonsplit structural updates.
|
||||
// This prevents transient teardown/recreate that can momentarily detach the hosted terminal view.
|
||||
.id(panel.id)
|
||||
.background(Color.clear)
|
||||
|
||||
// Search overlay
|
||||
if let searchState = panel.searchState {
|
||||
SurfaceSearchOverlay(
|
||||
surface: panel.surface,
|
||||
searchState: searchState,
|
||||
onClose: {
|
||||
panel.searchState = nil
|
||||
panel.hostedView.moveFocus()
|
||||
}
|
||||
)
|
||||
}
|
||||
}
|
||||
// Layering contract: terminal find UI is mounted in GhosttySurfaceScrollView (AppKit portal layer)
|
||||
// via `searchState`. Rendering `SurfaceSearchOverlay` in this SwiftUI container can hide it.
|
||||
GhosttyTerminalView(
|
||||
terminalSurface: panel.surface,
|
||||
isActive: isFocused,
|
||||
isVisibleInUI: isVisibleInUI,
|
||||
portalZPriority: portalPriority,
|
||||
showsInactiveOverlay: isSplit && !isFocused,
|
||||
showsUnreadNotificationRing: hasUnreadNotification,
|
||||
inactiveOverlayColor: appearance.unfocusedOverlayNSColor,
|
||||
inactiveOverlayOpacity: appearance.unfocusedOverlayOpacity,
|
||||
searchState: panel.searchState,
|
||||
reattachToken: panel.viewReattachToken,
|
||||
onFocus: { _ in onFocus() },
|
||||
onTriggerFlash: onTriggerFlash
|
||||
)
|
||||
// Keep the NSViewRepresentable identity stable across bonsplit structural updates.
|
||||
// This prevents transient teardown/recreate that can momentarily detach the hosted terminal view.
|
||||
.id(panel.id)
|
||||
.background(Color.clear)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3256,6 +3256,105 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
|
|||
state = hostedView.debugInactiveOverlayState()
|
||||
XCTAssertTrue(state.isHidden)
|
||||
}
|
||||
|
||||
func testSearchOverlayMountsAndUnmountsWithSearchState() {
|
||||
let surface = TerminalSurface(
|
||||
tabId: UUID(),
|
||||
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
|
||||
configTemplate: nil,
|
||||
workingDirectory: nil
|
||||
)
|
||||
let hostedView = surface.hostedView
|
||||
XCTAssertFalse(hostedView.debugHasSearchOverlay())
|
||||
|
||||
let searchState = TerminalSurface.SearchState(needle: "example")
|
||||
hostedView.setSearchOverlay(searchState: searchState)
|
||||
XCTAssertTrue(hostedView.debugHasSearchOverlay())
|
||||
|
||||
hostedView.setSearchOverlay(searchState: nil)
|
||||
XCTAssertFalse(hostedView.debugHasSearchOverlay())
|
||||
}
|
||||
|
||||
func testSearchOverlaySurvivesPortalRebindDuringSplitLikeChurn() {
|
||||
let window = NSWindow(
|
||||
contentRect: NSRect(x: 0, y: 0, width: 480, height: 320),
|
||||
styleMask: [.titled, .closable],
|
||||
backing: .buffered,
|
||||
defer: false
|
||||
)
|
||||
defer { window.orderOut(nil) }
|
||||
let portal = WindowTerminalPortal(window: window)
|
||||
|
||||
guard let contentView = window.contentView else {
|
||||
XCTFail("Expected content view")
|
||||
return
|
||||
}
|
||||
|
||||
let anchorA = NSView(frame: NSRect(x: 20, y: 20, width: 180, height: 140))
|
||||
let anchorB = NSView(frame: NSRect(x: 220, y: 20, width: 180, height: 140))
|
||||
contentView.addSubview(anchorA)
|
||||
contentView.addSubview(anchorB)
|
||||
|
||||
let surface = TerminalSurface(
|
||||
tabId: UUID(),
|
||||
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
|
||||
configTemplate: nil,
|
||||
workingDirectory: nil
|
||||
)
|
||||
let hostedView = surface.hostedView
|
||||
hostedView.setSearchOverlay(searchState: TerminalSurface.SearchState(needle: "split"))
|
||||
XCTAssertTrue(hostedView.debugHasSearchOverlay())
|
||||
|
||||
portal.bind(hostedView: hostedView, to: anchorA, visibleInUI: true)
|
||||
XCTAssertTrue(hostedView.debugHasSearchOverlay())
|
||||
|
||||
portal.bind(hostedView: hostedView, to: anchorB, visibleInUI: true)
|
||||
XCTAssertTrue(
|
||||
hostedView.debugHasSearchOverlay(),
|
||||
"Split-like anchor churn should not unmount terminal search overlay"
|
||||
)
|
||||
}
|
||||
|
||||
func testSearchOverlaySurvivesPortalVisibilityToggleDuringWorkspaceSwitchLikeChurn() {
|
||||
let window = NSWindow(
|
||||
contentRect: NSRect(x: 0, y: 0, width: 480, height: 320),
|
||||
styleMask: [.titled, .closable],
|
||||
backing: .buffered,
|
||||
defer: false
|
||||
)
|
||||
defer { window.orderOut(nil) }
|
||||
let portal = WindowTerminalPortal(window: window)
|
||||
|
||||
guard let contentView = window.contentView else {
|
||||
XCTFail("Expected content view")
|
||||
return
|
||||
}
|
||||
|
||||
let anchor = NSView(frame: NSRect(x: 40, y: 40, width: 220, height: 160))
|
||||
contentView.addSubview(anchor)
|
||||
|
||||
let surface = TerminalSurface(
|
||||
tabId: UUID(),
|
||||
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
|
||||
configTemplate: nil,
|
||||
workingDirectory: nil
|
||||
)
|
||||
let hostedView = surface.hostedView
|
||||
hostedView.setSearchOverlay(searchState: TerminalSurface.SearchState(needle: "workspace"))
|
||||
XCTAssertTrue(hostedView.debugHasSearchOverlay())
|
||||
|
||||
portal.bind(hostedView: hostedView, to: anchor, visibleInUI: true)
|
||||
XCTAssertTrue(hostedView.debugHasSearchOverlay())
|
||||
|
||||
portal.bind(hostedView: hostedView, to: anchor, visibleInUI: false)
|
||||
XCTAssertTrue(hostedView.debugHasSearchOverlay())
|
||||
|
||||
portal.bind(hostedView: hostedView, to: anchor, visibleInUI: true)
|
||||
XCTAssertTrue(
|
||||
hostedView.debugHasSearchOverlay(),
|
||||
"Workspace-switch-like visibility toggles should not unmount terminal search overlay"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@MainActor
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue