Automatically build mobile menus from navigation, and clean up external ProfileMenu API

Summary: Depends on D20355. Ref T13275. Ref T13247. Currently, "Hamburger" menus are not automatically built from navigation menus. However, this is (I'm almost completely sure?) a reasonable and appropriate default behavior, and saves us some code around profile menus. With this rule in place, we can remove `setApplicationMenu()` and `getApplicationMenu()` from `StandardPageView`, since they have no callers. This also updates a lot of profile menu callsites to a new API which is added in the next change. Test Plan: See the next two changes. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13275, T13247 Differential Revision: https://secure.phabricator.com/D20356
2024-12-23 14:00:56 +01:00 · 2019-03-31 11:48:09 -07:00 · 2019-03-31 11:48:09 -07:00 · 971a272bf6
commit 971a272bf6
parent 47bf382435
18 changed files with 91 additions and 154 deletions
--- a/src/applications/favorites/engineextension/PhabricatorFavoritesMainMenuBarExtension.php
+++ b/src/applications/favorites/engineextension/PhabricatorFavoritesMainMenuBarExtension.php
@ -20,7 +20,7 @@ final class PhabricatorFavoritesMainMenuBarExtension

    $dropdown = $this->newDropdown($viewer);
    if (!$dropdown) {
-      return null;
+      return array();
    }

    $favorites_menu = id(new PHUIButtonView())
@ -59,7 +59,8 @@ final class PhabricatorFavoritesMainMenuBarExtension
      $menu_engine->setController($controller);
    }

-    $filter_view = $menu_engine->buildNavigation();
+    $filter_view = $menu_engine->newProfileMenuItemViewList()
+      ->newNavigationView();

    $menu_view = $filter_view->getMenu();
    $item_views = $menu_view->getItems();
--- a/src/applications/home/controller/PhabricatorHomeController.php
+++ b/src/applications/home/controller/PhabricatorHomeController.php
@ -1,44 +1,4 @@
 <?php

-abstract class PhabricatorHomeController extends PhabricatorController {
-
-  private $home;
-  private $profileMenu;
-
-  public function buildApplicationMenu() {
-    $menu = $this->newApplicationMenu();
-
-    $profile_menu = $this->getProfileMenu();
-    if ($profile_menu) {
-      $menu->setProfileMenu($profile_menu);
-    }
-
-    return $menu;
-  }
-
-  protected function getProfileMenu() {
-    if (!$this->profileMenu) {
-      $viewer = $this->getViewer();
-      $applications = id(new PhabricatorApplicationQuery())
-        ->setViewer($viewer)
-        ->withClasses(array('PhabricatorHomeApplication'))
-        ->withInstalled(true)
-        ->execute();
-      $home = head($applications);
-      if (!$home) {
-        return null;
-      }
-
-      $engine = id(new PhabricatorHomeProfileMenuEngine())
-        ->setViewer($viewer)
-        ->setController($this)
-        ->setProfileObject($home)
-        ->setCustomPHID($viewer->getPHID());
-
-      $this->profileMenu = $engine->buildNavigation();
-    }
-
-    return $this->profileMenu;
-  }
-
-}
+abstract class PhabricatorHomeController
+  extends PhabricatorController {}
--- a/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
@ -30,8 +30,9 @@ final class PhabricatorPeopleProfileBadgesController
    $crumbs->addTextCrumb(pht('Badges'));
    $crumbs->setBorder(true);

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_BADGES);
+    $nav = $this->newNavigation(
+      $user,
+      PhabricatorPeopleProfileMenuEngine::ITEM_BADGES);

    // Best option?
    $badges = id(new PhabricatorBadgesQuery())
--- a/src/applications/people/controller/PhabricatorPeopleProfileCommitsController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileCommitsController.php
@ -32,8 +32,9 @@ final class PhabricatorPeopleProfileCommitsController
    $crumbs->addTextCrumb(pht('Recent Commits'));
    $crumbs->setBorder(true);

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_COMMITS);
+    $nav = $this->newNavigation(
+      $user,
+      PhabricatorPeopleProfileMenuEngine::ITEM_COMMITS);

    $view = id(new PHUITwoColumnView())
      ->setHeader($header)
