Page MenuHomePhabricator

D18070.diff
No OneTemporary

D18070.diff

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -12,8 +12,8 @@
'core.pkg.css' => '5284a0e0',
'core.pkg.js' => '1475bd91',
'darkconsole.pkg.js' => '1f9a31bc',
- 'differential.pkg.css' => 'a2755617',
- 'differential.pkg.js' => '9cab3335',
+ 'differential.pkg.css' => '1ccbf3a9',
+ 'differential.pkg.js' => 'b453b745',
'diffusion.pkg.css' => 'b93d9b8c',
'diffusion.pkg.js' => '84c8f8fd',
'favicon.ico' => '30672e08',
@@ -64,7 +64,7 @@
'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' => '983751ee',
+ 'rsrc/css/application/differential/changeset-view.css' => 'c3f44655',
'rsrc/css/application/differential/core.css' => '5b7b8ff4',
'rsrc/css/application/differential/phui-inline-comment.css' => 'ffd1a542',
'rsrc/css/application/differential/revision-comment.css' => '14b8565a',
@@ -393,8 +393,8 @@
'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' => 'aaaf4cb5',
- 'rsrc/js/application/diff/DiffChangesetList.js' => '85abc805',
+ 'rsrc/js/application/diff/DiffChangeset.js' => 'fc3919c8',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => 'ffa063d8',
'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',
@@ -526,7 +526,7 @@
'rsrc/js/phuix/PHUIXActionListView.js' => 'b5c256b8',
'rsrc/js/phuix/PHUIXActionView.js' => 'b3465b9b',
'rsrc/js/phuix/PHUIXAutocomplete.js' => 'f6699267',
- 'rsrc/js/phuix/PHUIXButtonView.js' => '0f13520b',
+ 'rsrc/js/phuix/PHUIXButtonView.js' => 'b3c515be',
'rsrc/js/phuix/PHUIXDropdownMenu.js' => '8018ee50',
'rsrc/js/phuix/PHUIXExample.js' => '68af71ca',
'rsrc/js/phuix/PHUIXFormControl.js' => '83e03671',
@@ -560,7 +560,7 @@
'conpherence-thread-manager' => '4d863052',
'conpherence-transaction-css' => '85129c68',
'd3' => 'a11a5ff2',
- 'differential-changeset-view-css' => '983751ee',
+ 'differential-changeset-view-css' => 'c3f44655',
'differential-core-view-css' => '5b7b8ff4',
'differential-revision-add-comment-css' => 'c47f8c40',
'differential-revision-comment-css' => '14b8565a',
@@ -772,8 +772,8 @@
'phabricator-darklog' => 'c8e1ffe3',
'phabricator-darkmessage' => 'c48cccdd',
'phabricator-dashboard-css' => 'fe5b1869',
- 'phabricator-diff-changeset' => 'aaaf4cb5',
- 'phabricator-diff-changeset-list' => '85abc805',
+ 'phabricator-diff-changeset' => 'fc3919c8',
+ 'phabricator-diff-changeset-list' => 'ffa063d8',
'phabricator-diff-inline' => '1d17130f',
'phabricator-drag-and-drop-file-upload' => '58dea2fa',
'phabricator-draggable-list' => 'bea6e7f4',
@@ -878,7 +878,7 @@
'phuix-action-list-view' => 'b5c256b8',
'phuix-action-view' => 'b3465b9b',
'phuix-autocomplete' => 'f6699267',
- 'phuix-button-view' => '0f13520b',
+ 'phuix-button-view' => 'b3c515be',
'phuix-dropdown-menu' => '8018ee50',
'phuix-form-control-view' => '83e03671',
'phuix-icon-view' => 'bff6884b',
@@ -960,10 +960,6 @@
'javelin-dom',
'javelin-router',
),
- '0f13520b' => array(
- 'javelin-install',
- 'javelin-dom',
- ),
'0f764c35' => array(
'javelin-install',
'javelin-util',
@@ -1514,9 +1510,6 @@
'javelin-dom',
'javelin-stratcom',
),
- '85abc805' => array(
- 'javelin-install',
- ),
'85ee8ce6' => array(
'aphront-dialog-view-css',
),
@@ -1621,9 +1614,6 @@
'javelin-mask',
'phabricator-drag-and-drop-file-upload',
),
- '983751ee' => array(
- 'phui-inline-comment-view-css',
- ),
'9a6dd75c' => array(
'javelin-behavior',
'javelin-stratcom',
@@ -1716,17 +1706,6 @@
'javelin-util',
'phabricator-prefab',
),
- 'aaaf4cb5' => array(
- 'javelin-dom',
- 'javelin-util',
- 'javelin-stratcom',
- 'javelin-install',
- 'javelin-workflow',
- 'javelin-router',
- 'javelin-behavior-device',
- 'javelin-vector',
- 'phabricator-diff-inline',
- ),
'ab2f381b' => array(
'javelin-request',
'javelin-behavior',
@@ -1785,6 +1764,10 @@
'javelin-behavior',
'phabricator-prefab',
),
+ 'b3c515be' => array(
+ 'javelin-install',
+ 'javelin-dom',
+ ),
'b3e7d692' => array(
'javelin-install',
),
@@ -1877,6 +1860,9 @@
'javelin-dom',
'javelin-vector',
),
+ 'c3f44655' => array(
+ 'phui-inline-comment-view-css',
+ ),
'c420b0b9' => array(
'javelin-behavior',
'javelin-behavior-device',
@@ -2155,6 +2141,17 @@
'javelin-behavior-device',
'phabricator-keyboard-shortcut',
),
+ 'fc3919c8' => array(
+ 'javelin-dom',
+ 'javelin-util',
+ 'javelin-stratcom',
+ 'javelin-install',
+ 'javelin-workflow',
+ 'javelin-router',
+ 'javelin-behavior-device',
+ 'javelin-vector',
+ 'phabricator-diff-inline',
+ ),
'fc91ab6c' => array(
'javelin-behavior',
'javelin-dom',
@@ -2166,6 +2163,10 @@
'javelin-view-visitor',
'javelin-util',
),
+ 'ffa063d8' => array(
+ 'javelin-install',
+ 'phuix-button-view',
+ ),
),
'packages' => array(
'conpherence.pkg.css' => array(
diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php
--- a/src/applications/differential/view/DifferentialChangesetListView.php
+++ b/src/applications/differential/view/DifferentialChangesetListView.php
@@ -275,6 +275,10 @@
pht('Hide or show the current file.'),
'You must select a file to hide or show.' =>
pht('You must select a file to hide or show.'),
+
+ 'Unsaved' => pht('Unsaved'),
+ 'Unsubmitted' => pht('Unsubmitted'),
+ 'Comments' => pht('Comments'),
),
));
diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css
--- a/webroot/rsrc/css/application/differential/changeset-view.css
+++ b/webroot/rsrc/css/application/differential/changeset-view.css
@@ -413,3 +413,7 @@
.diff-banner-has-unsubmitted {
background: {$sh-yellowbackground};
}
+
+.diff-banner-buttons {
+ float: right;
+}
diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js
--- a/webroot/rsrc/js/application/diff/DiffChangeset.js
+++ b/webroot/rsrc/js/application/diff/DiffChangeset.js
@@ -403,6 +403,8 @@
block.items.push(rows[ii]);
}
+ var last_inline = null;
+ var last_inline_item = null;
for (ii = 0; ii < blocks.length; ii++) {
block = blocks[ii];
@@ -422,16 +424,38 @@
for (var jj = 0; jj < block.items.length; jj++) {
var inline = this.getInlineForRow(block.items[jj]);
- items.push({
+ // When comments are being edited, they have a hidden row with
+ // the actual comment and then a visible row with the editor.
+
+ // In this case, we only want to generate one item, but it should
+ // use the editor as a scroll target. To accomplish this, check if
+ // this row has the same inline as the previous row. If so, update
+ // the last item to use this row's nodes.
+
+ if (inline === last_inline) {
+ last_inline_item.nodes.begin = block.items[jj];
+ last_inline_item.nodes.end = block.items[jj];
+ continue;
+ } else {
+ last_inline = inline;
+ }
+
+ last_inline_item = {
type: block.type,
changeset: this,
target: inline,
hidden: inline.isHidden(),
+ deleted: !inline.getID() && !inline.isEditing(),
nodes: {
begin: block.items[jj],
end: block.items[jj]
+ },
+ attributes: {
+ unsaved: inline.isEditing()
}
- });
+ };
+
+ items.push(last_inline_item);
}
}
}
diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js
--- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
+++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
@@ -1,6 +1,7 @@
/**
* @provides phabricator-diff-changeset-list
* @requires javelin-install
+ * phuix-button-view
* @javelin
*/
@@ -111,6 +112,7 @@
_rangeTarget: null,
_bannerNode: null,
+ _unsavedButton: null,
sleep: function() {
this._asleep = true;
@@ -258,7 +260,13 @@
_installJumpKey: function(key, label, delta, filter, show_hidden) {
filter = filter || null;
- var handler = JX.bind(this, this._onjumpkey, delta, filter, show_hidden);
+
+ var options = {
+ filter: filter,
+ hidden: show_hidden
+ };
+
+ var handler = JX.bind(this, this._onjumpkey, delta, options);
return this._installKey(key, label, handler);
},
@@ -440,9 +448,14 @@
.show();
},
- _onjumpkey: function(delta, filter, show_hidden, manager) {
+ _onjumpkey: function(delta, options) {
var state = this._getSelectionState();
+ var filter = options.filter || null;
+ var hidden = options.hidden || false;
+ var wrap = options.wrap || false;
+ var attribute = options.attribute || null;
+
var cursor = state.cursor;
var items = state.items;
@@ -452,6 +465,7 @@
return;
}
+ var did_wrap = false;
while (true) {
if (cursor === null) {
cursor = 0;
@@ -464,9 +478,22 @@
return;
}
- // If we've gone forward off the end of the list, bail out.
+ // If we've gone forward off the end of the list, figure out where we
+ // should end up.
if (cursor >= items.length) {
- return;
+ if (!wrap) {
+ // If we aren't wrapping around, we're done.
+ return;
+ }
+
+ if (did_wrap) {
+ // If we're already wrapped around, we're done.
+ return;
+ }
+
+ // Otherwise, wrap the cursor back to the top.
+ cursor = 0;
+ did_wrap = true;
}
// If we're selecting things of a particular type (like only files)
@@ -479,17 +506,31 @@
// If the item is hidden, don't select it when iterating with jump
// keys. It can still potentially be selected in other ways.
- if (!show_hidden) {
+ if (!hidden) {
if (items[cursor].hidden) {
continue;
}
}
+ // If the item has been deleted, don't select it when iterating. The
+ // cursor may remain on it until it is removed.
+ if (items[cursor].deleted) {
+ continue;
+ }
+
+ // If we're selecting things with a particular attribute, like
+ // "unsaved", skip items without the attribute.
+ if (attribute !== null) {
+ if (!(items[cursor].attributes || {})[attribute]) {
+ continue;
+ }
+ }
+
// Otherwise, we've found a valid item to select.
break;
}
- this._setSelectionState(items[cursor], manager);
+ this._setSelectionState(items[cursor]);
},
_getSelectionState: function() {
@@ -512,24 +553,34 @@
};
},
- _setSelectionState: function(item, manager) {
+ _setSelectionState: function(item) {
this._cursorItem = item;
- this._redrawSelection(manager, true);
+ this._redrawSelection(true);
return this;
},
- _redrawSelection: function(manager, scroll) {
+ _redrawSelection: function(scroll) {
var cursor = this._cursorItem;
if (!cursor) {
this.setFocus(null);
return;
}
+ // If this item has been removed from the document (for example: create
+ // a new empty comment, then use the "Unsaved" button to select it, then
+ // cancel it), we can still keep the cursor here but do not want to show
+ // a selection reticle over an invisible node.
+ if (cursor.deleted) {
+ this.setFocus(null);
+ return;
+ }
+
this.setFocus(cursor.nodes.begin, cursor.nodes.end);
- if (manager && scroll) {
- manager.scrollTo(cursor.nodes.begin);
+ if (scroll) {
+ var pos = JX.$V(cursor.nodes.begin);
+ JX.DOM.scrollToPosition(0, pos.y - 60);
}
return this;
@@ -1320,14 +1371,65 @@
'diff-banner-has-unsubmitted',
!!unsubmitted.length);
+ var unsaved_button = this._getUnsavedButton();
+ var pht = this.getTranslations();
+
+ if (unsaved.length) {
+ unsaved_button.setText(unsaved.length + ' ' + pht('Unsaved'));
+ JX.DOM.show(unsaved_button.getNode());
+ } else {
+ JX.DOM.hide(unsaved_button.getNode());
+ }
+
+ var path_view = [icon, ' ', changeset.getDisplayPath()];
+
+ var buttons_attrs = {
+ className: 'diff-banner-buttons'
+ };
+
+ var buttons_list = [
+ unsaved_button.getNode()
+ ];
+
+ var buttons_view = JX.$N('div', buttons_attrs, buttons_list);
+
var icon = new JX.PHUIXIconView()
.setIcon(changeset.getIcon())
.getNode();
- JX.DOM.setContent(node, [icon, ' ', changeset.getDisplayPath()]);
+ JX.DOM.setContent(node, [buttons_view, path_view]);
document.body.appendChild(node);
},
+ _getUnsavedButton: function() {
+ if (!this._unsavedButton) {
+ var button = new JX.PHUIXButtonView()
+ .setIcon('fa-commenting-o')
+ .setButtonType(JX.PHUIXButtonView.BUTTONTYPE_SIMPLE);
+
+ var node = button.getNode();
+
+ var onunsaved = JX.bind(this, this._onunsavedclick);
+ JX.DOM.listen(node, 'click', null, onunsaved);
+
+ this._unsavedButton = button;
+ }
+
+ return this._unsavedButton;
+ },
+
+ _onunsavedclick: function(e) {
+ e.kill();
+
+ var options = {
+ filter: 'comment',
+ wrap: true,
+ attribute: 'unsaved'
+ };
+
+ this._onjumpkey(1, options);
+ },
+
_getBannerNode: function() {
if (!this._bannerNode) {
var attributes = {
diff --git a/webroot/rsrc/js/phuix/PHUIXButtonView.js b/webroot/rsrc/js/phuix/PHUIXButtonView.js
--- a/webroot/rsrc/js/phuix/PHUIXButtonView.js
+++ b/webroot/rsrc/js/phuix/PHUIXButtonView.js
@@ -57,6 +57,7 @@
setText: function(text) {
JX.DOM.setContent(this._getTextNode(), text);
+ this._redraw();
return this;
},

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 22, 4:03 PM (8 h, 31 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6775276
Default Alt Text
D18070.diff (15 KB)

Event Timeline