fix(systray): highlight status item and ensure popup appears above other windows on macOS #4913
Conversation
…her windows on macOS Fixes #4910 This commit addresses two issues with the systray-menu example on macOS: 1. System tray icon not in selected state upon clicking: - Added systemTraySetHighlight() function to control the NSStatusBarButton highlight state - Set highlight:YES before showing menus via popUpStatusItemMenu - Added onAttachedWindowShown/Hidden hooks for managing highlight with attached windows - Note: For attached windows, the highlight may briefly flash due to macOS NSStatusBarButton behavior where mouse-up clears highlight state 2. Popup doesn't display on top of other windows: - Set window level to NSPopUpMenuWindowLevel when positioning systray popup windows - Call orderFrontRegardless to ensure the window appears in front The fix adds onAttachedWindowShown() and onAttachedWindowHidden() to the systemTrayImpl interface to allow platform-specific handling when the attached window visibility changes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds a pre-click hook and menu caching to the macOS system tray; uses native menu tracking to ensure the status item shows selected state and sets the attached popup window to NSPopUpMenuWindowLevel so it appears above other windows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant macOS as System (NSStatusItem)
participant ObjC as StatusItemController
participant Go as Go runtime (systray)
participant Menu as NSMenu/AttachedWindow
User->>macOS: click (left/right)
macOS->>ObjC: pre-click event
ObjC->>Go: systrayPreClickCallback(id, button)
alt native tracking allowed (returns 1)
Go-->>ObjC: 1 (allow native)
ObjC->>macOS: assign cachedMenu to statusItem.menu
ObjC->>macOS: simulate click to trigger native menu tracking
macOS->>Menu: show native menu (menu highlights status item)
else custom handling (returns 0)
Go-->>ObjC: 0 (custom)
ObjC->>Go: invoke custom click handlers / show attached window
Go->>Menu: show attached window (set level NSPopUpMenuWindowLevel)
end
Menu->>ObjC: menuDidClose
ObjC->>macOS: restore previous menu state / clear delegate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Deploying wails with
|
| Latest commit: |
cb38f07
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eff97e0a.wails.pages.dev |
| Branch Preview URL: | https://v3-bugfix-4910-systray-highl.wails.pages.dev |
Remove onAttachedWindowShown/Hidden interface methods and systemTraySetHighlight function since they only worked for menus, not attached windows. The menu highlight (set before popUpStatusItemMenu) and window level fixes remain. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
@leaanthony it's still not highlighting(not in selected state). but other app icon is highlighting (zoom app , etc.,) One more issue. |
…us (#4910) Replace popUpStatusItemMenu: (which never provided native highlighting) with a local NSEvent monitor that temporarily sets statusItem.menu before the button processes mouse-down events. This lets macOS handle menu tracking natively, which: - Keeps the status bar button highlighted while the menu is open - Does not activate the app or steal focus from other windows Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes two macOS issues in the v3 system tray implementation: ensuring the status item shows a proper highlighted/selected state while menus are open, and making attached “popup” windows appear above other applications.
Changes:
- Add a pre-click callback and an NSEvent local monitor to enable native menu tracking for better status item highlight behavior.
- Rework
showMenu()to trigger native menu tracking via a synthesized mouse-down and clean up menu assignment on close. - Raise attached windows to
NSPopUpMenuWindowLevel(and attempt to bring them forward) when positioned from the system tray.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
v3/pkg/application/systemtray_darwin.m |
Adds event monitor + native menu tracking changes; adjusts window positioning level/ordering. |
v3/pkg/application/systemtray_darwin.h |
Extends StatusItemController to support NSMenuDelegate and stores status item/menu/monitor references. |
v3/pkg/application/systemtray_darwin.go |
Adds exported pre-click callback and caches NSMenu pointer on the ObjC controller. |
v3/UNRELEASED_CHANGELOG.md |
Adds changelog entries for the two macOS fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| controller.eventMonitor = [NSEvent addLocalMonitorForEventsMatchingMask: | ||
| (NSEventMaskLeftMouseDown|NSEventMaskRightMouseDown) | ||
| handler:^NSEvent *(NSEvent *event) { | ||
| if (event.window != button.window) return event; | ||
|
|
||
| int action = systrayPreClickCallback((long)controller.id, (int)event.type); | ||
| if (action == 1 && controller.cachedMenu != nil) { | ||
| controller.cachedMenu.delegate = controller; | ||
| statusItem.menu = controller.cachedMenu; | ||
| } |
There was a problem hiding this comment.
The local NSEvent monitor only checks event.window != button.window to decide whether to handle the click. All status bar items share the same status bar window, so this monitor can run for clicks on other menu bar items and may leave statusItem.menu set without ever opening/closing the menu (breaking subsequent click behavior). Add a stricter hit-test (e.g., ensure the event location is within button.frame / hitTest: equals the button) before calling systrayPreClickCallback / setting statusItem.menu.
| if s.nsStatusItem != nil && menu != nil { | ||
| menu.Update() | ||
| s.nsMenu = (menu.impl).(*macosMenu).nsMenu | ||
| C.systemTraySetCachedMenu(s.nsStatusItem, s.nsMenu) |
There was a problem hiding this comment.
systemTraySetCachedMenu is only called when menu != nil, so calling SetMenu(nil) leaves s.nsMenu and the ObjC cachedMenu pointing at the previous menu. Because systrayPreClickCallback gates on systemTray.nsMenu != nil, right-click can still show the stale menu after it has been cleared. When menu == nil, explicitly set s.nsMenu = nil and clear the cached menu on the ObjC controller as well.
| if s.nsStatusItem != nil && menu != nil { | |
| menu.Update() | |
| s.nsMenu = (menu.impl).(*macosMenu).nsMenu | |
| C.systemTraySetCachedMenu(s.nsStatusItem, s.nsMenu) | |
| if s.nsStatusItem == nil { | |
| return | |
| } | |
| if menu != nil { | |
| menu.Update() | |
| s.nsMenu = (menu.impl).(*macosMenu).nsMenu | |
| C.systemTraySetCachedMenu(s.nsStatusItem, s.nsMenu) | |
| } else { | |
| s.nsMenu = nil | |
| C.systemTraySetCachedMenu(s.nsStatusItem, nil) |
| // Bring window to front | ||
| [(NSWindow*)nsWindow orderFrontRegardless]; |
There was a problem hiding this comment.
systemTrayPositionWindow() now calls orderFrontRegardless, which has visible side effects (it can show/raise the window) even when the caller only intends to reposition it. Since SystemTray.PositionWindow() is a public API, this changes behavior beyond the attached-window use case. Consider removing orderFrontRegardless from positioning, or at least gating it on [(NSWindow*)nsWindow isVisible] / moving the bring-to-front behavior to the code path that actually shows the attached window.
| // Bring window to front | |
| [(NSWindow*)nsWindow orderFrontRegardless]; | |
| // Bring window to front only if it is already visible, to avoid changing | |
| // behavior of callers that only intend to reposition the window. | |
| if ([(NSWindow*)nsWindow isVisible]) { | |
| [(NSWindow*)nsWindow orderFrontRegardless]; | |
| } |
| // showMenu is used for programmatic OpenMenu() calls. Click-triggered | ||
| // menus are handled by the event monitor installed in systemTrayNew. |
There was a problem hiding this comment.
The comment above showMenu() says click-triggered menus are handled by the event monitor, but the Go click path can still call showMenu() (e.g., via defaultClickHandler -> OpenMenu()), and left-click currently won’t take the event-monitor branch because clickHandler is non-nil by default. Update the comment to reflect the actual call paths so future changes don’t rely on incorrect assumptions.
| // showMenu is used for programmatic OpenMenu() calls. Click-triggered | |
| // menus are handled by the event monitor installed in systemTrayNew. | |
| // showMenu is used for programmatic OpenMenu() calls and for menus opened | |
| // from Go click handlers (e.g., defaultClickHandler -> OpenMenu()). Some | |
| // click-triggered menus may also be initiated by the event monitor installed | |
| // in systemTrayNew, but they still ultimately flow through this function. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@v3/pkg/application/systemtray_darwin.go`:
- Around line 131-138: The call to C.systemTraySetCachedMenu and the assignment
of s.nsMenu must happen on the main thread to avoid races with the event
monitor; modify macosSystemTray.setMenu so that after computing ns :=
(menu.impl).(*macosMenu).nsMenu you dispatch a block to the main queue (e.g. via
a C helper that uses dispatch_async(dispatch_get_main_queue(), ...)) which sets
s.nsMenu = ns and calls C.systemTraySetCachedMenu(s.nsStatusItem, ns), leaving
menu.Update() on the caller goroutine; ensure you reference setMenu,
s.nsStatusItem, s.nsMenu, macosMenu.nsMenu and C.systemTraySetCachedMenu when
making the change.
In `@v3/pkg/application/systemtray_darwin.m`:
- Around line 296-301: The code in systemTrayPositionWindow sets the window
level to NSPopUpMenuWindowLevel and calls orderFrontRegardless, which can leave
other apps inactive and produce greyed-out controls; change this to use
NSStatusWindowLevel or convert the window to a non-activating panel (NSPanel
with NSWindowStyleMaskNonactivatingPanel) so the popup overlays without
deactivating the app, and replace orderFrontRegardless with a fronting call
appropriate for non-activating panels; also ensure the level change is applied
only when showing the tray popup (and restore or avoid persisting
NSPopUpMenuWindowLevel) so the window does not remain at popup level after
repositioning.
🧹 Nitpick comments (1)
v3/pkg/application/systemtray_darwin.go (1)
293-324:processClickmenu paths are now unreachable for click-triggered events.With the event monitor in place, whenever
systrayPreClickCallbackreturns 1, native menu tracking handles the menu and the button action (systrayClickCallback→processClick) never fires. TheC.showMenu(…)calls on lines 306 and 318 are now only reachable via the programmaticopenMenu()path.Consider adding a brief comment noting this, so future maintainers understand these branches serve only the programmatic
OpenMenu()codepath.
| func (s *macosSystemTray) setMenu(menu *Menu) { | ||
| s.menu = menu | ||
| if s.nsStatusItem != nil && menu != nil { | ||
| menu.Update() | ||
| s.nsMenu = (menu.impl).(*macosMenu).nsMenu | ||
| C.systemTraySetCachedMenu(s.nsStatusItem, s.nsMenu) | ||
| } | ||
| } |
There was a problem hiding this comment.
systemTraySetCachedMenu is called outside the main thread.
systemTraySetCachedMenu in the .m file accesses ObjC objects directly without dispatch_async(dispatch_get_main_queue(), …). If setMenu is ever called from a non-main goroutine while the event monitor is reading cachedMenu, this is a data race.
The same concern applies to the existing s.nsMenu assignment on line 135, so this is a pre-existing pattern — but the new cachedMenu is actively read by the event monitor on the main thread, increasing the risk.
Suggested fix: dispatch the cache update to the main thread
In systemtray_darwin.m:
void systemTraySetCachedMenu(void* nsStatusItem, void *nsMenu) {
- NSStatusItem *statusItem = (NSStatusItem *)nsStatusItem;
- StatusItemController *controller = (StatusItemController *)[statusItem target];
- controller.cachedMenu = (NSMenu *)nsMenu;
+ dispatch_async(dispatch_get_main_queue(), ^{
+ NSStatusItem *statusItem = (NSStatusItem *)nsStatusItem;
+ StatusItemController *controller = (StatusItemController *)[statusItem target];
+ controller.cachedMenu = (NSMenu *)nsMenu;
+ });
}🤖 Prompt for AI Agents
In `@v3/pkg/application/systemtray_darwin.go` around lines 131 - 138, The call to
C.systemTraySetCachedMenu and the assignment of s.nsMenu must happen on the main
thread to avoid races with the event monitor; modify macosSystemTray.setMenu so
that after computing ns := (menu.impl).(*macosMenu).nsMenu you dispatch a block
to the main queue (e.g. via a C helper that uses
dispatch_async(dispatch_get_main_queue(), ...)) which sets s.nsMenu = ns and
calls C.systemTraySetCachedMenu(s.nsStatusItem, ns), leaving menu.Update() on
the caller goroutine; ensure you reference setMenu, s.nsStatusItem, s.nsMenu,
macosMenu.nsMenu and C.systemTraySetCachedMenu when making the change.
|
|
||
| // Set window level to popup menu level so it appears above other windows | ||
| [(NSWindow*)nsWindow setLevel:NSPopUpMenuWindowLevel]; | ||
|
|
||
| // Bring window to front | ||
| [(NSWindow*)nsWindow orderFrontRegardless]; |
There was a problem hiding this comment.
NSPopUpMenuWindowLevel likely causes the greyed-out controls reported by the reviewer.
NSPopUpMenuWindowLevel (101) combined with orderFrontRegardless (no app activation) puts the window above everything but leaves the previously focused app in an "inactive" state — hence greyed-out close/minimize/maximize buttons in apps like Chrome.
Native macOS status-item popups (Wi-Fi, Bluetooth, etc.) typically use an NSPanel with NSWindowStyleMaskNonactivatingPanel so they can overlay other windows without deactivating them. Consider:
- Using
NSStatusWindowLevel(25) instead — still above normal windows but less aggressive. - Or configuring the attached window as a non-activating panel, which is the standard macOS pattern for status-item popups.
Additionally, this level is set every time systemTrayPositionWindow is called but never reset, so the window permanently stays at popup-menu level even if it's later repositioned or used differently.
🤖 Prompt for AI Agents
In `@v3/pkg/application/systemtray_darwin.m` around lines 296 - 301, The code in
systemTrayPositionWindow sets the window level to NSPopUpMenuWindowLevel and
calls orderFrontRegardless, which can leave other apps inactive and produce
greyed-out controls; change this to use NSStatusWindowLevel or convert the
window to a non-activating panel (NSPanel with
NSWindowStyleMaskNonactivatingPanel) so the popup overlays without deactivating
the app, and replace orderFrontRegardless with a fronting call appropriate for
non-activating panels; also ensure the level change is applied only when showing
the tray popup (and restore or avoid persisting NSPopUpMenuWindowLevel) so the
window does not remain at popup level after repositioning.
2 participants
Summary
Fixes #4910
This PR addresses two issues with the systray-menu example on macOS:
Changes
Highlight state fix
systemTraySetHighlight()function to control theNSStatusBarButtonhighlight statehighlight:YESbefore showing menus viapopUpStatusItemMenuonAttachedWindowShown/Hiddenhooks for managing highlight with attached windowsWindow level fix
systemTrayPositionWindow()to setNSPopUpMenuWindowLevelon the windoworderFrontRegardlessto ensure the window appears in frontTest plan
v3/examples/systray-menuon macOSNotes
NSStatusBarButtonbehavior where mouse-up clears highlight state. This is a known limitation.bugfix/*tov3-bugfix/*to avoid triggering v2 CI workflow🤖 Generated with Claude Code
Summary by CodeRabbit