--- a/src/applications/people/controller/PhabricatorPeopleProfileController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php
@ -4,7 +4,6 @@ abstract class PhabricatorPeopleProfileController
  extends PhabricatorPeopleController {

  private $user;
-  private $profileMenu;

  public function shouldRequireAdmin() {
    return false;
@ -19,34 +18,6 @@ abstract class PhabricatorPeopleProfileController
    return $this->user;
  }

-  public function buildApplicationMenu() {
-    $menu = $this->newApplicationMenu();
-
-    $profile_menu = $this->getProfileMenu();
-    if ($profile_menu) {
-      $menu->setProfileMenu($profile_menu);
-    }
-
-    return $menu;
-  }
-
-  protected function getProfileMenu() {
-    if (!$this->profileMenu) {
-      $user = $this->getUser();
-      if ($user) {
-        $viewer = $this->getViewer();
-
-        $engine = id(new PhabricatorPeopleProfileMenuEngine())
-          ->setViewer($viewer)
-          ->setProfileObject($user);
-
-        $this->profileMenu = $engine->buildNavigation();
-      }
-    }
-
-    return $this->profileMenu;
-  }
-
  protected function buildApplicationCrumbs() {
    $crumbs = parent::buildApplicationCrumbs();

@ -138,4 +109,24 @@ abstract class PhabricatorPeopleProfileController
    return $header;
  }

+  final protected function newNavigation(
+    PhabricatorUser $user,
+    $item_identifier) {
+
+    $viewer = $this->getViewer();
+
+    $engine = id(new PhabricatorPeopleProfileMenuEngine())
+      ->setViewer($viewer)
+      ->setController($this)
+      ->setProfileObject($user);
+
+    $view_list = $engine->newProfileMenuItemViewList();
+
+    $view_list->setSelectedViewWithItemIdentifier($item_identifier);
+
+    $navigation = $view_list->newNavigationView();
+
+    return $navigation;
+  }
+
 }
--- a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
@ -29,8 +29,9 @@ final class PhabricatorPeopleProfileManageController
    $properties = $this->buildPropertyView($user);
    $name = $user->getUsername();

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_MANAGE);
+    $nav = $this->newNavigation(
+      $user,
+      PhabricatorPeopleProfileMenuEngine::ITEM_MANAGE);

    $crumbs = $this->buildApplicationCrumbs();
    $crumbs->addTextCrumb(pht('Manage'));
--- a/src/applications/people/controller/PhabricatorPeopleProfileRevisionsController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileRevisionsController.php
@ -32,8 +32,9 @@ final class PhabricatorPeopleProfileRevisionsController
    $crumbs->addTextCrumb(pht('Recent Revisions'));
    $crumbs->setBorder(true);

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_REVISIONS);
+    $nav = $this->newNavigation(
+      $user,
+      PhabricatorPeopleProfileMenuEngine::ITEM_REVISIONS);

    $view = id(new PHUITwoColumnView())
      ->setHeader($header)
--- a/src/applications/people/controller/PhabricatorPeopleProfileTasksController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileTasksController.php
@ -32,8 +32,9 @@ final class PhabricatorPeopleProfileTasksController
    $crumbs->addTextCrumb(pht('Assigned Tasks'));
    $crumbs->setBorder(true);

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_TASKS);
+    $nav = $this->newNavigation(
+      $user,
+      PhabricatorPeopleProfileMenuEngine::ITEM_TASKS);

    $view = id(new PHUITwoColumnView())
      ->setHeader($header)
--- a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
@ -64,15 +64,16 @@ final class PhabricatorPeopleProfileViewController
          $calendar,
        ));

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_PROFILE);
+    $navigation = $this->newNavigation(
+      $user,
+      PhabricatorPeopleProfileMenuEngine::ITEM_PROFILE);

    $crumbs = $this->buildApplicationCrumbs();
    $crumbs->setBorder(true);

    return $this->newPage()
      ->setTitle($user->getUsername())
