Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
75 changes: 61 additions & 14 deletions mac/Sources/CodeBurnMenubar/CodeBurnApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@ struct CodeBurnApp: App {
}

@MainActor
final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {
final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate, NSMenuDelegate {
private var statusItem: NSStatusItem!
private var popover: NSPopover!
private var rightClickMonitor: Any?
private var lastContextMenuPresentedAt: Date = .distantPast
/// Held only while the right-click menu is open. Cleared in menuDidClose so
/// left-click goes back to the popover action instead of re-showing the menu.
private var contextMenu: NSMenu?
fileprivate let store = AppStore()
let updateChecker = UpdateChecker()
/// True while the displays are asleep. Refresh ticks skip spawning
Expand Down Expand Up @@ -909,15 +912,25 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {
button.sendAction(on: [.leftMouseUp, .rightMouseUp])

// macOS 27 no longer routes any right-mouse event to the status-item
// button's target/action. A global monitor still observes right-mouse-down;
// button's target/action. A global monitor still observes right-mouse-up;
// we hit-test it against our own status-item window and present the menu
// ourselves. Harmless and stable on 15/26 too (the debounce in
// showContextMenu prevents a double-present if the legacy path also fires).
rightClickMonitor = NSEvent.addGlobalMonitorForEvents(matching: [.rightMouseDown]) { [weak self] _ in
// ourselves.
//
// Must be mouse-*up*, not mouse-down: presenting on down starts menu
// tracking while the button is still held, so the matching rightMouseUp
// is treated as an outside click and the menu flashes then dismisses.
// Presenting on up (after the click completes) keeps it open. Harmless
// on 15/26 too (the debounce in showContextMenu prevents a double-present
// if the legacy path also fires).
rightClickMonitor = NSEvent.addGlobalMonitorForEvents(matching: StatusItemContextMenuPolicy.presentEventMask) { [weak self] _ in
guard let self,
let button = self.statusItem.button,
let window = button.window,
window.frame.contains(NSEvent.mouseLocation) else { return }
// Defer one turn so menu presentation is not nested inside the
// monitor callback. Safe on mouse-*up* (the click is already
// complete); on mouse-down a deferred present was killed by the
// matching up event and the menu only flashed.
DispatchQueue.main.async { self.showContextMenu(from: button) }
}

Expand Down Expand Up @@ -1126,9 +1139,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {
private func showContextMenu(from button: NSStatusBarButton) {
// Debounce: on macOS <= 26 both the legacy action path and the global
// monitor can fire for a single right-click. Present at most once per click.
let now = Date()
guard now.timeIntervalSince(lastContextMenuPresentedAt) > 0.3 else { return }
lastContextMenuPresentedAt = now
// Policy lives in StatusItemContextMenuPolicy so the gate is unit-tested (#802).
guard StatusItemContextMenuPolicy.acceptPresent(
now: Date(),
lastPresentedAt: &lastContextMenuPresentedAt
) else { return }

// Don't let an open popover steal the click / sit under the menu.
if popover?.isShown == true {
popover.performClose(nil)
}

let menu = NSMenu()

Expand Down Expand Up @@ -1160,12 +1180,39 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate {
quitItem.target = self
menu.addItem(quitItem)

// Present directly. The previous `statusItem.menu = menu; button.performClick`
// trick relies on the click -> action path that macOS 27 changed; popUp is
// version-stable. Open a few px below the status item so the menu clears the
// menu bar: anchoring flush clips the top edge and makes macOS engage menu
// scrolling (a scroll chevron appears and the first row slides up on hover).
menu.popUp(positioning: nil, at: NSPoint(x: 0, y: button.bounds.height + 6), in: button)
// Present via the status item's own menu slot. AppKit positions and tracks
// that menu correctly under the status item (no scroll chevron / first-row
// jump). Manual NSMenu.popUp(at:in:) is what caused the jump: the menu was
// tracked against a point while the cursor still sat on the status item
// above it, so the first mouse move engaged scroll mode.
//
// #472 dropped this pattern because assigning `statusItem.menu` from the
// right-mouse *action* never ran on macOS 27 (right-clicks no longer reach
// the button action). We still open from our global rightMouseUp monitor
// (or the legacy action on ≤26); once we set `statusItem.menu` ourselves,
// performClick is just "open the attached menu" and works on 27 too.
//
// menuDidClose clears `statusItem.menu` so the next left-click hits our
// action (popover) instead of re-opening this menu.
menu.delegate = self
contextMenu = menu
statusItem.menu = menu
button.performClick(nil)
}

// MARK: - NSMenuDelegate

// AppKit invokes menu callbacks on the main thread. Clear the status-item
// menu slot so the next left-click hits our action (popover) again.
nonisolated func menuDidClose(_ menu: NSMenu) {
// Hop explicitly — don't assumeIsolated across the NSMenuDelegate boundary
// under Swift 6 strict concurrency (NSMenu isn't Sendable).
DispatchQueue.main.async { [weak self] in
guard let self else { return }
// Always clear: we only ever attach our own context menu to the item.
self.statusItem.menu = nil
self.contextMenu = nil
}
}

/// One-line "today" summary for the context menu's usage row.
Expand Down
47 changes: 47 additions & 0 deletions mac/Sources/CodeBurnMenubar/StatusItemContextMenuPolicy.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import AppKit
import Foundation

/// Pure policy for the status-item right-click menu (#802).
///
/// Keeps the event-mask, debounce, and presentation choices out of AppDelegate
/// so they can be unit-tested without spinning up an NSStatusItem. The bugs this
/// encodes:
///
/// 1. **Flash** — presenting on `rightMouseDown` lets the matching `rightMouseUp`
/// dismiss the menu immediately. Monitor (and legacy action) use mouse-*up*.
/// 2. **Jump/scroll** — manual `NSMenu.popUp(at:in:)` tracks poorly while the
/// cursor still sits on the status item above the menu. Attach via
/// `statusItem.menu` + `performClick` instead.
/// 3. **Double-present** — on macOS ≤26 both the button action and the global
/// monitor can fire for one click; debounce collapses them.
enum StatusItemContextMenuPolicy {
/// Global-monitor / action event. Must be mouse-up (see flash note above).
static let presentEventMask: NSEvent.EventTypeMask = .rightMouseUp

/// Minimum gap between presents. Covers the dual-path race on macOS ≤26.
static let presentDebounceSeconds: TimeInterval = 0.3

/// How the menu is shown once a present is accepted.
enum Presentation: Equatable {
/// Assign `statusItem.menu` then `button.performClick`. AppKit positions
/// and tracks the menu under the status item. Clear menu in menuDidClose.
case statusItemMenu
/// Manual `NSMenu.popUp(at:in:)`. Causes scroll-chevron jump on mouse move
/// when the cursor starts above the menu. Kept only as a named anti-pattern
/// so tests can lock that we do *not* use it.
case manualPopUp
}

static let presentation: Presentation = .statusItemMenu

/// Returns true and advances `lastPresentedAt` when a new present is allowed.
static func acceptPresent(
now: Date,
lastPresentedAt: inout Date,
debounce: TimeInterval = presentDebounceSeconds
) -> Bool {
guard now.timeIntervalSince(lastPresentedAt) > debounce else { return false }
lastPresentedAt = now
return true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import AppKit
import XCTest
@testable import CodeBurnMenubar

/// Locks the right-click menu policy that fixed flash + scroll-jump (#802).
/// Does not drive a real NSStatusItem — that needs manual/AppKit integration.
final class StatusItemContextMenuPolicyTests: XCTestCase {
func testPresentEventMaskIsRightMouseUpNotDown() {
// Presenting on mouse-down lets the matching up dismiss the menu (flash).
XCTAssertEqual(
StatusItemContextMenuPolicy.presentEventMask,
NSEvent.EventTypeMask.rightMouseUp
)
XCTAssertNotEqual(
StatusItemContextMenuPolicy.presentEventMask,
NSEvent.EventTypeMask.rightMouseDown
)
// Mask must include up and must not include down (single-bit masks here).
XCTAssertTrue(StatusItemContextMenuPolicy.presentEventMask.contains(.rightMouseUp))
XCTAssertFalse(StatusItemContextMenuPolicy.presentEventMask.contains(.rightMouseDown))
}

func testPresentationUsesStatusItemMenuNotManualPopUp() {
// Manual popUp tracks against a point while the cursor sits on the status
// item above the menu → scroll chevron / Today-row jump on mouse move.
XCTAssertEqual(
StatusItemContextMenuPolicy.presentation,
.statusItemMenu
)
XCTAssertNotEqual(
StatusItemContextMenuPolicy.presentation,
.manualPopUp
)
}

func testDebounceAcceptsFirstPresent() {
var last = Date.distantPast
let now = Date(timeIntervalSince1970: 1_000)
XCTAssertTrue(
StatusItemContextMenuPolicy.acceptPresent(now: now, lastPresentedAt: &last)
)
XCTAssertEqual(last, now)
}

func testDebounceRejectsWithinWindow() {
let t0 = Date(timeIntervalSince1970: 1_000)
var last = t0
// Just inside the 0.3s window
let t1 = t0.addingTimeInterval(0.299)
XCTAssertFalse(
StatusItemContextMenuPolicy.acceptPresent(now: t1, lastPresentedAt: &last)
)
XCTAssertEqual(last, t0, "reject must not advance lastPresentedAt")
}

func testDebounceAcceptsAfterWindow() {
let t0 = Date(timeIntervalSince1970: 1_000)
var last = t0
let t1 = t0.addingTimeInterval(StatusItemContextMenuPolicy.presentDebounceSeconds + 0.001)
XCTAssertTrue(
StatusItemContextMenuPolicy.acceptPresent(now: t1, lastPresentedAt: &last)
)
XCTAssertEqual(last, t1)
}

func testDebounceBoundaryIsStrictlyGreaterThan() {
// Gate uses `>` not `>=`: exactly debounce seconds later is still rejected.
let t0 = Date(timeIntervalSince1970: 1_000)
var last = t0
let exact = t0.addingTimeInterval(StatusItemContextMenuPolicy.presentDebounceSeconds)
XCTAssertFalse(
StatusItemContextMenuPolicy.acceptPresent(now: exact, lastPresentedAt: &last)
)
XCTAssertEqual(last, t0)
}

func testCustomDebounceOverride() {
var last = Date(timeIntervalSince1970: 0)
let now = Date(timeIntervalSince1970: 0.5)
XCTAssertFalse(
StatusItemContextMenuPolicy.acceptPresent(now: now, lastPresentedAt: &last, debounce: 1.0)
)
XCTAssertTrue(
StatusItemContextMenuPolicy.acceptPresent(now: now, lastPresentedAt: &last, debounce: 0.4)
)
}
}