Skip to content

ci(macos): capture menu panel screenshots for visual review - #41

Draft
ssddOnTop wants to merge 14 commits into
mainfrom
ci/macos-menu-screenshots
Draft

ssddOnTop wants to merge 14 commits into
mainfrom
ci/macos-menu-screenshots

Conversation

@ssddOnTop

Copy link
Copy Markdown
Collaborator

Stacked on #39. Not for merge — this exists to run CI and produce screenshots so the Liquid Glass work in #39 can be reviewed visually. Close once the images have been checked.

Why

The panel's backdrop is chosen at runtime, so neither source review nor the unit tests show what it looks like. The effect also depends on what is behind the window, so it has to be captured from a real running app on a real macOS version.

What

  • ScreenshotMode.swift — env-gated on FORGE_SCREENSHOT_DIR, returns early in applicationDidFinishLaunching before any service/runtime work. Drives the real PopoverController over 5 service states × light/dark × both backends.
  • scripts/capture-screenshots.sh — builds from source, or renders a prebuilt binary via BINARY.
  • CI screenshots job — builds once on macOS 26, renders the same binary on macOS 26 and 15.

Two details that make the captures meaningful:

  • Window capture, not view capture. Glass is composited by the window server from what sits behind the window, so a view-level render would show content with no material at all.
  • A patterned stage window behind the panel. Over an empty desktop the material has nothing to sample and reads as flat grey.

Coverage

Both materials are exercised. MenuBackdrop.forced pins the backend so a macOS 26 run captures glass and the fallback; the override can only step down, since glass cannot be synthesised where AppKit lacks it. CI additionally runs the same binary on macOS 15 so the fallback is seen on a genuinely older AppKit.

The matrix is [26, 15] — GitHub deprecated its macOS 14 image and retired 13, so 13–14 cannot be exercised on hosted runners despite being supported.

Artifacts

menu-panel-screenshots-macos-26 and -15. Files are prefixed glass- or legacy-; environment.txt records the OS and native backend.

Fixes from the previous attempt

The first run reported green while producing nothing: the bare binary aborted with Library not loaded: @rpath/Sparkle.framework, masked by continue-on-error. Now the executable is staged alongside Sparkle.framework matching its @executable_path/../Frameworks rpath, tarred (upload-artifact follows symlinks rather than preserving them), and a report step emits an explicit warning when capture fails.

Caveat

GitHub runners are virtualized. Captures are reliable for layout, colour, radius, and gross failures; blur fidelity on real hardware may differ.

The status icon changed shade during every launch. `updateStatusItem`
derived `contentTintColor` from the service phase, and startup always
walked through several phases before settling:

  1. The first paint runs before `serviceController.start()`, when the
     snapshot still holds its default `.stopped` phase, so the icon was
     drawn dimmed with `.secondaryLabelColor`.
  2. Installing/starting then applied `.controlAccentColor`.
  3. `.ready` finally cleared the tint.

Because the logo is a template image, the tint was the only thing
determining its shade, so this read as the logo spontaneously changing
colour — appearing dark-then-light on a light menu bar and the reverse
on a dark one.

Drop the phase tinting entirely and set the image once at setup. The
icon is now constant and matches the menu bar like any other status
item. Phase remains visible through the tooltip, the accessibility
label, and the panel contents.

Also stop rebuilding the image on every snapshot: it never varies, so
that only re-rasterised the bezier path.
The panel read as an app card rather than a menu bar utility: every row
was set in medium weight, rows were 30pt tall against a native 22pt, and
the 280pt width left ~110pt of dead space beside the longest label.

Rather than estimate from a screenshot, the target values were taken from
AppKit directly -- `NSFont.menuFont(ofSize: 0)` reports 13pt at weight 5
(regular), and differencing the reported heights of NSMenus built with N
items yields exactly 22pt per item, an 11pt separator slot, 10pt of total
chrome, and a 171pt intrinsic width for these labels.

  body weight        medium      -> regular
  prominent rows     semibold    -> regular, accent colour
  row height         30pt + 1pt  -> 22pt, no inter-row gap
  panel width        280pt       -> 220pt
  corner radius      12pt        -> 10pt
  vertical padding   6pt         -> 5pt
  toggle switch      .small      -> .mini
  key equivalent     12pt        -> 13pt, recedes via colour alone

Steady-state height drops from ~147pt to ~104pt.

The medium weight was introduced on the theory that regular reads thin
over a translucent backdrop. It does not: the vibrancy material and
`labelColor` already carry the contrast, and the extra weight only made
every row look emphasised.

Dropping semibold would have left `isProminent` a silent no-op, since
weight was the only thing it selected. It now drives the resting title
colour instead, which is how native menus signal emphasis.

