Page MenuHomePhabricator

D17919.id43105.diff
No OneTemporary

D17919.id43105.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' => '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
+ // <td /> 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 <td /> 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 "<th />" 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();
});
});

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 22, 11:23 PM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7710558
Default Alt Text
D17919.id43105.diff (18 KB)

Event Timeline