Add Jump to Latest to the notifications popover - #1167

Sources/Update/UpdateTitlebarAccessory.swift (1)

979-996: Consider extracting shared shortcut decoding logic.

The jumpToUnreadShortcut, hasUnreadNotifications, and decodeShortcut(from:fallback:) implementations are duplicated between NotificationsPopoverView and NotificationsPage (lines 134-151 in NotificationsPage.swift). Consider extracting these to a shared extension or helper to reduce duplication.

♻️ Suggested shared helper location

You could add these as extensions on TerminalNotificationStore or create a small helper struct:

// e.g., in a shared file or extension
extension TerminalNotificationStore {
    var hasUnreadNotifications: Bool {
        notifications.contains(where: { !$0.isRead })
    }
}

// For shortcut decoding, a static helper could work:
enum ShortcutDecoder {
    static func decode(from data: Data, fallback: StoredShortcut) -> StoredShortcut {
        guard !data.isEmpty,
              let shortcut = try? JSONDecoder().decode(StoredShortcut.self, from: data) else {
            return fallback
        }
        return shortcut
    }
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Sources/Update/UpdateTitlebarAccessory.swift` around lines 979 - 996, Extract
the duplicated logic for shortcut decoding and unread checks into a shared
helper: move decodeShortcut(from:fallback:) and jumpToUnreadShortcut usage out
of NotificationsPopoverView and NotificationsPage into a reusable utility (e.g.,
a ShortcutDecoder type with static func decode(from:fallback:) or similar) and
move hasUnreadNotifications into an extension on TerminalNotificationStore as
var hasUnreadNotifications: Bool; then update NotificationsPopoverView and
NotificationsPage to call ShortcutDecoder.decode(...) and
TerminalNotificationStore.hasUnreadNotifications respectively so the duplicated
implementations (jumpToUnreadShortcut, hasUnreadNotifications,
decodeShortcut(from:fallback:)) are removed.

cmuxUITests/MultiWindowNotificationsUITests.swift (1)

186-188: Consider documenting the hardcoded shortcut assumption.

The assertion XCTAssertEqual(shortcutBadge.label, "⌘⇧U") assumes the default shortcut for jumpToUnread is Cmd+Shift+U. This is fine for validating default behavior, but consider adding a brief comment noting this tests the default shortcut in case the default changes in the future.

 let shortcutBadge = app.staticTexts["notificationsPopover.jumpToLatest.shortcut"]
 XCTAssertTrue(shortcutBadge.waitForExistence(timeout: 2.0), "Expected Jump to Latest shortcut badge")
+// Validates default jumpToUnread shortcut (Cmd+Shift+U)
 XCTAssertEqual(shortcutBadge.label, "⌘⇧U")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmuxUITests/MultiWindowNotificationsUITests.swift` around lines 186 - 188,
Add a short inline comment above the Shortcut assertion to document that this
test expects the app's default keyboard shortcut for jumpToUnread (accessed via
the UI element identifier "notificationsPopover.jumpToLatest.shortcut" and the
variable shortcutBadge) to be "⌘⇧U"; update the comment to mention that if the
app's default shortcut changes the test should be adjusted accordingly so future
reviewers know this is an intentional hardcoded expectation.

Sources/Update/UpdateTitlebarAccessory.swift (1)

991-997: Consider consolidating duplicate decodeShortcut helper.

This method is duplicated verbatim in at least four files (NotificationsPage.swift, WorkspaceContentView.swift, cmuxApp.swift, and here). Consider extracting it to a shared location (e.g., as a static method on StoredShortcut or KeyboardShortcutSettings) to reduce maintenance burden.

♻️ Suggested consolidation

Add to KeyboardShortcutSettings or StoredShortcut:

extension StoredShortcut {
    static func decode(from data: Data, fallback: StoredShortcut) -> StoredShortcut {
        guard !data.isEmpty,
              let shortcut = try? JSONDecoder().decode(StoredShortcut.self, from: data) else {
            return fallback
        }
        return shortcut
    }
}

Then replace all call sites with:

private var jumpToUnreadShortcut: StoredShortcut {
    StoredShortcut.decode(from: jumpToUnreadShortcutData, fallback: KeyboardShortcutSettings.Action.jumpToUnread.defaultShortcut)
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Sources/Update/UpdateTitlebarAccessory.swift` around lines 991 - 997, Several
files duplicate the decodeShortcut implementation; extract it to a single shared
API such as an extension on StoredShortcut (e.g.,
StoredShortcut.decode(from:fallback:)) and replace the local private func
decodeShortcut(...) implementations with calls to that new static method. Update
callers like the computed property jumpToUnreadShortcut to use
StoredShortcut.decode(from: jumpToUnreadShortcutData, fallback:
KeyboardShortcutSettings.Action.jumpToUnread.defaultShortcut) and remove the
private decodeShortcut function from UpdateTitlebarAccessory.swift and the other
files (NotificationsPage.swift, WorkspaceContentView.swift, cmuxApp.swift).

Sources/Update/UpdateTitlebarAccessory.swift (1)

988-994: Consider centralizing shortcut decoding.

Lines 988-994 are now another copy of the same JSON decode/fallback helper already used in Sources/NotificationsPage.swift, Sources/WorkspaceContentView.swift, and Sources/cmuxApp.swift. Pulling this into a shared helper on KeyboardShortcutSettings or StoredShortcut would reduce drift the next time the stored format changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Sources/Update/UpdateTitlebarAccessory.swift` around lines 988 - 994, The
decodeShortcut(from:fallback:) helper in UpdateTitlebarAccessory (the private
function decodeShortcut) duplicates logic used elsewhere; move this JSON decode
+ fallback logic into a single shared method on either KeyboardShortcutSettings
or StoredShortcut (e.g., StoredShortcut.decode(from:fallback:) or
KeyboardShortcutSettings.decodeShortcut(from:fallback:)) and replace the local
decodeShortcut implementation with calls to that new static/instance helper from
UpdateTitlebarAccessory, NotificationsPage, WorkspaceContentView, and cmuxApp so
all code uses the centralized decoder and avoids drift when the stored format
changes.