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; },