From 19a8f09e5a086b7c9ecd0c2daa9fbcc7942d4bd4 Mon Sep 17 00:00:00 2001 From: Tommaso Negri Date: Sun, 12 Jul 2026 15:23:22 +0200 Subject: [PATCH] fix an issue with horizontal window margins not respected --- .../PHThemeWindow/PHThemeWindowAnchor.swift | 46 +++++++------- .../PHThemeWindow/PHThemeWindowMargin.swift | 25 +++++++- Sources/phbar/Models/PHTheme/theme.toml | 5 +- Sources/phbar/Views/BarWindow.swift | 25 +++++--- Tests/phbarTests/phbarTests.swift | 63 +++++++++++++++---- 5 files changed, 112 insertions(+), 52 deletions(-) diff --git a/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowAnchor.swift b/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowAnchor.swift index 78aa1df..44c71d0 100644 --- a/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowAnchor.swift +++ b/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowAnchor.swift @@ -17,44 +17,40 @@ enum PHThemeWindowAnchor: String, Decodable { extension PHThemeWindowAnchor { /// Compute the top-left origin (in screen coordinates) of a window of `size` - /// placed against this anchor within `screen`, offset by `margin` along the - /// anchored edges. + /// placed against this anchor within `rect`. /// - /// Edges the anchor does not reference are ignored, so margins only affect - /// the relevant sides. - func origin(in screen: CGRect, size: CGSize, margin: PHThemeWindowMargin) -> CGPoint { - let top = margin.top ?? 0 - let bottom = margin.bottom ?? 0 - let leading = margin.leading ?? 0 - let trailing = margin.trailing ?? 0 - - // Default: centered on both axes. + /// `rect` is the content rectangle the window is placed within — typically the + /// screen inset by the window's margin (see `PHThemeWindowMargin.inset(of:)`). + /// Single-edge anchors pin to that edge and center along the opposite axis; + /// corner anchors pin to two edges; `.center` centers on both axes. + func origin(in rect: CGRect, size: CGSize) -> CGPoint { + // Default: centered on both axes within the rect. var point = CGPoint( - x: screen.midX - size.width / 2, - y: screen.midY - size.height / 2 + x: rect.midX - size.width / 2, + y: rect.midY - size.height / 2 ) switch self { case .top: - point.y = screen.maxY - CGFloat(top) - size.height + point.y = rect.maxY - size.height case .bottom: - point.y = screen.minY + CGFloat(bottom) + point.y = rect.minY case .leading: - point.x = screen.minX + CGFloat(leading) + point.x = rect.minX case .trailing: - point.x = screen.maxX - CGFloat(trailing) - size.width + point.x = rect.maxX - size.width case .topLeading: - point.x = screen.minX + CGFloat(leading) - point.y = screen.maxY - CGFloat(top) - size.height + point.x = rect.minX + point.y = rect.maxY - size.height case .topTrailing: - point.x = screen.maxX - CGFloat(trailing) - size.width - point.y = screen.maxY - CGFloat(top) - size.height + point.x = rect.maxX - size.width + point.y = rect.maxY - size.height case .bottomLeading: - point.x = screen.minX + CGFloat(leading) - point.y = screen.minY + CGFloat(bottom) + point.x = rect.minX + point.y = rect.minY case .bottomTrailing: - point.x = screen.maxX - CGFloat(trailing) - size.width - point.y = screen.minY + CGFloat(bottom) + point.x = rect.maxX - size.width + point.y = rect.minY case .center: break } diff --git a/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowMargin.swift b/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowMargin.swift index d10afca..e157f18 100644 --- a/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowMargin.swift +++ b/Sources/phbar/Models/PHTheme/PHThemeWindow/PHThemeWindowMargin.swift @@ -1,9 +1,12 @@ +import CoreGraphics import Foundation -/// Insets (in points) from the screen edges implied by a window's anchor. +/// Insets (in points) from the screen edges, defining the content rectangle a +/// window is sized and placed within. /// -/// Only the edges referenced by the anchor are consumed; the rest are ignored, -/// so a margin only affects the relevant sides of the placement. +/// Margins compose with `width`/`height`: a `width = "100%"` window spans the +/// space *between* the leading/trailing margins rather than the full screen. +/// Unspecified edges are left untouched (treated as 0). struct PHThemeWindowMargin: Decodable, Equatable { let top: Double? let bottom: Double? @@ -21,4 +24,20 @@ struct PHThemeWindowMargin: Decodable, Equatable { self.leading = leading self.trailing = trailing } + + /// Returns `rect` inset by the specified edges; unspecified edges are left + /// untouched (treated as 0). Uses AppKit's coordinate system (origin at the + /// bottom-left, y increasing upward): `top` reduces the max-Y edge. + func inset(of rect: CGRect) -> CGRect { + let l = leading ?? 0 + let r = trailing ?? 0 + let t = top ?? 0 + let b = bottom ?? 0 + return CGRect( + x: rect.minX + l, + y: rect.minY + b, + width: rect.width - l - r, + height: rect.height - t - b + ) + } } diff --git a/Sources/phbar/Models/PHTheme/theme.toml b/Sources/phbar/Models/PHTheme/theme.toml index 4def825..a499af3 100644 --- a/Sources/phbar/Models/PHTheme/theme.toml +++ b/Sources/phbar/Models/PHTheme/theme.toml @@ -7,16 +7,13 @@ name = "default" anchor = "top" height = 30 width = "100%" -shadow = false -blur = 0.0 +margin = { leading = 30, top = 30, trailing = 30, bottom = 30 } [[window]] name = "bottom" anchor = "bottom" height = 30 width = "100%" -shadow = false -blur = 0.0 [[text]] name = "default" diff --git a/Sources/phbar/Views/BarWindow.swift b/Sources/phbar/Views/BarWindow.swift index 30b0269..8eb9234 100644 --- a/Sources/phbar/Views/BarWindow.swift +++ b/Sources/phbar/Views/BarWindow.swift @@ -52,13 +52,14 @@ extension BarWindow { let screenFrame = controller.screen.frame let themeWindow = controller.window - let size = CGSize( - width: (themeWindow.width ?? .percentage(1.0)).resolve(against: screenFrame.width), - height: CGFloat(themeWindow.height ?? 30) - ) - - // Absolute origin wins over anchor/margin. + // Absolute origin bypasses the anchor/margin system entirely: width + // resolves against the full screen and the window sits at the exact + // coordinates given. if let origin = themeWindow.origin { + let size = CGSize( + width: (themeWindow.width ?? .percentage(1.0)).resolve(against: screenFrame.width), + height: CGFloat(themeWindow.height ?? 30) + ) return NSRect( x: CGFloat(origin.x), y: CGFloat(origin.y), @@ -67,10 +68,18 @@ extension BarWindow { ) } - let anchor = themeWindow.anchor ?? .top + // Margins define a content rectangle within the screen: the window is + // sized against it (so `width = "100%"` spans only between the margins, + // not the full screen) and placed by the anchor inside it. let margin = themeWindow.margin ?? .init() + let contentRect = margin.inset(of: screenFrame) + let size = CGSize( + width: (themeWindow.width ?? .percentage(1.0)).resolve(against: contentRect.width), + height: CGFloat(themeWindow.height ?? 30) + ) + let anchor = themeWindow.anchor ?? .top return NSRect( - origin: anchor.origin(in: screenFrame, size: size, margin: margin), + origin: anchor.origin(in: contentRect, size: size), size: size ) } diff --git a/Tests/phbarTests/phbarTests.swift b/Tests/phbarTests/phbarTests.swift index 08e8852..f5ac6b2 100644 --- a/Tests/phbarTests/phbarTests.swift +++ b/Tests/phbarTests/phbarTests.swift @@ -488,40 +488,43 @@ private struct DimensionWrapper: Decodable { @Test func anchorPlacesAtTopEdge() { let screen = CGRect(x: 0, y: 0, width: 1000, height: 800) let size = CGSize(width: 1000, height: 30) - let origin = PHThemeWindowAnchor.top.origin(in: screen, size: size, margin: .init()) + let origin = PHThemeWindowAnchor.top.origin(in: screen, size: size) #expect(origin == CGPoint(x: 0, y: 770)) } @Test func anchorRespectsTopMargin() { let screen = CGRect(x: 0, y: 0, width: 1000, height: 800) let size = CGSize(width: 1000, height: 30) - let origin = PHThemeWindowAnchor.top.origin(in: screen, size: size, margin: .init(top: 10)) + let rect = PHThemeWindowMargin(top: 10).inset(of: screen) + let origin = PHThemeWindowAnchor.top.origin(in: rect, size: size) #expect(origin == CGPoint(x: 0, y: 760)) } @Test func anchorPlacesBottomLeadingCorner() { let screen = CGRect(x: 0, y: 0, width: 1000, height: 800) let size = CGSize(width: 200, height: 40) - let origin = PHThemeWindowAnchor.bottomLeading.origin( - in: screen, size: size, margin: .init(bottom: 8, leading: 12) - ) + let rect = PHThemeWindowMargin(bottom: 8, leading: 12).inset(of: screen) + let origin = PHThemeWindowAnchor.bottomLeading.origin(in: rect, size: size) #expect(origin == CGPoint(x: 12, y: 8)) } @Test func anchorTrailingCentersVertically() { let screen = CGRect(x: 0, y: 0, width: 1000, height: 800) let size = CGSize(width: 200, height: 40) - let origin = PHThemeWindowAnchor.trailing.origin( - in: screen, size: size, margin: .init(trailing: 20) - ) + let rect = PHThemeWindowMargin(trailing: 20).inset(of: screen) + let origin = PHThemeWindowAnchor.trailing.origin(in: rect, size: size) #expect(origin == CGPoint(x: 780, y: 380)) } -@Test func anchorCentersOnBothAxesIgnoringMargin() { +@Test func anchorCentersWithinContentRect() { let screen = CGRect(x: 0, y: 0, width: 1000, height: 800) let size = CGSize(width: 200, height: 40) - let origin = PHThemeWindowAnchor.center.origin(in: screen, size: size, margin: .init(top: 999)) - #expect(origin == CGPoint(x: 400, y: 380)) + // `.center` centers within the inset rect: a top margin lifts the center, + // it doesn't ignore the margin. + let rect = PHThemeWindowMargin(top: 100).inset(of: screen) + let origin = PHThemeWindowAnchor.center.origin(in: rect, size: size) + // rect: y ∈ [0, 700], midY = 350 → 350 − 20 = 330 + #expect(origin == CGPoint(x: 400, y: 330)) } // MARK: - Monitor → window/blocks resolver @@ -625,7 +628,15 @@ private func makeBlocksConfigDir() throws -> URL { @MainActor @Test func computeFrameDefaultsToFullScreenTopBar() throws { let screen = try #require(NSScreen.main) - let theme = try PHTheme.load("voltage") + // A window that omits geometry exercises computeFrame's defaults: + // width 100%, top anchor, no margin → a full-screen top bar. Built locally + // (rather than loading the bundled theme) so the test tracks the engine's + // defaults, not the theme file's editorial margins. + let theme = PHTheme( + windows: [PHThemeWindow(name: "default")], + texts: nil, + styles: nil + ) let config = PHConfig(window: "default", blocks: nil, env: nil, monitors: nil) let blocks = try PHBlock.load(from: "[[block]]\ncommand = \"echo x\"") let controller = BarController(config: config, screen: screen, theme: theme, blocks: blocks, debug: false) @@ -655,6 +666,34 @@ private func makeBlocksConfigDir() throws -> URL { #expect(frame.height == 30) } +@MainActor +@Test func computeFrameInsetsFullWidthBarByMargins() throws { + // Regression: `width = "100%"` previously ignored leading/trailing margins + // and spanned edge to edge. Margins now inset the content rectangle, so the + // bar spans only between them. + let screen = try #require(NSScreen.main) + let sf = screen.frame + let theme = PHTheme( + windows: [PHThemeWindow( + name: "inset", + width: .percentage(1.0), + anchor: .top, + margin: PHThemeWindowMargin(leading: 20, trailing: 20) + )], + texts: nil, + styles: nil + ) + let config = PHConfig(window: "inset", blocks: nil, env: nil, monitors: nil) + let blocks = try PHBlock.load(from: "[[block]]\ncommand = \"echo x\"") + let controller = BarController(config: config, screen: screen, theme: theme, blocks: blocks, debug: false) + + let frame = BarWindow.computeFrame(from: controller) + + #expect(frame.width == sf.width - 40) + #expect(frame.minX == sf.minX + 20) + #expect(frame.maxX == sf.maxX - 20) +} + // MARK: - PHPaths (config directory cascade) @Test func pathsCandidatesIncludeAbsoluteXdgFirst() {