Allow PhabricatorMenuView to have items inserted in the middle

Summary:
Make `PhabricatorMenuView` more flexible, so callers can add items to the beginning/end/middle.

In particular, this allows event handlers to receive a $menu and call `addMenuItemToLabel('activity', ...)` or similar, for D4708.

Test Plan: Unit tests. Browsed site. Home page, Conpherence, and other pages with menus look correct.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4792

src/__phutil_library_map__.php

@@ -980,6 +980,7 @@ phutil_register_library_map(array(
     'PhabricatorMarkupInterface' => 'infrastructure/markup/PhabricatorMarkupInterface.php',
     'PhabricatorMenuItemView' => 'view/layout/PhabricatorMenuItemView.php',
     'PhabricatorMenuView' => 'view/layout/PhabricatorMenuView.php',
+    'PhabricatorMenuViewTestCase' => 'view/layout/__tests__/PhabricatorMenuViewTestCase.php',
     'PhabricatorMercurialGraphStream' => 'applications/repository/daemon/PhabricatorMercurialGraphStream.php',
     'PhabricatorMetaMTAAttachment' => 'applications/metamta/storage/PhabricatorMetaMTAAttachment.php',
     'PhabricatorMetaMTAConfigOptions' => 'applications/config/option/PhabricatorMetaMTAConfigOptions.php',
@@ -2398,6 +2399,7 @@ phutil_register_library_map(array(
     'PhabricatorMarkupCache' => 'PhabricatorCacheDAO',
     'PhabricatorMenuItemView' => 'AphrontTagView',
     'PhabricatorMenuView' => 'AphrontTagView',
+    'PhabricatorMenuViewTestCase' => 'PhabricatorTestCase',
     'PhabricatorMetaMTAConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorMetaMTAController' => 'PhabricatorController',
     'PhabricatorMetaMTADAO' => 'PhabricatorLiskDAO',

src/applications/auth/application/PhabricatorApplicationAuth.php

@@ -22,7 +22,6 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication {
       $item->setIcon('power');
       $item->setWorkflow(true);
       $item->setHref('/logout/');
-      $item->setSortOrder(2.0);
       $item->setSelected(($controller instanceof PhabricatorLogoutController));
       $items[] = $item;
     }

src/applications/diviner/application/PhabricatorApplicationDiviner.php

@@ -44,7 +44,6 @@ final class PhabricatorApplicationDiviner extends PhabricatorApplication {
       $item->setName(pht('%s Help', $application->getName()));
       $item->setIcon('help');
       $item->setHref($application->getHelpURI());
-      $item->setSortOrder(0.1);
       $items[] = $item;
     }
 

src/applications/people/application/PhabricatorApplicationPeople.php

@@ -58,7 +58,6 @@ final class PhabricatorApplicationPeople extends PhabricatorApplication {
       $item = new PhabricatorMenuItemView();
       $item->setName($user->getUsername());
       $item->setHref('/p/'.$user->getUsername().'/');
-      $item->setSortOrder(0.0);
       $item->addClass('phabricator-core-menu-item-profile');
 
       $classes = array(

src/applications/settings/application/PhabricatorApplicationSettings.php

@@ -44,7 +44,6 @@ final class PhabricatorApplicationSettings extends PhabricatorApplication {
       $item->setIcon('settings');
       $item->setSelected($selected);
       $item->setHref('/settings/');
-      $item->setSortOrder(0.90);
       $items[] = $item;
     }
 

src/view/layout/AphrontSideNavFilterView.php

@@ -121,7 +121,10 @@ final class AphrontSideNavFilterView extends AphrontView {
   }
 
   public function addCustomBlock($block) {
-    $this->menu->appendChild($block);
+    $this->menu->addMenuItem(
+      id(new PhabricatorMenuItemView())
+        ->setType(PhabricatorMenuItemView::TYPE_CUSTOM)
+        ->appendChild($block));
     return $this;
   }
 

src/view/layout/PhabricatorMenuItemView.php

@@ -6,13 +6,13 @@ final class PhabricatorMenuItemView extends AphrontTagView {
   const TYPE_SPACER   = 'type-spacer';
   const TYPE_LABEL    = 'type-label';
   const TYPE_BUTTON   = 'type-button';
+  const TYPE_CUSTOM   = 'type-custom';
 
   private $name;
   private $href;
   private $type = self::TYPE_LINK;
   private $isExternal;
   private $key;
-  private $sortOrder = 1.0;
   private $icon;
   private $selected;
 
@@ -88,15 +88,6 @@ final class PhabricatorMenuItemView extends AphrontTagView {
     return $this->isExternal;
   }
 
-  public function setSortOrder($order) {
-    $this->sortOrder = $order;
-    return $this;
-  }
-
-  public function getSortOrder() {
-    return $this->sortOrder;
-  }
-
   protected function getTagName() {
     return $this->href ? 'a' : 'div';
   }

src/view/layout/PhabricatorMenuView.php

@@ -4,6 +4,10 @@ final class PhabricatorMenuView extends AphrontTagView {
 
   private $items = array();
 
+  protected function canAppendChild() {
+    return false;
+  }
+
   public function newLabel($name) {
     $item = id(new PhabricatorMenuItemView())
       ->setType(PhabricatorMenuItemView::TYPE_LABEL)
@@ -37,13 +41,83 @@ final class PhabricatorMenuView extends AphrontTagView {
   }
 
   public function addMenuItem(PhabricatorMenuItemView $item) {
-    $key = $item->getKey();
+    return $this->addMenuItemAfter(null, $item);
-    $this->items[] = $item;
+  }
-    $this->appendChild($item);
 
+  public function addMenuItemAfter($key, PhabricatorMenuItemView $item) {
+    if ($key === null) {
+      $this->items[] = $item;
+      return $this;
+    }
+
+    if (!$this->getItem($key)) {
+      throw new Exception("No such key '{$key}' to add menu item after!");
+    }
+
+    $result = array();
+    foreach ($this->items as $other) {
+      $result[] = $other;
+      if ($other->getKey() == $key) {
+        $result[] = $item;
+      }
+    }
+
+    $this->items = $result;
     return $this;
   }
 
+  public function addMenuItemBefore($key, PhabricatorMenuItemView $item) {
+    if ($key === null) {
+      array_unshift($this->items, $item);
+      return $this;
+    }
+
+    $this->requireKey($key);
+
+    $result = array();
+    foreach ($this->items as $other) {
+      if ($other->getKey() == $key) {
+        $result[] = $item;
+      }
+      $result[] = $other;
+    }
+
+    $this->items = $result;
+    return $this;
+  }
+
+  public function addMenuItemToLabel($key, PhabricatorMenuItemView $item) {
+    $this->requireKey($key);
+
+    $other = $this->getItem($key);
+    if ($other->getType() != PhabricatorMenuItemView::TYPE_LABEL) {
+      throw new Exception("Menu item '{$key}' is not a label!");
+    }
+
+    $seen = false;
+    $after = null;
+    foreach ($this->items as $other) {
+      if (!$seen) {
+        if ($other->getKey() == $key) {
+          $seen = true;
+        }
+      } else {
+        if ($other->getType() == PhabricatorMenuItemView::TYPE_LABEL) {
+          break;
+        }
+      }
+      $after = $other->getKey();
+    }
+
+    return $this->addMenuItemAfter($after, $item);
+  }
+
+  private function requireKey($key) {
+    if (!$this->getItem($key)) {
+      throw new Exception("No menu item with key '{$key}' exists!");
+    }
+  }
+
   public function getItem($key) {
     $key = (string)$key;
 
@@ -83,4 +157,8 @@ final class PhabricatorMenuView extends AphrontTagView {
       'class' => 'phabricator-menu-view',
     );
   }
+
+  protected function getTagContent() {
+    return $this->renderSingleView($this->items);
+  }
 }

src/view/layout/__tests__/PhabricatorMenuViewTestCase.php

@@ -0,0 +1,141 @@
+<?php
+
+final class PhabricatorMenuViewTestCase extends PhabricatorTestCase {
+
+  public function testAppend() {
+    $menu = $this->newABCMenu();
+
+    $this->assertMenuKeys(
+      array(
+        'a',
+        'b',
+        'c',
+      ),
+      $menu);
+  }
+
+  public function testAppendAfter() {
+    $menu = $this->newABCMenu();
+
+    $caught = null;
+    try {
+      $menu->addMenuItemAfter('x', $this->newLink('test1'));
+    } catch (Exception $ex) {
+      $caught = $ex;
+    }
+    $this->assertEqual(true, $caught instanceof Exception);
+
+    $menu->addMenuItemAfter('a', $this->newLink('test2'));
+    $menu->addMenuItemAfter(null, $this->newLink('test3'));
+    $menu->addMenuItemAfter('a', $this->newLink('test4'));
+    $menu->addMenuItemAfter('test3', $this->newLink('test5'));
+
+    $this->assertMenuKeys(
+      array(
+        'a',
+        'test4',
+        'test2',
+        'b',
+        'c',
+        'test3',
+        'test5',
+      ),
+      $menu);
+  }
+
+  public function testAppendBefore() {
+    $menu = $this->newABCMenu();
+
+    $caught = null;
+    try {
+      $menu->addMenuItemBefore('x', $this->newLink('test1'));
+    } catch (Exception $ex) {
+      $caught = $ex;
+    }
+    $this->assertEqual(true, $caught instanceof Exception);
+
+    $menu->addMenuItemBefore('b', $this->newLink('test2'));
+    $menu->addMenuItemBefore(null, $this->newLink('test3'));
+    $menu->addMenuItemBefore('a', $this->newLink('test4'));
+    $menu->addMenuItemBefore('test3', $this->newLink('test5'));
+
+    $this->assertMenuKeys(
+      array(
+        'test5',
+        'test3',
+        'test4',
+        'a',
+        'test2',
+        'b',
+        'c',
+      ),
+      $menu);
+  }
+
+  public function testAppendLabel() {
+    $menu = new PhabricatorMenuView();
+    $menu->addMenuItem($this->newLabel('fruit'));
+    $menu->addMenuItem($this->newLabel('animals'));
+
+    $caught = null;
+    try {
+      $menu->addMenuItemToLabel('x', $this->newLink('test1'));
+    } catch (Exception $ex) {
+      $caught = $ex;
+    }
+    $this->assertEqual(true, $caught instanceof Exception);
+
+    $menu->addMenuItemToLabel('fruit', $this->newLink('apple'));
+    $menu->addMenuItemToLabel('fruit', $this->newLink('banana'));
+
+    $menu->addMenuItemToLabel('animals', $this->newLink('dog'));
+    $menu->addMenuItemToLabel('animals', $this->newLink('cat'));
+
+    $menu->addMenuItemToLabel('fruit', $this->newLink('cherry'));
+
+    $this->assertMenuKeys(
+      array(
+        'fruit',
+          'apple',
+          'banana',
+          'cherry',
+        'animals',
+          'dog',
+          'cat',
+      ),
+      $menu);
+  }
+
+  private function newLink($key) {
+    return id(new PhabricatorMenuItemView())
+      ->setKey($key)
+      ->setHref('#')
+      ->setName('Link');
+  }
+
+  private function newLabel($key) {
+    return id(new PhabricatorMenuItemView())
+      ->setType(PhabricatorMenuItemView::TYPE_LABEL)
+      ->setKey($key)
+      ->setName('Label');
+  }
+
+  private function newABCMenu() {
+    $menu = new PhabricatorMenuView();
+
+    $menu->addMenuItem($this->newLink('a'));
+    $menu->addMenuItem($this->newLink('b'));
+    $menu->addMenuItem($this->newLink('c'));
+
+    return $menu;
+  }
+
+  private function assertMenuKeys(array $expect, PhabricatorMenuView $menu) {
+    $items = $menu->getItems();
+    $keys = mpull($items, 'getKey');
+    $keys = array_values($keys);
+
+    $this->assertEqual($expect, $keys);
+  }
+
+}

src/view/page/menu/PhabricatorMainMenuIconView.php

@@ -5,7 +5,6 @@ final class PhabricatorMainMenuIconView extends AphrontView {
   private $classes = array();
   private $href;
   private $name;
-  private $sortOrder = 0.5;
   private $workflow;
   private $style;
 
@@ -42,22 +41,6 @@ final class PhabricatorMainMenuIconView extends AphrontView {
     return $this;
   }
 
-  /**
-   * Provide a float, where 0.0 is the profile item and 1.0 is the logout
-   * item. Normally you should pick something between the two.
-   *
-   * @param float Sort order.
-   * @return this
-   */
-  public function setSortOrder($sort_order) {
-    $this->sortOrder = $sort_order;
-    return $this;
-  }
-
-  public function getSortOrder() {
-    return $this->sortOrder;
-  }
-
   public function render() {
     $name = $this->getName();
     $href = $this->getHref();

src/view/page/menu/PhabricatorMainMenuView.php

@@ -164,7 +164,6 @@ final class PhabricatorMainMenuView extends AphrontView {
     $controller = $this->getController();
 
     $applications = PhabricatorApplication::getAllInstalledApplications();
-    $applications = msort($applications, 'getName');
 
     $core = array();
     $more = array();
@@ -184,7 +183,7 @@ final class PhabricatorMainMenuView extends AphrontView {
         if ($application->getApplicationGroup() == $group_core) {
           $core[] = $item;
         } else {
-          $more[] = $item;
+          $more[$application->getName()] = $item;
         }
       }
 
@@ -200,7 +199,7 @@ final class PhabricatorMainMenuView extends AphrontView {
     $view->addClass('phabricator-core-menu');
 
     $search = $this->renderSearch();
-    $view->appendChild($search);
+    $view->addMenuItem($search);
 
     $view
       ->newLabel(pht('Home'))
@@ -235,7 +234,6 @@ final class PhabricatorMainMenuView extends AphrontView {
     }
 
     if ($actions) {
-      $actions = msort($actions, 'getSortOrder');
       $view->addMenuItem(
         id(new PhabricatorMenuItemView())
           ->addClass('phabricator-core-item-device')
@@ -260,6 +258,7 @@ final class PhabricatorMainMenuView extends AphrontView {
           ->addClass('phabricator-core-item-device')
           ->setType(PhabricatorMenuItemView::TYPE_LABEL)
           ->setName(pht('More Applications')));
+      ksort($more);
       foreach ($more as $item) {
         $item->addClass('phabricator-core-item-device');
         $view->addMenuItem($item);