Fix Mark Tab as Unread no-op on focused tab (#249)
* Fix manual unread clear race on focused tab * Add mark-as-read tab action and show ring for manual unread * Flash then clear manual unread on tab focus
This commit is contained in:
parent
12374c4a76
commit
ede5b701bb
7 changed files with 189 additions and 7 deletions
|
|
@ -3031,7 +3031,7 @@ struct CMUXCLI {
|
|||
new-terminal-right | new-browser-right
|
||||
reload | duplicate
|
||||
pin | unpin
|
||||
mark-unread
|
||||
mark-read | mark-unread
|
||||
|
||||
Flags:
|
||||
--action <name> Action name (required if not positional)
|
||||
|
|
|
|||
|
|
@ -2046,7 +2046,7 @@ class TerminalController {
|
|||
"close_left", "close_right", "close_others",
|
||||
"new_terminal_right", "new_browser_right",
|
||||
"reload", "duplicate",
|
||||
"pin", "unpin", "mark_unread"
|
||||
"pin", "unpin", "mark_read", "mark_unread"
|
||||
]
|
||||
|
||||
var result: V2CallResult = .err(code: "invalid_params", message: "Unknown tab action", data: [
|
||||
|
|
@ -2160,6 +2160,10 @@ class TerminalController {
|
|||
workspace.setPanelPinned(panelId: surfaceId, pinned: false)
|
||||
finish(["pinned": false])
|
||||
|
||||
case "mark_read", "mark_as_read":
|
||||
workspace.markPanelRead(surfaceId)
|
||||
finish()
|
||||
|
||||
case "mark_unread", "mark_as_unread":
|
||||
workspace.markPanelUnread(surfaceId)
|
||||
finish()
|
||||
|
|
|
|||
|
|
@ -103,6 +103,9 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
@Published private(set) var panelCustomTitles: [UUID: String] = [:]
|
||||
@Published private(set) var pinnedPanelIds: Set<UUID> = []
|
||||
@Published private(set) var manualUnreadPanelIds: Set<UUID> = []
|
||||
private var manualUnreadMarkedAt: [UUID: Date] = [:]
|
||||
nonisolated private static let manualUnreadFocusGraceInterval: TimeInterval = 0.2
|
||||
nonisolated private static let manualUnreadClearDelayAfterFocusFlash: TimeInterval = 0.2
|
||||
@Published var statusEntries: [String: SidebarStatusEntry] = [:]
|
||||
@Published var logEntries: [SidebarLogEntry] = []
|
||||
@Published var progress: SidebarProgressState?
|
||||
|
|
@ -388,7 +391,10 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
|
||||
private func syncUnreadBadgeStateForPanel(_ panelId: UUID) {
|
||||
guard let tabId = surfaceIdFromPanelId(panelId) else { return }
|
||||
let shouldShowUnread = manualUnreadPanelIds.contains(panelId) || hasUnreadNotification(panelId: panelId)
|
||||
let shouldShowUnread = Self.shouldShowUnreadIndicator(
|
||||
hasUnreadNotification: hasUnreadNotification(panelId: panelId),
|
||||
isManuallyUnread: manualUnreadPanelIds.contains(panelId)
|
||||
)
|
||||
if let existing = bonsplitController.tab(tabId), existing.showsNotificationBadge == shouldShowUnread {
|
||||
return
|
||||
}
|
||||
|
|
@ -481,14 +487,45 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
func markPanelUnread(_ panelId: UUID) {
|
||||
guard panels[panelId] != nil else { return }
|
||||
guard manualUnreadPanelIds.insert(panelId).inserted else { return }
|
||||
manualUnreadMarkedAt[panelId] = Date()
|
||||
syncUnreadBadgeStateForPanel(panelId)
|
||||
}
|
||||
|
||||
func markPanelRead(_ panelId: UUID) {
|
||||
guard panels[panelId] != nil else { return }
|
||||
AppDelegate.shared?.notificationStore?.markRead(forTabId: id, surfaceId: panelId)
|
||||
clearManualUnread(panelId: panelId)
|
||||
}
|
||||
|
||||
func clearManualUnread(panelId: UUID) {
|
||||
guard manualUnreadPanelIds.remove(panelId) != nil else { return }
|
||||
let didRemoveUnread = manualUnreadPanelIds.remove(panelId) != nil
|
||||
manualUnreadMarkedAt.removeValue(forKey: panelId)
|
||||
guard didRemoveUnread else { return }
|
||||
syncUnreadBadgeStateForPanel(panelId)
|
||||
}
|
||||
|
||||
static func shouldClearManualUnread(
|
||||
previousFocusedPanelId: UUID?,
|
||||
nextFocusedPanelId: UUID,
|
||||
isManuallyUnread: Bool,
|
||||
markedAt: Date?,
|
||||
now: Date = Date(),
|
||||
sameTabGraceInterval: TimeInterval = manualUnreadFocusGraceInterval
|
||||
) -> Bool {
|
||||
guard isManuallyUnread else { return false }
|
||||
|
||||
if let previousFocusedPanelId, previousFocusedPanelId != nextFocusedPanelId {
|
||||
return true
|
||||
}
|
||||
|
||||
guard let markedAt else { return true }
|
||||
return now.timeIntervalSince(markedAt) >= sameTabGraceInterval
|
||||
}
|
||||
|
||||
static func shouldShowUnreadIndicator(hasUnreadNotification: Bool, isManuallyUnread: Bool) -> Bool {
|
||||
hasUnreadNotification || isManuallyUnread
|
||||
}
|
||||
|
||||
// MARK: - Title Management
|
||||
|
||||
var hasCustomTitle: Bool {
|
||||
|
|
@ -571,6 +608,7 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
panelCustomTitles = panelCustomTitles.filter { validSurfaceIds.contains($0.key) }
|
||||
pinnedPanelIds = pinnedPanelIds.filter { validSurfaceIds.contains($0) }
|
||||
manualUnreadPanelIds = manualUnreadPanelIds.filter { validSurfaceIds.contains($0) }
|
||||
manualUnreadMarkedAt = manualUnreadMarkedAt.filter { validSurfaceIds.contains($0.key) }
|
||||
surfaceListeningPorts = surfaceListeningPorts.filter { validSurfaceIds.contains($0.key) }
|
||||
surfaceTTYNames = surfaceTTYNames.filter { validSurfaceIds.contains($0.key) }
|
||||
recomputeListeningPorts()
|
||||
|
|
@ -1157,8 +1195,10 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
}
|
||||
if detached.manuallyUnread {
|
||||
manualUnreadPanelIds.insert(detached.panelId)
|
||||
manualUnreadMarkedAt[detached.panelId] = .distantPast
|
||||
} else {
|
||||
manualUnreadPanelIds.remove(detached.panelId)
|
||||
manualUnreadMarkedAt.removeValue(forKey: detached.panelId)
|
||||
}
|
||||
|
||||
guard let newTabId = bonsplitController.createTab(
|
||||
|
|
@ -1178,6 +1218,7 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
panelCustomTitles.removeValue(forKey: detached.panelId)
|
||||
pinnedPanelIds.remove(detached.panelId)
|
||||
manualUnreadPanelIds.remove(detached.panelId)
|
||||
manualUnreadMarkedAt.removeValue(forKey: detached.panelId)
|
||||
panelSubscriptions.removeValue(forKey: detached.panelId)
|
||||
return nil
|
||||
}
|
||||
|
|
@ -1611,6 +1652,7 @@ extension Workspace: BonsplitDelegate {
|
|||
}
|
||||
|
||||
private func applyTabSelectionNow(tabId: TabID, inPane pane: PaneID) {
|
||||
let previousFocusedPanelId = focusedPanelId
|
||||
if bonsplitController.allPaneIds.contains(pane) {
|
||||
if bonsplitController.focusedPaneId != pane {
|
||||
bonsplitController.focusPane(pane)
|
||||
|
|
@ -1650,7 +1692,24 @@ extension Workspace: BonsplitDelegate {
|
|||
}
|
||||
|
||||
panel.focus()
|
||||
clearManualUnread(panelId: panelId)
|
||||
let isManuallyUnread = manualUnreadPanelIds.contains(panelId)
|
||||
let markedAt = manualUnreadMarkedAt[panelId]
|
||||
if Self.shouldClearManualUnread(
|
||||
previousFocusedPanelId: previousFocusedPanelId,
|
||||
nextFocusedPanelId: panelId,
|
||||
isManuallyUnread: isManuallyUnread,
|
||||
markedAt: markedAt
|
||||
) {
|
||||
triggerFocusFlash(panelId: panelId)
|
||||
let clearDelay = Self.manualUnreadClearDelayAfterFocusFlash
|
||||
if clearDelay <= 0 {
|
||||
clearManualUnread(panelId: panelId)
|
||||
} else {
|
||||
DispatchQueue.main.asyncAfter(deadline: .now() + clearDelay) { [weak self] in
|
||||
self?.clearManualUnread(panelId: panelId)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Converge AppKit first responder with bonsplit's selected tab in the focused pane.
|
||||
// Without this, keyboard input can remain on a different terminal than the blue tab indicator.
|
||||
|
|
@ -1803,6 +1862,7 @@ extension Workspace: BonsplitDelegate {
|
|||
panelCustomTitles.removeValue(forKey: panelId)
|
||||
pinnedPanelIds.remove(panelId)
|
||||
manualUnreadPanelIds.remove(panelId)
|
||||
manualUnreadMarkedAt.removeValue(forKey: panelId)
|
||||
panelSubscriptions.removeValue(forKey: panelId)
|
||||
surfaceTTYNames.removeValue(forKey: panelId)
|
||||
PortScanner.shared.unregisterPanel(workspaceId: id, panelId: panelId)
|
||||
|
|
@ -2118,6 +2178,9 @@ extension Workspace: BonsplitDelegate {
|
|||
guard let panelId = panelIdFromSurfaceId(tab.id) else { return }
|
||||
let shouldPin = !pinnedPanelIds.contains(panelId)
|
||||
setPanelPinned(panelId: panelId, pinned: shouldPin)
|
||||
case .markAsRead:
|
||||
guard let panelId = panelIdFromSurfaceId(tab.id) else { return }
|
||||
markPanelRead(panelId)
|
||||
case .markAsUnread:
|
||||
guard let panelId = panelIdFromSurfaceId(tab.id) else { return }
|
||||
markPanelUnread(panelId)
|
||||
|
|
|
|||
|
|
@ -42,7 +42,10 @@ struct WorkspaceContentView: View {
|
|||
let isFocused = isWorkspaceInputActive && workspace.focusedPanelId == panel.id
|
||||
let isSelectedInPane = workspace.bonsplitController.selectedTab(inPane: paneId)?.id == tab.id
|
||||
let isVisibleInUI = isWorkspaceVisible && isSelectedInPane
|
||||
let hasUnreadNotification = notificationStore.hasUnreadNotification(forTabId: workspace.id, surfaceId: panel.id)
|
||||
let hasUnreadNotification = Workspace.shouldShowUnreadIndicator(
|
||||
hasUnreadNotification: notificationStore.hasUnreadNotification(forTabId: workspace.id, surfaceId: panel.id),
|
||||
isManuallyUnread: workspace.manualUnreadPanelIds.contains(panel.id)
|
||||
)
|
||||
PanelContentView(
|
||||
panel: panel,
|
||||
isFocused: isFocused,
|
||||
|
|
|
|||
108
cmuxTests/WorkspaceManualUnreadTests.swift
Normal file
108
cmuxTests/WorkspaceManualUnreadTests.swift
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
import XCTest
|
||||
|
||||
#if canImport(cmux_DEV)
|
||||
@testable import cmux_DEV
|
||||
#elseif canImport(cmux)
|
||||
@testable import cmux
|
||||
#endif
|
||||
|
||||
final class WorkspaceManualUnreadTests: XCTestCase {
|
||||
func testShouldClearManualUnreadWhenFocusMovesToDifferentPanel() {
|
||||
let previousFocusedPanelId = UUID()
|
||||
let nextFocusedPanelId = UUID()
|
||||
|
||||
XCTAssertTrue(
|
||||
Workspace.shouldClearManualUnread(
|
||||
previousFocusedPanelId: previousFocusedPanelId,
|
||||
nextFocusedPanelId: nextFocusedPanelId,
|
||||
isManuallyUnread: true,
|
||||
markedAt: Date()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testShouldNotClearManualUnreadWhenFocusStaysOnSamePanelWithinGrace() {
|
||||
let panelId = UUID()
|
||||
let now = Date()
|
||||
|
||||
XCTAssertFalse(
|
||||
Workspace.shouldClearManualUnread(
|
||||
previousFocusedPanelId: panelId,
|
||||
nextFocusedPanelId: panelId,
|
||||
isManuallyUnread: true,
|
||||
markedAt: now.addingTimeInterval(-0.05),
|
||||
now: now,
|
||||
sameTabGraceInterval: 0.2
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testShouldClearManualUnreadWhenFocusStaysOnSamePanelAfterGrace() {
|
||||
let panelId = UUID()
|
||||
let now = Date()
|
||||
|
||||
XCTAssertTrue(
|
||||
Workspace.shouldClearManualUnread(
|
||||
previousFocusedPanelId: panelId,
|
||||
nextFocusedPanelId: panelId,
|
||||
isManuallyUnread: true,
|
||||
markedAt: now.addingTimeInterval(-0.25),
|
||||
now: now,
|
||||
sameTabGraceInterval: 0.2
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testShouldNotClearManualUnreadWhenNotManuallyUnread() {
|
||||
XCTAssertFalse(
|
||||
Workspace.shouldClearManualUnread(
|
||||
previousFocusedPanelId: UUID(),
|
||||
nextFocusedPanelId: UUID(),
|
||||
isManuallyUnread: false,
|
||||
markedAt: Date()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testShouldNotClearManualUnreadWhenNoPreviousFocusAndWithinGrace() {
|
||||
let now = Date()
|
||||
|
||||
XCTAssertFalse(
|
||||
Workspace.shouldClearManualUnread(
|
||||
previousFocusedPanelId: nil,
|
||||
nextFocusedPanelId: UUID(),
|
||||
isManuallyUnread: true,
|
||||
markedAt: now.addingTimeInterval(-0.05),
|
||||
now: now,
|
||||
sameTabGraceInterval: 0.2
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testShouldShowUnreadIndicatorWhenNotificationIsUnread() {
|
||||
XCTAssertTrue(
|
||||
Workspace.shouldShowUnreadIndicator(
|
||||
hasUnreadNotification: true,
|
||||
isManuallyUnread: false
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testShouldShowUnreadIndicatorWhenManualUnreadIsSet() {
|
||||
XCTAssertTrue(
|
||||
Workspace.shouldShowUnreadIndicator(
|
||||
hasUnreadNotification: false,
|
||||
isManuallyUnread: true
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
func testShouldHideUnreadIndicatorWhenNeitherNotificationNorManualUnreadExists() {
|
||||
XCTAssertFalse(
|
||||
Workspace.shouldShowUnreadIndicator(
|
||||
hasUnreadNotification: false,
|
||||
isManuallyUnread: false
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
@ -146,6 +146,10 @@ def main() -> int:
|
|||
by_tab_only = c._call("tab.action", {"tab_id": tab_ref, "action": "mark_unread"}) or {}
|
||||
_must(str(by_tab_only.get("tab_ref") or "").startswith("tab:"), f"Expected tab_ref in tab_id-only result: {by_tab_only}")
|
||||
_must(str(by_tab_only.get("workspace_id") or "") == ws_id, f"tab_id-only action should resolve target workspace: {by_tab_only}")
|
||||
|
||||
mark_read = c._call("tab.action", {"tab_id": tab_ref, "action": "mark_read"}) or {}
|
||||
_must(str(mark_read.get("tab_ref") or "").startswith("tab:"), f"Expected tab_ref in mark_read result: {mark_read}")
|
||||
_must(str(mark_read.get("workspace_id") or "") == ws_id, f"mark_read should resolve target workspace: {mark_read}")
|
||||
finally:
|
||||
if ws_other:
|
||||
try:
|
||||
|
|
|
|||
2
vendor/bonsplit
vendored
2
vendor/bonsplit
vendored
|
|
@ -1 +1 @@
|
|||
Subproject commit cf929c887af79ea8b881e39da5b8c4ee1d6b9009
|
||||
Subproject commit 198736e4e2db10931c263eb221b2592fc86e80e7
|
||||
Loading…
Add table
Add a link
Reference in a new issue