On Dashboard tab panels in edit mode, make the "Tab Name" and the "Dropdown Edit Caret" into different links

Summary:
Ref T13272. In edit mode, tab panels now have a dropdown menu. However, this sort of overrlaps with the actual action of clicking the tab to select it.

Separate these into different click targets so that "select tab X" and "open dropdown menu for X" are different operations.

This is more work than it appears because:

  - We have an "action icon" already, used when you put a dashboard on a portal/home to create an "Edit" link. It makes sense to attach dropdowns to this, but it has some hard-coded stuff.
  - In applications with a "Create <thing>" in the crumbs (like Maniphest), we may use a dropdown menu if there are multiple create forms available. However, this menu renders in a weird way by reading all the properties out of an actual "View" object and building something else.
  - The "list of tabs" stuff shares code with different "list of tabs" navigation used by Diffusion and Instances.

..but I think I fixed everything and didn't break anything.

Test Plan:
  - Clicked "select tab" and "open dropdown menu" as separate actions.
  - Viewed Diffusion, Maniphest with multiple create forms, Instances.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272

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

resources/celerity/map.php

@@ -9,8 +9,8 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '3c8a0668',
     'conpherence.pkg.js' => '020aebcf',
-    'core.pkg.css' => 'dacb981b',
+    'core.pkg.css' => '9d654dff',
-    'core.pkg.js' => 'c783d8f6',
+    'core.pkg.js' => '350acda5',
     'differential.pkg.css' => '8d8360fb',
     'differential.pkg.js' => '67e02996',
     'diffusion.pkg.css' => '42c75c37',
@@ -134,7 +134,7 @@ return array(
     'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '490e2e2e',
     'rsrc/css/phui/object-item/phui-oi-list-view.css' => 'f14f2422',
     'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => '6a30fa46',
-    'rsrc/css/phui/phui-action-list.css' => '8862282e',
+    'rsrc/css/phui/phui-action-list.css' => '48a45c51',
     'rsrc/css/phui/phui-action-panel.css' => '6c386cbf',
     'rsrc/css/phui/phui-badge.css' => '666e25ad',
     'rsrc/css/phui/phui-basic-nav-view.css' => '56ebd66d',
@@ -164,7 +164,7 @@ return array(
     'rsrc/css/phui/phui-invisible-character-view.css' => 'c694c4a4',
     'rsrc/css/phui/phui-left-right.css' => '68513c34',
     'rsrc/css/phui/phui-lightbox.css' => '4ebf22da',
-    'rsrc/css/phui/phui-list.css' => '470b1adb',
+    'rsrc/css/phui/phui-list.css' => '734a1039',
     'rsrc/css/phui/phui-object-box.css' => 'f434b6be',
     'rsrc/css/phui/phui-pager.css' => 'd022c7ad',
     'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8',
@@ -374,7 +374,7 @@ return array(
     'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '09ecf50c',
     'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '076bd092',
     'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
-    'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '9b1cbd76',
+    'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '8d4490a2',
     'rsrc/js/application/diff/DiffChangeset.js' => 'd0a85a85',
     'rsrc/js/application/diff/DiffChangesetList.js' => '04023d82',
     'rsrc/js/application/diff/DiffInline.js' => 'a4a14a94',
@@ -597,7 +597,7 @@ return array(
     'javelin-behavior-dashboard-async-panel' => '09ecf50c',
     'javelin-behavior-dashboard-move-panels' => '076bd092',
     'javelin-behavior-dashboard-query-panel-select' => '1e413dc9',
-    'javelin-behavior-dashboard-tab-panel' => '9b1cbd76',
+    'javelin-behavior-dashboard-tab-panel' => '8d4490a2',
     'javelin-behavior-day-view' => '727a5a61',
     'javelin-behavior-desktop-notifications-control' => '070679fe',
     'javelin-behavior-detect-timezone' => '78bc5d94',
@@ -757,7 +757,7 @@ return array(
     'path-typeahead' => 'ad486db3',
     'people-picture-menu-item-css' => 'fe8e07cf',
     'people-profile-css' => '2ea2daa1',
-    'phabricator-action-list-view-css' => '8862282e',
+    'phabricator-action-list-view-css' => '48a45c51',
     'phabricator-busy' => '5202e831',
     'phabricator-chatlog-css' => 'abdc76ee',
     'phabricator-content-source-view-css' => 'cdf0d579',
@@ -847,7 +847,7 @@ return array(
     'phui-invisible-character-view-css' => 'c694c4a4',
     'phui-left-right-css' => '68513c34',
     'phui-lightbox-css' => '4ebf22da',
-    'phui-list-view-css' => '470b1adb',
+    'phui-list-view-css' => '734a1039',
     'phui-object-box-css' => 'f434b6be',
     'phui-oi-big-ui-css' => 'fa74cc35',
     'phui-oi-color-css' => 'b517bfa0',
@@ -1644,6 +1644,11 @@ return array(
       'phabricator-shaped-request',
       'conpherence-thread-manager',
     ),
+    '8d4490a2' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-stratcom',
+    ),
     '8e0aa661' => array(
       'javelin-install',
       'javelin-dom',
@@ -1736,11 +1741,6 @@ return array(
       'javelin-install',
       'javelin-util',
     ),
-    '9b1cbd76' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-stratcom',
-    ),
     '9cec214e' => array(
       'javelin-behavior',
       'javelin-stratcom',

src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php

@@ -19,6 +19,11 @@ final class PhabricatorDashboardPanelViewController
       return new Aphront404Response();
     }
 
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $viewer,
+      $panel,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
     $title = $panel->getMonogram().' '.$panel->getName();
     $crumbs = $this->buildApplicationCrumbs();
     $crumbs->addTextCrumb(
@@ -29,7 +34,6 @@ final class PhabricatorDashboardPanelViewController
 
     $header = $this->buildHeaderView($panel);
     $curtain = $this->buildCurtainView($panel);
-    $properties = $this->buildPropertyView($panel);
 
     $timeline = $this->buildTransactionTimeline(
       $panel,
@@ -40,6 +44,7 @@ final class PhabricatorDashboardPanelViewController
       ->setPanel($panel)
       ->setPanelPHID($panel->getPHID())
       ->setParentPanelPHIDs(array())
+      ->setEditMode(true)
       ->renderPanel();
 
     $preview = id(new PHUIBoxView())
@@ -50,10 +55,9 @@ final class PhabricatorDashboardPanelViewController
       ->setHeader($header)
       ->setCurtain($curtain)
       ->setMainColumn(array(
-        $properties,
+        $rendered_panel,
         $timeline,
-      ))
+      ));
-      ->setFooter($rendered_panel);
 
     return $this->newPage()
       ->setTitle($title)
@@ -124,51 +128,4 @@ final class PhabricatorDashboardPanelViewController
     return $curtain;
   }
 
-  private function buildPropertyView(PhabricatorDashboardPanel $panel) {
-    $viewer = $this->getViewer();
-
-    $properties = id(new PHUIPropertyListView())
-      ->setUser($viewer);
-
-    $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
-      $viewer,
-      $panel);
-
-    $panel_type = $panel->getImplementation();
-    if ($panel_type) {
-      $type_name = $panel_type->getPanelTypeName();
-    } else {
-      $type_name = phutil_tag(
-        'em',
-        array(),
-        nonempty($panel->getPanelType(), pht('null')));
-    }
-
-    $properties->addProperty(
-      pht('Panel Type'),
-      $type_name);
-
-    $properties->addProperty(
-      pht('Editable By'),
-      $descriptions[PhabricatorPolicyCapability::CAN_EDIT]);
-
-    $dashboard_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
-      $panel->getPHID(),
-      PhabricatorDashboardPanelHasDashboardEdgeType::EDGECONST);
-
-    $does_not_appear = pht(
-      'This panel does not appear on any dashboards.');
-
-    $properties->addProperty(
-      pht('Appears On'),
-      $dashboard_phids
-        ? $viewer->renderHandleList($dashboard_phids)
-        : phutil_tag('em', array(), $does_not_appear));
-
-    return id(new PHUIObjectBoxView())
-      ->setHeaderText(pht('Details'))
-      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
-      ->addPropertyList($properties);
-  }
-
 }

src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php

@@ -15,6 +15,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
   private $dashboardID;
   private $movable = true;
   private $panelHandle;
+  private $editMode;
 
   public function setDashboardID($id) {
     $this->dashboardID = $id;
@@ -44,7 +45,12 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
   }
 
   public function isEditMode() {
-    return ($this->getHeaderMode() === self::HEADER_MODE_EDIT);
+    return $this->editMode;
+  }
+
+  public function setEditMode($mode) {
+    $this->editMode = $mode;
+    return $this;
   }
 
   /**

src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php

@@ -76,6 +76,7 @@ final class PhabricatorDashboardRenderingEngine extends Phobject {
           ->setPanelPHID($panel_phid)
           ->setParentPanelPHIDs(array())
           ->setHeaderMode($h_mode)
+          ->setEditMode($is_editable)
           ->setPanelHandle($handles[$panel_phid]);
 
         $panel = idx($panels, $panel_phid);

src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php

@@ -166,7 +166,9 @@ final class PhabricatorDashboardTabsPanelType
             ->setHref($details_uri)
             ->setDisabled(!$subpanel));
 
-        $tab_view->setDropdownMenu($dropdown_menu);
+        $tab_view
+          ->setActionIcon('fa-caret-down', '#')
+          ->setDropdownMenu($dropdown_menu);
       }
 
       $list->addMenuItem($tab_view);
@@ -193,8 +195,10 @@ final class PhabricatorDashboardTabsPanelType
       $list->addMenuItem(
         id(new PHUIListItemView())
           ->setHref('#')
+          ->setDisabled(true)
           ->setSelected(false)
-          ->setName(pht('Add Tab...'))
+          ->setName(pht("\xC2\xB7 \xC2\xB7 \xC2\xB7"))
+          ->setActionIcon('fa-caret-down', '#')
           ->setDropdownMenu($actions));
     }
 

src/view/phui/PHUICrumbsView.php

@@ -50,6 +50,12 @@ final class PHUICrumbsView extends AphrontView {
 
     $action_view = null;
     if ($this->actions) {
+      // TODO: This block of code takes "PHUIListItemView" objects and turns
+      // them into some weird abomination by reading most of their properties
+      // out. Some day, this workflow should render the items and CSS should
+      // resytle them in place without needing a wholly separate set of
+      // DOM nodes.
+
       $actions = array();
       foreach ($this->actions as $action) {
         if ($action->getType() == PHUIListItemView::TYPE_DIVIDER) {

src/view/phui/PHUIListItemView.php

@@ -35,7 +35,7 @@ final class PHUIListItemView extends AphrontTagView {
   private $actionIconHref;
   private $count;
   private $rel;
-  private $hasDropdown;
+  private $dropdownMenu;
 
   public function setOpenInNewWindow($open_in_new_window) {
     $this->openInNewWindow = $open_in_new_window;
@@ -65,11 +65,19 @@ final class PHUIListItemView extends AphrontTagView {
   }
 
   public function setDropdownMenu(PhabricatorActionListView $actions) {
-    Javelin::initBehavior('phui-dropdown-menu');
 
+    $this->dropdownMenu = $actions;
+
+    // TODO: "PHUICrumbsView" currently creates a bad copy of list items
+    // by reading some of their properties. To survive this copy step, we
+    // need to mutate "$this" immediately or the "Create Object" dropdown
+    // when multiple create forms exist breaks.
+
+    if (!$this->actionIcon) {
+      Javelin::initBehavior('phui-dropdown-menu');
       $this->addSigil('phui-dropdown-menu');
       $this->setMetadata($actions->getDropdownMenuMetadata());
-    $this->hasDropdown = true;
+    }
 
     return $this;
   }
@@ -237,12 +245,19 @@ final class PHUIListItemView extends AphrontTagView {
       $classes[] = 'phui-list-item-has-action-icon';
     }
 
-    if ($this->hasDropdown) {
+    if ($this->dropdownMenu) {
       $classes[] = 'dropdown';
+      if (!$this->actionIcon) {
+        throw new Exception(
+          pht(
+            'List item views can not currently render a dropdown without '.
+            'an action icon, because no application uses one. Clean up '.
+            'PHUICrumbsView, then add this capability.'));
+      }
     }
 
     return array(
-      'class' => implode(' ', $classes),
+      'class' => $classes,
     );
   }
 
@@ -345,19 +360,7 @@ final class PHUIListItemView extends AphrontTagView {
       $classes[] = 'phui-list-item-indented';
     }
 
-    $action_link = null;
+    $action_link = $this->newActionIconView();
-    if ($this->actionIcon) {
-      $action_icon = id(new PHUIIconView())
-        ->setIcon($this->actionIcon)
-        ->addClass('phui-list-item-action-icon');
-      $action_link = phutil_tag(
-        'a',
-        array(
-          'href' => $this->actionIconHref,
-          'class' => 'phui-list-item-action-href',
-        ),
-        $action_icon);
-    }
 
     $count = null;
     if ($this->count) {
@@ -369,12 +372,6 @@ final class PHUIListItemView extends AphrontTagView {
         $this->count);
     }
 
-    if ($this->hasDropdown) {
-      $caret = phutil_tag('span', array('class' => 'caret'), '');
-    } else {
-      $caret = null;
-    }
-
     $icons = $this->getIcons();
 
     $list_item = javelin_tag(
@@ -393,11 +390,42 @@ final class PHUIListItemView extends AphrontTagView {
         $icons,
         $this->renderChildren(),
         $name,
-        $caret,
         $count,
       ));
 
     return array($list_item, $action_link);
   }
 
+  private function newActionIconView() {
+    $action_icon = $this->actionIcon;
+    $action_href = $this->actionIconHref;
+
+    if ($action_icon === null) {
+      return null;
+    }
+
+    $icon_view = id(new PHUIIconView())
+      ->setIcon($action_icon)
+      ->addClass('phui-list-item-action-icon');
+
+    if ($this->dropdownMenu) {
+      Javelin::initBehavior('phui-dropdown-menu');
+      $sigil = 'phui-dropdown-menu';
+      $metadata = $this->dropdownMenu->getDropdownMenuMetadata();
+    } else {
+      $sigil = null;
+      $metadata = null;
+    }
+
+    return javelin_tag(
+      'a',
+      array(
+        'href' => $action_href,
+        'class' => 'phui-list-item-action-href',
+        'sigil' => $sigil,
+        'meta' => $metadata,
+      ),
+      $icon_view);
+  }
+
 }

webroot/rsrc/css/phui/phui-action-list.css

@@ -214,13 +214,26 @@
     color: {$sky};
 }
 
-.phui-list-navbar .phui-list-item-view.dropdown .phui-list-item-href {
+.phui-list-navbar .phui-list-item-href {
-  padding-right: 28px;
+  display: inline-block;
 }
 
-.phui-list-navbar .phui-list-item-view .caret {
+.phui-list-navbar .phui-list-item-disabled .phui-list-item-href {
-  position: absolute;
+  color: {$lightgreytext};
-  top: 6px;
+}
-  right: 12px;
+
-  border-top: 7px solid {$greytext};
+.phui-list-navbar .phui-list-item-action-href {
+  display: inline-block;
+  padding: 8px 16px;
+  line-height: 16px;
+}
+
+.phui-list-navbar .phui-list-item-action-href .phui-icon-view {
+  color: {$darkgreytext};
+}
+
+.device-desktop
+  .phui-list-navbar .phui-list-item-action-href:hover {
+  background-color: rgba({$alphablue}, 0.07);
+  color: {$sky};
 }

webroot/rsrc/css/phui/phui-list.css

@@ -110,10 +110,6 @@
   border-right: 1px solid {$thinblueborder};
 }
 
-.phui-list-view.phui-list-navbar > li > * {
-  display: block;
-}
-
 .phui-list-navbar .phui-list-item-href {
   color: {$bluetext};
   padding: 8px 16px;
@@ -265,7 +261,7 @@
 
 /* - Action Icon ----------------------------------------------------------- */
 
-.phui-list-item-has-action-icon .phui-list-item-action-href {
+.phui-list-sidenav .phui-list-item-has-action-icon .phui-list-item-action-href {
   position: absolute;
   width: 28px;
   top: 0;
@@ -277,24 +273,27 @@
   display: none;
 }
 
-.phui-list-item-has-action-icon.phui-list-item-selected .phui-list-item-href {
+.phui-list-sidenav .phui-list-item-has-action-icon.phui-list-item-selected
+  .phui-list-item-href {
   padding-right: 32px;
 }
 
-.phui-list-item-has-action-icon.phui-list-item-selected
+.phui-list-sidenav .phui-list-item-has-action-icon.phui-list-item-selected
   .phui-list-item-action-href {
     display: block;
 }
 
-.phui-list-item-has-action-icon .phui-list-item-action-href:hover {
+.phui-list-sidenav .phui-list-item-has-action-icon
+  .phui-list-item-action-href:hover {
   background-color: rgba({$alphablack},.05);
 }
 
-.phui-list-item-has-action-icon .phui-list-item-action-icon {
+.phui-list-sidenav .phui-list-item-has-action-icon .phui-list-item-action-icon {
   opacity: 0.5;
 }
 
-.phui-list-item-has-action-icon .phui-list-item-action-href:hover
+.phui-list-sidenav .phui-list-item-has-action-icon
+  .phui-list-item-action-href:hover
   .phui-list-item-action-icon {
   opacity: 1;
 }

webroot/rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js

@@ -8,6 +8,14 @@
 JX.behavior('dashboard-tab-panel', function() {
 
   JX.Stratcom.listen('click', 'dashboard-tab-panel-tab', function(e) {
+    // On dashboard panels in edit mode, the user may click the dropdown caret
+    // within a tab to open the context menu. If they do, this click should
+    // just open the menu, not select the tab. For now, pass the event here
+    // to let the menu handler act on it.
+    if (JX.Stratcom.pass(e)) {
+      return;
+    }
+
     e.kill();
 
     var ii;