From e15009b76e1b085fabb0efe58a03869999ca202a Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 May 2017 05:48:38 -0700 Subject: [PATCH] Show "reply" inlines as replies in the objective list Summary: Ref T12733. Show a "reply" icon for replies, and make them stack directly under their parent. Test Plan: {F4968500} Reviewers: chad Reviewed By: chad Maniphest Tasks: T12733 Differential Revision: https://secure.phabricator.com/D17981 --- resources/celerity/map.php | 52 +++++++++---------- .../rsrc/js/application/diff/DiffInline.js | 5 ++ .../js/application/diff/ScrollObjective.js | 18 +++++++ .../application/diff/ScrollObjectiveList.js | 9 +++- 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 46443e7ecc..1d430926a6 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ return array( 'core.pkg.js' => '599698a7', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '7d4cfa59', - 'differential.pkg.js' => '06cddcc0', + 'differential.pkg.js' => '886eadff', 'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.js' => '84c8f8fd', 'favicon.ico' => '30672e08', @@ -392,9 +392,9 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', 'rsrc/js/application/diff/DiffChangeset.js' => 'cf4e2140', 'rsrc/js/application/diff/DiffChangesetList.js' => 'a716ca27', - 'rsrc/js/application/diff/DiffInline.js' => '4478f8ac', - 'rsrc/js/application/diff/ScrollObjective.js' => '9df4e4e2', - 'rsrc/js/application/diff/ScrollObjectiveList.js' => '085dd101', + 'rsrc/js/application/diff/DiffInline.js' => '1478c3b2', + 'rsrc/js/application/diff/ScrollObjective.js' => '7e8877e7', + 'rsrc/js/application/diff/ScrollObjectiveList.js' => '6120e99a', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', @@ -779,7 +779,7 @@ return array( 'phabricator-dashboard-css' => 'fe5b1869', 'phabricator-diff-changeset' => 'cf4e2140', 'phabricator-diff-changeset-list' => 'a716ca27', - 'phabricator-diff-inline' => '4478f8ac', + 'phabricator-diff-inline' => '1478c3b2', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', 'phabricator-fatal-config-template-css' => '8f18fa41', @@ -799,8 +799,8 @@ return array( 'phabricator-phtize' => 'd254d646', 'phabricator-prefab' => 'c5af80a2', 'phabricator-remarkup-css' => 'd1a5e11e', - 'phabricator-scroll-objective' => '9df4e4e2', - 'phabricator-scroll-objective-list' => '085dd101', + 'phabricator-scroll-objective' => '7e8877e7', + 'phabricator-scroll-objective-list' => '6120e99a', 'phabricator-search-results-css' => 'f87d23ad', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', @@ -956,15 +956,6 @@ return array( 'javelin-stratcom', 'javelin-util', ), - '085dd101' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-scrollbar', - 'phabricator-scroll-objective', - ), '08f4ccc3' => array( 'phui-oi-list-view-css', ), @@ -990,6 +981,9 @@ return array( 'javelin-dom', 'javelin-typeahead-normalizer', ), + '1478c3b2' => array( + 'javelin-dom', + ), '1499a8cb' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1203,9 +1197,6 @@ return array( 'javelin-workflow', 'javelin-workboard-controller', ), - '4478f8ac' => array( - 'javelin-dom', - ), '44959b73' => array( 'javelin-util', 'javelin-uri', @@ -1393,6 +1384,15 @@ return array( 'javelin-stratcom', 'javelin-dom', ), + '6120e99a' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-scrollbar', + 'phabricator-scroll-objective', + ), '61cbc29a' => array( 'javelin-magical-init', 'javelin-util', @@ -1501,6 +1501,13 @@ return array( '7e41274a' => array( 'javelin-install', ), + '7e8877e7' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + ), '7ebaeed3' => array( 'herald-rule-editor', 'javelin-behavior', @@ -1671,13 +1678,6 @@ return array( '9d9685d6' => array( 'phui-oi-list-view-css', ), - '9df4e4e2' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - ), '9f36c42d' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js index 1f0d503b9f..b0a8202e54 100644 --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -223,6 +223,7 @@ JX.install('DiffInline', { var color = 'bluegrey'; var tooltip = this._snippet; var anchor = this._row; + var should_stack = false; if (this._isEditing) { icon = 'fa-star'; @@ -244,6 +245,9 @@ JX.install('DiffInline', { } else if (this._isGhost) { icon = 'fa-comment-o'; color = 'grey'; + } else if (this._replyToCommentPHID) { + icon = 'fa-reply'; + should_stack = true; } objective @@ -251,6 +255,7 @@ JX.install('DiffInline', { .setIcon(icon) .setColor(color) .setTooltip(tooltip) + .setShouldStack(should_stack) .show(); }, diff --git a/webroot/rsrc/js/application/diff/ScrollObjective.js b/webroot/rsrc/js/application/diff/ScrollObjective.js index b8fb169914..1e596bd816 100644 --- a/webroot/rsrc/js/application/diff/ScrollObjective.js +++ b/webroot/rsrc/js/application/diff/ScrollObjective.js @@ -26,6 +26,7 @@ JX.install('ScrollObjective', { _visible: false, _callback: false, + _stack: false, getNode: function() { if (!this._node) { @@ -104,6 +105,23 @@ JX.install('ScrollObjective', { return this; }, + + /** + * Should this objective always stack immediately under the previous + * objective? + * + * This allows related objectives (like "comment, reply, reply") to be + * rendered in a tight sequence. + */ + setShouldStack: function(stack) { + this._stack = stack; + return this; + }, + + shouldStack: function() { + return this._stack; + }, + show: function() { this._visible = true; return this; diff --git a/webroot/rsrc/js/application/diff/ScrollObjectiveList.js b/webroot/rsrc/js/application/diff/ScrollObjectiveList.js index 7aae1fb1b4..ce47877d1a 100644 --- a/webroot/rsrc/js/application/diff/ScrollObjectiveList.js +++ b/webroot/rsrc/js/application/diff/ScrollObjectiveList.js @@ -112,7 +112,8 @@ JX.install('ScrollObjectiveList', { items.push({ offset: offset, - node: objective_node + node: objective_node, + objective: objective }); } @@ -130,7 +131,11 @@ JX.install('ScrollObjectiveList', { offset = item.offset; if (min !== null) { - offset = Math.max(offset, min); + if (item.objective.shouldStack()) { + offset = min; + } else { + offset = Math.max(offset, min); + } } min = offset + 15;