mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-17 12:16:59 +00:00
fix(bar): keep menu bar panel on the clicked screen in multi-display setups
The MenuBarExtra panel's default collectionBehavior included canJoinAllSpaces, so the bar could be visible on every Space and display at once. The WindowAppearanceForcer bridge now strips canJoinAllSpaces and applies moveToActiveSpace. Panel and Settings window placement also keyed off the key-window screen (NSScreen.main / window.center()) rather than the display where the status item was clicked. Both now anchor to the screen under the cursor at click time via the pure BarScreenPicker helper, covered by new ccs-bar-check geometry assertions. Closes #1502 Closes #1503
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user