diff --git a/macos-bar/Sources/CCSBarApp/SettingsWindowController.swift b/macos-bar/Sources/CCSBarApp/SettingsWindowController.swift index 2e966208..7e2ac290 100644 --- a/macos-bar/Sources/CCSBarApp/SettingsWindowController.swift +++ b/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 diff --git a/macos-bar/Sources/CCSBarApp/WindowAppearanceForcer.swift b/macos-bar/Sources/CCSBarApp/WindowAppearanceForcer.swift index b9b8d6e8..0e6d6cb9 100644 --- a/macos-bar/Sources/CCSBarApp/WindowAppearanceForcer.swift +++ b/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,18 @@ 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. +/// Reading `NSEvent.mouseLocation` at open-time gives the screen of interaction; +/// if the panel is on a different screen we reposition it to match (#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 { @@ -23,22 +39,28 @@ 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) } + DispatchQueue.main.async { apply(to: probe, isFirstOpen: true) } 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 (isFirstOpen: false) to avoid fighting + // SwiftUI's own layout passes once the panel is already shown. + DispatchQueue.main.async { apply(to: nsView, isFirstOpen: false) } } - /// 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) { + private func apply(to view: NSView, isFirstOpen: Bool) { guard let window = view.window else { return } + + // (1) Appearance. switch appearance { case .system: window.appearance = nil @@ -47,5 +69,88 @@ 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. + // NSEvent.mouseLocation reports the click position in global (screen) + // coordinates at the time the panel opens. If the panel landed on a + // different screen than the one that was clicked, move it there. + // This is a no-op on a single-display setup or when the panel is already + // on the correct screen. + if isFirstOpen { + anchorToClickedScreen(window: window) + } + } + + /// Repositions `window` to the screen that contains the current mouse cursor + /// 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) { + let mouseGlobal = NSEvent.mouseLocation + 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 } } diff --git a/macos-bar/Sources/CCSBarCheck/main.swift b/macos-bar/Sources/CCSBarCheck/main.swift index 5bbc2db7..0932ca85 100644 --- a/macos-bar/Sources/CCSBarCheck/main.swift +++ b/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)