Skip to content

feat(macos): replace popover with native-style borderless menu panel - #39

Closed
ssddOnTop wants to merge 10 commits into
mainfrom
feat/macos-menu-ui-polish
Closed

ssddOnTop wants to merge 10 commits into
mainfrom
feat/macos-menu-ui-polish

Conversation

@ssddOnTop

Copy link
Copy Markdown
Collaborator

No description provided.

@ssddOnTop ssddOnTop added the test-release Run the full Release workflow as a dry run on this PR (artifact only, no release touched) label Aug 2, 2026
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.
@ssddOnTop
ssddOnTop enabled auto-merge (squash) August 2, 2026 20:06
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 and others added 6 commits August 4, 2026 00:39
… 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.
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.
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.
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.
…lish

# Conflicts:
#	macos/Sources/ForgeMenuBar/AppDelegate.swift
@ssddOnTop

Copy link
Copy Markdown
Collaborator Author

fixed in #48

@ssddOnTop ssddOnTop closed this Aug 6, 2026
auto-merge was automatically disabled August 6, 2026 08:11

Pull request was closed

@ssddOnTop
ssddOnTop deleted the feat/macos-menu-ui-polish branch August 6, 2026 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test-release Run the full Release workflow as a dry run on this PR (artifact only, no release touched)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant