fix(darwin): use statusItem.menu + NSMenuDelegate for robust multi-display rendering#114
Open
yangglive wants to merge 3 commits into
Open
fix(darwin): use statusItem.menu + NSMenuDelegate for robust multi-display rendering#114yangglive wants to merge 3 commits into
yangglive wants to merge 3 commits into
Conversation
…mode Upstream show_menu passes atLocation:NSMakePoint(0, height+6), which places the menu's top edge above the status item button. On MacBook notch displays, combined with the system-reserved menubar safe area, AppKit mis-measures the available space and flips the menu into scroll mode (a ^ arrow appears at the top of the menu and some rows are hidden). External displays aren't affected, so the bug only manifests on the laptop's internal screen. Anchoring at (0, 0) lets AppKit measure from the button itself and renders the menu normally on both notch and external screens.
…endering
The previous (0,0) anchor fix only resolved the notch-screen scroll-mode
issue on the built-in display. On external monitors — especially when
"Displays have separate Spaces" is enabled and each screen gets its own
mirrored menubar — AppKit still measured available height against the
wrong screen and flipped the menu into scroll mode ("^" arrow at top).
Root cause: popUpMenuPositioningItem:atLocation:inView: doesn't reliably
route the popup to the clicked mirror; AppKit picks a screen based on the
button's window geometry, not the event's screen.
Fix: attach the menu permanently via statusItem.menu = self->menu so
AppKit handles click-to-open natively on each mirror. Drop the manual
NSEvent local monitor, button.action/leftMouseClicked, and the
RightClickDetector subview — they existed to synthesise clicks from Go
callbacks, but break under the native-menu path.
Trade-off: upstream systray's tappedLeft / tappedRight custom hooks are
bypassed in this fork. Kopi doesn't need them; upstream should keep them
behind a runtime switch if they want this approach.
Additional fix: NSStatusItem's auto-highlight on button press doesn't
fire reliably under the statusItem.menu path, so drive the press-state
explicitly via NSMenuDelegate's menuWillOpen: / menuDidClose: — toggling
button.highlighted around menu open/close. Scoped to the top-level menu
so submenus don't retrigger the status button state.
e493b2b to
18c86f7
Compare
…table When all checkable items in the status item menu are in NSOffState, AppKit drops the leading state column (the checkmark indent), narrowing the popup. Toggling any item back on snaps the column back — the menu width visibly oscillates as the user clicks around. NSMenu does not expose a public showsStateColumn property (despite some forum suggestions; the runtime silently ignores the call). The reliable public-API fix is NSMenu.minimumWidth — pin a width that comfortably fits the longest item, and AppKit no longer narrows the popup below it. Note: items still re-align horizontally when the state column appears or disappears (native NSMenu behavior — kopi accepts this trade for keeping the standard macOS look). 220pt is sized for kopi's longest entry "Open Settings…" with checkmark indent + tooltip buffer.
18c86f7 to
6424f9b
Compare
Member
Does this mean we can no longer differentiate between left and right click? that is not OK. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On macOS, the current
popUpMenuPositioningItem:atLocation:inView:approach is fragile in two scenarios:^arrow appears at the top and some items get hidden.Both symptoms look the same (
^truncation), but they have different root causes. A previous revision of this PR only addressed #1 by changing the anchor to(0, 0). That wasn't enough for #2.Fix
Attach the menu permanently to the status item via
statusItem.menu = self->menuinapplicationDidFinishLaunching. AppKit then handles click-to-open natively on each mirror with correct geometry measurement per display.Consequences:
NSEvent addLocalMonitorForEventsMatchingMaskclick interceptor.button.action = @selector(leftMouseClicked).RightClickDetectorsubview.show_menubecomes a no-op (kept for ABI compat with Go'sC.show_menu()fallback callers).The native-menu path also broke the status button's press-state auto-highlight. Restore it explicitly via
NSMenuDelegate:menuWillOpen:→button.highlighted = YESmenuDidClose:→button.highlighted = NOScoped to the top-level menu only, so submenus don't retrigger the status-button highlight.
Trade-off
The existing
tappedLeft/tappedRightcustom-callback hooks are bypassed under this path — AppKit owns click handling now. Apps that rely on those hooks would regress. If upstream wants to land this, one option is gating the behavior behind a build tag or runtime switch; I'm happy to iterate on the shape.Testing
Verified on macOS 15 with:
No
^scroll mode in any configuration. Press-state highlight fires correctly when the menu opens on either screen.