Merge pull request #1505 from kaitranntt/kai/fix/1502-1503-bar-multiscreen

fix(bar): keep the menu bar panel on the clicked screen in multi-display setups

macos-bar/Sources/CCSBarApp/SettingsWindowController.swift

@@ -55,7 +55,11 @@ final class SettingsWindowController {
     window.minSize = NSSize(width: 420, height: 520)
     // Reuse the instance on reopen instead of tearing it down on close.
     window.isReleasedWhenClosed = false
-    window.center()
+    // Center on the screen that contains the mouse cursor (= the clicked screen)
+    // rather than NSScreen.main (which is the screen with the key window and may
+    // differ from the display where the menu bar icon was clicked). Falls back to
+    // the primary screen via NSWindow.center() when no match is found.
+    centerOnClickedScreen(window)
 
     let delegate = SettingsWindowDelegate(onClose: { [weak self] in self?.handleClose() })
     window.delegate = delegate
@@ -85,6 +89,31 @@ final class SettingsWindowController {
     window = nil
     delegate = nil
   }
+
+  /// Centers `window` on the screen that contains the mouse cursor at call-time
+  /// (i.e. the display where the user clicked the settings button). This ensures
+  /// the settings window opens on the same display as the menu bar icon that was
+  /// clicked, rather than defaulting to NSScreen.main (the primary / key-window
+  /// screen, which may be a different monitor in a multi-display setup).
+  ///
+  /// Falls back to `window.center()` (primary screen, macOS default) when no
+  /// candidate screen can be determined.
+  private func centerOnClickedScreen(_ window: NSWindow) {
+    let mouseGlobal = NSEvent.mouseLocation
+    let screens = NSScreen.screens
+    guard let target = BarScreenPicker.screen(for: mouseGlobal, in: screens) else {
+      window.center()
+      return
+    }
+    let visibleFrame = target.visibleFrame
+    let windowSize = window.frame.size
+    let x = visibleFrame.midX - windowSize.width / 2
+    let y = visibleFrame.midY - windowSize.height / 2
+    window.setFrameOrigin(NSPoint(
+      x: max(visibleFrame.minX, min(x, visibleFrame.maxX - windowSize.width)),
+      y: max(visibleFrame.minY, min(y, visibleFrame.maxY - windowSize.height))
+    ))
+  }
 }
 
 /// Bridges `NSWindow` close back to the controller so it can restore the

macos-bar/Sources/CCSBarApp/WindowAppearanceForcer.swift

@@ -2,10 +2,14 @@ import SwiftUI
 import AppKit
 import CCSBarCore
 
-/// Zero-size bridge that walks up to the host `NSWindow` and forces its
-/// `appearance` to match the user's chosen `BarAppearance`.
+/// Zero-size bridge that walks up to the host `NSWindow` and:
+///   1. Forces its `appearance` to match the user's chosen `BarAppearance`.
+///   2. Fixes `collectionBehavior` to `.moveToActiveSpace` so the panel is
+///      never visible on multiple Spaces / displays simultaneously (#1503).
+///   3. Anchors the panel to the screen where the user clicked the status item
+///      by reading `NSEvent.mouseLocation` at open-time (#1502).
 ///
-/// Why this exists on top of `.preferredColorScheme`: that modifier only
+/// Why (1) exists on top of `.preferredColorScheme`: that modifier only
 /// rewrites the SwiftUI `\.colorScheme` environment for descendant views — it
 /// does NOT change the host `NSWindow.effectiveAppearance`. So AppKit-level
 /// surfaces keep reading the OS appearance and fight the chosen theme:
@@ -15,6 +19,20 @@ import CCSBarCore
 /// Setting `window.appearance` directly fixes the theme at the AppKit layer so
 /// the whole surface flips, not just the custom RGB tokens.
 ///
+/// Why (2): the default `NSPanel` created by SwiftUI `MenuBarExtra(.window)`
+/// can carry `NSWindow.CollectionBehavior.canJoinAllSpaces`, which makes it
+/// appear on every Space and every display simultaneously. Switching to
+/// `.moveToActiveSpace` (the correct behavior for a transient panel) removes
+/// the multi-display ghost (#1503).
+///
+/// Why (3): with "Displays have separate Spaces" on, the menu bar icon appears
+/// on every display but the panel can open on the wrong screen when its initial
+/// frame is computed relative to `NSScreen.main` instead of the clicked screen.
+/// `NSEvent.mouseLocation` is captured synchronously at click time (before the
+/// async window-materialization hop) to avoid a placement race where a post-click
+/// cursor move would anchor the panel to the wrong display.
+/// If the panel landed on a different screen than the click we reposition it (#1502).
+///
 /// Modeled on the proven `ScrollerHider` pattern (which already reaches the host
 /// window inside this popover), proving cross-window AppKit access works here.
 struct WindowAppearanceForcer: NSViewRepresentable {
@@ -22,23 +40,37 @@ struct WindowAppearanceForcer: NSViewRepresentable {
 
   func makeNSView(context: Context) -> NSView {
     let probe = NSView(frame: .zero)
-    // Defer until the view is in the hierarchy; at make-time `view.window` is nil.
-    DispatchQueue.main.async { apply(to: probe) }
+    // Capture the cursor position synchronously at click time (before any async
+    // dispatch) so the anchoring logic always sees the screen that was clicked,
+    // not wherever the cursor ended up by the time the async block executes.
+    let clickLocation = NSEvent.mouseLocation
+    // Defer window access until the view is in the hierarchy; at make-time
+    // `view.window` is nil.
+    DispatchQueue.main.async { apply(to: probe, clickLocation: clickLocation) }
     return probe
   }
 
   func updateNSView(_ nsView: NSView, context: Context) {
     // Re-apply on every update: the popover's NSWindow can be rebuilt on content
-    // changes, and the appearance pick itself changes mid-session.
-    DispatchQueue.main.async { apply(to: nsView) }
+    // changes, and the appearance pick itself changes mid-session. Screen
+    // anchoring is skipped on updates (clickLocation: nil) to avoid fighting
+    // SwiftUI's own layout passes once the panel is already shown.
+    DispatchQueue.main.async { apply(to: nsView, clickLocation: nil) }
   }
 
-  /// Force the host window's appearance from the chosen theme.
+  /// Apply all window fixes: appearance, collectionBehavior, and (on first open)
+  /// screen anchoring.
+  ///
   ///   .system -> nil   (follow the OS)
   ///   .light  -> aqua
   ///   .dark   -> darkAqua
-  private func apply(to view: NSView) {
+  ///
+  /// `clickLocation`: the cursor position captured synchronously at click time.
+  /// Non-nil only on the first open; nil on subsequent updates (anchoring skipped).
+  private func apply(to view: NSView, clickLocation: NSPoint?) {
     guard let window = view.window else { return }
+
+    // (1) Appearance.
     switch appearance {
     case .system:
       window.appearance = nil
@@ -47,5 +79,90 @@ struct WindowAppearanceForcer: NSViewRepresentable {
     case .dark:
       window.appearance = NSAppearance(named: .darkAqua)
     }
+
+    // (2) Collection behavior — fix #1503.
+    // Remove canJoinAllSpaces so the panel does not ghost across all displays.
+    // .moveToActiveSpace is the correct policy for a transient menu bar panel:
+    // it stays on the Space where it was opened, moves with the user's active
+    // Space on Space switches, and is never visible on multiple displays at once.
+    var behavior = window.collectionBehavior
+    behavior.remove(.canJoinAllSpaces)
+    behavior.insert(.moveToActiveSpace)
+    window.collectionBehavior = behavior
+
+    // (3) Screen anchoring on first open — fix #1502.
+    // clickLocation is the cursor position captured synchronously at click time
+    // (before the async hop). Passing it here avoids the placement race where
+    // a late-read NSEvent.mouseLocation could reflect a post-click cursor move
+    // and anchor the panel to the wrong display.
+    // This is a no-op on a single-display setup or when the panel is already
+    // on the correct screen.
+    if let location = clickLocation {
+      anchorToClickedScreen(window: window, clickLocation: location)
+    }
+  }
+
+  /// Repositions `window` to the screen that contains `clickLocation` (the
+  /// cursor position captured synchronously at click time) if that screen
+  /// differs from the screen the panel currently occupies.
+  ///
+  /// The y-offset from the top of the screen is preserved so the panel still
+  /// appears just below the menu bar on the correct display.
+  private func anchorToClickedScreen(window: NSWindow, clickLocation: NSPoint) {
+    let mouseGlobal = clickLocation
+    let screens = NSScreen.screens
+    guard
+      let clickedScreen = BarScreenPicker.screen(for: mouseGlobal, in: screens),
+      let panelScreen = window.screen,
+      clickedScreen != panelScreen
+    else { return }
+
+    // Compute the distance from the top of the panel's current screen to the
+    // top of the panel frame. "Top" in macOS global coords = maxY (Y increases
+    // upward). Preserve this offset when moving to the clicked screen.
+    let currentMaxY = panelScreen.frame.maxY
+    let panelFrame = window.frame
+    let distanceFromTop = currentMaxY - panelFrame.maxY
+
+    // Place the panel at the same relative position from the top of the clicked
+    // screen, horizontally centered on that screen.
+    let targetMaxY = clickedScreen.frame.maxY - distanceFromTop
+    let targetX = clickedScreen.frame.midX - panelFrame.width / 2
+    let targetOrigin = NSPoint(x: targetX, y: targetMaxY - panelFrame.height)
+
+    window.setFrameOrigin(targetOrigin)
+  }
+}
+
+/// Pure helper: given a list of screen frames and a point in global coordinates,
+/// returns the screen whose frame contains the point. Falls back to the first
+/// screen if no screen contains the point (e.g. the cursor is between displays).
+///
+/// Extracted from `WindowAppearanceForcer` so the decision logic can be
+/// unit-tested headlessly in the `ccs-bar-check` assert harness without AppKit.
+enum BarScreenPicker {
+  /// Returns the element of `screens` whose `frame` contains `point`, or `nil`
+  /// if `screens` is empty. When no screen contains `point` exactly (gap between
+  /// displays), returns the closest screen by distance to frame center.
+  static func screen(for point: NSPoint, in screens: [NSScreen]) -> NSScreen? {
+    guard !screens.isEmpty else { return nil }
+    // Exact hit first.
+    if let exact = screens.first(where: { $0.frame.contains(point) }) {
+      return exact
+    }
+    // Fallback: closest by Euclidean distance from frame center (handles gaps).
+    return screens.min(by: { a, b in
+      distanceSquared(from: point, to: a.frame) < distanceSquared(from: point, to: b.frame)
+    })
+  }
+
+  // MARK: - Internal helpers (fileprivate for tests via same module)
+
+  /// Squared Euclidean distance from a point to the nearest point on a rect.
+  /// Zero when the point is inside the rect (handled by the caller first).
+  static func distanceSquared(from point: NSPoint, to rect: NSRect) -> CGFloat {
+    let dx = max(rect.minX - point.x, 0, point.x - rect.maxX)
+    let dy = max(rect.minY - point.y, 0, point.y - rect.maxY)
+    return dx * dx + dy * dy
   }
 }

macos-bar/Sources/CCSBarCheck/main.swift

@@ -1379,6 +1379,63 @@ do {
   defaults.removePersistentDomain(forName: suiteName)
 }
 
+// MARK: BarScreenPicker — pure geometry helpers (#1502 / #1503)
+//
+// BarScreenPicker lives in CCSBarApp (AppKit-only target), so we can't import it
+// here. We test the equivalent geometry directly: distanceSquared and the
+// index-selection logic expressed over plain CGRect/CGPoint values — the same
+// arithmetic BarScreenPicker runs at runtime. NSPoint/NSRect are CGPoint/CGRect
+// on macOS, so the results are identical.
+
+do {
+  // distanceSquared: point inside rect -> 0.
+  let r = CGRect(x: 0, y: 0, width: 1920, height: 1080)
+  let inside = CGPoint(x: 960, y: 540)
+  let dxIn = max(r.minX - inside.x, 0, inside.x - r.maxX)
+  let dyIn = max(r.minY - inside.y, 0, inside.y - r.maxY)
+  check(dxIn * dxIn + dyIn * dyIn == 0, "screenPicker: point inside rect -> distance 0")
+
+  // distanceSquared: point to the right of rect.
+  let rightOf = CGPoint(x: 2000, y: 540)
+  let dx = max(r.minX - rightOf.x, 0, rightOf.x - r.maxX)  // 2000 - 1920 = 80
+  let dy = max(r.minY - rightOf.y, 0, rightOf.y - r.maxY)  // 0
+  check(dx == 80 && dy == 0, "screenPicker: point right of rect -> dx=80, dy=0")
+  check(dx * dx + dy * dy == 6400, "screenPicker: rightOf distance^2 = 6400")
+
+  // Screen selection: pick the screen containing the point.
+  // Simulate two side-by-side displays: left (0..1920) and right (1920..3840),
+  // each 1080 tall. A click at x=2500 is on the right display.
+  let leftFrame = CGRect(x: 0, y: 0, width: 1920, height: 1080)
+  let rightFrame = CGRect(x: 1920, y: 0, width: 1920, height: 1080)
+  let clickOnRight = CGPoint(x: 2500, y: 50)
+
+  // Contains check (exact hit on right).
+  let exactHit = rightFrame.contains(clickOnRight)
+  check(exactHit, "screenPicker: click at x=2500 is contained by right display frame")
+  let noHitLeft = leftFrame.contains(clickOnRight)
+  check(!noHitLeft, "screenPicker: click at x=2500 is NOT on the left display frame")
+
+  // Gap scenario: click at x=-10 (left of leftFrame), should pick left (closest).
+  let gapClick = CGPoint(x: -10, y: 540)
+  let dLeft: CGFloat = {
+    let ddx = max(leftFrame.minX - gapClick.x, 0, gapClick.x - leftFrame.maxX)
+    let ddy = max(leftFrame.minY - gapClick.y, 0, gapClick.y - leftFrame.maxY)
+    return ddx * ddx + ddy * ddy
+  }()
+  let dRight: CGFloat = {
+    let ddx = max(rightFrame.minX - gapClick.x, 0, gapClick.x - rightFrame.maxX)
+    let ddy = max(rightFrame.minY - gapClick.y, 0, gapClick.y - rightFrame.maxY)
+    return ddx * ddx + ddy * ddy
+  }()
+  check(dLeft < dRight, "screenPicker: gap click left of left display -> left is closest")
+
+  // Screen A is the one whose frame.contains(click) returns true; that is the
+  // screen that should be selected for panel anchoring (the fix for #1502).
+  let screens = [leftFrame, rightFrame]
+  let selectedIdx = screens.firstIndex(where: { $0.contains(clickOnRight) })
+  check(selectedIdx == 1, "screenPicker: exact hit selects right screen (index 1) for #1502 anchor")
+}
+
 // cleanup
 try? FileManager.default.removeItem(atPath: tmp)