Page MenuHomePhabricator
Files F15455901

D21248.diff
No OneTemporary
Actions

D21248.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' => '0e3cf785',
'conpherence.pkg.js' => '020aebcf',
'core.pkg.css' => 'a560707d',
- 'core.pkg.js' => '1e667bcb',
+ 'core.pkg.js' => '0efaf0ac',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => 'd71d4531',
- 'differential.pkg.js' => 'b3e29cb8',
+ 'differential.pkg.js' => '06a7949c',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => 'a98c0bf7',
'maniphest.pkg.css' => '35995d6d',
@@ -380,7 +380,7 @@
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
'rsrc/js/application/diff/DiffChangeset.js' => '20715b98',
- 'rsrc/js/application/diff/DiffChangesetList.js' => '40d6c41c',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => '4a3639a1',
'rsrc/js/application/diff/DiffInline.js' => '6227a0e3',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
@@ -488,7 +488,7 @@
'rsrc/js/core/behavior-linked-container.js' => '74446546',
'rsrc/js/core/behavior-more.js' => '506aa3f4',
'rsrc/js/core/behavior-object-selector.js' => '98ef467f',
- 'rsrc/js/core/behavior-oncopy.js' => 'ff7b3f22',
+ 'rsrc/js/core/behavior-oncopy.js' => 'b475aae5',
'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '54262396',
'rsrc/js/core/behavior-read-only-warning.js' => 'b9109f8f',
'rsrc/js/core/behavior-redirect.js' => '407ee861',
@@ -523,7 +523,7 @@
'rsrc/js/phuix/PHUIXActionView.js' => 'a8f573a9',
'rsrc/js/phuix/PHUIXAutocomplete.js' => '2fbe234d',
'rsrc/js/phuix/PHUIXButtonView.js' => '55a24e84',
- 'rsrc/js/phuix/PHUIXDropdownMenu.js' => '7acfd98b',
+ 'rsrc/js/phuix/PHUIXDropdownMenu.js' => 'b557770a',
'rsrc/js/phuix/PHUIXExample.js' => 'c2c500a7',
'rsrc/js/phuix/PHUIXFormControl.js' => '38c1f3fb',
'rsrc/js/phuix/PHUIXFormationColumnView.js' => '4bcc1f78',
@@ -648,7 +648,7 @@
'javelin-behavior-phabricator-line-linker' => '590e6527',
'javelin-behavior-phabricator-notification-example' => '29819b75',
'javelin-behavior-phabricator-object-selector' => '98ef467f',
- 'javelin-behavior-phabricator-oncopy' => 'ff7b3f22',
+ 'javelin-behavior-phabricator-oncopy' => 'b475aae5',
'javelin-behavior-phabricator-remarkup-assist' => '54262396',
'javelin-behavior-phabricator-reveal-content' => 'b105a3a6',
'javelin-behavior-phabricator-search-typeahead' => '1cb7d027',
@@ -775,7 +775,7 @@
'phabricator-darkmessage' => '26cd4b73',
'phabricator-dashboard-css' => '5a205b9d',
'phabricator-diff-changeset' => '20715b98',
- 'phabricator-diff-changeset-list' => '40d6c41c',
+ 'phabricator-diff-changeset-list' => '4a3639a1',
'phabricator-diff-inline' => '6227a0e3',
'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b',
@@ -886,7 +886,7 @@
'phuix-action-view' => 'a8f573a9',
'phuix-autocomplete' => '2fbe234d',
'phuix-button-view' => '55a24e84',
- 'phuix-dropdown-menu' => '7acfd98b',
+ 'phuix-dropdown-menu' => 'b557770a',
'phuix-form-control-view' => '38c1f3fb',
'phuix-formation-column-view' => '4bcc1f78',
'phuix-formation-flank-view' => '6648270a',
@@ -1259,11 +1259,6 @@
'javelin-behavior',
'javelin-uri',
),
- '40d6c41c' => array(
- 'javelin-install',
- 'phuix-button-view',
- 'phabricator-diff-tree-view',
- ),
'42c44e8b' => array(
'javelin-behavior',
'javelin-workflow',
@@ -1332,6 +1327,11 @@
'490e2e2e' => array(
'phui-oi-list-view-css',
),
+ '4a3639a1' => array(
+ 'javelin-install',
+ 'phuix-button-view',
+ 'phabricator-diff-tree-view',
+ ),
'4a7fb02b' => array(
'javelin-behavior',
'javelin-dom',
@@ -1614,13 +1614,6 @@
'javelin-install',
'javelin-dom',
),
- '7acfd98b' => array(
- 'javelin-install',
- 'javelin-util',
- 'javelin-dom',
- 'javelin-vector',
- 'javelin-stratcom',
- ),
'7ad020a5' => array(
'javelin-behavior',
'javelin-dom',
@@ -1975,6 +1968,10 @@
'javelin-workboard-card-template',
'javelin-workboard-order-template',
),
+ 'b475aae5' => array(
+ 'javelin-behavior',
+ 'javelin-dom',
+ ),
'b49fd60c' => array(
'multirow-row-manager',
'trigger-rule',
@@ -1982,6 +1979,13 @@
'b517bfa0' => array(
'phui-oi-list-view-css',
),
+ 'b557770a' => array(
+ 'javelin-install',
+ 'javelin-util',
+ 'javelin-dom',
+ 'javelin-vector',
+ 'javelin-stratcom',
+ ),
'b58d1a2a' => array(
'javelin-behavior',
'javelin-behavior-device',
@@ -2203,10 +2207,6 @@
'owners-path-editor',
'javelin-behavior',
),
- 'ff7b3f22' => array(
- 'javelin-behavior',
- 'javelin-dom',
- ),
),
'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
@@ -365,6 +365,14 @@
pht('Jump to the comment area.'),
'Show Changeset' => pht('Show Changeset'),
+
+ 'You must select source text to create a new inline comment.' =>
+ pht('You must select source text to create a new inline comment.'),
+
+ 'New Inline Comment' => pht('New Inline Comment'),
+
+ 'Add new inline comment on selected source text.' =>
+ pht('Add new inline comment on selected source text.'),
),
));
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
@@ -59,6 +59,10 @@
null,
onrangeup);
+ var onrange = JX.bind(this, this._ifawake, this._onSelectRange);
+ JX.enableDispatch(window, 'selectionchange');
+ JX.Stratcom.listen('selectionchange', null, onrange);
+
this._setupInlineCommentListeners();
},
@@ -194,6 +198,10 @@
this._installKey('R', 'inline', label,
JX.bind(this, this._onkeyreply, true));
+ label = pht('Add new inline comment on selected source text.');
+ this._installKey('c', 'inline', label,
+ JX.bind(this, this._onKeyCreate));
+
label = pht('Edit selected inline comment.');
this._installKey('e', 'inline', label, this._onkeyedit);
@@ -417,6 +425,24 @@
this._warnUser(pht('You must select a comment to edit.'));
},
+ _onKeyCreate: function() {
+ var start = this._sourceSelectionStart;
+ var end = this._sourceSelectionEnd;
+
+ if (!this._sourceSelectionStart) {
+ var pht = this.getTranslations();
+ this._warnUser(
+ pht(
+ 'You must select source text to create a new inline comment.'));
+ return;
+ }
+
+ this._setSourceSelection(null, null);
+
+ var changeset = start.changeset;
+ changeset.newInlineForRange(start.targetNode, end.targetNode);
+ },
+
_onkeydone: function() {
var cursor = this._cursorItem;
@@ -2197,8 +2223,278 @@
inline.activateMenu(node, e);
break;
}
- }
+ },
+
+ _onSelectRange: function(e) {
+ this._updateSourceSelection();
+ },
+
+ _updateSourceSelection: function() {
+ var ranges = this._getSelectedRanges();
+
+ // If we have zero or more than one range, don't do anything.
+ if (ranges.length === 1) {
+ for (var ii = 0; ii < ranges.length; ii++) {
+ var range = ranges[ii];
+
+ var head = range.startContainer;
+ var last = range.endContainer;
+
+ var head_loc = this._getFragmentLocation(head);
+ var last_loc = this._getFragmentLocation(last);
+
+ if (head_loc === null || last_loc === null) {
+ break;
+ }
+
+ if (head_loc.changesetID !== last_loc.changesetID) {
+ break;
+ }
+
+ head_loc.offset += range.startOffset;
+ last_loc.offset += range.endOffset;
+
+ this._setSourceSelection(head_loc, last_loc);
+ return;
+ }
+ }
+
+ this._setSourceSelection(null, null);
+ },
+
+ _setSourceSelection: function(start, end) {
+ var start_updated =
+ !this._isSameSourceSelection(this._sourceSelectionStart, start);
+
+ var end_updated =
+ !this._isSameSourceSelection(this._sourceSelectionEnd, end);
+
+ if (!start_updated && !end_updated) {
+ return;
+ }
+
+ this._sourceSelectionStart = start;
+ this._sourceSelectionEnd = end;
+
+ if (!start) {
+ this._closeSourceSelectionMenu();
+ return;
+ }
+
+ var menu;
+ if (this._sourceSelectionMenu) {
+ menu = this._sourceSelectionMenu;
+ } else {
+ menu = this._newSourceSelectionMenu();
+ this._sourceSelectionMenu = menu;
+ }
+
+ var pos = JX.$V(start.node)
+ .add(0, -menu.getMenuNodeDimensions().y)
+ .add(0, -24);
+
+ menu.setPosition(pos);
+ menu.open();
+ },
+
+ _newSourceSelectionMenu: function() {
+ var pht = this.getTranslations();
+
+ var menu = new JX.PHUIXDropdownMenu(null)
+ .setWidth(240);
+
+ // We need to disable autofocus for this menu, since it operates on the
+ // text selection in the document. If we leave this enabled, opening the
+ // menu immediately discards the selection.
+ menu.setDisableAutofocus(true);
+
+ var list = new JX.PHUIXActionListView();
+ menu.setContent(list.getNode());
+
+ var oncreate = JX.bind(this, this._onSourceSelectionMenuAction, 'create');
+
+ var comment_item = new JX.PHUIXActionView()
+ .setIcon('fa-comment-o')
+ .setName(pht('New Inline Comment'))
+ .setKeyCommand('c')
+ .setHandler(oncreate);
+
+ list.addItem(comment_item);
+
+ return menu;
+ },
+
+ _onSourceSelectionMenuAction: function(action, e) {
+ e.kill();
+ this._closeSourceSelectionMenu();
+
+ switch (action) {
+ case 'create':
+ this._onKeyCreate();
+ break;
+ }
+ },
+
+ _closeSourceSelectionMenu: function() {
+ if (this._sourceSelectionMenu) {
+ this._sourceSelectionMenu.close();
+ }
+ },
+
+ _isSameSourceSelection: function(u, v) {
+ if (u === null && v === null) {
+ return true;
+ }
+
+ if (u === null && v !== null) {
+ return false;
+ }
+
+ if (u !== null && v === null) {
+ return false;
+ }
+
+ return (
+ (u.changesetID === v.changesetID) &&
+ (u.line === v.line) &&
+ (u.displayColumn === v.displayColumn) &&
+ (u.offset === v.offset)
+ );
+ },
+
+ _getFragmentLocation: function(fragment) {
+ // Find the changeset containing the fragment.
+ var changeset = null;
+ try {
+ var node = JX.DOM.findAbove(
+ fragment,
+ 'div',
+ 'differential-changeset');
+
+ changeset = this.getChangesetForNode(node);
+ if (!changeset) {
+ return null;
+ }
+ } catch (ex) {
+ return null;
+ }
+
+ // Find the line number and display column for the fragment.
+ var line = null;
+ var column_count = -1;
+ var offset = null;
+ var target_node = null;
+ var td;
+ try {
+
+ // NOTE: In Safari, you can carefully select an entire line and then
+ // move your mouse down slightly, causing selection of an empty
+ // document fragment which is an immediate child of the next "<tr />".
+
+ // If the fragment is a direct child of a "<tr />" parent, assume the
+ // user has done this and select the last child of the previous row
+ // instead. It's possible there are other ways to do this, so this may
+ // not always be the right rule.
+
+ // Otherwise, select the containing "<td />".
+
+ var is_end;
+ if (JX.DOM.isType(fragment.parentNode, 'tr')) {
+ // Assume this is Safari, and that the user has carefully selected a
+ // row and then moved their mouse down a few pixels to select the
+ // invisible fragment at the beginning of the next row.
+ var cells = fragment.parentNode.previousSibling.childNodes;
+ td = cells[cells.length - 1];
+ is_end = true;
+ } else {
+ td = JX.DOM.findAbove(fragment, 'td');
+ is_end = false;
+ }
+
+ var cursor = td;
+ while (cursor) {
+ if (cursor.getAttribute('data-copy-mode')) {
+ column_count++;
+ }
+
+ var n = parseInt(cursor.getAttribute('data-n'));
+
+ if (n) {
+ if (line === null) {
+ target_node = cursor;
+ line = n;
+ }
+ }
+
+ cursor = cursor.previousSibling;
+ }
+
+ if (!line) {
+ return null;
+ }
+
+ if (column_count < 0) {
+ return null;
+ }
+
+ var seen = 0;
+ for (var ii = 0; ii < td.childNodes.length; ii++) {
+ var child = td.childNodes[ii];
+ if (child === fragment) {
+ offset = seen;
+ break;
+ }
+ seen += child.textContent.length;
+ }
+
+ if (offset === null) {
+ if (is_end) {
+ offset = seen;
+ } else {
+ offset = 0;
+ }
+ }
+ } catch (ex) {
+ return null;
+ }
+ var changeset_id;
+ if (column_count > 0) {
+ changeset_id = changeset.getRightChangesetID();
+ } else {
+ changeset_id = changeset.getLeftChangesetID();
+ }
+
+ return {
+ node: td,
+ changeset: changeset,
+ changesetID: changeset_id,
+ line: line,
+ displayColumn: column_count,
+ offset: offset,
+ targetNode: target_node
+ };
+ },
+
+ _getSelectedRanges: function() {
+ var ranges = [];
+
+ if (!window.getSelection) {
+ return ranges;
+ }
+
+ var selection = window.getSelection();
+ for (var ii = 0; ii < selection.rangeCount; ii++) {
+ var range = selection.getRangeAt(ii);
+ if (range.collapsed) {
+ continue;
+ }
+
+ ranges.push(range);
+ }
+
+ return ranges;
+ }
}
});
diff --git a/webroot/rsrc/js/core/behavior-oncopy.js b/webroot/rsrc/js/core/behavior-oncopy.js
--- a/webroot/rsrc/js/core/behavior-oncopy.js
+++ b/webroot/rsrc/js/core/behavior-oncopy.js
@@ -119,7 +119,7 @@
// When the selection range changes, apply CSS classes if the selection is
// nonempty. We don't want to make visual changes to the document immediately
- // when the user press the mouse button, since we aren't yet sure that
+ // when the user presses the mouse button, since we aren't yet sure that
// they are starting a selection: instead, wait for them to actually select
// something.
function onchangeselect() {
diff --git a/webroot/rsrc/js/phuix/PHUIXDropdownMenu.js b/webroot/rsrc/js/phuix/PHUIXDropdownMenu.js
--- a/webroot/rsrc/js/phuix/PHUIXDropdownMenu.js
+++ b/webroot/rsrc/js/phuix/PHUIXDropdownMenu.js
@@ -21,11 +21,13 @@
construct : function(node) {
this._node = node;
- JX.DOM.listen(
- this._node,
- 'click',
- null,
- JX.bind(this, this._onclick));
+ if (node) {
+ JX.DOM.listen(
+ this._node,
+ 'click',
+ null,
+ JX.bind(this, this._onclick));
+ }
JX.Stratcom.listen(
'mousedown',
@@ -54,7 +56,8 @@
width: null,
align: 'right',
offsetX: 0,
- offsetY: 0
+ offsetY: 0,
+ disableAutofocus: false
},
members: {
@@ -62,6 +65,8 @@
_menu: null,
_open: false,
_content: null,
+ _position: null,
+ _visible: false,
setContent: function(content) {
JX.DOM.setContent(this._getMenuNode(), content);
@@ -94,6 +99,12 @@
return this;
},
+ setPosition: function(pos) {
+ this._position = pos;
+ this._setMenuNodePosition(pos);
+ return this;
+ },
+
_getMenuNode: function() {
if (!this._menu) {
var attrs = {
@@ -161,7 +172,10 @@
},
_show : function() {
- document.body.appendChild(this._menu);
+ if (!this._visible) {
+ this._visible = true;
+ document.body.appendChild(this._menu);
+ }
if (this.getWidth()) {
new JX.Vector(this.getWidth(), null).setDim(this._menu);
@@ -169,23 +183,28 @@
this._adjustposition();
- JX.DOM.alterClass(this._node, 'phuix-dropdown-open', true);
-
- this._node.setAttribute('aria-expanded', 'true');
+ if (this._node) {
+ JX.DOM.alterClass(this._node, 'phuix-dropdown-open', true);
+ this._node.setAttribute('aria-expanded', 'true');
+ }
// Try to highlight the first link in the menu for assistive technologies.
- var links = JX.DOM.scry(this._menu, 'a');
- if (links[0]) {
- JX.DOM.focus(links[0]);
+ if (!this.getDisableAutofocus()) {
+ var links = JX.DOM.scry(this._menu, 'a');
+ if (links[0]) {
+ JX.DOM.focus(links[0]);
+ }
}
},
_hide : function() {
+ this._visible = false;
JX.DOM.remove(this._menu);
- JX.DOM.alterClass(this._node, 'phuix-dropdown-open', false);
-
- this._node.setAttribute('aria-expanded', 'false');
+ if (this._node) {
+ JX.DOM.alterClass(this._node, 'phuix-dropdown-open', false);
+ this._node.setAttribute('aria-expanded', 'false');
+ }
},
_adjustposition : function() {
@@ -193,6 +212,15 @@
return;
}
+ if (this._position) {
+ this._setMenuNodePosition(this._position);
+ return;
+ }
+
+ if (!this._node) {
+ return;
+ }
+
var m = JX.Vector.getDim(this._menu);
var v = JX.$V(this._node);
@@ -236,11 +264,28 @@
break;
}
- v = v.add(this.getOffsetX(), this.getOffsetY());
+ this._setMenuNodePosition(v);
+ },
+ _setMenuNodePosition: function(v) {
+ v = v.add(this.getOffsetX(), this.getOffsetY());
v.setPos(this._menu);
},
+ getMenuNodeDimensions: function() {
+ if (!this._visible) {
+ document.body.appendChild(this._menu);
+ }
+
+ var dim = JX.Vector.getDim(this._menu);
+
+ if (!this._visible) {
+ JX.DOM.remove(this._menu);
+ }
+
+ return dim;
+ },
+
_onkey: function(e) {
// When the user presses escape with a menu open, close the menu and
// refocus the button which activates the menu. In particular, this makes
@@ -255,7 +300,10 @@
}
this.close();
- JX.DOM.focus(this._node);
+
+ if (this._node) {
+ JX.DOM.focus(this._node);
+ }
e.prevent();
}

File Metadata

Mime Type
text/plain
Expires
Mon, Mar 31, 5:37 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7342421
Default Alt Text
D21248.diff (19 KB)

Event Timeline

Phabricator · Built by Phacility