When editing a tab panel from a dashboard, redirect back to the dashboard

Summary:
Depends on D20396. Ref T13272. Currently, using the dropdowns to edit a tab panel from a dashboard redirects you to the tab panel page.

Instead, redirect back to the context page (usually, a dashboard -- but theoretically a containing tab panel, since you can put tab panels inside tab panels).

Also, fix some JS issues with non-integer panel keys. I've moved panel keys from "0, 1, 2, ..." to having arbitrary keys to make some operations less flimsy/error-prone, but this needs some JS tweaks.

Test Plan: Edited a tab panel from a dashboard, got sent sensibly back to the dashboard.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272

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

resources/celerity/map.php

@@ -10,7 +10,7 @@ return array(
     'conpherence.pkg.css' => '3c8a0668',
     'conpherence.pkg.js' => '020aebcf',
     'core.pkg.css' => '9d654dff',
-    'core.pkg.js' => '350acda5',
+    'core.pkg.js' => '794952ae',
     'differential.pkg.css' => '8d8360fb',
     'differential.pkg.js' => '67e02996',
     'diffusion.pkg.css' => '42c75c37',
@@ -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' => '8d4490a2',
+    'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
     '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' => '8d4490a2',
+    'javelin-behavior-dashboard-tab-panel' => '0116d3e8',
     'javelin-behavior-day-view' => '727a5a61',
     'javelin-behavior-desktop-notifications-control' => '070679fe',
     'javelin-behavior-detect-timezone' => '78bc5d94',
@@ -902,6 +902,11 @@ return array(
     'unhandled-exception-css' => '9ecfc00d',
   ),
   'requires' => array(
+    '0116d3e8' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-stratcom',
+    ),
     '01384686' => array(
       'javelin-behavior',
       'javelin-uri',
@@ -1644,11 +1649,6 @@ return array(
       'phabricator-shaped-request',
       'conpherence-thread-manager',
     ),
-    '8d4490a2' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-stratcom',
-    ),
     '8e0aa661' => array(
       'javelin-install',
       'javelin-dom',

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

@@ -3,6 +3,17 @@
 final class PhabricatorDashboardPanelTabsController
   extends PhabricatorDashboardController {
 
+  private $contextObject;
+
+  private function setContextObject($context_object) {
+    $this->contextObject = $context_object;
+    return $this;
+  }
+
+  private function getContextObject() {
+    return $this->contextObject;
+  }
+
   public function handleRequest(AphrontRequest $request) {
     $viewer = $this->getViewer();
 
@@ -86,6 +97,43 @@ final class PhabricatorDashboardPanelTabsController
       }
     }
 
+    // Tab panels may be edited from the panel page, or from the context of
+    // a dashboard. If we're editing from a dashboard, we want to redirect
+    // back to the dashboard after making changes.
+
+    $context_phid = $request->getStr('contextPHID');
+    $context = null;
+    if (strlen($context_phid)) {
+      $context = id(new PhabricatorObjectQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($context_phid))
+        ->executeOne();
+      if (!$context) {
+        return new Aphront404Response();
+      }
+
+      switch (phid_get_type($context_phid)) {
+        case PhabricatorDashboardDashboardPHIDType::TYPECONST:
+          $cancel_uri = $context->getURI();
+          break;
+        case PhabricatorDashboardPanelPHIDType::TYPECONST:
+          $cancel_uri = $context->getURI();
+          break;
+        default:
+          return $this->newDialog()
+            ->setTitle(pht('Context Object Unsupported'))
+            ->appendParagraph(
+              pht(
+                'Context object ("%s") has unsupported type. Panels should '.
+                'be rendered from the context of a dashboard or another '.
+                'panel.',
+                $context_phid))
+            ->addCancelButton($cancel_uri);
+      }
+
+      $this->setContextObject($context);
+    }
+
     switch ($op) {
       case 'add':
         return $this->handleAddOperation($panel, $after, $cancel_uri);
@@ -176,10 +224,9 @@ final class PhabricatorDashboardPanelTabsController
           ->setLabel(pht('Panel'))
           ->setValue($v_panel));
 
-    return $this->newDialog()
+    return $this->newEditDialog()
       ->setTitle(pht('Choose Dashboard Panel'))
       ->setErrors($errors)
-      ->setWidth(AphrontDialogView::WIDTH_FORM)
       ->addHiddenInput('after', $after)
       ->appendForm($form)
       ->addCancelButton($cancel_uri)
@@ -205,7 +252,7 @@ final class PhabricatorDashboardPanelTabsController
       return id(new AphrontRedirectResponse())->setURI($cancel_uri);
     }
 
-    return $this->newDialog()
+    return $this->newEditDialog()
       ->setTitle(pht('Remove tab?'))
       ->addHiddenInput('target', $target)
       ->appendParagraph(pht('Really remove this tab?'))
@@ -243,7 +290,7 @@ final class PhabricatorDashboardPanelTabsController
           ->setName('name')
           ->setLabel(pht('Tab Name')));
 