-      ->setNavigation($nav)
+      ->setNavigation($navigation)
      ->setCrumbs($crumbs)
      ->setPageObjectPHIDs(
        array(
--- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
@ -172,7 +172,9 @@ final class PhabricatorProjectBoardViewController
        return $content;
      }

-      $nav = $this->newWorkboardProfileMenu();
+      $nav = $this->newNavigation(
+        $project,
+        PhabricatorProject::ITEM_WORKBOARD);

      $crumbs = $this->buildApplicationCrumbs();
      $crumbs->addTextCrumb(pht('Workboard'));
@ -719,7 +721,9 @@ final class PhabricatorProjectBoardViewController
      ->appendChild($board)
      ->addClass('project-board-wrapper');

-    $nav = $this->newWorkboardProfileMenu();
+    $nav = $this->newNavigation(
+      $project,
+      PhabricatorProject::ITEM_WORKBOARD);

    $divider = id(new PHUIListItemView())
      ->setType(PHUIListItemView::TYPE_DIVIDER);
@ -1503,15 +1507,4 @@ final class PhabricatorProjectBoardViewController
      ->addCancelButton($profile_uri);
  }

-  private function newWorkboardProfileMenu() {
-    $default_item = id(new PhabricatorProfileMenuItemConfiguration())
-      ->setBuiltinKey(PhabricatorProject::ITEM_WORKBOARD);
-
-    $menu = parent::getProfileMenu($default_item);
-
-    $menu->addClass('project-board-nav');
-
-    return $menu;
-  }
-
 }
--- a/src/applications/project/controller/PhabricatorProjectController.php
+++ b/src/applications/project/controller/PhabricatorProjectController.php
@ -84,30 +84,6 @@ abstract class PhabricatorProjectController extends PhabricatorController {
    return null;
  }

-  public function buildApplicationMenu() {
-    $menu = $this->newApplicationMenu();
-
-    $profile_menu = $this->getProfileMenu();
-    if ($profile_menu) {
-      $menu->setProfileMenu($profile_menu);
-    }
-
-    $menu->setSearchEngine(new PhabricatorProjectSearchEngine());
-
-    return $menu;
-  }
-
-  protected function getProfileMenu($default_item = null) {
-    if (!$this->profileMenu) {
-      $engine = $this->getProfileMenuEngine();
-      if ($engine) {
-        $this->profileMenu = $engine->buildNavigation($default_item);
-      }
-    }
-
-    return $this->profileMenu;
-  }
-
  protected function buildApplicationCrumbs() {
    return $this->newApplicationCrumbs('profile');
  }
@ -207,4 +183,23 @@ abstract class PhabricatorProjectController extends PhabricatorController {
    return implode(', ', $result);
  }

+  final protected function newNavigation(
+    PhabricatorProject $project,
+    $item_identifier) {
+
+    $engine = $this->getProfileMenuEngine();
+
+    $view_list = $engine->newProfileMenuItemViewList();
+
+    $view_list->setSelectedViewWithItemIdentifier($item_identifier);
+
+    $navigation = $view_list->newNavigationView();
+
+    if ($item_identifier === PhabricatorProject::ITEM_WORKBOARD) {
+      $navigation->addClass('project-board-nav');
+    }
+
+    return $navigation;
+  }
+
 }
--- a/src/applications/project/controller/PhabricatorProjectManageController.php
+++ b/src/applications/project/controller/PhabricatorProjectManageController.php
@ -37,8 +37,9 @@ final class PhabricatorProjectManageController
      new PhabricatorProjectTransactionQuery());
    $timeline->setShouldTerminate(true);

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorProject::ITEM_MANAGE);
+    $nav = $this->newNavigation(
+      $project,
+      PhabricatorProject::ITEM_MANAGE);

    $crumbs = $this->buildApplicationCrumbs();
    $crumbs->addTextCrumb(pht('Manage'));
--- a/src/applications/project/controller/PhabricatorProjectMembersViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectMembersViewController.php
@ -36,8 +36,9 @@ final class PhabricatorProjectMembersViewController
      ->setUserPHIDs($project->getWatcherPHIDs())
      ->setShowNote(true);

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorProject::ITEM_MEMBERS);
+    $nav = $this->newNavigation(
+      $project,
+      PhabricatorProject::ITEM_MEMBERS);

    $crumbs = $this->buildApplicationCrumbs();
    $crumbs->addTextCrumb(pht('Members'));
