diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2f3b6236fd..83d988c22d 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php index acd98344ea..2bb0926cde 100644 --- a/src/applications/auth/application/PhabricatorApplicationAuth.php +++ b/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; } diff --git a/src/applications/diviner/application/PhabricatorApplicationDiviner.php b/src/applications/diviner/application/PhabricatorApplicationDiviner.php index 9d9ae40616..23a1f5a8ca 100644 --- a/src/applications/diviner/application/PhabricatorApplicationDiviner.php +++ b/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; } diff --git a/src/applications/people/application/PhabricatorApplicationPeople.php b/src/applications/people/application/PhabricatorApplicationPeople.php index 0f0f05385e..0fbfa0882c 100644 --- a/src/applications/people/application/PhabricatorApplicationPeople.php +++ b/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( diff --git a/src/applications/settings/application/PhabricatorApplicationSettings.php b/src/applications/settings/application/PhabricatorApplicationSettings.php index 613ec7168a..c05d9a2a9d 100644 --- a/src/applications/settings/application/PhabricatorApplicationSettings.php +++ b/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; } diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index b754ea5608..1ff467b523 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/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; } diff --git a/src/view/layout/PhabricatorMenuItemView.php b/src/view/layout/PhabricatorMenuItemView.php index e66abecb8a..952a09d1eb 100644 --- a/src/view/layout/PhabricatorMenuItemView.php +++ b/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'; } diff --git a/src/view/layout/PhabricatorMenuView.php b/src/view/layout/PhabricatorMenuView.php index b5e7fc8a8d..bf1d601eb2 100644 --- a/src/view/layout/PhabricatorMenuView.php +++ b/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(); - $this->items[] = $item; - $this->appendChild($item); + return $this->addMenuItemAfter(null, $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); + } } diff --git a/src/view/layout/__tests__/PhabricatorMenuViewTestCase.php b/src/view/layout/__tests__/PhabricatorMenuViewTestCase.php new file mode 100644 index 0000000000..a87c90289e --- /dev/null +++ b/src/view/layout/__tests__/PhabricatorMenuViewTestCase.php @@ -0,0 +1,141 @@ +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); + } + +} diff --git a/src/view/page/menu/PhabricatorMainMenuIconView.php b/src/view/page/menu/PhabricatorMainMenuIconView.php index a94c903b55..822790535c 100644 --- a/src/view/page/menu/PhabricatorMainMenuIconView.php +++ b/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(); diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php index 73c8ff9789..9dc83bc73d 100644 --- a/src/view/page/menu/PhabricatorMainMenuView.php +++ b/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);