-    return $this->newDialog()
+    return $this->newEditDialog()
       ->setTitle(pht('Rename Panel'))
       ->addHiddenInput('target', $target)
       ->appendForm($form)
@@ -292,4 +339,16 @@ final class PhabricatorDashboardPanelTabsController
     return $config;
   }
 
+  protected function newEditDialog() {
+    $dialog = $this->newDialog()
+      ->setWidth(AphrontDialogView::WIDTH_FORM);
+
+    $context = $this->getContextObject();
+    if ($context) {
+      $dialog->addHiddenInput('contextPHID', $context->getPHID());
+    }
+
+    return $dialog;
+  }
+
 }

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

@@ -42,6 +42,7 @@ final class PhabricatorDashboardPanelViewController
     $rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine())
       ->setViewer($viewer)
       ->setPanel($panel)
+      ->setContextObject($panel)
       ->setPanelPHID($panel->getPHID())
       ->setParentPanelPHIDs(array())
       ->setEditMode(true)
@@ -69,18 +70,11 @@ final class PhabricatorDashboardPanelViewController
     $viewer = $this->getViewer();
     $id = $panel->getID();
 
-    $button = id(new PHUIButtonView())
-      ->setTag('a')
-      ->setText(pht('View Panel'))
-      ->setIcon('fa-columns')
-      ->setHref($this->getApplicationURI("panel/render/{$id}/"));
-
     $header = id(new PHUIHeaderView())
       ->setUser($viewer)
       ->setHeader($panel->getName())
       ->setPolicyObject($panel)