--- a/src/applications/project/controller/PhabricatorProjectProfileController.php
+++ b/src/applications/project/controller/PhabricatorProjectProfileController.php
@ -74,8 +74,9 @@ final class PhabricatorProjectProfileController
      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
      ->setUserPHIDs($project->getWatcherPHIDs());

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorProject::ITEM_PROFILE);
+    $nav = $this->newNavigation(
+      $project,
+      PhabricatorProject::ITEM_PROFILE);

    $stories = id(new PhabricatorFeedQuery())
      ->setViewer($viewer)
--- a/src/applications/project/controller/PhabricatorProjectSubprojectsController.php
+++ b/src/applications/project/controller/PhabricatorProjectSubprojectsController.php
@ -77,8 +77,9 @@ final class PhabricatorProjectSubprojectsController
      $milestones,
      $subprojects);

-    $nav = $this->getProfileMenu();
-    $nav->selectFilter(PhabricatorProject::ITEM_SUBPROJECTS);
+    $nav = $this->newNavigation(
+      $project,
+      PhabricatorProject::ITEM_SUBPROJECTS);

    $crumbs = $this->buildApplicationCrumbs();
    $crumbs->addTextCrumb(pht('Subprojects'));
--- a/src/applications/project/controller/PhabricatorProjectViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectViewController.php
@ -18,7 +18,7 @@ final class PhabricatorProjectViewController
    $project = $this->getProject();

    $engine = $this->getProfileMenuEngine();
-    $default = $engine->getDefaultItem();
+    $default = $engine->getDefaultMenuItemConfiguration();

    // If defaults are broken somehow, serve the manage page. See T13033 for
    // discussion.
--- a/src/applications/search/controller/PhabricatorApplicationSearchController.php
+++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php
@ -387,7 +387,6 @@ final class PhabricatorApplicationSearchController
    require_celerity_resource('application-search-view-css');

    return $this->newPage()
-      ->setApplicationMenu($this->buildApplicationMenu())
      ->setTitle(pht('Query: %s', $title))
      ->setCrumbs($crumbs)
      ->setNavigation($nav)
@ -611,7 +610,6 @@ final class PhabricatorApplicationSearchController
      ->setFooter($lists);

    return $this->newPage()
-      ->setApplicationMenu($this->buildApplicationMenu())
      ->setTitle(pht('Saved Queries'))
      ->setCrumbs($crumbs)
      ->setNavigation($nav)
--- a/src/view/page/PhabricatorStandardPageView.php
+++ b/src/view/page/PhabricatorStandardPageView.php
@ -32,18 +32,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
    return $this->showFooter;
  }

-  public function setApplicationMenu($application_menu) {
-    // NOTE: For now, this can either be a PHUIListView or a
-    // PHUIApplicationMenuView.
-
-    $this->applicationMenu = $application_menu;
-    return $this;
-  }
-
-  public function getApplicationMenu() {
-    return $this->applicationMenu;
-  }
-
  public function setApplicationName($application_name) {
    $this->applicationName = $application_name;
    return $this;
@ -345,7 +333,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
      $menu->setController($this->getController());
    }

-    $application_menu = $this->getApplicationMenu();
+    $application_menu = $this->applicationMenu;
    if ($application_menu) {
      if ($application_menu instanceof PHUIApplicationMenuView) {
        $crumbs = $this->getCrumbs();
@ -865,13 +853,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
  public function produceAphrontResponse() {
    $controller = $this->getController();

-    if (!$this->getApplicationMenu()) {
-      $application_menu = $controller->buildApplicationMenu();
-      if ($application_menu) {
-        $this->setApplicationMenu($application_menu);
-      }
-    }
-
    $viewer = $this->getUser();
    if ($viewer && $viewer->getPHID()) {
      $object_phids = $this->pageObjects;
@ -887,6 +868,14 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
      $response = id(new AphrontAjaxResponse())
        ->setContent($content);
    } else {
+      // See T13247. Try to find some navigational menu items to create a
+      // mobile navigation menu from.
+      $application_menu = $controller->buildApplicationMenu();
+      if (!$application_menu) {
+        $application_menu = $this->getNavigation()->getMenu();
+      }
+      $this->applicationMenu = $application_menu;
+
      $content = $this->render();

      $response = id(new AphrontWebpageResponse())