From 2b8bbae5fb276b8ec7c26f6a82335eba680e8357 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 16 Oct 2020 14:01:55 -0700 Subject: [PATCH] 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 --- resources/celerity/map.php | 16 +++++++-------- .../DifferentialRevisionViewController.php | 5 +++++ .../diff/view/PHUIDiffGraphView.php | 12 ++++++++++- .../graph/PhabricatorObjectGraph.php | 20 +++++++++++++++++-- .../diffusion/behavior-commit-graph.js | 6 +++--- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 4b33b32da5..0ae7849bfb 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -15,7 +15,7 @@ return array( '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 @@ return array( '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 @@ return array( '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 @@ return array( '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 @@ return array( 'javelin-dom', 'phabricator-notification', ), + 'ac10c917' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + ), 'ac2b1e01' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index ccb4b85867..e37fa2b529 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -510,6 +510,11 @@ final class DifferentialRevisionViewController ->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; diff --git a/src/infrastructure/diff/view/PHUIDiffGraphView.php b/src/infrastructure/diff/view/PHUIDiffGraphView.php index 589526feb6..6fa1b7c239 100644 --- a/src/infrastructure/diff/view/PHUIDiffGraphView.php +++ b/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 @@ final class PHUIDiffGraphView extends Phobject { 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 @@ final class PHUIDiffGraphView extends Phobject { 'diffusion-commit-graph', array( 'count' => $count, - 'autoheight' => true, + 'height' => $this->getHeight(), )); return $graph; diff --git a/src/infrastructure/graph/PhabricatorObjectGraph.php b/src/infrastructure/graph/PhabricatorObjectGraph.php index 67130e8c05..93e538b3de 100644 --- a/src/infrastructure/graph/PhabricatorObjectGraph.php +++ b/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 @@ abstract class PhabricatorObjectGraph 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 @@ abstract class PhabricatorObjectGraph $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(); diff --git a/webroot/rsrc/js/application/diffusion/behavior-commit-graph.js b/webroot/rsrc/js/application/diffusion/behavior-commit-graph.js index c0aa9bcb58..e8946ca9b4 100644 --- a/webroot/rsrc/js/application/diffusion/behavior-commit-graph.js +++ b/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;