:lipstick: Organize Menu and MenuItem's code.

diff --git a/atom/browser/api/lib/menu-item.coffee b/atom/browser/api/lib/menu-item.coffee
index 2dda5a9..bbac726 100644 (file)
--- a/atom/browser/api/lib/menu-item.coffee
+++ b/atom/browser/api/lib/menu-item.coffee
@@ -3,25 +3,6 @@ v8Util = process.atomBinding 'v8_util'
 
 nextCommandId = 0
 
-overrideProperty = (item, name, defaultValue) ->
-  item[name] ?= defaultValue
-
-  return unless process.platform is 'win32'
-  v8Util.setHiddenValue item, name, item[name]
-  Object.defineProperty item, name,
-    enumerable: true
-    get: -> v8Util.getHiddenValue item, name
-    set: (val) ->
-      v8Util.setHiddenValue item, name, val
-      item.menu?._updateStates()
-
-overrideReadOnlyProperty = (item, name, defaultValue) ->
-  item[name] ?= defaultValue
-  Object.defineProperty item, name,
-    enumerable: true
-    writable: false
-    value: item[name]
-
 class MenuItem
   @types = ['normal', 'separator', 'submenu', 'checkbox', 'radio']
 
@@ -33,22 +14,45 @@ class MenuItem
     @type = 'submenu' if not @type? and @submenu?
     throw new Error('Invalid submenu') if @type is 'submenu' and @submenu?.constructor isnt Menu
 
-    overrideReadOnlyProperty this, 'type', 'normal'
-    overrideReadOnlyProperty this, 'accelerator', null
-    overrideReadOnlyProperty this, 'submenu', null
-    overrideProperty this, 'label', ''
-    overrideProperty this, 'sublabel', ''
-    overrideProperty this, 'enabled', true
-    overrideProperty this, 'visible', true
-    overrideProperty this, 'checked', false
+    @overrideReadOnlyProperty 'type', 'normal'
+    @overrideReadOnlyProperty 'accelerator'
+    @overrideReadOnlyProperty 'submenu'
+    @overrideProperty 'label', ''
+    @overrideProperty 'sublabel', ''
+    @overrideProperty 'enabled', true
+    @overrideProperty 'visible', true
+    @overrideProperty 'checked', false
 
     throw new Error("Unknown menu type #{@type}") if MenuItem.types.indexOf(@type) is -1
 
     @commandId = ++nextCommandId
     @click = =>
+      # Manually flip the checked flags when clicked.
+      @checked = !@checked if @type in ['checkbox', 'radio']
+
       if typeof click is 'function'
         click this, BrowserWindow.getFocusedWindow()
       else if typeof @selector is 'string'
         Menu.sendActionToFirstResponder @selector
 
+  overrideProperty: (name, defaultValue=null) ->
+    this[name] ?= defaultValue
+
+    # Update states when property is changed on Windows.
+    return unless process.platform is 'win32'
+    v8Util.setHiddenValue this, name, this[name]
+    Object.defineProperty this, name,
+      enumerable: true
+      get: => v8Util.getHiddenValue this, name
+      set: (val) =>
+        v8Util.setHiddenValue this, name, val
+        @menu?._updateStates()
+
+  overrideReadOnlyProperty: (name, defaultValue=null) ->
+    this[name] ?= defaultValue
+    Object.defineProperty this, name,
+      enumerable: true
+      writable: false
+      value: this[name]
+
 module.exports = MenuItem

diff --git a/atom/browser/api/lib/menu.coffee b/atom/browser/api/lib/menu.coffee
index a2cdd2e..0390144 100644 (file)
--- a/atom/browser/api/lib/menu.coffee
+++ b/atom/browser/api/lib/menu.coffee
@@ -46,13 +46,7 @@ Menu::insert = (pos, item) ->
       isCommandIdEnabled: (commandId) => @commandsMap[commandId]?.enabled
       isCommandIdVisible: (commandId) => @commandsMap[commandId]?.visible
       getAcceleratorForCommandId: (commandId) => @commandsMap[commandId]?.accelerator
-      executeCommand: (commandId) =>
-        activeItem = @commandsMap[commandId]
-        # Manually flip the checked flags when clicked.
-        if activeItem?
-          if activeItem.type in ['checkbox', 'radio']
-            activeItem.checked = !activeItem.checked
-          activeItem.click()
+      executeCommand: (commandId) => @commandsMap[commandId]?.click()
       menuWillShow: =>
         # Make sure radio groups have at least one menu item seleted.
         for id, group of @groupsMap
@@ -69,7 +63,7 @@ Menu::insert = (pos, item) ->
     when 'submenu' then @insertSubMenu pos, item.commandId, item.label, item.submenu
     when 'radio'
       # Grouping radio menu items.
-      item.groupId = generateGroupId(@items, pos)
+      item.overrideReadOnlyProperty 'groupId', generateGroupId(@items, pos)
       @groupsMap[item.groupId] ?= []
       @groupsMap[item.groupId].push item
 
@@ -78,7 +72,7 @@ Menu::insert = (pos, item) ->
         enumerable: true
         get: -> v8Util.getHiddenValue item, 'checked'
         set: (val) =>
-          for otherItem in @groupsMap[item.groupId]
+          for otherItem in @groupsMap[item.groupId] when otherItem isnt item
             v8Util.setHiddenValue otherItem, 'checked', false
           v8Util.setHiddenValue item, 'checked', true
 
@@ -90,7 +84,7 @@ Menu::insert = (pos, item) ->
   @setSublabel pos, item.sublabel if item.sublabel?
 
   # Make menu accessable to items.
-  item.menu = this
+  item.overrideReadOnlyProperty 'menu', this
 
   # Remember the items.
   @items.splice pos, 0, item