Improves #1373

* Don't pollute Menu API.
* Add examples in docs.
* Moves the docs from menu-item.md to menu.md.

atom/browser/api/lib/menu.coffee

@@ -23,6 +23,37 @@ generateGroupId = (items, pos) ->
       break if item.type is 'separator'
   ++nextGroupId
 
+# Returns the index of item according to |id|.
+indexOfItemById = (items, id) ->
+  return i for item, i in items when item.id is id
+  -1
+
+# Returns the index of where to insert the item according to |position|.
+indexToInsertByPosition = (items, position) ->
+  return items.length unless position
+
+  [query, id] = position.split '='
+  insertIndex = indexOfItemById items, id
+  if insertIndex is -1 and query isnt 'endof'
+    console.warn "Item with id '#{id}' is not found"
+    return items.length
+
+  switch query
+    when 'after'
+      insertIndex++
+    when 'endof'
+      # If the |id| doesn't exist, then create a new group with the |id|.
+      if insertIndex is -1
+        items.push id: id, type: 'separator'
+        insertIndex = items.length - 1
+
+      # Find the end of the group.
+      insertIndex++
+      while insertIndex < items.length and items[insertIndex].type isnt 'separator'
+        insertIndex++
+
+  insertIndex
+
 Menu = bindings.Menu
 Menu::__proto__ = EventEmitter.prototype
 
@@ -113,43 +144,6 @@ Menu.getApplicationMenu = -> applicationMenu
 
 Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder
 
-Menu._indexOfItemWithId = (insertedItems, id) ->
-  for each, index in insertedItems
-    if id is each.id
-      return index
-  -1
-
-Menu._itemIndexForPosition = (insertedItems, position) ->
-  insertIndex = insertedItems.length
-
-  if position
-    [query, id] = position.split '='
-    switch query
-      when 'before'
-        insertIndex = Menu._indexOfItemWithId insertedItems, id
-      when 'after'
-        insertIndex = Menu._indexOfItemWithId insertedItems, id
-        unless insertIndex is -1
-          insertIndex++
-      when 'endof'
-        insertIndex = Menu._indexOfItemWithId insertedItems, id
-        if insertIndex is -1
-          separatorItem = id: id, type: 'separator'
-          insertIndex = insertedItems.length
-          insertedItems.push separatorItem
-
-        insertIndex++
-        item = insertedItems[insertIndex]
-        while (insertIndex < insertedItems.length) and item.type != 'separator'
-          insertIndex++
-          item = insertedItems[insertIndex]
-
-    if insertIndex is -1
-      console.warn "Could not position item at position #{position}"
-      insertIndex = insertedItems.length
-
-  insertIndex
-
 Menu.buildFromTemplate = (template) ->
   throw new TypeError('Invalid template for Menu') unless Array.isArray template
 
@@ -157,11 +151,12 @@ Menu.buildFromTemplate = (template) ->
   insertIndex = 0
 
   for item in template
-    position = item.position
+    if item.position
-    if position
+      insertIndex = indexToInsertByPosition positionedTemplate, item.position
-      insertIndex = Menu._itemIndexForPosition positionedTemplate, position
+    else
+      # If no |position| is specified, insert after last item.
+      insertIndex++
     positionedTemplate.splice insertIndex, 0, item
-    insertIndex++
 
   menu = new Menu
 

docs/api/menu-item.md

@@ -5,54 +5,21 @@
 ### new MenuItem(options)
 
 * `options` Object
-
   * `click` Function - Callback when the menu item is clicked
-
   * `selector` String - Call the selector of first responder when clicked (OS
      X only)
-
   * `type` String - Can be `normal`, `separator`, `submenu`, `checkbox` or
      `radio`
-
   * `label` String
-
   * `sublabel` String
-
   * `accelerator` [Accelerator](accelerator.md)
-
   * `icon` [NativeImage](native-image.md)
-
   * `enabled` Boolean
-
   * `visible` Boolean
-
   * `checked` Boolean
-
   * `submenu` Menu - Should be specified for `submenu` type menu item, when
      it's specified the `type: 'submenu'` can be omitted for the menu item
-
   * `id` String - Unique within a single menu. If defined then it can be used
      as a reference to this item by the position attribute.
-
+  * `position` String - This field allows fine-grained definition of the
-  * `position` String - This field allows fine-grained definition of the specific
+     specific location within a given menu.
-     location within a given menu.
-
-     It has the form "[placement]=[id]" where placement is one of `before`,
-     `after`, or `endof` and `id` is the id of an existing item in the menu.
-
-     - `before` - Inserts this item before the id referenced item. If the
-        referenced item doesn't exist the item will be inserted at the end of
-        the menu.
-
-     - `after` - Inserts this item after id referenced item. If the referenced
-        item doesn't exist the item will be inserted at the end of the menu.
-
-     - `endof` - Inserts this item at the end of the logical group containing
-        the id referenced item. (Groups are created by separator items). If
-        the referenced item doesn't exist a new separator group is created with
-        the given id and this item is inserted after that separator.
-
-    When an item is positioned following unpositioned items are inserted after
-    it, until a new item is positioned. So if you want to position a group of
-    menu items in the same location you only need to specify a position for
-    the first item.

docs/api/menu.md

@@ -249,3 +249,78 @@ no matter what label you set. To change it you have to change your app's name
 by modifying your app bundle's `Info.plist` file. See
 [About Information Property List Files](https://developer.apple.com/library/ios/documentation/general/Reference/InfoPlistKeyReference/Articles/AboutInformationPropertyListFiles.html)
 for more.
+
+
+## Menu item position
+
+You can make use of `position` and `id` to control how the item would be placed
+when building a menu with `Menu.buildFromTemplate`.
+
+The `position` attribute of `MenuItem` has the form `[placement]=[id]` where
+placement is one of `before`, `after`, or `endof` and `id` is the unique ID of
+an existing item in the menu:
+
+* `before` - Inserts this item before the id referenced item. If the
+  referenced item doesn't exist the item will be inserted at the end of
+  the menu.
+* `after` - Inserts this item after id referenced item. If the referenced
+  item doesn't exist the item will be inserted at the end of the menu.
+* `endof` - Inserts this item at the end of the logical group containing
+  the id referenced item. (Groups are created by separator items). If
+  the referenced item doesn't exist a new separator group is created with
+  the given id and this item is inserted after that separator.
+
+When an item is positioned following unpositioned items are inserted after
+it, until a new item is positioned. So if you want to position a group of
+menu items in the same location you only need to specify a position for
+the first item.
+
+### Examples
+
+Template:
+
+```javascript
+[
+  {label: '4', id: '4'}
+  {label: '5', id: '5'}
+  {label: '1', id: '1', position: 'before=4'}
+  {label: '2', id: '2'}
+  {label: '3', id: '3'}
+]
+```
+
+Menu:
+
+```
+- 1
+- 2
+- 3
+- 4
+- 5
+```
+
+Template:
+
+```javascript
+[
+  {label: 'a', position: 'endof=letters'}
+  {label: '1', position: 'endof=numbers'}
+  {label: 'b', position: 'endof=letters'}
+  {label: '2', position: 'endof=numbers'}
+  {label: 'c', position: 'endof=letters'}
+  {label: '3', position: 'endof=numbers'}
+]
+```
+
+Menu:
+
+```
+- ---
+- a
+- b
+- c
+- ---
+- 1
+- 2
+- 3
+```