From 429a0ce4e699b507165b7672bf4cdd42d65c5da6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 May 2017 04:58:09 -0700 Subject: [PATCH] (stable) Make Differential objective markers show a brighter "editing" state Summary: Ref T12733. - While editing a comment, show a pink star ({icon star, color=pink}) with a tooltip. - Slight UI tweaks, including draft comments getting an indigo pencil ({icon pencil, color=indigo}). Test Plan: {F4968470} Reviewers: chad Reviewed By: chad Maniphest Tasks: T12733 Differential Revision: https://secure.phabricator.com/D17977 --- resources/celerity/map.php | 40 +++++++++---------- .../view/DifferentialChangesetListView.php | 2 + .../differential/changeset-view.css | 5 ++- .../rsrc/js/application/diff/DiffInline.js | 29 +++++++++++++- webroot/rsrc/js/core/ToolTip.js | 4 ++ 5 files changed, 56 insertions(+), 24 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index bbd3c5e317..97b4f15a35 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,10 +10,10 @@ return array( 'conpherence.pkg.css' => 'ff161f2d', 'conpherence.pkg.js' => 'b5b51108', 'core.pkg.css' => '5ffe8b79', - 'core.pkg.js' => 'a8eda64a', + 'core.pkg.js' => '599698a7', 'darkconsole.pkg.js' => '1f9a31bc', - 'differential.pkg.css' => 'bf87589e', - 'differential.pkg.js' => '24d1acf0', + 'differential.pkg.css' => '7d4cfa59', + 'differential.pkg.js' => 'f94e941c', 'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.js' => '84c8f8fd', 'favicon.ico' => '30672e08', @@ -64,7 +64,7 @@ return array( 'rsrc/css/application/dashboard/dashboard.css' => 'fe5b1869', 'rsrc/css/application/diff/inline-comment-summary.css' => '51efda3a', 'rsrc/css/application/differential/add-comment.css' => 'c47f8c40', - 'rsrc/css/application/differential/changeset-view.css' => 'fa476ec0', + 'rsrc/css/application/differential/changeset-view.css' => 'acfd58f6', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => 'ffd1a542', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -392,7 +392,7 @@ 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' => '77e14b60', + 'rsrc/js/application/diff/DiffInline.js' => 'fa07d36e', 'rsrc/js/application/diff/ScrollObjective.js' => '2e069f79', 'rsrc/js/application/diff/ScrollObjectiveList.js' => '085dd101', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', @@ -480,7 +480,7 @@ return array( 'rsrc/js/core/ShapedRequest.js' => '7cbe244b', 'rsrc/js/core/TextAreaUtils.js' => '320810c8', 'rsrc/js/core/Title.js' => '485aaa6c', - 'rsrc/js/core/ToolTip.js' => '74caa17f', + 'rsrc/js/core/ToolTip.js' => '358b8c04', 'rsrc/js/core/behavior-active-nav.js' => 'e379b58e', 'rsrc/js/core/behavior-audio-source.js' => '59b251eb', 'rsrc/js/core/behavior-autofocus.js' => '7319e029', @@ -567,7 +567,7 @@ return array( 'conpherence-thread-manager' => '4d863052', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => 'fa476ec0', + 'differential-changeset-view-css' => 'acfd58f6', 'differential-core-view-css' => '5b7b8ff4', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', @@ -779,7 +779,7 @@ return array( 'phabricator-dashboard-css' => 'fe5b1869', 'phabricator-diff-changeset' => 'cf4e2140', 'phabricator-diff-changeset-list' => 'a716ca27', - 'phabricator-diff-inline' => '77e14b60', + 'phabricator-diff-inline' => 'fa07d36e', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', 'phabricator-fatal-config-template-css' => '8f18fa41', @@ -808,7 +808,7 @@ return array( 'phabricator-standard-page-view' => 'eb5b80c5', 'phabricator-textareautils' => '320810c8', 'phabricator-title' => '485aaa6c', - 'phabricator-tooltip' => '74caa17f', + 'phabricator-tooltip' => '358b8c04', 'phabricator-ui-example-css' => '528b19de', 'phabricator-uiexample-javelin-view' => 'd4a14807', 'phabricator-uiexample-reactor-button' => 'd19198c8', @@ -1137,6 +1137,12 @@ return array( 'javelin-dom', 'javelin-workflow', ), + '358b8c04' => array( + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-vector', + ), '3ab51e2c' => array( 'javelin-behavior', 'javelin-behavior-device', @@ -1462,12 +1468,6 @@ return array( 'javelin-vector', 'javelin-dom', ), - '74caa17f' => array( - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-vector', - ), '76b9fc3e' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1480,9 +1480,6 @@ return array( 'javelin-reactor', 'javelin-util', ), - '77e14b60' => array( - 'javelin-dom', - ), '782ab6e7' => array( 'javelin-behavior', 'javelin-dom', @@ -1788,6 +1785,9 @@ return array( 'phuix-autocomplete', 'javelin-mask', ), + 'acfd58f6' => array( + 'phui-inline-comment-view-css', + ), 'ae95d984' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2206,8 +2206,8 @@ return array( 'javelin-install', 'javelin-dom', ), - 'fa476ec0' => array( - 'phui-inline-comment-view-css', + 'fa07d36e' => array( + 'javelin-dom', ), 'fbe497e7' => array( 'javelin-behavior', diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index ea8a5e42f1..880fcb872d 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -232,6 +232,8 @@ final class DifferentialChangesetListView extends AphrontView { 'Loading...' => pht('Loading...'), + 'Editing Comment' => pht('Editing Comment'), + 'Jump to next change.' => pht('Jump to next change.'), 'Jump to previous change.' => pht('Jump to previous change.'), 'Jump to next file.' => pht('Jump to next file.'), diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 6a2552fbd1..6a29e1269b 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -419,7 +419,7 @@ tr.differential-inline-loading { border-style: solid; border-color: rgba(255, 255, 255, 0.95); border-width: 1px 0 1px 1px; - box-shadow: -1px 0 2px rgba(255, 255, 255, 0.25); + box-shadow: -1px 0 2px rgba(255, 255, 255, 0.10); overflow: hidden; } @@ -433,7 +433,8 @@ tr.differential-inline-loading { position: absolute; box-sizing: border-box; cursor: pointer; - left: 8px; + text-align: middle; + left: 7px; } .scroll-objective .phui-icon-view { diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js index e66fb544c5..32cf0ac5a1 100644 --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -32,6 +32,7 @@ JX.install('DiffInline', { _isDraft: null, _isFixed: null, + _isEditing: false, bindToRow: function(row) { this._row = row; @@ -176,6 +177,12 @@ JX.install('DiffInline', { return this._changeset; }, + setEditing: function(editing) { + this._isEditing = editing; + this.updateObjective(); + return this; + }, + _onobjective: function() { this.getChangeset().getChangesetList().selectInline(this); }, @@ -194,12 +201,20 @@ JX.install('DiffInline', { return; } + var pht = changeset.getChangesetList().getTranslations(); + var icon = 'fa-comment'; var color = 'bluegrey'; + var tooltip = null; - if (this._isDraft) { + if (this._isEditing) { + icon = 'fa-star'; + color = 'pink'; + tooltip = pht('Editing Comment'); + } else if (this._isDraft) { // This inline is an unsubmitted draft. icon = 'fa-pencil'; + color = 'indigo'; } else if (this._isFixed) { // This inline has been marked done. icon = 'fa-check'; @@ -212,6 +227,7 @@ JX.install('DiffInline', { objective .setIcon(icon) .setColor(color) + .setTooltip(tooltip) .show(); }, @@ -511,7 +527,12 @@ JX.install('DiffInline', { this._undoRow = this._drawRows(template, cursor, mode, text); }, + _drawContentRows: function(rows) { + return this._drawRows(rows, null, 'content'); + }, + _drawEditRows: function(rows) { + this.setEditing(true); return this._drawRows(rows, null, 'edit'); }, @@ -560,6 +581,8 @@ JX.install('DiffInline', { 'click', 'inline-edit-cancel', JX.bind(this, this._oncancel, row_meta))); + } else if (type == 'content') { + // No special listeners for these rows. } else { row_meta.listeners.push( JX.DOM.listen( @@ -645,6 +668,7 @@ JX.install('DiffInline', { } this._removeRow(row); + this.setEditing(false); this.setInvisible(false); @@ -670,6 +694,7 @@ JX.install('DiffInline', { this.setLoading(false); this.setInvisible(false); + this.setEditing(false); this._onupdate(response); }, @@ -677,7 +702,7 @@ JX.install('DiffInline', { _onupdate: function(response) { var new_row; if (response.markup) { - new_row = this._drawEditRows(JX.$H(response.markup).getNode()).node; + new_row = this._drawContentRows(JX.$H(response.markup).getNode()).node; } // TODO: Save the old row so the action it's undo-able if it was a diff --git a/webroot/rsrc/js/core/ToolTip.js b/webroot/rsrc/js/core/ToolTip.js index 64a2deb301..635c9466ad 100644 --- a/webroot/rsrc/js/core/ToolTip.js +++ b/webroot/rsrc/js/core/ToolTip.js @@ -21,6 +21,10 @@ JX.install('Tooltip', { return; } + if (content === null) { + return; + } + if (__DEV__) { switch (align) { case 'N':