Set an explicit height when drawing the dependent revision graph

Summary:
See PHI1900. Recent changes to how commit graphs are drawn made the height automatic in most cases, but it fails in Differential because the element isn't initially visible so the computed height is 0.

Just give them an explicit height so they show up again.

Test Plan: Viewed graphs in Maniphest, Differential, and Diffusion; saw them all render properly.

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

Author: epriestley

resources/celerity/map.php

@@ -15,7 +15,7 @@
     'differential.pkg.css' => '5c459f92',
     'differential.pkg.js' => '5080baf4',
     'diffusion.pkg.css' => '42c75c37',
-    'diffusion.pkg.js' => '8ee48a4b',
+    'diffusion.pkg.js' => '78c9885d',
     'maniphest.pkg.css' => '35995d6d',
     'maniphest.pkg.js' => 'c9308721',
     'rsrc/audio/basic/alert.mp3' => '17889334',
@@ -394,7 +394,7 @@
     'rsrc/js/application/diffusion/ExternalEditorLinkEngine.js' => '48a8641f',
     'rsrc/js/application/diffusion/behavior-audit-preview.js' => 'b7b73831',
     'rsrc/js/application/diffusion/behavior-commit-branches.js' => '4b671572',
-    'rsrc/js/application/diffusion/behavior-commit-graph.js' => '3be6ef4f',
+    'rsrc/js/application/diffusion/behavior-commit-graph.js' => 'ac10c917',
     'rsrc/js/application/diffusion/behavior-locate-file.js' => '87428eb2',
     'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => 'c715c123',
     'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => '6a85bc5a',
@@ -623,7 +623,7 @@
     'javelin-behavior-differential-diff-radios' => '925fe8cd',
     'javelin-behavior-differential-populate' => 'b86ef6c2',
     'javelin-behavior-diffusion-commit-branches' => '4b671572',
-    'javelin-behavior-diffusion-commit-graph' => '3be6ef4f',
+    'javelin-behavior-diffusion-commit-graph' => 'ac10c917',
     'javelin-behavior-diffusion-locate-file' => '87428eb2',
     'javelin-behavior-diffusion-pull-lastmodified' => 'c715c123',
     'javelin-behavior-document-engine' => '243d6c22',
@@ -1250,11 +1250,6 @@
       'phuix-button-view',
       'javelin-external-editor-link-engine',
     ),
-    '3be6ef4f' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-stratcom',
-    ),
     '3dc5ad43' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1927,6 +1922,11 @@
       'javelin-dom',
       'phabricator-notification',
     ),
+    'ac10c917' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-stratcom',
+    ),
     'ac2b1e01' => array(
       'javelin-behavior',
       'javelin-stratcom',

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -510,6 +510,11 @@ public function handleRequest(AphrontRequest $request) {
       ->setLoadEntireGraph(true)
       ->loadGraph();
     if (!$stack_graph->isEmpty()) {
+      // See PHI1900. The graph UI element now tries to figure out the correct
+      // height automatically, but currently can't in this case because the
+      // element is not visible when the page loads. Set an explicit height.
+      $stack_graph->setHeight(34);
+
       $stack_table = $stack_graph->newGraphTable();
 
       $parent_type = DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST;

src/infrastructure/diff/view/PHUIDiffGraphView.php

@@ -4,6 +4,7 @@ final class PHUIDiffGraphView extends Phobject {
 
   private $isHead = true;
   private $isTail = true;
+  private $height;
 
   public function setIsHead($is_head) {
     $this->isHead = $is_head;
@@ -23,6 +24,15 @@ public function getIsTail() {
     return $this->isTail;
   }
 
+  public function setHeight($height) {
+    $this->height = $height;
+    return $this;
+  }
+
+  public function getHeight() {
+    return $this->height;
+  }
+
   public function renderRawGraph(array $parents) {
     // This keeps our accumulated information about each line of the
     // merge/branch graph.
@@ -205,7 +215,7 @@ public function renderGraph(array $parents) {
       'diffusion-commit-graph',
       array(
         'count' => $count,
-        'autoheight' => true,
+        'height' => $this->getHeight(),
       ));
 
     return $graph;

src/infrastructure/graph/PhabricatorObjectGraph.php

@@ -11,6 +11,7 @@ abstract class PhabricatorObjectGraph
   private $loadEntireGraph = false;
   private $limit;
   private $adjacent;
+  private $height;
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -34,6 +35,15 @@ public function getLimit() {
     return $this->limit;
   }
 
+  public function setHeight($height) {
+    $this->height = $height;
+    return $this;
+  }
+
+  public function getHeight() {
+    return $this->height;
+  }
+
   final public function setRenderOnlyAdjacentNodes($adjacent) {
     $this->adjacent = $adjacent;
     return $this;
@@ -193,8 +203,14 @@ final public function newGraphTable() {
 
     $ancestry = array_select_keys($ancestry, $order);
 
-    $traces = id(new PHUIDiffGraphView())
-      ->renderGraph($ancestry);
+    $graph_view = id(new PHUIDiffGraphView());
+
+    $height = $this->getHeight();
+    if ($height !== null) {
+      $graph_view->setHeight($height);
+    }
+
+    $traces = $graph_view->renderGraph($ancestry);
 
     $ii = 0;
     $rows = array();

webroot/rsrc/js/application/diffusion/behavior-commit-graph.js

@@ -62,10 +62,10 @@ JX.behavior('diffusion-commit-graph', function(config) {
     };
 
     var h;
-    if (config.autoheight) {
-      h = JX.Vector.getDim(nodes[ii].parentNode).y;
+    if (config.height) {
+      h = config.height;
     } else {
-      h = 34;
+      h = JX.Vector.getDim(nodes[ii].parentNode).y;
     }
 
     var w = cell * config.count;