From 83e99fb691aa350d6990d3e467a3af1ba0d47213 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 30 May 2017 14:00:02 -0700 Subject: [PATCH] Add a class to the Differential banner when unsubmitted/unsaved changes are present Summary: Ref T12733. This adds classes for unsubmitted/unsaved changes, and lays some groundwork for additional buttons. Test Plan: - Added, edited, deleted comments. - Saw bar background color update appropriately. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12733 Differential Revision: https://secure.phabricator.com/D18045 --- resources/celerity/map.php | 60 +++++++++---------- .../differential/changeset-view.css | 5 ++ .../rsrc/js/application/diff/DiffChangeset.js | 5 ++ .../js/application/diff/DiffChangesetList.js | 42 +++++++++++++ .../rsrc/js/application/diff/DiffInline.js | 16 +++++ 5 files changed, 98 insertions(+), 30 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b6cf443b35..d6ad7e41fd 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -12,8 +12,8 @@ return array( 'core.pkg.css' => 'bb7f0446', 'core.pkg.js' => '1475bd91', 'darkconsole.pkg.js' => '1f9a31bc', - 'differential.pkg.css' => '9ebe4f44', - 'differential.pkg.js' => '40f4acb3', + 'differential.pkg.css' => 'a2755617', + 'differential.pkg.js' => '5bf658f0', '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' => '2971e2a2', + 'rsrc/css/application/differential/changeset-view.css' => '983751ee', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => 'ffd1a542', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -391,9 +391,9 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/diff/DiffChangeset.js' => '3359ad02', - 'rsrc/js/application/diff/DiffChangesetList.js' => 'b42eb5ff', - 'rsrc/js/application/diff/DiffInline.js' => '45d37835', + 'rsrc/js/application/diff/DiffChangeset.js' => '1f6748ae', + 'rsrc/js/application/diff/DiffChangesetList.js' => '85abc805', + 'rsrc/js/application/diff/DiffInline.js' => '1d17130f', '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', @@ -566,7 +566,7 @@ return array( 'conpherence-thread-manager' => '4d863052', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => '2971e2a2', + 'differential-changeset-view-css' => '983751ee', 'differential-core-view-css' => '5b7b8ff4', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', @@ -777,9 +777,9 @@ return array( 'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', - 'phabricator-diff-changeset' => '3359ad02', - 'phabricator-diff-changeset-list' => 'b42eb5ff', - 'phabricator-diff-inline' => '45d37835', + 'phabricator-diff-changeset' => '1f6748ae', + 'phabricator-diff-changeset-list' => '85abc805', + 'phabricator-diff-inline' => '1d17130f', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', 'phabricator-fatal-config-template-css' => '8f18fa41', @@ -1019,6 +1019,9 @@ return array( 'javelin-request', 'javelin-uri', ), + '1d17130f' => array( + 'javelin-dom', + ), '1def2711' => array( 'javelin-install', 'javelin-dom', @@ -1035,6 +1038,17 @@ return array( 'javelin-uri', 'javelin-routable', ), + '1f6748ae' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), '1f6794f6' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1072,9 +1086,6 @@ return array( 'javelin-install', 'javelin-util', ), - '2971e2a2' => array( - 'phui-inline-comment-view-css', - ), '2ae077e1' => array( 'javelin-behavior', 'javelin-dom', @@ -1110,17 +1121,6 @@ return array( 'javelin-dom', 'javelin-workflow', ), - '3359ad02' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), '358b8c04' => array( 'javelin-install', 'javelin-util', @@ -1203,9 +1203,6 @@ return array( 'javelin-behavior', 'javelin-dom', ), - '45d37835' => array( - 'javelin-dom', - ), '469c0d9e' => array( 'javelin-behavior', 'javelin-dom', @@ -1541,6 +1538,9 @@ return array( 'javelin-dom', 'javelin-stratcom', ), + '85abc805' => array( + 'javelin-install', + ), '85ee8ce6' => array( 'aphront-dialog-view-css', ), @@ -1652,6 +1652,9 @@ return array( 'javelin-mask', 'phabricator-drag-and-drop-file-upload', ), + '983751ee' => array( + 'phui-inline-comment-view-css', + ), '988040b4' => array( 'javelin-install', 'javelin-dom', @@ -1819,9 +1822,6 @@ return array( 'b3e7d692' => array( 'javelin-install', ), - 'b42eb5ff' => array( - 'javelin-install', - ), 'b59e1e96' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 39df37003c..f1be2b5f99 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -408,3 +408,8 @@ tr.differential-inline-loading { .diff-banner-path { color: {$greytext}; } + +.diff-banner-has-unsaved, +.diff-banner-has-unsubmitted { + background: {$sh-yellowbackground}; +} diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index 8e70c5ac47..6b5ebf5317 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -679,6 +679,11 @@ JX.install('DiffChangeset', { return null; }, + getInlines: function() { + this._rebuildAllInlines(); + return this._inlines; + }, + _rebuildAllInlines: function() { var rows = JX.DOM.scry(this._node, 'tr'); for (var ii = 0; ii < rows.length; ii++) { diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index 60575e2aa2..b79f05296e 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -818,6 +818,11 @@ JX.install('DiffChangesetList', { this._redrawSelection(); this._redrawHover(); + // Force a banner redraw after a resize event. Particularly, this makes + // sure the inline state updates immediately after an inline edit + // operation, even if the changeset itself has not changed. + this._bannerChangeset = null; + this._redrawBanner(); }, @@ -1278,6 +1283,43 @@ JX.install('DiffChangesetList', { return; } + var changesets = this._changesets; + var unsaved = []; + var unsubmitted = []; + var undone = []; + var all = []; + + for (var ii = 0; ii < changesets.length; ii++) { + var inlines = changesets[ii].getInlines(); + for (var jj = 0; jj < inlines.length; jj++) { + var inline = inlines[jj]; + + if (inline.isDeleted()) { + continue; + } + + all.push(inline); + + if (inline.isEditing()) { + unsaved.push(inline); + } else if (inline.isDraft()) { + unsubmitted.push(inline); + } else if (!inline.isDone()) { + undone.push(inline); + } + } + } + + JX.DOM.alterClass( + node, + 'diff-banner-has-unsaved', + !!unsaved.length); + + JX.DOM.alterClass( + node, + 'diff-banner-has-unsubmitted', + !!unsubmitted.length); + var icon = new JX.PHUIXIconView() .setIcon(changeset.getIcon()) .getNode(); diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js index b0eb9ca2cb..6d897c27ae 100644 --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -81,6 +81,22 @@ JX.install('DiffInline', { return this; }, + isDraft: function() { + return this._isDraft; + }, + + isDone: function() { + return this._isFixed; + }, + + isEditing: function() { + return this._isEditing; + }, + + isDeleted: function() { + return this._isDeleted; + }, + bindToRange: function(data) { this._displaySide = data.displaySide; this._number = parseInt(data.number, 10);