Fix notification ring dismissal on direct terminal clicks (#1126)
* Add regression test for terminal notification click dismissal * Dismiss terminal notifications on direct clicks * Add regression for focused terminal notification ring * Keep focused terminal notifications unread until click * Verify direct notification dismiss triggers flash * Use focus-flash path for direct notification dismiss * Align notification dismiss flash with ring geometry
This commit is contained in:
parent
78b901617c
commit
b824147dcb
7 changed files with 186 additions and 24 deletions
|
|
@ -9816,6 +9816,7 @@ private struct TabItemView: View {
|
|||
let modifiers = NSEvent.modifierFlags
|
||||
let isCommand = modifiers.contains(.command)
|
||||
let isShift = modifiers.contains(.shift)
|
||||
let wasSelected = tabManager.selectedTabId == tab.id
|
||||
|
||||
if isShift, let lastIndex = lastSidebarSelectionIndex {
|
||||
let lower = min(lastIndex, index)
|
||||
|
|
@ -9838,6 +9839,12 @@ private struct TabItemView: View {
|
|||
|
||||
lastSidebarSelectionIndex = index
|
||||
tabManager.selectTab(tab)
|
||||
if wasSelected, !isCommand, !isShift {
|
||||
tabManager.dismissNotificationOnDirectInteraction(
|
||||
tabId: tab.id,
|
||||
surfaceId: tabManager.focusedSurfaceId(for: tab.id)
|
||||
)
|
||||
}
|
||||
selection = .tabs
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -4547,6 +4547,12 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
|
|||
dlog("terminal.mouseDown surface=\(terminalSurface?.id.uuidString.prefix(5) ?? "nil") mods=[\(debugModifierString(event.modifierFlags))] clickCount=\(event.clickCount) point=(\(String(format: "%.0f", debugPoint.x)),\(String(format: "%.0f", debugPoint.y)))")
|
||||
#endif
|
||||
window?.makeFirstResponder(self)
|
||||
if let terminalSurface {
|
||||
AppDelegate.shared?.tabManager?.dismissNotificationOnDirectInteraction(
|
||||
tabId: terminalSurface.tabId,
|
||||
surfaceId: terminalSurface.id
|
||||
)
|
||||
}
|
||||
guard let surface = surface else { return }
|
||||
let point = convert(event.locationInWindow, from: nil)
|
||||
ghostty_surface_mouse_pos(surface, point.x, bounds.height - point.y, modsFromEvent(event))
|
||||
|
|
@ -5017,6 +5023,16 @@ private final class GhosttyPassthroughVisualEffectView: NSVisualEffectView {
|
|||
}
|
||||
|
||||
final class GhosttySurfaceScrollView: NSView {
|
||||
enum FlashStyle {
|
||||
case standardFocus
|
||||
case notificationDismiss
|
||||
}
|
||||
|
||||
private enum NotificationRingMetrics {
|
||||
static let inset: CGFloat = 2
|
||||
static let cornerRadius: CGFloat = 6
|
||||
}
|
||||
|
||||
private let backgroundView: NSView
|
||||
private let scrollView: GhosttyScrollView
|
||||
private let documentView: NSView
|
||||
|
|
@ -5457,7 +5473,7 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
_ = setFrameIfNeeded(notificationRingOverlayView, to: bounds)
|
||||
_ = setFrameIfNeeded(flashOverlayView, to: bounds)
|
||||
updateNotificationRingPath()
|
||||
updateFlashPath()
|
||||
updateFlashPath(style: .standardFocus)
|
||||
synchronizeScrollView()
|
||||
synchronizeSurfaceView()
|
||||
let didCoreSurfaceChange = synchronizeCoreSurface()
|
||||
|
|
@ -5892,7 +5908,7 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
}
|
||||
#endif
|
||||
|
||||
func triggerFlash() {
|
||||
func triggerFlash(style: FlashStyle = .standardFocus) {
|
||||
DispatchQueue.main.async { [weak self] in
|
||||
guard let self else { return }
|
||||
#if DEBUG
|
||||
|
|
@ -5900,7 +5916,7 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
Self.recordFlash(for: surfaceId)
|
||||
}
|
||||
#endif
|
||||
self.updateFlashPath()
|
||||
self.updateFlashPath(style: style)
|
||||
self.flashLayer.removeAllAnimations()
|
||||
self.flashLayer.opacity = 0
|
||||
let animation = CAKeyframeAnimation(keyPath: "opacity")
|
||||
|
|
@ -6642,17 +6658,27 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
updateOverlayRingPath(
|
||||
layer: notificationRingLayer,
|
||||
bounds: notificationRingOverlayView.bounds,
|
||||
inset: 2,
|
||||
radius: 6
|
||||
inset: NotificationRingMetrics.inset,
|
||||
radius: NotificationRingMetrics.cornerRadius
|
||||
)
|
||||
}
|
||||
|
||||
private func updateFlashPath() {
|
||||
private func updateFlashPath(style: FlashStyle) {
|
||||
let inset: CGFloat
|
||||
let radius: CGFloat
|
||||
switch style {
|
||||
case .standardFocus:
|
||||
inset = CGFloat(FocusFlashPattern.ringInset)
|
||||
radius = CGFloat(FocusFlashPattern.ringCornerRadius)
|
||||
case .notificationDismiss:
|
||||
inset = NotificationRingMetrics.inset
|
||||
radius = NotificationRingMetrics.cornerRadius
|
||||
}
|
||||
updateOverlayRingPath(
|
||||
layer: flashLayer,
|
||||
bounds: flashOverlayView.bounds,
|
||||
inset: CGFloat(FocusFlashPattern.ringInset),
|
||||
radius: CGFloat(FocusFlashPattern.ringCornerRadius)
|
||||
inset: inset,
|
||||
radius: radius
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -190,6 +190,10 @@ final class TerminalPanel: Panel, ObservableObject {
|
|||
hostedView.triggerFlash()
|
||||
}
|
||||
|
||||
func triggerNotificationDismissFlash() {
|
||||
hostedView.triggerFlash(style: .notificationDismiss)
|
||||
}
|
||||
|
||||
func applyWindowBackgroundIfActive() {
|
||||
surface.applyWindowBackgroundIfActive()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1819,19 +1819,37 @@ class TabManager: ObservableObject {
|
|||
guard !shouldSuppressFlash else { return }
|
||||
guard AppFocusState.isAppActive() else { return }
|
||||
guard let panelId = focusedPanelId(for: tabId) else { return }
|
||||
markPanelReadOnFocusIfActive(tabId: tabId, panelId: panelId)
|
||||
_ = dismissNotificationIfActive(tabId: tabId, surfaceId: panelId, triggerFlash: true)
|
||||
}
|
||||
|
||||
private func markPanelReadOnFocusIfActive(tabId: UUID, panelId: UUID) {
|
||||
guard selectedTabId == tabId else { return }
|
||||
guard !suppressFocusFlash else { return }
|
||||
guard AppFocusState.isAppActive() else { return }
|
||||
guard let notificationStore = AppDelegate.shared?.notificationStore else { return }
|
||||
guard notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: panelId) else { return }
|
||||
if let tab = tabs.first(where: { $0.id == tabId }) {
|
||||
_ = dismissNotificationIfActive(tabId: tabId, surfaceId: panelId, triggerFlash: true)
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
func dismissNotificationOnDirectInteraction(tabId: UUID, surfaceId: UUID?) -> Bool {
|
||||
dismissNotificationIfActive(tabId: tabId, surfaceId: surfaceId, triggerFlash: true)
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
private func dismissNotificationIfActive(
|
||||
tabId: UUID,
|
||||
surfaceId: UUID?,
|
||||
triggerFlash: Bool
|
||||
) -> Bool {
|
||||
guard selectedTabId == tabId else { return false }
|
||||
guard AppFocusState.isAppActive() else { return false }
|
||||
guard let notificationStore = AppDelegate.shared?.notificationStore else { return false }
|
||||
guard notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: surfaceId) else { return false }
|
||||
if triggerFlash,
|
||||
let panelId = surfaceId,
|
||||
let tab = tabs.first(where: { $0.id == tabId }) {
|
||||
tab.triggerNotificationFocusFlash(panelId: panelId, requiresSplit: false, shouldFocus: false)
|
||||
}
|
||||
notificationStore.markRead(forTabId: tabId, surfaceId: panelId)
|
||||
notificationStore.markRead(forTabId: tabId, surfaceId: surfaceId)
|
||||
return true
|
||||
}
|
||||
|
||||
private func enqueuePanelTitleUpdate(tabId: UUID, panelId: UUID, title: String) {
|
||||
|
|
|
|||
|
|
@ -838,14 +838,7 @@ final class TerminalNotificationStore: ObservableObject {
|
|||
let isFocusedSurface = surfaceId == nil || focusedSurfaceId == surfaceId
|
||||
let isFocusedPanel = isActiveTab && isFocusedSurface
|
||||
let isAppFocused = AppFocusState.isAppFocused()
|
||||
if isAppFocused && isFocusedPanel {
|
||||
if !idsToClear.isEmpty {
|
||||
notifications = updated
|
||||
center.removeDeliveredNotificationsOffMain(withIdentifiers: idsToClear)
|
||||
center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear)
|
||||
}
|
||||
return
|
||||
}
|
||||
let shouldSuppressExternalDelivery = isAppFocused && isFocusedPanel
|
||||
|
||||
if WorkspaceAutoReorderSettings.isEnabled() {
|
||||
AppDelegate.shared?.tabManager?.moveTabToTopForNotification(tabId)
|
||||
|
|
@ -867,7 +860,9 @@ final class TerminalNotificationStore: ObservableObject {
|
|||
center.removeDeliveredNotificationsOffMain(withIdentifiers: idsToClear)
|
||||
center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear)
|
||||
}
|
||||
notificationDeliveryHandler(self, notification)
|
||||
if !shouldSuppressExternalDelivery {
|
||||
notificationDeliveryHandler(self, notification)
|
||||
}
|
||||
}
|
||||
|
||||
func markRead(id: UUID) {
|
||||
|
|
|
|||
|
|
@ -3260,7 +3260,7 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
if requiresSplit && !isSplit {
|
||||
return
|
||||
}
|
||||
terminalPanel.triggerFlash()
|
||||
terminalPanel.triggerNotificationDismissFlash()
|
||||
}
|
||||
|
||||
func triggerDebugFlash(panelId: UUID) {
|
||||
|
|
|
|||
|
|
@ -7446,6 +7446,118 @@ final class NotificationDockBadgeTests: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
@MainActor
|
||||
final class TerminalNotificationDirectInteractionTests: XCTestCase {
|
||||
private func makeWindow() -> NSWindow {
|
||||
let window = NSWindow(
|
||||
contentRect: NSRect(x: 0, y: 0, width: 480, height: 320),
|
||||
styleMask: [.titled, .closable],
|
||||
backing: .buffered,
|
||||
defer: false
|
||||
)
|
||||
window.contentView = NSView(frame: window.contentRect(forFrameRect: window.frame))
|
||||
return window
|
||||
}
|
||||
|
||||
private func makeMouseEvent(type: NSEvent.EventType, location: NSPoint, window: NSWindow) -> NSEvent {
|
||||
guard let event = NSEvent.mouseEvent(
|
||||
with: type,
|
||||
location: location,
|
||||
modifierFlags: [],
|
||||
timestamp: ProcessInfo.processInfo.systemUptime,
|
||||
windowNumber: window.windowNumber,
|
||||
context: nil,
|
||||
eventNumber: 0,
|
||||
clickCount: 1,
|
||||
pressure: 1.0
|
||||
) else {
|
||||
fatalError("Failed to create \(type) mouse event")
|
||||
}
|
||||
return event
|
||||
}
|
||||
|
||||
private func surfaceView(in hostedView: GhosttySurfaceScrollView) -> NSView? {
|
||||
hostedView.subviews
|
||||
.compactMap { $0 as? NSScrollView }
|
||||
.first?
|
||||
.documentView?
|
||||
.subviews
|
||||
.first
|
||||
}
|
||||
|
||||
func testTerminalMouseDownDismissesUnreadWhenSurfaceIsAlreadyFirstResponder() {
|
||||
let appDelegate = AppDelegate.shared ?? AppDelegate()
|
||||
let manager = TabManager()
|
||||
let store = TerminalNotificationStore.shared
|
||||
let window = makeWindow()
|
||||
|
||||
let originalTabManager = appDelegate.tabManager
|
||||
let originalNotificationStore = appDelegate.notificationStore
|
||||
let originalAppFocusOverride = AppFocusState.overrideIsFocused
|
||||
|
||||
store.replaceNotificationsForTesting([])
|
||||
store.configureNotificationDeliveryHandlerForTesting { _, _ in }
|
||||
appDelegate.tabManager = manager
|
||||
appDelegate.notificationStore = store
|
||||
|
||||
defer {
|
||||
store.replaceNotificationsForTesting([])
|
||||
store.resetNotificationDeliveryHandlerForTesting()
|
||||
appDelegate.tabManager = originalTabManager
|
||||
appDelegate.notificationStore = originalNotificationStore
|
||||
AppFocusState.overrideIsFocused = originalAppFocusOverride
|
||||
window.orderOut(nil)
|
||||
}
|
||||
|
||||
guard let workspace = manager.selectedWorkspace,
|
||||
let terminalPanel = workspace.focusedTerminalPanel else {
|
||||
XCTFail("Expected an initial focused terminal panel")
|
||||
return
|
||||
}
|
||||
|
||||
guard let contentView = window.contentView else {
|
||||
XCTFail("Expected content view")
|
||||
return
|
||||
}
|
||||
|
||||
let hostedView = terminalPanel.hostedView
|
||||
hostedView.frame = contentView.bounds
|
||||
hostedView.autoresizingMask = [.width, .height]
|
||||
contentView.addSubview(hostedView)
|
||||
contentView.layoutSubtreeIfNeeded()
|
||||
hostedView.layoutSubtreeIfNeeded()
|
||||
|
||||
guard let surfaceView = surfaceView(in: hostedView) else {
|
||||
XCTFail("Expected terminal surface view")
|
||||
return
|
||||
}
|
||||
|
||||
GhosttySurfaceScrollView.resetFlashCounts()
|
||||
AppFocusState.overrideIsFocused = true
|
||||
XCTAssertTrue(window.makeFirstResponder(surfaceView))
|
||||
|
||||
store.addNotification(
|
||||
tabId: workspace.id,
|
||||
surfaceId: terminalPanel.id,
|
||||
title: "Unread",
|
||||
subtitle: "",
|
||||
body: ""
|
||||
)
|
||||
XCTAssertTrue(store.hasUnreadNotification(forTabId: workspace.id, surfaceId: terminalPanel.id))
|
||||
|
||||
AppFocusState.overrideIsFocused = true
|
||||
let pointInWindow = surfaceView.convert(NSPoint(x: 20, y: 20), to: nil)
|
||||
let event = makeMouseEvent(type: .leftMouseDown, location: pointInWindow, window: window)
|
||||
surfaceView.mouseDown(with: event)
|
||||
let drained = expectation(description: "flash drained")
|
||||
DispatchQueue.main.async { drained.fulfill() }
|
||||
wait(for: [drained], timeout: 1.0)
|
||||
|
||||
XCTAssertFalse(store.hasUnreadNotification(forTabId: workspace.id, surfaceId: terminalPanel.id))
|
||||
XCTAssertEqual(GhosttySurfaceScrollView.flashCount(for: terminalPanel.id), 1)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
final class MenuBarBadgeLabelFormatterTests: XCTestCase {
|
||||
func testBadgeLabelFormatting() {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue