Page MenuHomePhabricator
Files F15430699

D17927.diff
No OneTemporary
Actions

D17927.diff
View Options

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -10,10 +10,10 @@
'conpherence.pkg.css' => 'ff161f2d',
'conpherence.pkg.js' => 'b5b51108',
'core.pkg.css' => 'ee5f28cd',
- 'core.pkg.js' => '8c5f913d',
+ 'core.pkg.js' => '0f87a6eb',
'darkconsole.pkg.js' => '1f9a31bc',
'differential.pkg.css' => 'ea471cb0',
- 'differential.pkg.js' => 'ae6f5198',
+ 'differential.pkg.js' => '85c19957',
'diffusion.pkg.css' => 'b93d9b8c',
'diffusion.pkg.js' => '84c8f8fd',
'favicon.ico' => '30672e08',
@@ -237,7 +237,7 @@
'rsrc/externals/javelin/ext/view/__tests__/ViewInterpreter.js' => '7a94d6a5',
'rsrc/externals/javelin/ext/view/__tests__/ViewRenderer.js' => '6ea96ac9',
'rsrc/externals/javelin/lib/Cookie.js' => '62dfea03',
- 'rsrc/externals/javelin/lib/DOM.js' => '805b806a',
+ 'rsrc/externals/javelin/lib/DOM.js' => '4976858c',
'rsrc/externals/javelin/lib/History.js' => 'd4505101',
'rsrc/externals/javelin/lib/JSON.js' => '69adf288',
'rsrc/externals/javelin/lib/Leader.js' => '7f243deb',
@@ -391,12 +391,11 @@
'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' => '68758d99',
- 'rsrc/js/application/diff/DiffChangesetList.js' => '57c491b5',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => '842e2676',
'rsrc/js/application/diff/DiffInline.js' => '1afe9760',
'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' => '1c6bc8cf',
'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',
@@ -619,7 +618,6 @@
'javelin-behavior-device' => 'bb1dd507',
'javelin-behavior-diff-preview-link' => '051c7832',
'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
- 'javelin-behavior-differential-edit-inline-comments' => '1c6bc8cf',
'javelin-behavior-differential-feedback-preview' => 'b064af76',
'javelin-behavior-differential-populate' => '5e41c819',
'javelin-behavior-differential-toggle-files' => 'ca3f91eb',
@@ -715,7 +713,7 @@
'javelin-color' => '7e41274a',
'javelin-cookie' => '62dfea03',
'javelin-diffusion-locate-file-source' => 'c93358e3',
- 'javelin-dom' => '805b806a',
+ 'javelin-dom' => '4976858c',
'javelin-dynval' => 'f6555212',
'javelin-event' => '2ee659ce',
'javelin-fx' => '54b612ba',
@@ -780,7 +778,7 @@
'phabricator-darkmessage' => 'c48cccdd',
'phabricator-dashboard-css' => 'fe5b1869',
'phabricator-diff-changeset' => '68758d99',
- 'phabricator-diff-changeset-list' => '57c491b5',
+ 'phabricator-diff-changeset-list' => '842e2676',
'phabricator-diff-inline' => '1afe9760',
'phabricator-drag-and-drop-file-upload' => '58dea2fa',
'phabricator-draggable-list' => 'bea6e7f4',
@@ -1040,13 +1038,6 @@
'javelin-request',
'javelin-uri',
),
- '1c6bc8cf' => array(
- 'javelin-behavior',
- 'javelin-stratcom',
- 'javelin-dom',
- 'javelin-util',
- 'javelin-vector',
- ),
'1def2711' => array(
'javelin-install',
'javelin-dom',
@@ -1250,6 +1241,13 @@
'javelin-uri',
'phabricator-notification',
),
+ '4976858c' => array(
+ 'javelin-magical-init',
+ 'javelin-install',
+ 'javelin-util',
+ 'javelin-vector',
+ 'javelin-stratcom',
+ ),
'49ae8328' => array(
'javelin-behavior',
'javelin-dom',
@@ -1331,9 +1329,6 @@
'javelin-vector',
'javelin-dom',
),
- '57c491b5' => array(
- 'javelin-install',
- ),
'58dea2fa' => array(
'javelin-install',
'javelin-util',
@@ -1529,13 +1524,6 @@
'javelin-vector',
'javelin-stratcom',
),
- '805b806a' => array(
- 'javelin-magical-init',
- 'javelin-install',
- 'javelin-util',
- 'javelin-vector',
- 'javelin-stratcom',
- ),
'834a1173' => array(
'javelin-behavior',
'javelin-scrollbar',
@@ -1544,6 +1532,9 @@
'javelin-install',
'javelin-dom',
),
+ '842e2676' => array(
+ 'javelin-install',
+ ),
'8499b6ab' => array(
'javelin-behavior',
'javelin-dom',
@@ -2421,7 +2412,6 @@
'phabricator-drag-and-drop-file-upload',
'phabricator-shaped-request',
'javelin-behavior-differential-feedback-preview',
- 'javelin-behavior-differential-edit-inline-comments',
'javelin-behavior-differential-populate',
'javelin-behavior-differential-diff-radios',
'javelin-behavior-aphront-drag-and-drop-textarea',
diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php
--- a/resources/celerity/packages.php
+++ b/resources/celerity/packages.php
@@ -193,7 +193,6 @@
'phabricator-shaped-request',
'javelin-behavior-differential-feedback-preview',
- 'javelin-behavior-differential-edit-inline-comments',
'javelin-behavior-differential-populate',
'javelin-behavior-differential-diff-radios',
'javelin-behavior-aphront-drag-and-drop-textarea',
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
@@ -274,13 +274,6 @@
),
));
- if ($this->inlineURI) {
- Javelin::initBehavior('differential-edit-inline-comments', array(
- 'uri' => $this->inlineURI,
- 'stage' => 'differential-review-stage',
- ));
- }
-
if ($this->header) {
$header = $this->header;
} else {
diff --git a/webroot/rsrc/externals/javelin/lib/DOM.js b/webroot/rsrc/externals/javelin/lib/DOM.js
--- a/webroot/rsrc/externals/javelin/lib/DOM.js
+++ b/webroot/rsrc/externals/javelin/lib/DOM.js
@@ -891,7 +891,7 @@
* it.
*
* @param Node Node to look above.
- * @param string Tag name, like 'a' or 'textarea'.
+ * @param string Optional tag name, like 'a' or 'textarea'.
* @param string Optionally, sigil which selected node must have.
* @return Node Matching node.
*
@@ -911,7 +911,7 @@
if (!result) {
break;
}
- if (JX.DOM.isType(result, tagname)) {
+ if (!tagname || JX.DOM.isType(result, tagname)) {
if (!sigil || JX.Stratcom.hasSigil(result, sigil)) {
break;
}
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
@@ -62,6 +62,21 @@
['mouseover', 'mouseout'],
'differential-inline-comment',
onhover);
+
+ var onrangedown = JX.bind(this, this._ifawake, this._onrangedown);
+ JX.Stratcom.listen(
+ 'mousedown',
+ ['differential-changeset', 'tag:th'],
+ onrangedown);
+
+ var onrangemove = JX.bind(this, this._ifawake, this._onrangemove);
+ JX.Stratcom.listen(
+ ['mouseover', 'mouseout'],
+ ['differential-changeset', 'tag:th'],
+ onrangemove);
+
+ var onrangeup = JX.bind(this, this._ifawake, this._onrangeup);
+ JX.Stratcom.listen('mouseup', null, onrangeup);
},
properties: {
@@ -85,6 +100,10 @@
_hoverOrigin: null,
_hoverTarget: null,
+ _rangeActive: false,
+ _rangeOrigin: null,
+ _rangeTarget: null,
+
sleep: function() {
this._asleep = true;
@@ -953,8 +972,18 @@
this._redrawHover();
},
+ _setHoverRange: function(origin, target) {
+ this._hoverOrigin = origin;
+ this._hoverTarget = target;
+
+ this._redrawHover();
+ },
+
resetHover: function() {
this._setHoverInline(null);
+
+ this._hoverOrigin = null;
+ this._hoverTarget = null;
},
_redrawHover: function() {
@@ -1056,6 +1085,126 @@
getDisplaySideFromHeader: function(th) {
return (th.parentNode.firstChild != th) ? 'right' : 'left';
+ },
+
+ _onrangedown: function(e) {
+ if (!e.isNormalMouseEvent()) {
+ return;
+ }
+
+ if (e.getIsTouchEvent()) {
+ return;
+ }
+
+ if (this._rangeActive) {
+ return;
+ }
+
+ var target = e.getTarget();
+ var number = this.getLineNumberFromHeader(target);
+ if (!number) {
+ return;
+ }
+
+ e.kill();
+ this._rangeActive = true;
+
+ this._rangeOrigin = target;
+ this._rangeTarget = target;
+
+ this._setHoverRange(this._rangeOrigin, this._rangeTarget);
+ },
+
+ _onrangemove: function(e) {
+ if (e.getIsTouchEvent()) {
+ return;
+ }
+
+ var target = e.getTarget();
+
+ // Don't update the range if this "<th />" doesn't correspond to a line
+ // number. For instance, this may be a dead line number, like the empty
+ // line numbers on the left hand side of a newly added file.
+ var number = this.getLineNumberFromHeader(target);
+ if (!number) {
+ return;
+ }
+
+ if (this._rangeActive) {
+ var origin = this._hoverOrigin;
+
+ // Don't update the reticle if we're selecting a line range and the
+ // "<th />" under the cursor is on the wrong side of the file. You can
+ // only leave inline comments on the left or right side of a file, not
+ // across lines on both sides.
+ var origin_side = this.getDisplaySideFromHeader(origin);
+ var target_side = this.getDisplaySideFromHeader(target);
+ if (origin_side != target_side) {
+ return;
+ }
+
+ // Don't update the reticle if we're selecting a line range and the
+ // "<th />" under the cursor corresponds to a different file. You can
+ // only leave inline comments on lines in a single file, not across
+ // multiple files.
+ var origin_table = JX.DOM.findAbove(origin, 'table');
+ var target_table = JX.DOM.findAbove(target, 'table');
+ if (origin_table != target_table) {
+ return;
+ }
+ }
+
+ var is_out = (e.getType() == 'mouseout');
+ if (is_out) {
+ if (this._rangeActive) {
+ // If we're dragging a range, just leave the state as it is. This
+ // allows you to drag over something invalid while selecting a
+ // range without the range flickering or getting lost.
+ } else {
+ // Otherwise, clear the current range.
+ this.resetHover();
+ }
+ return;
+ }
+
+ if (this._rangeActive) {
+ this._rangeTarget = target;
+ } else {
+ this._rangeOrigin = target;
+ this._rangeTarget = target;
+ }
+
+ this._setHoverRange(this._rangeOrigin, this._rangeTarget);
+ },
+
+ _onrangeup: function(e) {
+ if (!this._rangeActive) {
+ return;
+ }
+
+ e.kill();
+
+ var origin = this._rangeOrigin;
+ var target = this._rangeTarget;
+
+ // If the user dragged a range from the bottom to the top, swap the node
+ // order around.
+ if (JX.$V(origin).y > JX.$V(target).y) {
+ var tmp = target;
+ target = origin;
+ origin = tmp;
+ }
+
+ var node = JX.DOM.findAbove(origin, null, 'differential-changeset');
+ var changeset = this.getChangesetForNode(node);
+
+ changeset.newInlineForRange(origin, target);
+
+ this._rangeActive = false;
+ this._rangeOrigin = null;
+ this._rangeTarget = null;
+
+ this.resetHover();
}
}
diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js
deleted file mode 100644
--- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js
+++ /dev/null
@@ -1,209 +0,0 @@
-/**
- * @provides javelin-behavior-differential-edit-inline-comments
- * @requires javelin-behavior
- * javelin-stratcom
- * javelin-dom
- * javelin-util
- * javelin-vector
- */
-
-JX.behavior('differential-edit-inline-comments', function(config) {
-
- var selecting = false;
- var reticle = JX.$N('div', {className: 'differential-reticle'});
- JX.DOM.hide(reticle);
-
- var origin = null;
- var target = null;
- var root = null;
- var changeset = null;
-
- function updateReticle() {
- JX.DOM.getContentFrame().appendChild(reticle);
-
- var top = origin;
- var bot = target;
- 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);
- }
-
- function hideReticle() {
- JX.DOM.hide(reticle);
- }
-
- function isOnRight(node) {
- return node.parentNode.firstChild != node;
- }
-
- function getRowNumber(th_node) {
- try {
- return parseInt(th_node.id.match(/^C\d+[ON]L(\d+)$/)[1], 10);
- } catch (x) {
- return undefined;
- }
- }
-
- JX.Stratcom.listen(
- 'mousedown',
- ['differential-changeset', 'tag:th'],
- function(e) {
- if (e.isRightButton() ||
- getRowNumber(e.getTarget()) === undefined) {
- return;
- }
-
- if (selecting) {
- return;
- }
-
- selecting = true;
- root = e.getNode('differential-changeset');
-
- origin = target = e.getTarget();
-
- var data = e.getNodeData('differential-changeset');
- if (isOnRight(target)) {
- changeset = data.right;
- } else {
- changeset = data.left;
- }
-
- updateReticle();
-
- e.kill();
- });
-
- JX.Stratcom.listen(
- ['mouseover', 'mouseout'],
- ['differential-changeset', 'tag:th'],
- function(e) {
- if (e.getIsTouchEvent()) {
- 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
- // empty line numbers on the left hand side of a newly added file.
- return;
- }
-
- if (selecting) {
- if (isOnRight(e.getTarget()) != isOnRight(origin)) {
- // Don't update the reticle if we're selecting a line range and the
- // "<th />" under the cursor is on the wrong side of the file. You
- // can only leave inline comments on the left or right side of a
- // file, not across lines on both sides.
- return;
- }
-
- if (e.getNode('differential-changeset') !== root) {
- // Don't update the reticle if we're selecting a line range and
- // the "<th />" under the cursor corresponds to a different file.
- // You can only leave inline comments on lines in a single file,
- // not across multiple files.
- return;
- }
- }
-
- if (e.getType() == 'mouseout') {
- if (selecting) {
- // Don't hide the reticle if we're selecting, since we want to
- // keep showing the line range that will be used if the mouse is
- // released.
- return;
- }
- hideReticle();
- } else {
- target = e.getTarget();
- if (!selecting) {
- // If we're just hovering the mouse and not selecting a line range,
- // set the origin to the current row so we highlight it.
- origin = target;
- }
-
- updateReticle();
- }
- });
-
- JX.Stratcom.listen(
- 'mouseup',
- null,
- function(e) {
- if (!selecting) {
- return;
- }
-
- var o = getRowNumber(origin);
- var t = getRowNumber(target);
-
- var insert;
- var len;
- if (t < o) {
- len = (o - t);
- o = t;
- insert = origin.parentNode;
- } else {
- len = (t - o);
- insert = target.parentNode;
- }
-
- var view = JX.DiffChangeset.getForNode(root);
-
- view.newInlineForRange(origin, target);
-
- selecting = false;
- origin = null;
- target = null;
-
- e.kill();
- });
-
- JX.Stratcom.listen(
- ['mouseover', 'mouseout'],
- 'differential-inline-comment',
- function(e) {
- if (e.getIsTouchEvent()) {
- return;
- }
- hideReticle();
- });
-
-});

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 25, 8:36 AM (3 d, 17 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7714804
Default Alt Text
D17927.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility