opendev/gerrit
Code Issues Proposed changes

Merge "Fix comment selection and copy behavior"

Browse Source
This commit is contained in:
Wyatt Allen
2016-09-26 20:45:02 +00:00
committed by Gerrit Code Review
parent 75b98f77d9 708d34fb1f
commit d4cd492d0f
2 changed files with 172 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

146
polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
View File

@@ -14,6 +14,16 @@
(function() {
'use strict';
/**
* Possible CSS classes indicating the state of selection. Dynamically added/
* removed based on where the user clicks within the diff.
*/
var SelectionClass = {
COMMENT: 'selected-comment',
LEFT: 'selected-left',
RIGHT: 'selected-right',
};
Polymer({
is: 'gr-diff-selection',
@@ -32,7 +42,7 @@
},
attached: function() {
this.classList.add('selected-right');
this.classList.add(SelectionClass.RIGHT);
},
get diffBuilder() {
@@ -48,41 +58,91 @@
if (!lineEl) {
return;
}
var commentSelected =
e.target.parentNode.classList.contains('gr-diff-comment');
var side = this.diffBuilder.getSideByLineEl(lineEl);
var targetClass = 'selected-' + side;
var alternateClass = 'selected-' + (side === 'left' ? 'right' : 'left');
var targetClasses = [];
targetClasses.push(side === 'left' ?
SelectionClass.LEFT :
SelectionClass.RIGHT);
if (this.classList.contains(alternateClass)) {
this.classList.remove(alternateClass);
if (commentSelected) {
targetClasses.push(SelectionClass.COMMENT);
}
if (!this.classList.contains(targetClass)) {
this.classList.add(targetClass);
// Remove any selection classes that do not belong.
for (var key in SelectionClass) {
if (SelectionClass.hasOwnProperty(key)) {
var className = SelectionClass[key];
if (targetClasses.indexOf(className) === -1) {
this.classList.remove(SelectionClass[key]);
}
}
}
// Add new selection classes iff they are not already present.
for (var i = 0; i < targetClasses.length; i++) {
if (!this.classList.contains(targetClasses[i])) {
this.classList.add(targetClasses[i]);
}
}
},
/**
* Utility function to determine whether an element is a descendant of
* another element with the particular className.
*
* @param {!Element} element
* @param {!string} className
* @return {boolean}
*/
_elementDescendedFromClass: function(element, className) {
while (!element.classList.contains(className)) {
if (!element.parentElement ||
element === this.diffBuilder.diffElement) {
return false;
}
element = element.parentElement;
}
return true;
},
_handleCopy: function(e) {
var el = e.target;
while (!el.classList.contains('content')) {
if (!el.parentElement) {
var commentSelected = false;
if (this._elementDescendedFromClass(e.target, SelectionClass.COMMENT)) {
commentSelected = true;
} else {
if (!this._elementDescendedFromClass(e.target, 'content')) {
return;
}
el = el.parentElement;
}
var lineEl = this.diffBuilder.getLineElByChild(e.target);
if (!lineEl) {
return;
}
var side = this.diffBuilder.getSideByLineEl(lineEl);
var text = this._getSelectedText(side);
e.clipboardData.setData('Text', text);
e.preventDefault();
var text = this._getSelectedText(side, commentSelected);
if (text) {
e.clipboardData.setData('Text', text);
e.preventDefault();
}
},
_getSelectedText: function(side) {
/**
* Get the text of the current window selection. If commentSelected is
* true, it returns only the text of comments within the selection.
* Otherwise it returns the text of the selected diff region.
*
* @param {!string} The side that is selected.
* @param {boolean} Whether or not a comment is selected.
* @return {string} The selected text.
*/
_getSelectedText: function(side, commentSelected) {
var sel = window.getSelection();
if (sel.rangeCount != 1) {
return; // No multi-select support yet.
}
if (commentSelected) {
return this._getCommentLines(sel, side);
}
var range = GrRangeNormalizer.normalize(sel.getRangeAt(0));
var startLineEl = this.diffBuilder.getLineElByChild(range.startContainer);
var endLineEl = this.diffBuilder.getLineElByChild(range.endContainer);
@@ -93,6 +153,16 @@
range.endOffset, side);
},
/**
* Query the diff object for the selected lines.
*
* @param {int} startLineNum
* @param {int} startOffset
* @param {int} endLineNum
* @param {int} endOffset
* @param {!string} side The side that is currently selected.
* @return {string} The selected diff text.
*/
_getRangeFromDiff: function(startLineNum, startOffset, endLineNum,
endOffset, side) {
var lines = this._getDiffLines(side).slice(startLineNum - 1, endLineNum);
@@ -104,11 +174,16 @@
return lines.join('\n');
},
/**
* Query the diff object for the lines from a particular side.
*
* @param {!string} side The side that is currently selected.
* @return {string[]} An array of strings indexed by line number.
*/
_getDiffLines: function(side) {
if (this._linesCache[side]) {
return this._linesCache[side];
}
var lines = [];
var chunk;
var key = side === 'left' ? 'a' : 'b';
@@ -122,9 +197,46 @@
lines = lines.concat(chunk[key]);
}
}
this._linesCache[side] = lines;
return lines;
},
/**
* Query the diffElement for comments and check whether they lie inside the
* selection range.
*
* @param {!Selection} sel The selection of the window.
* @param {!string} side The side that is currently selected.
* @return {string} The selected comment text.
*/
_getCommentLines: function(sel, side) {
var range = sel.getRangeAt(0);
var content = [];
// Fall back to default copy behavior if the selection lies within one
// comment body.
if (this._elementDescendedFromClass(range.commonAncestorContainer,
'message')) {
return;
}
// Query the diffElement for comments.
var messages = this.diffBuilder.diffElement.querySelectorAll(
'.side-by-side [data-side="' + side +
'"] .message *, .unified .message *');
for (var i = 0; i < messages.length; i++) {
var el = messages[i];
// Check if the comment element exists inside the selection.
if (sel.containsNode(el, true)) {
content.push(el.textContent);
}
}
// Deal with offsets.
content[0] = content[0].substring(range.startOffset);
if (range.endOffset) {
content[content.length - 1] =
content[content.length - 1].substring(0, range.endOffset);
}
return content.join('\n');
},
});
})();

46
polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
View File

@@ -27,11 +27,18 @@ limitations under the License.
<test-fixture id="basic">
<template>
<gr-diff-selection>
<table class="side-by-side">
<table id="diffTable" class="side-by-side">
<tr>
<td class="lineNum left" data-value="1">1</td>
<td class="content">
<div class="contentText" data-side="left">ba ba</div>
<div data-side="left">
<div class="gr-diff-comment-thread">
<div class="message">
<span>This is a comment</span>
</div>
</div>
</div>
</td>
<td class="lineNum right" data-value="1">1</td>
<td class="content">
@@ -46,12 +53,26 @@ limitations under the License.
<td class="lineNum right" data-value="2">2</td>
<td class="content">
<div class="contentText" data-side="right">more more more</div>
<div data-side="right">
<div class="gr-diff-comment-thread">
<div class="message">
<span>This is a comment on the right</span>
</div>
</div>
</div>
</td>
</tr>
<tr>
<td class="lineNum left" data-value="3">3</td>
<td class="content">
<div class="contentText" data-side="left">ga ga</div>
<div data-side="left">
<div class="gr-diff-comment-thread">
<div class="message">
<span>This is a different comment</span>
</div>
</div>
</div>
</td>
<td class="lineNum right" data-value="3">3</td>
<td class="other">
@@ -84,6 +105,7 @@ limitations under the License.
element._cachedDiffBuilder = {
getLineElByChild: sinon.stub().returns({}),
getSideByLineEl: sinon.stub(),
diffElement: element.querySelector('#diffTable'),
};
element.diff = {
content: [
@@ -130,11 +152,11 @@ limitations under the License.
assert.isFalse(element._getSelectedText.called);
});
test('asks for text for right side Elements', function() {
test('asks for text for left side Elements', function() {
element._cachedDiffBuilder.getSideByLineEl.returns('left');
sinon.stub(element, '_getSelectedText');
emulateCopyOn(element.querySelector('div.contentText'));
assert.deepEqual(['left'], element._getSelectedText.lastCall.args);
assert.deepEqual(['left', false], element._getSelectedText.lastCall.args);
});
test('reacts to copy for content Elements', function() {
@@ -145,6 +167,8 @@ limitations under the License.
test('copy event is prevented for content Elements', function() {
sinon.stub(element, '_getSelectedText');
element._cachedDiffBuilder.getSideByLineEl.returns('left');
element._getSelectedText.returns('test');
var event = emulateCopyOn(element.querySelector('div.contentText'));
assert.isTrue(event.preventDefault.called);
});
@@ -175,6 +199,22 @@ limitations under the License.
element.querySelectorAll('div.contentText')[4].firstChild, 2);
selection.addRange(range);
assert.equal(element._getSelectedText('left'), 'ba\nzin\nga');
selection.removeAllRanges();
});
test('copies comments', function() {
element.classList.add('selected-left');
element.classList.add('selected-comment');
element.classList.remove('selected-right');
var selection = window.getSelection();
var range = document.createRange();
range.setStart(element.querySelector('.message *').firstChild, 3);
range.setEnd(
element.querySelectorAll('.message *')[2].firstChild, 16);
selection.addRange(range);
assert.equal('s is a comment\nThis is a differ',
element._getSelectedText('left', true));
selection.removeAllRanges();
});
});
</script>