Merge pull request #1134 from atom/menu-icon

Enable setting icon for MenuItem

atom/browser/api/atom_api_menu.cc

@@ -6,7 +6,9 @@
 
 #include "atom/browser/native_window.h"
 #include "atom/common/native_mate_converters/accelerator_converter.h"
+#include "atom/common/native_mate_converters/image_converter.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
+#include "native_mate/callback.h"
 #include "native_mate/constructor.h"
 #include "native_mate/dictionary.h"
 #include "native_mate/object_template_builder.h"
@@ -17,30 +19,6 @@ namespace atom {
 
 namespace api {
 
-namespace {
-
-// Call method of delegate object.
-v8::Handle<v8::Value> CallDelegate(v8::Isolate* isolate,
-                                   v8::Handle<v8::Value> default_value,
-                                   v8::Handle<v8::Object> menu,
-                                   const char* method,
-                                   int command_id) {
-  v8::Handle<v8::Value> delegate = menu->Get(
-      MATE_STRING_NEW(isolate, "delegate"));
-  if (!delegate->IsObject())
-    return default_value;
-
-  v8::Handle<v8::Function> function = v8::Handle<v8::Function>::Cast(
-      delegate->ToObject()->Get(MATE_STRING_NEW(isolate, method)));
-  if (!function->IsFunction())
-    return default_value;
-
-  v8::Handle<v8::Value> argv = MATE_INTEGER_NEW(isolate, command_id);
-  return function->Call(isolate->GetCurrentContext()->Global(), 1, &argv);
-}
-
-}  // namespace
-
 Menu::Menu()
     : model_(new ui::SimpleMenuModel(this)),
       parent_(NULL) {
@@ -49,37 +27,30 @@ Menu::Menu()
 Menu::~Menu() {
 }
 
+void Menu::AfterInit(v8::Isolate* isolate) {
+  mate::Dictionary wrappable(isolate, GetWrapper(isolate));
+  mate::Dictionary delegate;
+  if (!wrappable.Get("delegate", &delegate))
+    return;
+
+  delegate.Get("isCommandIdChecked", &is_checked_);
+  delegate.Get("isCommandIdEnabled", &is_enabled_);
+  delegate.Get("isCommandIdVisible", &is_visible_);
+  delegate.Get("getAcceleratorForCommandId", &get_accelerator_);
+  delegate.Get("executeCommand", &execute_command_);
+  delegate.Get("menuWillShow", &menu_will_show_);
+}
+
 bool Menu::IsCommandIdChecked(int command_id) const {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  return CallDelegate(isolate,
-                      MATE_FALSE(isolate),
-                      const_cast<Menu*>(this)->GetWrapper(isolate),
-                      "isCommandIdChecked",
-                      command_id)->BooleanValue();
+  return is_checked_.Run(command_id);
 }
 
 bool Menu::IsCommandIdEnabled(int command_id) const {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  return CallDelegate(isolate,
-                      MATE_TRUE(isolate),
-                      const_cast<Menu*>(this)->GetWrapper(isolate),
-                      "isCommandIdEnabled",
-                      command_id)->BooleanValue();
+  return is_enabled_.Run(command_id);
 }
 
 bool Menu::IsCommandIdVisible(int command_id) const {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  return CallDelegate(isolate,
-                      MATE_TRUE(isolate),
-                      const_cast<Menu*>(this)->GetWrapper(isolate),
-                      "isCommandIdVisible",
-                      command_id)->BooleanValue();
+  return is_visible_.Run(command_id);
 }
 
 bool Menu::GetAcceleratorForCommandId(int command_id,
@@ -87,69 +58,16 @@ bool Menu::GetAcceleratorForCommandId(int command_id,
   v8::Isolate* isolate = v8::Isolate::GetCurrent();
   v8::Locker locker(isolate);
   v8::HandleScope handle_scope(isolate);
-  v8::Handle<v8::Value> shortcut = CallDelegate(isolate,
-                                                MATE_UNDEFINED(isolate),
-                                                GetWrapper(isolate),
-                                                "getAcceleratorForCommandId",
-                                                command_id);
-  return mate::ConvertFromV8(isolate, shortcut, accelerator);
-}
-
-bool Menu::IsItemForCommandIdDynamic(int command_id) const {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  return CallDelegate(isolate,
-                      MATE_FALSE(isolate),
-                      const_cast<Menu*>(this)->GetWrapper(isolate),
-                      "isItemForCommandIdDynamic",
-                      command_id)->BooleanValue();
-}
-
-base::string16 Menu::GetLabelForCommandId(int command_id) const {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  v8::Handle<v8::Value> result = CallDelegate(
-      isolate,
-      MATE_FALSE(isolate),
-      const_cast<Menu*>(this)->GetWrapper(isolate),
-      "getLabelForCommandId",
-      command_id);
-  base::string16 label;
-  mate::ConvertFromV8(isolate, result, &label);
-  return label;
-}
-
-base::string16 Menu::GetSublabelForCommandId(int command_id) const {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  v8::Handle<v8::Value> result = CallDelegate(
-      isolate,
-      MATE_FALSE(isolate),
-      const_cast<Menu*>(this)->GetWrapper(isolate),
-      "getSubLabelForCommandId",
-      command_id);
-  base::string16 label;
-  mate::ConvertFromV8(isolate, result, &label);
-  return label;
+  v8::Handle<v8::Value> val = get_accelerator_.Run(command_id);
+  return mate::ConvertFromV8(isolate, val, accelerator);
 }
 
 void Menu::ExecuteCommand(int command_id, int event_flags) {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  CallDelegate(isolate, MATE_FALSE(isolate), GetWrapper(isolate),
-               "executeCommand", command_id);
+  execute_command_.Run(command_id);
 }
 
 void Menu::MenuWillShow(ui::SimpleMenuModel* source) {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  CallDelegate(isolate, MATE_FALSE(isolate), GetWrapper(isolate),
-               "menuWillShow", -1);
+  menu_will_show_.Run();
 }
 
 void Menu::AttachToWindow(Window* window) {
@@ -186,6 +104,10 @@ void Menu::InsertSubMenuAt(int index,
   model_->InsertSubMenuAt(index, command_id, label, menu->model_.get());
 }
 
+void Menu::SetIcon(int index, const gfx::Image& image) {
+  model_->SetIcon(index, image);
+}
+
 void Menu::SetSublabel(int index, const base::string16& sublabel) {
   model_->SetSublabel(index, sublabel);
 }
@@ -235,6 +157,7 @@ void Menu::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("insertRadioItem", &Menu::InsertRadioItemAt)
       .SetMethod("insertSeparator", &Menu::InsertSeparatorAt)
       .SetMethod("insertSubMenu", &Menu::InsertSubMenuAt)
+      .SetMethod("setIcon", &Menu::SetIcon)
       .SetMethod("setSublabel", &Menu::SetSublabel)
       .SetMethod("clear", &Menu::Clear)
       .SetMethod("getIndexOfCommandId", &Menu::GetIndexOfCommandId)

atom/browser/api/atom_api_menu.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "atom/browser/api/atom_api_window.h"
+#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "native_mate/wrappable.h"
@@ -16,8 +17,6 @@ namespace atom {
 
 namespace api {
 
-class MenuMac;
-
 class Menu : public mate::Wrappable,
              public ui::SimpleMenuModel::Delegate {
  public:
@@ -40,16 +39,15 @@ class Menu : public mate::Wrappable,
   Menu();
   virtual ~Menu();
 
+  // mate::Wrappable:
+  void AfterInit(v8::Isolate* isolate) override;
+
   // ui::SimpleMenuModel::Delegate implementations:
   bool IsCommandIdChecked(int command_id) const override;
   bool IsCommandIdEnabled(int command_id) const override;
   bool IsCommandIdVisible(int command_id) const override;
-  bool GetAcceleratorForCommandId(
-      int command_id,
-      ui::Accelerator* accelerator) override;
-  bool IsItemForCommandIdDynamic(int command_id) const override;
-  base::string16 GetLabelForCommandId(int command_id) const override;
-  base::string16 GetSublabelForCommandId(int command_id) const override;
+  bool GetAcceleratorForCommandId(int command_id,
+                                  ui::Accelerator* accelerator) override;
   void ExecuteCommand(int command_id, int event_flags) override;
   void MenuWillShow(ui::SimpleMenuModel* source) override;
 
@@ -74,6 +72,7 @@ class Menu : public mate::Wrappable,
                        int command_id,
                        const base::string16& label,
                        Menu* menu);
+  void SetIcon(int index, const gfx::Image& image);
   void SetSublabel(int index, const base::string16& sublabel);
   void Clear();
   int GetIndexOfCommandId(int command_id);
@@ -85,6 +84,14 @@ class Menu : public mate::Wrappable,
   bool IsEnabledAt(int index) const;
   bool IsVisibleAt(int index) const;
 
+  // Stored delegate methods.
+  base::Callback<bool(int)> is_checked_;
+  base::Callback<bool(int)> is_enabled_;
+  base::Callback<bool(int)> is_visible_;
+  base::Callback<v8::Handle<v8::Value>(int)> get_accelerator_;
+  base::Callback<void(int)> execute_command_;
+  base::Callback<void()> menu_will_show_;
+
   DISALLOW_COPY_AND_ASSIGN(Menu);
 };
 

atom/browser/api/lib/menu-item.coffee

@@ -9,13 +9,14 @@ class MenuItem
   constructor: (options) ->
     Menu = require 'menu'
 
-    {click, @selector, @type, @label, @sublabel, @accelerator, @enabled, @visible, @checked, @submenu} = options
+    {click, @selector, @type, @label, @sublabel, @accelerator, @icon, @enabled, @visible, @checked, @submenu} = options
 
     @type = 'submenu' if not @type? and @submenu?
     throw new Error('Invalid submenu') if @type is 'submenu' and @submenu?.constructor isnt Menu
 
     @overrideReadOnlyProperty 'type', 'normal'
     @overrideReadOnlyProperty 'accelerator'
+    @overrideReadOnlyProperty 'icon'
     @overrideReadOnlyProperty 'submenu'
     @overrideProperty 'label', ''
     @overrideProperty 'sublabel', ''

atom/browser/api/lib/menu.coffee

@@ -35,6 +35,7 @@ Menu::_init = ->
     isCommandIdEnabled: (commandId) => @commandsMap[commandId]?.enabled
     isCommandIdVisible: (commandId) => @commandsMap[commandId]?.visible
     getAcceleratorForCommandId: (commandId) => @commandsMap[commandId]?.accelerator
+    getIconForCommandId: (commandId) => @commandsMap[commandId]?.icon
     executeCommand: (commandId) => @commandsMap[commandId]?.click()
     menuWillShow: =>
       # Make sure radio groups have at least one menu item seleted.
@@ -82,6 +83,7 @@ Menu::insert = (pos, item) ->
       @insertRadioItem pos, item.commandId, item.label, item.groupId
 
   @setSublabel pos, item.sublabel if item.sublabel?
+  @setIcon pos, item.icon if item.icon?
 
   # Make menu accessable to items.
   item.overrideReadOnlyProperty 'menu', this

docs/api/menu-item.md

@@ -13,6 +13,7 @@
   * `label` String
   * `sublabel` String
   * `accelerator` [Accelerator](accelerator.md)
+  * `icon` [NativeImage](native-image.md)
   * `enabled` Boolean
   * `visible` Boolean
   * `checked` Boolean

vendor/native_mate

@@ -1 +1 @@
-Subproject commit 8d537ee2b6da29c1aa38928590d4c56700e1c69b
+Subproject commit d0db7bfb586afe9f491bd4cb368353d2660ecfe1