-      ->setHeaderIcon('fa-columns')
+      ->setHeaderIcon('fa-window-maximize');
-      ->addActionLink($button);
 
     if (!$panel->getIsArchived()) {
       $header->setStatus('fa-check', 'bluegrey', pht('Active'));

src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php

@@ -16,6 +16,16 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
   private $movable = true;
   private $panelHandle;
   private $editMode;
+  private $contextObject;
+
+  public function setContextObject($object) {
+    $this->contextObject = $object;
+    return $this;
+  }
+
+  public function getContextObject() {
+    return $this->contextObject;
+  }
 
   public function setDashboardID($id) {
     $this->dashboardID = $id;

src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php

@@ -73,6 +73,7 @@ final class PhabricatorDashboardRenderingEngine extends Phobject {
           ->setViewer($viewer)
           ->setDashboardID($dashboard->getID())
           ->setEnableAsyncRendering(true)
+          ->setContextObject($dashboard)
           ->setPanelPHID($panel_phid)
           ->setParentPanelPHIDs(array())
           ->setHeaderMode($h_mode)

src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php

@@ -51,14 +51,16 @@ final class PhabricatorDashboardTabsPanelType
     $is_edit = $engine->isEditMode();
     $config = $this->getPanelConfiguration($panel);
 
+    $context_object = $engine->getContextObject();
+    if (!$context_object) {
+      $context_object = $panel;
+    }
+
+    $context_phid = $context_object->getPHID();
+
     $list = id(new PHUIListView())
       ->setType(PHUIListView::NAVBAR_LIST);
 
-    $node_ids = array();
-    foreach ($config as $idx => $tab_spec) {
-      $node_ids[$idx] = celerity_generate_unique_node_id();
-    }
-
     $ids = ipull($config, 'panelID');
     if ($ids) {
       $panels = id(new PhabricatorDashboardPanelQuery())
@@ -72,13 +74,16 @@ final class PhabricatorDashboardTabsPanelType
     $id = $panel->getID();
 
     $add_uri = urisprintf('/dashboard/panel/tabs/%d/add/', $id);
-    $add_uri = new PhutilURI($add_uri);
+    $add_uri = id(new PhutilURI($add_uri))
+      ->replaceQueryParam('contextPHID', $context_phid);
 
     $remove_uri = urisprintf('/dashboard/panel/tabs/%d/remove/', $id);
-    $remove_uri = new PhutilURI($remove_uri);
+    $remove_uri = id(new PhutilURI($remove_uri))
+      ->replaceQueryParam('contextPHID', $context_phid);
 
     $rename_uri = urisprintf('/dashboard/panel/tabs/%d/rename/', $id);
-    $rename_uri = new PhutilURI($rename_uri);
+    $rename_uri = id(new PhutilURI($rename_uri))
+      ->replaceQueryParam('contextPHID', $context_phid);
 
     $selected = 0;
 
@@ -102,7 +107,7 @@ final class PhabricatorDashboardTabsPanelType
         ->setHref('#')
         ->setSelected($idx == $selected)
         ->addSigil('dashboard-tab-panel-tab')
-        ->setMetadata(array('idx' => $idx))
+        ->setMetadata(array('panelKey' => $idx))
         ->setName($name);
 
       if ($is_edit) {
@@ -212,6 +217,7 @@ final class PhabricatorDashboardTabsPanelType
     // remains selected across page loads.
 
     $content = array();
+    $panel_list = array();
     $no_headers = PhabricatorDashboardPanelRenderingEngine::HEADER_MODE_NONE;
     foreach ($config as $idx => $tab_spec) {
       $panel_id = idx($tab_spec, 'panelID');
@@ -221,6 +227,7 @@ final class PhabricatorDashboardTabsPanelType
         $panel_content = id(new PhabricatorDashboardPanelRenderingEngine())
           ->setViewer($viewer)
           ->setEnableAsyncRendering(true)
+          ->setContextObject($context_object)
           ->setParentPanelPHIDs($parent_phids)
           ->setPanel($subpanel)
           ->setPanelPHID($subpanel->getPHID())
@@ -231,13 +238,20 @@ final class PhabricatorDashboardTabsPanelType
         $panel_content = pht('(Invalid Panel)');
       }
 
+      $content_id = celerity_generate_unique_node_id();
+
       $content[] = phutil_tag(
         'div',
         array(
-          'id' => $node_ids[$idx],
+          'id' => $content_id,
           'style' => ($idx == $selected) ? null : 'display: none',
         ),
         $panel_content);
+
+      $panel_list[] = array(
+        'panelKey' => (string)$idx,
+        'panelContentID' => $content_id,
+      );
     }
 
     if (!$content) {
@@ -269,7 +283,7 @@ final class PhabricatorDashboardTabsPanelType
       array(
         'sigil' => 'dashboard-tab-panel-container',
         'meta' => array(
-          'panels' => $node_ids,
+          'panels' => $panel_list,
         ),
       ),
       array(

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

@@ -18,26 +18,32 @@ JX.behavior('dashboard-tab-panel', function() {
 
     e.kill();
 
-    var ii;
+    var selected_key = e.getNodeData('dashboard-tab-panel-tab').panelKey;
-    var idx = e.getNodeData('dashboard-tab-panel-tab').idx;
 
     var root = e.getNode('dashboard-tab-panel-container');
     var data = JX.Stratcom.getData(root);
 
+
+    var ii;
     // Give the tab the user clicked a selected style, and remove it from
     // the other tabs.
     var tabs = JX.DOM.scry(root, 'li', 'dashboard-tab-panel-tab');
     for (ii = 0; ii < tabs.length; ii++) {
-      JX.DOM.alterClass(tabs[ii], 'phui-list-item-selected', (ii == idx));
+      var tab = tabs[ii];
+      var tab_data = JX.Stratcom.getData(tab);
+      var is_selected = (tab_data.panelKey === selected_key);
+      JX.DOM.alterClass(tabs[ii], 'phui-list-item-selected', is_selected);
     }
 
     // Switch the visible content to correspond to whatever the user clicked.
     for (ii = 0; ii < data.panels.length; ii++) {
-      var panel = JX.$(data.panels[ii]);
+      var panel = data.panels[ii];
-      if (ii == idx) {
+      var node = JX.$(panel.panelContentID);
-        JX.DOM.show(panel);
+
+      if (panel.panelKey == selected_key) {
+        JX.DOM.show(node);
       } else {
-        JX.DOM.hide(panel);
+        JX.DOM.hide(node);
       }
     }