Skip to content

Fix workspace unread dismissal on explicit tab focus #1001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lawrencecchen wants to merge 2 commits into from
Closed

Fix workspace unread dismissal on explicit tab focus #1001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12543db
test: restore unread dismissal expectations
lawrencecchen Mar 6, 2026
740762d
fix: dismiss unread on explicit tab focus
lawrencecchen Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions Sources/TabManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1601,8 +1601,17 @@ class TabManager: ObservableObject {
suppressFocusFlash = false
guard !shouldSuppressFlash else { return }
guard AppFocusState.isAppActive() else { return }
guard let panelId = focusedPanelId(for: tabId) else { return }
markPanelReadOnFocusIfActive(tabId: tabId, panelId: panelId)
if let panelId = focusedPanelId(for: tabId) {
guard let notificationStore = AppDelegate.shared?.notificationStore else { return }
if notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: panelId) {
markPanelReadOnFocusIfActive(tabId: tabId, panelId: panelId)
return
}
}
guard selectedTabId == tabId else { return }
guard let notificationStore = AppDelegate.shared?.notificationStore else { return }
guard notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: nil) else { return }
notificationStore.markRead(forTabId: tabId, surfaceId: nil)
}

private func markPanelReadOnFocusIfActive(tabId: UUID, panelId: UUID) {
Expand Down Expand Up @@ -1738,7 +1747,6 @@ class TabManager: ObservableObject {
guard let targetPanelId,
tab.panels[targetPanelId] != nil else { return }
guard let notificationStore = AppDelegate.shared?.notificationStore else { return }
guard notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: targetPanelId) else { return }
tab.triggerNotificationFocusFlash(panelId: targetPanelId, requiresSplit: false, shouldFocus: true)
notificationStore.markRead(forTabId: tabId, surfaceId: targetPanelId)
}
Expand Down
47 changes: 47 additions & 0 deletions cmuxTests/CmuxWebViewKeyEquivalentTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7412,6 +7412,53 @@ final class NotificationDockBadgeTests: XCTestCase {
XCTAssertEqual(store.latestNotification(forTabId: tabB)?.id, notificationBUnread.id)
}

func testSelectingWorkspaceMarksSurfaceLessNotificationRead() {
let store = TerminalNotificationStore.shared
let previousAppDelegate = AppDelegate.shared
let previousFocusOverride = AppFocusState.overrideIsFocused
let appDelegate = AppDelegate()
let tabManager = TabManager()
defer {
AppDelegate.shared = previousAppDelegate
AppFocusState.overrideIsFocused = previousFocusOverride
}
appDelegate.tabManager = tabManager
appDelegate.notificationStore = store
AppDelegate.shared = appDelegate
AppFocusState.overrideIsFocused = true

guard let originalTabId = tabManager.selectedTabId else {
XCTFail("Expected selected tab for workspace-level notification selection test")
return
}
guard let originalWorkspace = tabManager.tabs.first(where: { $0.id == originalTabId }) else {
XCTFail("Expected original workspace for workspace-level notification selection test")
return
}

let notification = TerminalNotification(
id: UUID(),
tabId: originalTabId,
surfaceId: nil,
title: "Unread",
subtitle: "",
body: "should clear after explicit workspace selection",
createdAt: Date(),
isRead: false
)
store.replaceNotificationsForTesting([notification])

_ = tabManager.addWorkspace(select: true)
tabManager.selectWorkspace(originalWorkspace)

let drained = expectation(description: "workspace-level selection side effects drained")
DispatchQueue.main.async { drained.fulfill() }
wait(for: [drained], timeout: 1.0)

XCTAssertEqual(tabManager.selectedTabId, originalTabId)
XCTAssertFalse(store.hasUnreadNotification(forTabId: originalTabId, surfaceId: nil))
XCTAssertTrue(store.notifications[0].isRead)
}
func testNotificationIndexesUpdateAfterReadAndClearMutations() {
Comment on lines +7461 to 7462

@greptile-apps greptile-apps Bot Mar 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add a blank line between test methods for consistency with surrounding tests:

Suggested change
}
func testNotificationIndexesUpdateAfterReadAndClearMutations() {
}
func testNotificationIndexesUpdateAfterReadAndClearMutations() {

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

let tab = UUID()
let surfaceUnread = UUID()
Expand Down