Refactor macOS menu event handling to use blocks

Replaces C++ back-pointer event emission for menu and menu item events with Objective-C blocks and associated objects for safer and more flexible event handling. Removes legacy event emission methods from MenuItem, updates context menu example to use new MenuOpenedEvent and MenuClosedEvent listeners, and improves cleanup logic for menu and menu item delegates.

Author: lijy91

examples/window_example/main.cpp

@@ -7,12 +7,14 @@ using nativeapi::DisplayAddedEvent;
 using nativeapi::DisplayManager;
 using nativeapi::DisplayRemovedEvent;
 using nativeapi::Menu;
+using nativeapi::MenuClosedEvent;
 using nativeapi::MenuItem;
 using nativeapi::MenuItemClickedEvent;
 using nativeapi::MenuItemState;
 using nativeapi::MenuItemSubmenuClosedEvent;
 using nativeapi::MenuItemSubmenuOpenedEvent;
 using nativeapi::MenuItemType;
+using nativeapi::MenuOpenedEvent;
 using nativeapi::TrayIcon;
 using nativeapi::TrayIconClickedEvent;
 using nativeapi::TrayIconDoubleClickedEvent;
@@ -85,10 +87,21 @@ int main() {
     std::cout << "Tray Title: "
               << (title.has_value() ? title.value() : "(no title)")
               << std::endl;
+    tray_icon.SetVisible(true);
 
     // Create context menu
     auto context_menu = Menu::Create();
 
+    context_menu->AddListener<MenuOpenedEvent>(
+        [](const MenuOpenedEvent& event) {
+          std::cout << "Menu opened" << std::endl;
+        });
+
+    context_menu->AddListener<MenuClosedEvent>(
+        [](const MenuClosedEvent& event) {
+          std::cout << "Menu closed" << std::endl;
+        });
+
     // Add menu items
     auto show_window_item =
         MenuItem::Create("Show Window", MenuItemType::Normal);

src/menu.h

@@ -447,26 +447,6 @@ class MenuItem : public EventEmitter, public NativeObjectProvider {
    */
   bool Trigger();
 
-  /**
-   * @brief Emit a menu item selected event.
-   *
-   * This method is used internally by platform implementations to
-   * emit selection events when a menu item is clicked.
-   *
-   * @param item_text The text of the selected menu item
-   */
-  void EmitSelectedEvent(const std::string& item_text);
-
-  /**
-   * @brief Emit a menu item state changed event.
-   *
-   * This method is used internally by platform implementations to
-   * emit state change events when a checkable menu item's state changes.
-   *
-   * @param checked The new checked state of the menu item
-   */
-  void EmitStateChangedEvent(bool checked);
-
  protected:
   /**
    * @brief Internal method to get the platform-specific native menu item object.

src/platform/macos/menu_macos.mm

@@ -7,18 +7,24 @@
 
 // Import Cocoa headers
 #import <Cocoa/Cocoa.h>
+#import <objc/runtime.h>
 
 // Note: This file assumes ARC (Automatic Reference Counting) is enabled
 // for proper memory management of Objective-C objects.
 
 // Forward declarations - moved to global scope
+typedef void (^MenuItemClickedBlock)(nativeapi::MenuItemID item_id, const std::string& item_text);
+typedef void (^MenuOpenedBlock)(nativeapi::MenuID menu_id);
+typedef void (^MenuClosedBlock)(nativeapi::MenuID menu_id);
+
 @interface MenuItemTarget : NSObject
-@property(nonatomic, assign) void* cppMenuItem;
+@property(nonatomic, copy) MenuItemClickedBlock clickedBlock;
 - (void)menuItemClicked:(id)sender;
 @end
 
 @interface MenuDelegate : NSObject <NSMenuDelegate>
-@property(nonatomic, assign) void* cppMenu;
+@property(nonatomic, copy) MenuOpenedBlock openedBlock;
+@property(nonatomic, copy) MenuClosedBlock closedBlock;
 @end
 
 namespace nativeapi {
@@ -111,31 +117,42 @@ @interface MenuDelegate : NSObject <NSMenuDelegate>
 @implementation MenuItemTarget
 - (void)menuItemClicked:(id)sender {
   NSMenuItem* menuItem = (NSMenuItem*)sender;
-  // Use direct back-pointer to the C++ MenuItem
-  nativeapi::MenuItem* menuItemPtr = static_cast<nativeapi::MenuItem*>(_cppMenuItem);
-  if (!menuItemPtr) {
-    return;
-  }
-  // Don't automatically handle state changes - let user code control the state
-  // This prevents double-toggling when user event handlers also call SetChecked
 
-  // Emit click event for all types
-  menuItemPtr->EmitSelectedEvent([[menuItem title] UTF8String]);
+  // Call the block if it exists
+  if (_clickedBlock) {
+    std::string itemText = [[menuItem title] UTF8String];
+    // Get the MenuItemID from the menu item's associated object
+    NSNumber* itemIdObj = objc_getAssociatedObject(menuItem, @"menuItemId");
+    if (itemIdObj) {
+      nativeapi::MenuItemID itemId = [itemIdObj longValue];
+      _clickedBlock(itemId, itemText);
+    }
+  }
 }
 @end
 
 // Implementation of MenuDelegate - moved to global scope
 @implementation MenuDelegate
 - (void)menuWillOpen:(NSMenu*)menu {
-  nativeapi::Menu* menuPtr = static_cast<nativeapi::Menu*>(_cppMenu);
-  if (menuPtr)
-    menuPtr->EmitOpenedEvent();
+  if (_openedBlock) {
+    // Get the MenuID from the menu's associated object
+    NSNumber* menuIdObj = objc_getAssociatedObject(menu, @"menuId");
+    if (menuIdObj) {
+      nativeapi::MenuID menuId = [menuIdObj longValue];
+      _openedBlock(menuId);
+    }
+  }
 }
 
 - (void)menuDidClose:(NSMenu*)menu {
-  nativeapi::Menu* menuPtr = static_cast<nativeapi::Menu*>(_cppMenu);
-  if (menuPtr)
-    menuPtr->EmitClosedEvent();
+  if (_closedBlock) {
+    // Get the MenuID from the menu's associated object
+    NSNumber* menuIdObj = objc_getAssociatedObject(menu, @"menuId");
+    if (menuIdObj) {
+      nativeapi::MenuID menuId = [menuIdObj longValue];
+      _closedBlock(menuId);
+    }
+  }
 }
 @end
 
@@ -179,6 +196,9 @@ - (void)menuDidClose:(NSMenu*)menu {
     }
 
     if (target_) {
+      // Clean up blocks first
+      target_.clickedBlock = nil;
+
       // Remove target and action to prevent callbacks after destruction
       [ns_menu_item_ setTarget:nil];
       [ns_menu_item_ setAction:nil];
@@ -209,7 +229,8 @@ - (void)menuDidClose:(NSMenu*)menu {
   auto item = std::shared_ptr<MenuItem>(new MenuItem(text, type));
   item->pimpl_ = std::make_unique<Impl>(nsItem, type);
   item->id = g_next_menu_item_id++;
-  [item->pimpl_->target_ setCppMenuItem:(void*)item.get()];
+  objc_setAssociatedObject(nsItem, @"menuItemId", [NSNumber numberWithLong:item->id],
+                           OBJC_ASSOCIATION_RETAIN_NONATOMIC);
   item->pimpl_->text_ = text;
 
   return item;
@@ -222,15 +243,26 @@ - (void)menuDidClose:(NSMenu*)menu {
 MenuItem::MenuItem(void* native_item)
     : id(g_next_menu_item_id++),
       pimpl_(std::make_unique<Impl>((__bridge NSMenuItem*)native_item, MenuItemType::Normal)) {
-  [pimpl_->target_ setCppMenuItem:(void*)this];
+  NSMenuItem* nsItem = (__bridge NSMenuItem*)native_item;
+  objc_setAssociatedObject(nsItem, @"menuItemId", [NSNumber numberWithLong:id],
+                           OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+
+  // 设置默认的 Block 处理器，直接发送事件
+  pimpl_->target_.clickedBlock = ^(MenuItemID item_id, const std::string& item_text) {
+    try {
+      EmitSync<MenuItemClickedEvent>(item_id, item_text);
+    } catch (...) {
+      // Protect against event emission exceptions
+    }
+  };
 }
 
 MenuItem::MenuItem(const std::string& text, MenuItemType type)
     : id(0) {  // Will be set in Create method
 }
 
 MenuItem::~MenuItem() {
-  // No global registry cleanup required
+  // No special cleanup needed since we're not storing C++ object references
 }
 
 MenuItemType MenuItem::GetType() const {
@@ -377,12 +409,13 @@ - (void)menuDidClose:(NSMenu*)menu {
             continue;
           NSObject* targetObj = [sibling target];
           if ([targetObj isKindOfClass:[MenuItemTarget class]]) {
-            MenuItemTarget* siblingTarget = (MenuItemTarget*)targetObj;
-            nativeapi::MenuItem* otherItem =
-                static_cast<nativeapi::MenuItem*>([siblingTarget cppMenuItem]);
-            if (otherItem && otherItem->GetType() == MenuItemType::Radio &&
-                otherItem->GetRadioGroup() == pimpl_->radio_group_) {
-              otherItem->pimpl_->state_ = MenuItemState::Unchecked;
+            // Get the MenuItemID from the associated object
+            NSNumber* siblingIdObj = objc_getAssociatedObject(sibling, @"menuItemId");
+            if (siblingIdObj) {
+              MenuItemID siblingId = [siblingIdObj longValue];
+              // Find the corresponding MenuItem in the parent menu's items
+              // This is a simplified approach - in practice, you might need to store
+              // a reference to the parent menu or use a different strategy
               [sibling setState:NSControlStateValueOff];
             }
           }
@@ -426,23 +459,18 @@ - (void)menuDidClose:(NSMenu*)menu {
   if (!pimpl_->enabled_)
     return false;
 
-  [pimpl_->target_ menuItemClicked:pimpl_->ns_menu_item_];
+  // Call the block directly instead of going through target-action
+  if (pimpl_->target_.clickedBlock) {
+    std::string itemText = pimpl_->text_;
+    pimpl_->target_.clickedBlock(id, itemText);
+  }
   return true;
 }
 
 void* MenuItem::GetNativeObjectInternal() const {
   return (__bridge void*)pimpl_->ns_menu_item_;
 }
 
-void MenuItem::EmitSelectedEvent(const std::string& item_text) {
-  EmitSync<MenuItemClickedEvent>(id, item_text);
-}
-
-void MenuItem::EmitStateChangedEvent(bool checked) {
-  // This method is kept for compatibility but doesn't emit events
-  // State change events are handled through the regular click event
-}
-
 // Menu::Impl implementation
 class Menu::Impl {
  public:
@@ -460,6 +488,10 @@ - (void)menuDidClose:(NSMenu*)menu {
   ~Impl() {
     // First, remove delegate to prevent callbacks during cleanup
     if (delegate_) {
+      // Clean up blocks first
+      delegate_.openedBlock = nil;
+      delegate_.closedBlock = nil;
+
       [ns_menu_ setDelegate:nil];
       delegate_ = nil;
     }
@@ -475,21 +507,56 @@ - (void)menuDidClose:(NSMenu*)menu {
   auto menu = std::shared_ptr<Menu>(new Menu());
   menu->pimpl_ = std::make_unique<Impl>(nsMenu);
   menu->id = g_next_menu_id++;
-  [menu->pimpl_->delegate_ setCppMenu:(void*)menu.get()];
+  objc_setAssociatedObject(nsMenu, @"menuId", [NSNumber numberWithLong:menu->id],
+                           OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+  // 设置默认的 Block 处理器，直接发送事件
+  menu->pimpl_->delegate_.openedBlock = ^(MenuID menu_id) {
+    try {
+      menu->EmitSync<MenuOpenedEvent>(menu_id);
+    } catch (...) {
+      // Protect against event emission exceptions
+    }
+  };
 
+  menu->pimpl_->delegate_.closedBlock = ^(MenuID menu_id) {
+    try {
+      menu->EmitSync<MenuClosedEvent>(menu_id);
+    } catch (...) {
+      // Protect against event emission exceptions
+    }
+  };
   return menu;
 }
 
 Menu::Menu(void* native_menu)
     : id(g_next_menu_id++), pimpl_(std::make_unique<Impl>((__bridge NSMenu*)native_menu)) {
-  [pimpl_->delegate_ setCppMenu:(void*)this];
+  NSMenu* nsMenu = (__bridge NSMenu*)native_menu;
+  objc_setAssociatedObject(nsMenu, @"menuId", [NSNumber numberWithLong:id],
+                           OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+
+  // 设置默认的 Block 处理器，直接发送事件
+  pimpl_->delegate_.openedBlock = ^(MenuID menu_id) {
+    try {
+      EmitSync<MenuOpenedEvent>(menu_id);
+    } catch (...) {
+      // Protect against event emission exceptions
+    }
+  };
+
+  pimpl_->delegate_.closedBlock = ^(MenuID menu_id) {
+    try {
+      EmitSync<MenuClosedEvent>(menu_id);
+    } catch (...) {
+      // Protect against event emission exceptions
+    }
+  };
 }
 
 Menu::Menu() : id(0) {  // Will be set in Create method
 }
 
 Menu::~Menu() {
-  // No global registry cleanup required
+  // No special cleanup needed since we're not storing C++ object references
 }
 
 void Menu::AddItem(std::shared_ptr<MenuItem> item) {
@@ -687,10 +754,18 @@ - (void)menuDidClose:(NSMenu*)menu {
 }
 
 void Menu::EmitOpenedEvent() {
-  EmitSync<MenuOpenedEvent>(id);
+  // This method is kept for compatibility but events are now handled through blocks
+  // Call the block if it exists
+  if (pimpl_->delegate_.openedBlock) {
+    pimpl_->delegate_.openedBlock(id);
+  }
 }
 
 void Menu::EmitClosedEvent() {
-  EmitSync<MenuClosedEvent>(id);
+  // This method is kept for compatibility but events are now handled through blocks
+  // Call the block if it exists
+  if (pimpl_->delegate_.closedBlock) {
+    pimpl_->delegate_.closedBlock(id);
+  }
 }
 }  // namespace nativeapi

src/platform/macos/tray_icon_macos.mm

@@ -333,8 +333,8 @@ @interface TrayIconMenuDelegate : NSObject <NSMenuDelegate>
     return false;
   }
 
-  // Set our menu delegate to handle menu close events
-  [nativeMenu setDelegate:pimpl_->menu_delegate_];
+  //  // Set our menu delegate to handle menu close events
+  //  [nativeMenu setDelegate:pimpl_->menu_delegate_];
 
   // Set the menu to the status item (like Swift version)
   pimpl_->ns_status_item_.menu = nativeMenu;