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' => 'a560707d', 'core.pkg.js' => '0efaf0ac', 'dark-console.pkg.js' => '187792c2', - 'differential.pkg.css' => 'd71d4531', - 'differential.pkg.js' => 'ac6914bb', + 'differential.pkg.css' => 'b042ee8b', + 'differential.pkg.js' => '4b2b5659', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -63,7 +63,7 @@ 'rsrc/css/application/diff/diff-tree-view.css' => 'e2d3e222', 'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d', 'rsrc/css/application/differential/add-comment.css' => '7e5900d9', - 'rsrc/css/application/differential/changeset-view.css' => 'a5cc67cf', + 'rsrc/css/application/differential/changeset-view.css' => 'df3afa61', 'rsrc/css/application/differential/core.css' => '7300a73e', 'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b', 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', @@ -379,9 +379,9 @@ 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be', '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' => '9d5b137e', - 'rsrc/js/application/diff/DiffInline.js' => '6227a0e3', + 'rsrc/js/application/diff/DiffChangeset.js' => 'b6bb0240', + 'rsrc/js/application/diff/DiffChangesetList.js' => '2347e0a6', + 'rsrc/js/application/diff/DiffInline.js' => '417b3cdb', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', @@ -559,7 +559,7 @@ 'conpherence-transaction-css' => '3a3f5e7e', 'd3' => '9d068042', 'diff-tree-view-css' => 'e2d3e222', - 'differential-changeset-view-css' => 'a5cc67cf', + 'differential-changeset-view-css' => 'df3afa61', 'differential-core-view-css' => '7300a73e', 'differential-revision-add-comment-css' => '7e5900d9', 'differential-revision-comment-css' => '7dbc8d1d', @@ -774,9 +774,9 @@ 'phabricator-darklog' => '3b869402', 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', - 'phabricator-diff-changeset' => '20715b98', - 'phabricator-diff-changeset-list' => '9d5b137e', - 'phabricator-diff-inline' => '6227a0e3', + 'phabricator-diff-changeset' => 'b6bb0240', + 'phabricator-diff-changeset-list' => '2347e0a6', + 'phabricator-diff-inline' => '417b3cdb', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', 'phabricator-drag-and-drop-file-upload' => '4370900d', @@ -1082,19 +1082,6 @@ 'javelin-behavior', 'javelin-request', ), - '20715b98' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - 'phabricator-diff-path-view', - 'phuix-button-view', - ), '225bbb98' => array( 'javelin-install', 'javelin-reactor', @@ -1111,6 +1098,11 @@ 'javelin-request', 'javelin-typeahead-source', ), + '2347e0a6' => array( + 'javelin-install', + 'phuix-button-view', + 'phabricator-diff-tree-view', + ), 23631304 => array( 'phui-fontkit-css', ), @@ -1259,6 +1251,9 @@ 'javelin-behavior', 'javelin-uri', ), + '417b3cdb' => array( + 'javelin-dom', + ), '42c44e8b' => array( 'javelin-behavior', 'javelin-workflow', @@ -1503,9 +1498,6 @@ '60cd9241' => array( 'javelin-behavior', ), - '6227a0e3' => array( - 'javelin-dom', - ), '6337cf26' => array( 'javelin-behavior', 'javelin-dom', @@ -1816,11 +1808,6 @@ 'javelin-uri', 'phabricator-textareautils', ), - '9d5b137e' => array( - 'javelin-install', - 'phuix-button-view', - 'phabricator-diff-tree-view', - ), '9f081f05' => array( 'javelin-behavior', 'javelin-dom', @@ -1865,9 +1852,6 @@ 'javelin-install', 'javelin-dom', ), - 'a5cc67cf' => array( - 'phui-inline-comment-view-css', - ), 'a77e2cbd' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1999,6 +1983,19 @@ 'javelin-stratcom', 'javelin-dom', ), + 'b6bb0240' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + 'phabricator-diff-path-view', + 'phuix-button-view', + ), 'b7b73831' => array( 'javelin-behavior', 'javelin-dom', @@ -2124,6 +2121,9 @@ 'javelin-uri', 'phabricator-notification', ), + 'df3afa61' => array( + 'phui-inline-comment-view-css', + ), 'e150bd50' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php --- a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php +++ b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php @@ -208,6 +208,8 @@ 'gentle.highlight' => '#fdf3da', 'gentle.highlight.border' => '#c9b8a8', + 'highlight.bright' => '#fdf320', + 'paste.content' => '#fffef5', 'paste.border' => '#e9dbcd', 'paste.highlight' => '#fdf3da', diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -320,7 +320,9 @@ ->setLineLength($length) ->setContent((string)$this->getCommentText()) ->setReplyToCommentPHID($this->getReplyToCommentPHID()) - ->setIsEditing(true); + ->setIsEditing(true) + ->setStartOffset($request->getInt('startOffset')) + ->setEndOffset($request->getInt('endOffset')); $document_engine_key = $request->getStr('documentEngineKey'); if ($document_engine_key !== null) { diff --git a/src/infrastructure/diff/interface/PhabricatorInlineComment.php b/src/infrastructure/diff/interface/PhabricatorInlineComment.php --- a/src/infrastructure/diff/interface/PhabricatorInlineComment.php +++ b/src/infrastructure/diff/interface/PhabricatorInlineComment.php @@ -222,6 +222,24 @@ return $this->getStorageObject()->getAttribute('documentEngineKey'); } + public function setStartOffset($offset) { + $this->getStorageObject()->setAttribute('startOffset', $offset); + return $this; + } + + public function getStartOffset() { + return $this->getStorageObject()->getAttribute('startOffset'); + } + + public function setEndOffset($offset) { + $this->getStorageObject()->setAttribute('endOffset', $offset); + return $this; + } + + public function getEndOffset() { + return $this->getStorageObject()->getAttribute('endOffset'); + } + public function getDateModified() { return $this->getStorageObject()->getDateModified(); } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php @@ -91,6 +91,8 @@ 'isDraftDone' => $is_draft_done, 'isEditing' => $inline->getIsEditing(), 'documentEngineKey' => $inline->getDocumentEngineKey(), + 'startOffset' => $inline->getStartOffset(), + 'endOffset' => $inline->getEndOffset(), 'on_right' => $this->getIsOnRight(), ); 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 @@ -487,3 +487,27 @@ background: {$lightyellow}; color: {$blacktext}; } + +.differential-diff tr td.inline-hover { + background: {$gentle.highlight}; +} + +.differential-diff tr td.inline-hover-bright { + background: {$highlight.bright}; +} + +.inline-hover-container { + position: absolute; + color: {$lightgreytext}; + background: {$lightyellow}; +} + +.inline-hover-text { + padding-top: 2px; + padding-bottom: 2px; +} + +.inline-hover-text-bright { + color: {$blacktext}; + background: {$highlight.bright}; +} 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 @@ -711,7 +711,7 @@ return data.inline; }, - newInlineForRange: function(origin, target) { + newInlineForRange: function(origin, target, options) { var list = this.getChangesetList(); var src = list.getLineNumberFromHeader(origin); @@ -742,6 +742,8 @@ isNewFile: is_new }; + JX.copy(data, options || {}); + var inline = new JX.DiffInline() .setChangeset(this) .bindToRange(data); 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 @@ -439,8 +439,13 @@ this._setSourceSelection(null, null); + var config = { + startOffset: start.offset, + endOffset: end.offset + }; + var changeset = start.changeset; - changeset.newInlineForRange(start.targetNode, end.targetNode); + changeset.newInlineForRange(start.targetNode, end.targetNode, config); }, _onkeydone: function() { @@ -1241,6 +1246,10 @@ }, _setHoverInline: function(inline) { + if (inline && (this._hoverInline === inline)) { + return; + } + this._hoverInline = inline; if (inline) { @@ -1305,14 +1314,31 @@ }, _redrawHover: function() { + var ii; + + var map = this._hoverMap; + if (map) { + for (ii = 0; ii < map.length; ii++) { + JX.DOM.alterClass(map[ii].cellNode, 'inline-hover', false); + + if (map[ii].bright) { + JX.DOM.alterClass(map[ii].cellNode, 'inline-hover-bright', false); + } + + if (map[ii].hoverNode) { + JX.DOM.remove(map[ii].hoverNode); + } + } + this._hoverMap = null; + } + var reticle = this._getHoverNode(); + JX.DOM.remove(reticle); + 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) { @@ -1325,7 +1351,7 @@ // next sibling with a "data-copy-mode" attribute, which is a marker // for the cell with actual content in it. var content_cell = top; - while (content_cell && !content_cell.getAttribute('data-copy-mode')) { + while (content_cell && !this._isContentCell(content_cell)) { content_cell = content_cell.nextSibling; } @@ -1334,22 +1360,184 @@ return; } - var pos = JX.$V(content_cell) - .add(JX.Vector.getAggregateScrollForNode(content_cell)); + var inline = this._hoverInline; + if (!inline) { + var pos = JX.$V(content_cell) + .add(JX.Vector.getAggregateScrollForNode(content_cell)); - var dim = JX.$V(content_cell) - .add(JX.Vector.getAggregateScrollForNode(content_cell)) - .add(-pos.x, -pos.y) - .add(JX.Vector.getDim(content_cell)); + var dim = JX.$V(content_cell) + .add(JX.Vector.getAggregateScrollForNode(content_cell)) + .add(-pos.x, -pos.y) + .add(JX.Vector.getDim(content_cell)); - var bpos = JX.$V(bot) - .add(JX.Vector.getAggregateScrollForNode(bot)); - dim.y = (bpos.y - pos.y) + JX.Vector.getDim(bot).y; + 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); + pos.setPos(reticle); + dim.setDim(reticle); - JX.DOM.show(reticle); + JX.DOM.getContentFrame().appendChild(reticle); + JX.DOM.show(reticle); + + return; + } + + if (!inline.hoverMap) { + inline.hoverMap = this._newHoverMap(top, bot, content_cell, inline); + } + + map = inline.hoverMap; + for (ii = 0; ii < map.length; ii++) { + JX.DOM.alterClass(map[ii].cellNode, 'inline-hover', true); + if (map[ii].bright) { + JX.DOM.alterClass(map[ii].cellNode, 'inline-hover-bright', true); + } + if (map[ii].hoverNode) { + map[ii].cellNode.insertBefore( + map[ii].hoverNode, + map[ii].cellNode.firstChild); + } + } + this._hoverMap = map; + }, + + _newHoverMap: function(top, bot, content_cell, inline) { + var start = inline.getStartOffset() || 0; + var end = inline.getEndOffset() || 0; + + var head_row = JX.DOM.findAbove(top, 'tr'); + var last_row = JX.DOM.findAbove(bot, 'tr'); + + var cursor = head_row; + var rows = []; + var idx = null; + var ii; + do { + for (ii = 0; ii < cursor.childNodes.length; ii++) { + var child = cursor.childNodes[ii]; + if (!JX.DOM.isType(child, 'td')) { + continue; + } + + if (child === content_cell) { + idx = ii; + } + + if (ii === idx) { + if (!this._isContentCell(child)) { + break; + } + rows.push({ + cellNode: child + }); + } + } + + if (cursor === last_row) { + break; + } + + cursor = cursor.nextSibling; + } while (cursor); + + var info; + var content; + for (ii = 0; ii < rows.length; ii++) { + info = this._getSelectionOffset(rows[ii].cellNode, null); + + content = info.content; + content = content.replace(/\n+$/, ''); + + rows[ii].content = content; + } + + var attr_dull = { + className: 'inline-hover-text' + }; + + var attr_bright = { + className: 'inline-hover-text inline-hover-text-bright' + }; + + var attr_container = { + className: 'inline-hover-container' + }; + + var min = 0; + var max = rows.length - 1; + var offset_min; + var offset_max; + var len; + var node; + var text; + var any_highlight = false; + for (ii = 0; ii < rows.length; ii++) { + content = rows[ii].content; + len = content.length; + + if (ii === min) { + offset_min = start; + } else { + offset_min = 0; + } + + if (ii === max) { + offset_max = Math.min(end, len); + } else { + offset_max = len; + } + + var has_min = (offset_min > 0); + var has_max = (offset_max < len); + + if (has_min || has_max) { + any_highlight = true; + } + + rows[ii].min = offset_min; + rows[ii].max = offset_max; + rows[ii].hasMin = has_min; + rows[ii].hasMax = has_max; + } + + for (ii = 0; ii < rows.length; ii++) { + content = rows[ii].content; + offset_min = rows[ii].min; + offset_max = rows[ii].max; + + var has_highlight = (rows[ii].hasMin || rows[ii].hasMax); + + if (any_highlight) { + var parts = []; + + if (offset_min > 0) { + text = content.substring(0, offset_min); + node = JX.$N('span', attr_dull, text); + parts.push(node); + } + + if (len) { + text = content.substring(offset_min, offset_max); + node = JX.$N('span', attr_bright, text); + parts.push(node); + } + + if (offset_max < len) { + text = content.substring(offset_max, len); + node = JX.$N('span', attr_dull, text); + parts.push(node); + } + + rows[ii].hoverNode = JX.$N('div', attr_container, parts); + } else { + rows[ii].hoverNode = null; + } + + rows[ii].bright = (any_highlight && !has_highlight); + } + + return rows; }, _getHoverNode: function() { @@ -2461,19 +2649,13 @@ } } - 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; - } + var info = this._getSelectionOffset(td, fragment); - if (offset === null) { + if (info.found) { + offset = info.offset; + } else { if (is_end) { - offset = seen; + offset = info.offset; } else { offset = 0; } @@ -2500,6 +2682,42 @@ }; }, + _getSelectionOffset: function(node, target) { + if (!node.childNodes || !node.childNodes.length) { + return { + offset: node.textContent.length, + content: node.textContent, + found: false + }; + } + + var found = false; + var offset = 0; + var content = ''; + for (var ii = 0; ii < node.childNodes.length; ii++) { + var child = node.childNodes[ii]; + + if (child === target) { + found = true; + } + + var spec = this._getSelectionOffset(child, target); + + content += spec.content; + if (!found) { + offset += spec.offset; + } + + found = found || spec.found; + } + + return { + offset: offset, + content: content, + found: found + }; + }, + _getSelectedRanges: function() { var ranges = []; @@ -2518,7 +2736,12 @@ } return ranges; + }, + + _isContentCell: function(node) { + return !!node.getAttribute('data-copy-mode'); } + } }); 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 @@ -48,6 +48,9 @@ _skipFocus: false, _menu: null, + _startOffset: null, + _endOffset: null, + bindToRow: function(row) { this._row = row; @@ -93,6 +96,8 @@ this._snippet = data.snippet; this._menuItems = data.menuItems; this._documentEngineKey = data.documentEngineKey; + this._startOffset = data.startOffset; + this._endOffset = data.endOffset; this._isEditing = data.isEditing; @@ -147,6 +152,14 @@ return this._isGhost; }, + getStartOffset: function() { + return this._startOffset; + }, + + getEndOffset: function() { + return this._endOffset; + }, + bindToRange: function(data) { this._displaySide = data.displaySide; this._number = parseInt(data.number, 10); @@ -154,6 +167,8 @@ this._isNewFile = data.isNewFile; this._changesetID = data.changesetID; this._isNew = true; + this._startOffset = data.startOffset; + this._endOffset = data.endOffset; // Insert the comment after any other comments which already appear on // the same row. @@ -506,19 +521,35 @@ }, _newRequestData: function(operation, text) { - return { + var data = { op: operation, - id: this._id, + is_new: this.isNewFile(), on_right: ((this.getDisplaySide() == 'right') ? 1 : 0), renderer: this.getChangeset().getRendererKey(), - number: this.getLineNumber(), - length: this.getLineLength(), - is_new: this.isNewFile(), - changesetID: this.getChangesetID(), - replyToCommentPHID: this.getReplyToCommentPHID(), - text: text || null, - documentEngineKey: this._documentEngineKey, + text: text || null }; + + if (operation === 'new') { + var create_data = { + changesetID: this.getChangesetID(), + documentEngineKey: this._documentEngineKey, + replyToCommentPHID: this.getReplyToCommentPHID(), + startOffset: this._startOffset, + endOffset: this._endOffset, + number: this.getLineNumber(), + length: this.getLineLength() + }; + + JX.copy(data, create_data); + } else { + var edit_data = { + id: this._id + }; + + JX.copy(data, edit_data); + } + + return data; }, _oneditresponse: function(response) {