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' => 'ee5f28cd', 'core.pkg.js' => '8c5f913d', 'darkconsole.pkg.js' => '1f9a31bc', - 'differential.pkg.css' => '4ff77743', - 'differential.pkg.js' => '85543704', + 'differential.pkg.css' => 'ea471cb0', + 'differential.pkg.js' => '7f24021f', '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' => '69a3c268', + 'rsrc/css/application/differential/changeset-view.css' => '6a77323e', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => 'ffd1a542', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -390,13 +390,13 @@ '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' => 'c5742feb', - 'rsrc/js/application/diff/DiffChangesetList.js' => 'f1101e6e', - 'rsrc/js/application/diff/DiffInline.js' => 'a81c29d4', + 'rsrc/js/application/diff/DiffChangeset.js' => '34e513e2', + 'rsrc/js/application/diff/DiffChangesetList.js' => 'fcbf80e9', + 'rsrc/js/application/diff/DiffInline.js' => 'c2e9ff4c', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', - 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '89d11432', + 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => 'd59300f5', 'rsrc/js/application/differential/behavior-populate.js' => '5e41c819', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', 'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d', @@ -567,7 +567,7 @@ 'conpherence-thread-manager' => '4d863052', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => '69a3c268', + 'differential-changeset-view-css' => '6a77323e', 'differential-core-view-css' => '5b7b8ff4', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', @@ -619,7 +619,7 @@ 'javelin-behavior-device' => 'bb1dd507', 'javelin-behavior-diff-preview-link' => '051c7832', 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', - 'javelin-behavior-differential-edit-inline-comments' => '89d11432', + 'javelin-behavior-differential-edit-inline-comments' => 'd59300f5', 'javelin-behavior-differential-feedback-preview' => 'b064af76', 'javelin-behavior-differential-populate' => '5e41c819', 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', @@ -779,9 +779,9 @@ 'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', - 'phabricator-diff-changeset' => 'c5742feb', - 'phabricator-diff-changeset-list' => 'f1101e6e', - 'phabricator-diff-inline' => 'a81c29d4', + 'phabricator-diff-changeset' => '34e513e2', + 'phabricator-diff-changeset-list' => 'fcbf80e9', + 'phabricator-diff-inline' => 'c2e9ff4c', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', 'phabricator-fatal-config-template-css' => '8f18fa41', @@ -1129,6 +1129,17 @@ 'javelin-dom', 'javelin-workflow', ), + '34e513e2' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), '3ab51e2c' => array( 'javelin-behavior', 'javelin-behavior-device', @@ -1398,12 +1409,12 @@ '6882e80a' => array( 'javelin-dom', ), - '69a3c268' => array( - 'phui-inline-comment-view-css', - ), '69adf288' => array( 'javelin-install', ), + '6a77323e' => array( + 'phui-inline-comment-view-css', + ), '6ad39b6f' => array( 'javelin-install', 'javelin-event', @@ -1564,13 +1575,6 @@ 'phabricator-draggable-list', 'javelin-workboard-column', ), - '89d11432' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-util', - 'javelin-vector', - ), '8a41885b' => array( 'javelin-install', 'javelin-dom', @@ -1718,9 +1722,6 @@ 'javelin-stratcom', 'javelin-dom', ), - 'a81c29d4' => array( - 'javelin-dom', - ), 'a8beebea' => array( 'phui-oi-list-view-css', ), @@ -1909,23 +1910,15 @@ 'javelin-dom', 'javelin-vector', ), + 'c2e9ff4c' => array( + 'javelin-dom', + ), 'c420b0b9' => array( 'javelin-behavior', 'javelin-behavior-device', 'javelin-stratcom', 'phabricator-tooltip', ), - 'c5742feb' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), 'c587b80f' => array( 'javelin-install', ), @@ -2043,6 +2036,13 @@ 'javelin-dom', 'javelin-stratcom', ), + 'd59300f5' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-util', + 'javelin-vector', + ), 'd5a2d665' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2174,9 +2174,6 @@ 'javelin-workflow', 'javelin-json', ), - 'f1101e6e' => array( - 'javelin-install', - ), 'f50152ad' => array( 'phui-timeline-view-css', ), @@ -2224,6 +2221,9 @@ 'javelin-dom', 'phortune-credit-card-form', ), + 'fcbf80e9' => array( + 'javelin-install', + ), 'fe287620' => array( 'javelin-install', 'javelin-dom', 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 @@ -109,10 +109,6 @@ color: {$darkgreytext}; } -.differential-diff th.selected { - background-color: {$sh-yellowbackground}; -} - .differential-changeset-immutable .differential-diff th { cursor: auto; } @@ -308,6 +304,7 @@ top: 0; left: 0; box-sizing: border-box; + pointer-events: none; } .differential-diff .inline > td { 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 @@ -27,6 +27,9 @@ this._encoding = data.encoding; this._loaded = data.loaded; + this._leftID = data.left; + this._rightID = data.right; + this._inlines = []; }, @@ -48,8 +51,19 @@ _encoding: null, _undoTemplates: null, + _leftID: null, + _rightID: null, + _inlines: null, + getLeftChangesetID: function() { + return this._leftID; + }, + + getRightChangesetID: function() { + return this._rightID; + }, + /** * Has the content of this changeset been loaded? * 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 @@ -56,6 +56,12 @@ 'mousedown', ['differential-inline-comment', 'differential-inline-header'], onselect); + + var onhover = JX.bind(this, this._ifawake, this._onhover); + JX.Stratcom.listen( + ['mouseover', 'mouseout'], + 'differential-inline-comment', + onhover); }, properties: { @@ -74,16 +80,24 @@ _focusStart: null, _focusEnd: null, + _hoverNode: null, + _hoverInline: null, + _hoverOrigin: null, + _hoverTarget: null, + sleep: function() { this._asleep = true; this._redrawFocus(); + this._redrawSelection(); + this.resetHover(); }, wake: function() { this._asleep = false; this._redrawFocus(); + this._redrawSelection(); if (this._initialized) { return; @@ -428,6 +442,21 @@ return result; }, + _onhover: function(e) { + if (e.getIsTouchEvent()) { + return; + } + + var inline; + if (e.getType() == 'mouseout') { + inline = null; + } else { + inline = this._getInlineForEvent(e); + } + + this._setHoverInline(inline); + }, + _onmore: function(e) { e.kill(); @@ -669,6 +698,8 @@ _onresize: function() { this._redrawFocus(); + this._redrawSelection(); + this._redrawHover(); }, _onselect: function(e) { @@ -769,6 +800,11 @@ if (forms.length) { JX.DOM.invoke(forms[0], 'shouldRefresh'); } + + // Clear the mouse hover reticle after a substantive edit: we don't get + // a "mouseout" event if the row vanished because of row being removed + // after an edit. + this.reseHover(); }, setFocus: function(node, extended_node) { @@ -812,6 +848,112 @@ return this._focusNode; }, + _setHoverInline: function(inline) { + this._hoverInline = inline; + + if (inline) { + var changeset = inline.getChangeset(); + + var changeset_id; + var side = inline.getDisplaySide(); + if (side == 'right') { + changeset_id = changeset.getRightChangesetID(); + } else { + changeset_id = changeset.getLeftChangesetID(); + } + + var new_part; + if (inline.isNewFile()) { + new_part = 'N'; + } else { + new_part = 'O'; + } + + var prefix = 'C' + changeset_id + new_part + 'L'; + + var number = inline.getLineNumber(); + var length = inline.getLineLength(); + + var origin = JX.$(prefix + number); + var target = JX.$(prefix + (number + length)); + + this._hoverOrigin = origin; + this._hoverTarget = target; + } else { + this._hoverOrigin = null; + this._hoverTarget = null; + } + + this._redrawHover(); + }, + + resetHover: function() { + this._setHoverInline(null); + }, + + _redrawHover: function() { + var reticle = this._getHoverNode(); + if (!this._hoverOrigin || this.isAsleep()) { + JX.DOM.remove(reticle); + return; + } + + JX.DOM.getContentFrame().appendChild(reticle); + + var top = this._hoverOrigin; + var bot = this._hoverTarget; + if (JX.$V(top).y > JX.$V(bot).y) { + var tmp = top; + top = bot; + bot = tmp; + } + + // Find the leftmost cell that we're going to highlight: this is the next + // in the row. In 2up views, it should be directly adjacent. In + // 1up views, we may have to skip over the other line number column. + var l = top; + while (JX.DOM.isType(l, 'th')) { + l = l.nextSibling; + } + + // Find the rightmost cell that we're going to highlight: this is the + // farthest consecutive, adjacent in the row. Sometimes the left + // and right nodes are the same (left side of 2up view); sometimes we're + // going to highlight several nodes (copy + code + coverage). + var r = l; + while (r.nextSibling && JX.DOM.isType(r.nextSibling, 'td')) { + r = r.nextSibling; + } + + var pos = JX.$V(l) + .add(JX.Vector.getAggregateScrollForNode(l)); + + var dim = JX.$V(r) + .add(JX.Vector.getAggregateScrollForNode(r)) + .add(-pos.x, -pos.y) + .add(JX.Vector.getDim(r)); + + var bpos = JX.$V(bot) + .add(JX.Vector.getAggregateScrollForNode(bot)); + dim.y = (bpos.y - pos.y) + JX.Vector.getDim(bot).y; + + pos.setPos(reticle); + dim.setDim(reticle); + + JX.DOM.show(reticle); + }, + + _getHoverNode: function() { + if (!this._hoverNode) { + var attributes = { + className: 'differential-reticle' + }; + this._hoverNode = JX.$N('div', attributes); + } + + return this._hoverNode; + }, + _deleteInlineByID: function(id) { var uri = this.getInlineURI(); var data = { diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -56,8 +56,8 @@ this._displaySide = 'left'; } - this._number = data.number; - this._length = data.length; + this._number = parseInt(data.number, 10); + this._length = parseInt(data.length, 10); this._originalText = data.original; this._isNewFile = (this.getDisplaySide() == 'right') || @@ -70,8 +70,8 @@ bindToRange: function(data) { this._displaySide = data.displaySide; - this._number = data.number; - this._length = data.length; + this._number = parseInt(data.number, 10); + this._length = parseInt(data.length, 10); this._isNewFile = data.isNewFile; this._changesetID = data.changesetID; @@ -365,9 +365,13 @@ }, _oncreateresponse: function(response) { + try { var rows = JX.$H(response).getNode(); this._drawEditRows(rows); + } catch (e) { + JX.log(e); + } }, _ondeleteresponse: function() { @@ -409,6 +413,7 @@ var first_row = JX.DOM.scry(rows, 'tr')[0]; var first_meta; var row = first_row; + var anchor = cursor || this._row; cursor = cursor || this._row.nextSibling; var next_row; @@ -417,7 +422,11 @@ // into the document. next_row = row.nextSibling; - cursor.parentNode.insertBefore(row, cursor); + // Bind edit and undo rows to this DiffInline object so that + // interactions like hovering work properly. + JX.Stratcom.getData(row).inline = this; + + anchor.parentNode.insertBefore(row, cursor); cursor = row; var row_meta = { @@ -528,6 +537,8 @@ this._removeRow(row); this.setInvisible(false); + + this._didUpdate(true); }, _readText: function(row) { @@ -579,8 +590,9 @@ } this.getChangeset().getChangesetList().redrawCursor(); + this.getChangeset().getChangesetList().resetHover(); - // Emit a resize event so that UI elements like the keyboad focus + // Emit a resize event so that UI elements like the keyboard focus // reticle can redraw properly. JX.Stratcom.invoke('resize'); }, diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js --- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js +++ b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js @@ -11,7 +11,6 @@ var selecting = false; var reticle = JX.$N('div', {className: 'differential-reticle'}); - var old_cells = []; JX.DOM.hide(reticle); var origin = null; @@ -19,28 +18,6 @@ var root = null; var changeset = null; - var editor = null; - - function updateReticleForComment(e) { - root = e.getNode('differential-changeset'); - if (!root) { - return; - } - - var data = e.getNodeData('differential-inline-comment'); - var change = e.getNodeData('differential-changeset'); - - var id_part = data.on_right ? change.right : change.left; - var new_part = data.isNewFile ? 'N' : 'O'; - var prefix = 'C' + id_part + new_part + 'L'; - - origin = JX.$(prefix + data.number); - target = JX.$(prefix + (parseInt(data.number, 10) + - parseInt(data.length, 10))); - - updateReticle(); - } - function updateReticle() { JX.DOM.getContentFrame().appendChild(reticle); @@ -85,44 +62,10 @@ dim.setDim(reticle); JX.DOM.show(reticle); - - // Find all the cells in the same row position between the top and bottom - // cell, so we can highlight them. - var seq = 0; - var row = top.parentNode; - for (seq = 0; seq < row.childNodes.length; seq++) { - if (row.childNodes[seq] == top) { - break; - } - } - - var cells = []; - while (true) { - cells.push(row.childNodes[seq]); - if (row.childNodes[seq] == bot) { - break; - } - row = row.nextSibling; - } - - setSelectedCells(cells); - } - - function setSelectedCells(new_cells) { - updateSelectedCellsClass(old_cells, false); - updateSelectedCellsClass(new_cells, true); - old_cells = new_cells; - } - - function updateSelectedCellsClass(cells, selected) { - for (var ii = 0; ii < cells.length; ii++) { - JX.DOM.alterClass(cells[ii], 'selected', selected); - } } function hideReticle() { JX.DOM.hide(reticle); - setSelectedCells([]); } function isOnRight(node) { @@ -180,13 +123,6 @@ return; } - if (editor) { - // Don't update the reticle if we're editing a comment, since this - // would be distracting and we want to keep the lines corresponding - // to the comment highlighted during the edit. - return; - } - if (getRowNumber(e.getTarget()) === undefined) { // Don't update the reticle if this "" doesn't correspond to a // line number. For instance, this may be a dead line number, like the @@ -236,7 +172,7 @@ 'mouseup', null, function(e) { - if (editor || !selecting) { + if (!selecting) { return; } @@ -280,12 +216,7 @@ if (e.getIsTouchEvent()) { return; } - - if (e.getType() == 'mouseout') { - hideReticle(); - } else { - updateReticleForComment(e); - } + hideReticle(); }); });