Fix macOS system tray menu real-time updates using NSMenuDelegate #4828
Conversation
Implements NSMenuDelegate to enable real-time menu updates while the system tray menu is open on macOS. Previously, menu.Update() calls would not refresh the displayed menu until it was closed and reopened. Changes: - Added MenuDelegate class implementing NSMenuDelegate protocol - Implemented menuNeedsUpdate: and menuWillOpen: delegate methods - Added nsMenuDelegate field to macosSystemTray struct - Updated setMenu() to create and attach delegate to NSMenu - Updated run() to set up delegate during initialization Fixes #4630 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughThis change implements a macOS system tray menu delegate pattern to enable real-time menu updates. A new Changes
Sequence DiagramsequenceDiagram
participant Go as Go Code
participant Tray as macOS SystemTray
participant Delegate as MenuDelegate
participant NSMenu as NSMenu (native)
Note over Go,NSMenu: Initial Setup (run)
Go->>Tray: run() initializes NSStatusItem
Tray->>Delegate: createMenuDelegate(menuPtr, trayID)
Delegate-->>Tray: return delegate instance
Tray->>NSMenu: setMenuDelegate(nsMenu, delegate)
NSMenu->>Delegate: assign as delegate
Note over Go,NSMenu: Real-time Update Flow (user opens menu)
NSMenu->>Delegate: menuNeedsUpdate:(menu)
Delegate-->>Go: triggers on-demand callback
Go->>NSMenu: menu reflects live changes
Note over Go,NSMenu: Menu Update (programmatic)
Go->>Tray: setMenu(newMenu) on main thread
Tray->>Delegate: initialize/assign delegate if needed
Tray->>NSMenu: setMenuDelegate(nsMenu, delegate)
NSMenu->>Delegate: delegate now active
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
v3/pkg/application/systemtray_darwin.go (1)
114-118: Delegate'smenuPtrmay become stale on subsequent menu changes.The delegate is created once (when
nsMenuDelegate == nil) with the currents.nsMenupointer. IfsetMenuis called again with a different menu, the delegate retains the oldmenuPtrreference. While the current delegate implementation doesn't usemenuPtr, this could cause issues if future changes rely on it.Consider updating
menuPtron every call or documenting this limitation:🔎 Suggested fix
// Set up the delegate if not already done if s.nsMenuDelegate == nil { s.nsMenuDelegate = C.createMenuDelegate(s.nsMenu, C.long(s.id)) + } else { + // Update the delegate's menuPtr if menu changed + C.updateMenuDelegatePtr(s.nsMenuDelegate, s.nsMenu) } C.setMenuDelegate(s.nsMenu, s.nsMenuDelegate)This would require adding an
updateMenuDelegatePtrfunction in the Objective-C layer.v3/pkg/application/systemtray_darwin.m (1)
262-267: Consider delegate lifecycle management.The
MenuDelegateis allocated with[[MenuDelegate alloc] init]but there's no corresponding release when the system tray is destroyed. While the delegate likely lives for the application lifetime, if system trays can be dynamically created/destroyed, this could leak memory.If needed, store the delegate reference and release it in
systemTrayDestroy:// In systemTrayDestroy, if delegate is tracked: // [delegate release];
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v3/UNRELEASED_CHANGELOG.mdv3/pkg/application/systemtray_darwin.gov3/pkg/application/systemtray_darwin.hv3/pkg/application/systemtray_darwin.m
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-24T22:41:18.566Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
Applied to files:
v3/pkg/application/systemtray_darwin.go
🪛 LanguageTool
v3/UNRELEASED_CHANGELOG.md
[style] ~26-~26: This phrase is redundant (‘OS’ stands for ‘operating system’). Use simply “macOS”.
Context: ... --> ## Fixed - Fix macOS system tray menu real-time updates using NSMen...
(ACRONYM_TAUTOLOGY)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (go)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
v3/UNRELEASED_CHANGELOG.md (1)
26-26: LGTM!The changelog entry clearly describes the fix and references the correct issue number. Note: The static analysis warning about "macOS" is a false positive—"macOS" is Apple's official branding and is correct here.
v3/pkg/application/systemtray_darwin.go (2)
48-48: LGTM!The new
nsMenuDelegatefield appropriately holds the Objective-C delegate reference usingunsafe.Pointerfor CGo interop.
188-190: LGTM!The delegate setup during initial
run()is correctly placed aftermenu.Update()ensures thensMenupointer is valid.v3/pkg/application/systemtray_darwin.h (2)
10-13: LGTM!The
MenuDelegateinterface declaration correctly adoptsNSMenuDelegateprotocol with appropriate properties for CGo interop.
30-31: LGTM!Function declarations are well-defined and match the implementation signatures.
v3/pkg/application/systemtray_darwin.m (2)
19-32: Clarify the real-time update mechanism.Both delegate methods are effectively no-ops. The comment says "The Go side will handle the actual menu rebuilding through menu.Update()" but there's no callback to Go from these methods.
Could you clarify: does simply having a delegate attached cause macOS to refresh the menu display when the underlying
NSMenuitems change? If the intent is formenuNeedsUpdate:to trigger Go-side updates, a callback would be needed.
270-274: LGTM!The
setMenuDelegatefunction correctly casts and assigns the delegate to theNSMenu.
Deploying wails with
|
| Latest commit: |
86f96d5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96019536.wails.pages.dev |
| Branch Preview URL: | https://fix-systemtray-menu-realtime.wails.pages.dev |
Adds NSMenuDelegate methods menuWillOpen: and menuDidClose: to call the existing onMenuOpen and onMenuClose callbacks on SystemTray, bringing macOS to parity with Windows and Linux implementations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
|
In #4719, I reported two behaviors:
|
Summary
Implements NSMenuDelegate to enable real-time menu updates while the system tray menu is open on macOS. Previously,
menu.Update()calls would not refresh the displayed menu until it was closed and reopened.Also implements
onMenuOpenandonMenuClosecallbacks for macOS, bringing it to parity with Windows and Linux.Changes
MenuDelegateclass implementingNSMenuDelegateprotocolmenuNeedsUpdate:,menuWillOpen:, andmenuDidClose:delegate methodsnsMenuDelegatefield tomacosSystemTraystructsetMenu()to create and attach delegate to NSMenurun()to set up delegate during initializationsystrayMenuOpenCallbackandsystrayMenuCloseCallbackHow it works
MenuDelegateis attached to the NSMenu when menu is setmenu.Update()is called, NSMenu is rebuilt in-placemenuNeedsUpdate:before display and periodicallymenuWillOpen:triggersonMenuOpencallbackmenuDidClose:triggersonMenuClosecallbackPlatform parity
Fixes #4719
🤖 Generated with Claude Code