Verified against a render of the live panel: width 219pt, row pitch
21.5pt, separator slot 11pt, no truncation of the longest label, and the
switch vertically centred with clearance inside the shorter row.
@ssddOnTop
ssddOnTop changed the base branch from feat/macos-menu-ui-polish to main August 3, 2026 18:18
@ssddOnTop
ssddOnTop marked this pull request as ready for review August 3, 2026 18:19
@ssddOnTop
ssddOnTop marked this pull request as draft August 3, 2026 18:19
… notes

Reviewing the rendered panel rather than the source:

- Corner radius goes back to 10pt on both backends. The 16pt glass value
  was reasoned from the HIG rounding popovers more generously, but at
  this panel's size it read as bubbled rather than native. Row highlight
  geometry reverts with it, to dx: 5 / radius: 6 on both paths.

- The hover and keyboard-focus fills on glass use
  unemphasizedSelectedContentBackgroundColor instead of an accent fill
  and an accent-blue focus ring. The system's own menu bar popovers mark
  a row with a neutral translucent wash, and the saturated accent was
  what made this panel read as non-native. The label stays labelColor
  there; selectedMenuItemTextColor is white and vanished into the wash.

- 'Retry Runtime Installation' becomes 'Retry Install', which fits the
  220pt panel instead of eliding to 'Retry Runtime Installati...'.

- Error notes wrap instead of eliding. maximumNumberOfLines = 2 had no
  effect because lineBreakMode was .byTruncatingTail, which pins a label
  to one line; 'Runtime download failed. Check y...' now shows in full.
  Wrapping also needs preferredMaxLayoutWidth or the label reports a
  one-line intrinsic height and the second line gets no room.

Verified on macOS 14.5 via the legacy path: 10/10 captures, 135 tests.
@ssddOnTop
ssddOnTop force-pushed the ci/macos-menu-screenshots branch from 9b6d27c to a79cb06 Compare August 3, 2026 19:10
The NSBox separator reads as a hard border cutting across the panel,
which is wrong against Liquid Glass: the material has no internal
dividers of its own and a drawn line breaks its continuity. The two
app-level commands are now fenced off by the same groupSpacer() already
used at the other group boundaries, so the grouping survives without a
rule through it.
@ssddOnTop
ssddOnTop force-pushed the ci/macos-menu-screenshots branch from 406bc4c to 82b1a84 Compare August 3, 2026 21:03
cornerRadius rounds the glass itself, but the content view and its rows
kept painting square out to the bounds, so the corners showed pointy
tabs poking past the curve and a row highlight squared off the top and
bottom edge. clipsToBounds makes the content follow the same silhouette,
which is what the system popovers do.
@ssddOnTop
ssddOnTop force-pushed the ci/macos-menu-screenshots branch from 82b1a84 to fc9be2a Compare August 3, 2026 21:10
This reverts commit 01c46ab.

The separator was removed while chasing a border the user was seeing,
but the real cause was NSGlassEffectView not clipping its content to
cornerRadius, fixed separately in 5afadf6. The system popovers and
1Password both keep a hairline between groups, so the line belongs
here too; it just needed ends that follow the panel's curve.
The capture loop nested RunLoop.run(until:) inside the run loop AppKit was
still starting from applicationDidFinishLaunching, so the app stalled after
the first capture instead of advancing. macOS 26 produced 1 of 20 images and
macOS 15 produced none; both then hit the script timeout.

Each shot is now queued back onto the main queue with
DispatchQueue.main.asyncAfter, letting AppKit finish its own loop turn between
captures. The script timeout goes to 300s for headroom over the ~15s the
20-shot sequence needs.
Two async fixes failed to stop the app wedging after the first capture,
so the sequencing is removed rather than repaired. The app now takes
FORGE_SCREENSHOT_INDEX and captures exactly one shot before exiting, and
the script loops the binary once per shot with its own timeout. A wedge
can now cost at most one image instead of the whole run, and each shot
gets a clean AppKit state.

Verified locally: 10 of 10 captured, previously 1.

The screenshots job also no longer needs the test job. It only requires
a binary, not a verified release, so it now builds its own in a
dedicated job and reviewers get images without waiting on the suite and
packaging.
The per-process run captured 17 of 20 on macOS 26 and 9 of 10 on 15,
with the gaps scattered rather than clustered, so the remaining failures
are transient per-process rather than systematic. Shots are independent,
so a failed one is retried once. The outcome notice also reports the
actual image count instead of asserting success.
Liquid Glass is a real-time refraction shader, so a capture can look
flat for reasons invisible in the image: Reduce Transparency disables it
outright (and Increase Contrast silently forces that on), and hosted
runners are virtualized without GPU acceleration. The report now records
both accessibility settings and warns that CI captures are reliable for
layout, wording, colour and radius but not for how glassy the material
looks.
@ssddOnTop
ssddOnTop force-pushed the ci/macos-menu-screenshots branch from 83531de to 2809d46 Compare August 4, 2026 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant