From df6e898a088c4c793abbbc2006c899e042f40e66 Mon Sep 17 00:00:00 2001 From: Alvaro Date: Sat, 5 Sep 2026 20:15:10 +0200 Subject: [PATCH 1/2] Fix collapsed menu popover viewport --- AudioPriorityBar/Views/MenuBarView.swift | 2 +- Tests/Popover-UAT.md | 13 +++++++++ Tests/popover-uat.applescript | 37 ++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 Tests/Popover-UAT.md create mode 100644 Tests/popover-uat.applescript diff --git a/AudioPriorityBar/Views/MenuBarView.swift b/AudioPriorityBar/Views/MenuBarView.swift index 3ad55c2..d781028 100644 --- a/AudioPriorityBar/Views/MenuBarView.swift +++ b/AudioPriorityBar/Views/MenuBarView.swift @@ -82,7 +82,7 @@ struct MenuBarView: View { .padding(.horizontal, 16) .padding(.vertical, 14) } - .frame(maxHeight: 420) + .frame(height: 420) Divider() .padding(.horizontal, 12) diff --git a/Tests/Popover-UAT.md b/Tests/Popover-UAT.md new file mode 100644 index 0000000..07bafdb --- /dev/null +++ b/Tests/Popover-UAT.md @@ -0,0 +1,13 @@ +# Popover layout regression check + +Build with `./build.sh`. Run the target app in Custom mode so opening the menu does not change the selected audio devices. With macOS Accessibility access for the terminal, run: + +```sh +osascript Tests/popover-uat.applescript AudioPriorityBar "MacBook Air Microphone" +``` + +Replace the process name and optional device name for the test machine. The script checks that the list viewport is at least 200 points high, checks the named rows, closes the panel, then opens and checks it again. The original layout fails the height check on affected macOS versions. + +Also check the actual panel with few devices and in Edit mode with enough remembered devices to require scrolling. Confirm that the footer is visible and that scrolling reaches the last row. The fixed 420-point viewport intentionally leaves space when a list is short; this preserves scrolling without adding a content-measurement mechanism. + +The regression depends on native MenuBarExtra layout; a source-text assertion would not detect it. The script checks the rendered native viewport instead. diff --git a/Tests/popover-uat.applescript b/Tests/popover-uat.applescript new file mode 100644 index 0000000..e3ae723 --- /dev/null +++ b/Tests/popover-uat.applescript @@ -0,0 +1,37 @@ +-- Run with the target app running; optionally pass expected connected device names. +-- osascript Tests/popover-uat.applescript AudioPriorityBar "MacBook Air Microphone" +-- Checks the rendered viewport, then closes and reopens the popover and checks again. +-- Does not change audio devices, priorities, or automation settings. +on run arguments + if (count of arguments) < 1 then error "Pass the target app process name." + set appName to item 1 of arguments + set expectedNames to rest of arguments + tell application "System Events" to tell process appName + repeat with attempt from 1 to 2 + if (count of windows) is 0 then + set iconPosition to position of menu bar item "Volume High" of menu bar 2 + set iconSize to size of menu bar item "Volume High" of menu bar 2 + tell application "System Events" to click at {(item 1 of iconPosition) + (item 1 of iconSize) / 2, (item 2 of iconPosition) + (item 2 of iconSize) / 2} + end if + repeat 20 times + if (count of windows) > 0 then exit repeat + delay 0.1 + end repeat + set viewport to scroll area 1 of group 1 of window 1 + set viewportSize to size of viewport + if (item 2 of viewportSize) < 200 then error "Device list collapsed: height=" & (item 2 of viewportSize) + set rowNames to name of every static text of viewport + repeat with expectedName in expectedNames + if rowNames does not contain (expectedName as text) then error "Missing microphone: " & expectedName + end repeat + if attempt is 1 then + set iconPosition to position of menu bar item "Volume High" of menu bar 2 + set iconSize to size of menu bar item "Volume High" of menu bar 2 + tell application "System Events" to click at {(item 1 of iconPosition) + (item 1 of iconSize) / 2, (item 2 of iconPosition) + (item 2 of iconSize) / 2} + delay 0.1 + if (count of windows) is not 0 then error "Popover did not close when its menu icon was clicked" + end if + end repeat + return "PASS: device list stays visible after reopening; expected microphones are present. Height=" & (item 2 of viewportSize) + end tell +end run From c37abe7eda9b8c2bf0ec0f06032ae96a6bcba345 Mon Sep 17 00:00:00 2001 From: Alvaro Date: Sat, 5 Sep 2026 21:15:45 +0200 Subject: [PATCH 2/2] Cap the popover viewport to available display height --- AudioPriorityBar/Views/MenuBarView.swift | 47 ++++++++++++++++++- Tests/Popover-UAT.md | 26 ++++++++-- Tests/PopoverLayoutTests.swift | 60 ++++++++++++++++++++++++ Tests/popover-uat.applescript | 27 +++++++++-- Tests/run-popover-layout-tests.sh | 27 +++++++++++ 5 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 Tests/PopoverLayoutTests.swift create mode 100644 Tests/run-popover-layout-tests.sh diff --git a/AudioPriorityBar/Views/MenuBarView.swift b/AudioPriorityBar/Views/MenuBarView.swift index d781028..c216ce2 100644 --- a/AudioPriorityBar/Views/MenuBarView.swift +++ b/AudioPriorityBar/Views/MenuBarView.swift @@ -4,6 +4,8 @@ import AppKit struct MenuBarView: View { @EnvironmentObject var audioManager: AudioManager + var availableHeight: CGFloat? = nil + @State private var screenHeight = NSScreen.main?.visibleFrame.height ?? 600 var body: some View { VStack(spacing: 0) { @@ -82,7 +84,8 @@ struct MenuBarView: View { .padding(.horizontal, 16) .padding(.vertical, 14) } - .frame(height: 420) + // Reserve space for the fixed header/footer and the screen edge. + .frame(height: max(1, min(420, (availableHeight ?? screenHeight) - 160))) Divider() .padding(.horizontal, 12) @@ -133,6 +136,48 @@ struct MenuBarView: View { .animation(.easeInOut(duration: 0.2), value: audioManager.isEditMode) } .frame(width: 340) + .background(PopoverScreenHeight { screenHeight = $0 }) + } +} + +// Use the popover's own display, including when it is on a secondary screen. +private struct PopoverScreenHeight: NSViewRepresentable { + var onChange: (CGFloat) -> Void + + func makeNSView(context: Context) -> ScreenView { + let view = ScreenView() + view.onChange = onChange + return view + } + + func updateNSView(_ view: ScreenView, context: Context) { + view.onChange = onChange + } + + final class ScreenView: NSView { + var onChange: ((CGFloat) -> Void)? + private var observers: [NSObjectProtocol] = [] + + override func viewDidMoveToWindow() { + super.viewDidMoveToWindow() + observers.forEach(NotificationCenter.default.removeObserver) + observers = [] + guard let window else { return } + for name in [NSWindow.didChangeScreenNotification, NSApplication.didChangeScreenParametersNotification] { + observers.append(NotificationCenter.default.addObserver(forName: name, + object: name == NSWindow.didChangeScreenNotification ? window : nil, queue: .main) { [weak self] _ in + self?.reportHeight() + }) + } + reportHeight() + } + + private func reportHeight() { + guard let height = window?.screen?.visibleFrame.height else { return } + DispatchQueue.main.async { [weak self] in self?.onChange?(height) } + } + + deinit { observers.forEach(NotificationCenter.default.removeObserver) } } } diff --git a/Tests/Popover-UAT.md b/Tests/Popover-UAT.md index 07bafdb..e763a9f 100644 --- a/Tests/Popover-UAT.md +++ b/Tests/Popover-UAT.md @@ -1,4 +1,4 @@ -# Popover layout regression check +# Popover layout regression checks Build with `./build.sh`. Run the target app in Custom mode so opening the menu does not change the selected audio devices. With macOS Accessibility access for the terminal, run: @@ -6,8 +6,26 @@ Build with `./build.sh`. Run the target app in Custom mode so opening the menu d osascript Tests/popover-uat.applescript AudioPriorityBar "MacBook Air Microphone" ``` -Replace the process name and optional device name for the test machine. The script checks that the list viewport is at least 200 points high, checks the named rows, closes the panel, then opens and checks it again. The original layout fails the height check on affected macOS versions. +Replace the process name and optional device name for the test machine. The script checks a normal-display list viewport, checks the named rows, closes the panel, then opens and checks it again. The original layout fails with height 0 on affected macOS versions. -Also check the actual panel with few devices and in Edit mode with enough remembered devices to require scrolling. Confirm that the footer is visible and that scrolling reaches the last row. The fixed 420-point viewport intentionally leaves space when a list is short; this preserves scrolling without adding a content-measurement mechanism. +## Constrained height -The regression depends on native MenuBarExtra layout; a source-text assertion would not detect it. The script checks the rendered native viewport instead. +```sh +sh Tests/run-popover-layout-tests.sh +``` + +This compiles the real views and manager with a separate test entry point and a unique preference domain. It enables the existing Custom mode before manager construction and replaces only the displayed rows with fixtures. It does not select or change system audio devices. + +For 3 and 20 input rows, the test supplies display height limits of 553, 400, and 300 points. It checks that both the native panel and its fitting height stay within that limit without collapsing. The previous fixed 420-point viewport produces a 553-point panel and fails the 400-point case. + +To inspect the final 300-point case, including the footer and scrolling: + +```sh +POPOVER_LAYOUT_HOLD=1 sh Tests/run-popover-layout-tests.sh +``` + +While the fixture is open, `osascript Tests/popover-uat.applescript PopoverLayoutTests --window-only` checks its viewport and that all top-level buttons lie inside the native panel bounds. + +The command prints the temporary app path and leaves the fixture visible for two minutes. Check that Login, Edit, and Quit remain visible and that scrolling reaches Test microphone 20. This is a constrained-window test, not a physical display-resolution change. + +The app takes the visible height of the popover's own screen and reserves 160 points for its fixed header/footer and edge clearance. It caps the remaining list area at 420 points. The screen observer updates the budget when display parameters or the window's screen change. Check opening on each display after changing scaling or Dock placement when those configurations are available. diff --git a/Tests/PopoverLayoutTests.swift b/Tests/PopoverLayoutTests.swift new file mode 100644 index 0000000..8c2d8ef --- /dev/null +++ b/Tests/PopoverLayoutTests.swift @@ -0,0 +1,60 @@ +import AppKit +import SwiftUI + +@main +@MainActor +struct PopoverLayoutTests { + static func main() { + let app = NSApplication.shared + app.setActivationPolicy(.accessory) + app.finishLaunching() + guard let domain = Bundle.main.bundleIdentifier, + domain.hasPrefix("app.audioprioritybar.layouttests.") else { + fatalError("Run through run-popover-layout-tests.sh for isolated preferences.") + } + defer { UserDefaults.standard.removePersistentDomain(forName: domain) } + // Use the existing manual-mode preference before constructing the real manager. + UserDefaults.standard.setVolatileDomain(["customMode": true], forName: UserDefaults.argumentDomain) + let manager = AudioManager() + precondition(manager.isCustomMode) + let host = NSHostingView(rootView: MenuBarView().environmentObject(manager)) + let window = NSWindow(contentRect: NSRect(x: 100, y: 100, width: 340, height: 553), + styleMask: [.borderless], backing: .buffered, defer: false) + window.contentView = host + window.makeKeyAndOrderFront(nil) + defer { window.orderOut(nil) } + + // Replace only the displayed rows, so results do not depend on connected hardware. + manager.speakerDevices = [] + manager.headphoneDevices = [] + for count in [3, 20] { + manager.inputDevices = (1...count).map { + AudioDevice(id: UInt32($0), uid: "layout-test-\($0)", name: "Test microphone \($0)", type: .input, isConnected: false) + } + for height: CGFloat in [553, 400, 300] { + host.rootView = MenuBarView(availableHeight: height).environmentObject(manager) + window.setContentSize(NSSize(width: 340, height: height)) + host.layoutSubtreeIfNeeded() + RunLoop.main.run(until: Date().addingTimeInterval(0.2)) + guard host.bounds.height > 200 && host.bounds.height <= height + 1 else { + fputs("FAIL: requested \(height)-point panel, got \(host.bounds.height).\n", stderr) + exit(1) + } + guard host.fittingSize.height > 200 && host.fittingSize.height <= height else { + fputs("FAIL: ideal panel height is empty or exceeds the available height.\n", stderr) + exit(1) + } + print("PASS: \(count) rows, available \(Int(height)), actual \(Int(host.bounds.height)), ideal \(Int(host.fittingSize.height))") + } + } + if ProcessInfo.processInfo.environment["POPOVER_LAYOUT_HOLD"] == "1" { + print("Ready for native footer/scroll inspection at height 300; preview closes after two minutes.") + fflush(stdout) + Timer.scheduledTimer(withTimeInterval: 120, repeats: false) { _ in + UserDefaults.standard.removePersistentDomain(forName: domain) + app.terminate(nil) + } + app.run() + } + } +} diff --git a/Tests/popover-uat.applescript b/Tests/popover-uat.applescript index e3ae723..ec6e479 100644 --- a/Tests/popover-uat.applescript +++ b/Tests/popover-uat.applescript @@ -6,8 +6,19 @@ on run arguments if (count of arguments) < 1 then error "Pass the target app process name." set appName to item 1 of arguments set expectedNames to rest of arguments + set passes to 2 + set minimumViewportHeight to 200 + if expectedNames is not {} and item 1 of expectedNames is "--window-only" then + set expectedNames to rest of expectedNames + set passes to 1 + set minimumViewportHeight to 1 + end if tell application "System Events" to tell process appName - repeat with attempt from 1 to 2 + repeat 30 times + if (count of windows) > 0 or (count of menu bars) > 1 then exit repeat + delay 0.1 + end repeat + repeat with attempt from 1 to passes if (count of windows) is 0 then set iconPosition to position of menu bar item "Volume High" of menu bar 2 set iconSize to size of menu bar item "Volume High" of menu bar 2 @@ -19,12 +30,20 @@ on run arguments end repeat set viewport to scroll area 1 of group 1 of window 1 set viewportSize to size of viewport - if (item 2 of viewportSize) < 200 then error "Device list collapsed: height=" & (item 2 of viewportSize) + if (item 2 of viewportSize) < minimumViewportHeight then error "Device list collapsed: height=" & (item 2 of viewportSize) set rowNames to name of every static text of viewport repeat with expectedName in expectedNames if rowNames does not contain (expectedName as text) then error "Missing microphone: " & expectedName end repeat - if attempt is 1 then + set panelPosition to position of window 1 + set panelSize to size of window 1 + -- The top-level buttons include Login, Edit, and Quit, outside the scroll view. + repeat with panelButton in buttons of group 1 of window 1 + set controlPosition to position of panelButton + set controlSize to size of panelButton + if (item 2 of controlPosition) < (item 2 of panelPosition) - 1 or (item 2 of controlPosition) + (item 2 of controlSize) > (item 2 of panelPosition) + (item 2 of panelSize) + 1 then error "Header or footer button is clipped" + end repeat + if attempt is 1 and passes is 2 then set iconPosition to position of menu bar item "Volume High" of menu bar 2 set iconSize to size of menu bar item "Volume High" of menu bar 2 tell application "System Events" to click at {(item 1 of iconPosition) + (item 1 of iconSize) / 2, (item 2 of iconPosition) + (item 2 of iconSize) / 2} @@ -32,6 +51,6 @@ on run arguments if (count of windows) is not 0 then error "Popover did not close when its menu icon was clicked" end if end repeat - return "PASS: device list stays visible after reopening; expected microphones are present. Height=" & (item 2 of viewportSize) + return "PASS: viewport and top-level controls visible; expected microphones present. Passes=" & passes & "; height=" & (item 2 of viewportSize) end tell end run diff --git a/Tests/run-popover-layout-tests.sh b/Tests/run-popover-layout-tests.sh new file mode 100644 index 0000000..f84779f --- /dev/null +++ b/Tests/run-popover-layout-tests.sh @@ -0,0 +1,27 @@ +#!/bin/sh +set -eu +repo_dir=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) +test_dir=$(mktemp -d "${TMPDIR:-/tmp}/popover-layout.XXXXXX") +trap 'rm -rf "$test_dir"' EXIT HUP INT TERM +app_dir="$test_dir/PopoverLayoutTests.app" +mkdir -p "$app_dir/Contents/MacOS" +cat > "$app_dir/Contents/Info.plist" < + + +CFBundleIdentifierapp.audioprioritybar.layouttests.$(uuidgen) +CFBundleExecutablePopoverLayoutTests +LSUIElement + +PLIST +if [ "${POPOVER_LAYOUT_HOLD:-0}" = 1 ]; then printf '%s\n' "$app_dir"; fi +# Use the real views and manager, replacing only the app entry point. +sed '/^@main$/d' "$repo_dir/AudioPriorityBar/AudioPriorityBarApp.swift" > "$test_dir/AudioPriorityBarApp.swift" +xcrun swiftc -module-cache-path "$test_dir/modules" \ + "$test_dir/AudioPriorityBarApp.swift" \ + "$repo_dir"/AudioPriorityBar/Models/*.swift \ + "$repo_dir"/AudioPriorityBar/Services/*.swift \ + "$repo_dir"/AudioPriorityBar/Views/*.swift \ + "$repo_dir/Tests/PopoverLayoutTests.swift" \ + -o "$app_dir/Contents/MacOS/PopoverLayoutTests" +"$app_dir/Contents/MacOS/PopoverLayoutTests"