Range comments select-to-create

Creates action box, that creates range comment on mouse down and hotkey
over selected text in diff. Makes best effort in guessing correct start
and end points for the selection.

Known issues listed as TODO items in test and code.

Feature: Issue 3915
Change-Id: I0a3e41d062e559c8cdb4b847829429f65622eb72
This commit is contained in:
Viktar Donich
2016-06-06 12:20:31 -07:00
parent f6e5149bbf
commit b74a83162f
5 changed files with 449 additions and 11 deletions

View File

@@ -77,6 +77,12 @@ limitations under the License.
return null;
},
getLineNumberByChild: function(node) {
var lineEl = this.getLineElByChild(node);
return lineEl ?
parseInt(lineEl.getAttribute('data-value'), 10) : null;
},
renderLineRange: function(startLine, endLine, opt_side) {
var groups =
this._builder.getGroupsByLineRange(startLine, endLine, opt_side);

View File

@@ -20,7 +20,7 @@ limitations under the License.
<dom-module id="gr-diff-highlight">
<template>
<style>
:host {
.contentWrapper ::content {
position: relative;
}
.contentWrapper ::content .range {

View File

@@ -59,6 +59,11 @@
},
_enabledChanged: function() {
if (this.enabled) {
this.listen(document, 'selectionchange', '_handleSelectionChange');
} else {
this.unlisten(document, 'selectionchange', '_handleSelectionChange');
}
for (var eventName in this._enabledListeners) {
var methodName = this._enabledListeners[eventName];
if (this.enabled) {
@@ -88,6 +93,14 @@
}
},
_handleSelectionChange: function() {
// Can't use up or down events to handle selection started and/or ended in
// in comment threads or outside of diff.
// Debounce removeActionBox to give it a chance to react to click/tap.
this._removeActionBoxDebounced();
this.debounce('selectionChange', this._handleSelection, 200);
},
_handleRender: function() {
this._applyAllHighlights();
},
@@ -129,6 +142,111 @@
}, this);
},
/**
* Convert DOM Range selection to concrete numbers (line, column, side).
* Moves range end if it's not inside td.content.
* Returns null if selection end is not valid (outside of diff).
*
* @param {Node} node td.content child
* @param {number} offset offset within node
* @return {{
* node: Node,
* side: string,
* line: Number,
* column: Number
* }}
*/
_normalizeSelectionSide: function(node, offset) {
var column;
if (!this.contains(node)) {
return;
}
var lineEl = this.diffBuilder.getLineElByChild(node);
if (!lineEl) {
return;
}
var side = this.diffBuilder.getSideByLineEl(lineEl);
if (!side) {
return;
}
var line = this.diffBuilder.getLineNumberByChild(lineEl);
if (!line) {
return;
}
var content = this.diffBuilder.getContentByLineEl(lineEl);
if (!content) {
return;
}
if (!content.contains(node)) {
node = content;
column = 0;
} else {
var thread = content.querySelector('gr-diff-comment-thread');
if (thread && thread.contains(node)) {
column = this._getLength(content);
node = content;
} else {
column = this._convertOffsetToColumn(node, offset);
}
}
return {
node: node,
side: side,
line: line,
column: column,
};
},
_handleSelection: function() {
var selection = window.getSelection();
if (selection.rangeCount != 1) {
return;
}
var range = selection.getRangeAt(0);
if (range.collapsed) {
return;
}
var start =
this._normalizeSelectionSide(range.startContainer, range.startOffset);
if (!start) {
return;
}
var end =
this._normalizeSelectionSide(range.endContainer, range.endOffset);
if (!end) {
return;
}
if (start.side !== end.side ||
end.line < start.line ||
(start.line === end.line && start.column === end.column)) {
return;
}
// TODO (viktard): Drop empty first and last lines from selection.
var actionBox = document.createElement('gr-selection-action-box');
Polymer.dom(this.root).appendChild(actionBox);
actionBox.range = {
startLine: start.line,
startChar: start.column,
endLine: end.line,
endChar: end.column,
};
actionBox.side = start.side;
if (start.line === end.line) {
actionBox.placeAbove(range);
} else if (start.node instanceof Text) {
actionBox.placeAbove(start.node.splitText(start.column));
start.node.parentElement.normalize(); // Undo splitText from above.
} else if (start.node.classList.contains('content') &&
start.node.firstChild) {
actionBox.placeAbove(start.node.firstChild);
} else {
actionBox.placeAbove(start.node);
}
},
_renderCommentRange: function(comment, el) {
var lineEl = this.diffBuilder.getLineElByChild(el);
if (!lineEl) {
@@ -181,6 +299,10 @@
range.endLine, range.endChar, side);
},
_removeActionBoxDebounced: function() {
this.debounce('removeActionBox', this._removeActionBox, 10);
},
_removeActionBox: function() {
var actionBox = this.$$('gr-selection-action-box');
if (actionBox) {
@@ -188,6 +310,22 @@
}
},
_convertOffsetToColumn: function(el, offset) {
if (el instanceof Element && el.classList.contains('content')) {
return offset;
}
while (el.previousSibling ||
!el.parentElement.classList.contains('content')) {
if (el.previousSibling) {
el = el.previousSibling;
offset += this._getLength(el);
} else {
el = el.parentElement;
}
}
return offset;
},
/**
* Traverse diff content from right to left, call callback for each node.
* Stops if callback returns true.

View File

@@ -31,23 +31,22 @@ limitations under the License.
<tbody class="section both">
<tr class="diff-row side-by-side" left-type="both" right-type="both">
<td class="left lineNum" data-value="138"></td>
<td class="left lineNum" data-value="138">138</td>
<td class="content both darkHighlight">[14] Nam cum ad me in Cumanum salutandi causa uterque venisset,</td>
<td class="right lineNum" data-value="119"></td>
<td class="right lineNum" data-value="119">119</td>
<td class="content both darkHighlight">[14] Nam cum ad me in Cumanum salutandi causa uterque venisset,</td>
</tr>
</tbody>
<tbody class="section delta">
<tr class="diff-row side-by-side" left-type="remove" right-type="add">
<td class="left lineNum" data-value="140"></td>
<td class="left lineNum" data-value="140">140</td>
<!-- Next tag is formatted to eliminate zero-length text nodes. -->
<td class="content remove lightHighlight">na💢ti <hl class="foo">te, inquit</hl>, sumus <hl class="bar">aliquando</hl> otiosum, <hl>certe</hl> a udiam, <hl>quid</hl> sit, quod <hl>Epicurum</hl><gr-diff-comment-thread>
[Yet another random diff thread content here]
</gr-diff-comment-thread></td>
<td class="right lineNum" data-value="121"></td>
<td class="content add lightHighlight">
nacti ,
<td class="right lineNum" data-value="120">120</td>
<td class="content add lightHighlight">nacti ,
<hl>,</hl>
sumus otiosum, audiam, sit, quod
</td>
@@ -56,13 +55,52 @@ limitations under the License.
<tbody class="section both">
<tr class="diff-row side-by-side" left-type="both" right-type="both">
<td class="left lineNum" data-value="149"></td>
<td class="left lineNum" data-value="141"></td>
<td class="content both darkHighlight">nam et complectitur verbis, quod vult, et dicit plane, quod intellegam;</td>
<td class="right lineNum" data-value="130"></td>
<td class="content both darkHighlight">nam et complectitur verbis, quod vult, et dicit plane, quod intellegam;</td>
</tr>
</tbody>
<tbody class="section contextControl">
<tr class="diff-row side-by-side" left-type="contextControl" right-type="contextControl">
<td class="left contextLineNum" data-value="@@"></td>
<td>
<gr-button>+10↑</gr-button>
-
<gr-button>Show 21 common lines</gr-button>
-
<gr-button>+10↓</gr-button>
</td>
<td class="right contextLineNum" data-value="@@"></td>
<td>
<gr-button>+10↑</gr-button>
-
<gr-button>Show 21 common lines</gr-button>
-
<gr-button>+10↓</gr-button>
</td>
</tr>
</tbody>
<tbody class="section delta">
<tr class="diff-row side-by-side" left-type="blank" right-type="add">
<td class="left"></td>
<td class="blank darkHighlight"></td>
<td class="right lineNum" data-value="146"></td>
<td class="content add darkHighlight">[17] Quid igitur est? inquit; audire enim cupio, quid non probes. Principio, inquam,</td>
</tr>
</tbody>
<tbody class="section both">
<tr class="diff-row side-by-side" left-type="both" right-type="both">
<td class="left lineNum" data-value="165"></td>
<td class="content both darkHighlight">in physicis, quibus maxime gloriatur, primum totus est alienus. Democritea dicit</td>
<td class="right lineNum" data-value="147"></td>
<td class="content both darkHighlight">in physicis, quibus maxime gloriatur, primum totus est alienus. Democritea dicit</td>
</tr>
</tbody>
</table>
</gr-diff-highlight>
</template>
@@ -116,6 +154,28 @@ limitations under the License.
}
});
test('does not listen to selectionchange when disabled', function() {
sandbox.stub(element, '_handleSelection');
sandbox.stub(element, '_removeActionBox');
element.enabled = false;
document.dispatchEvent(new CustomEvent('selectionchange'));
element.flushDebouncer('selectionChange');
assert.isFalse(element._handleSelection.called);
element.flushDebouncer('removeActionBox');
assert.isFalse(element._removeActionBox.called);
});
test('listens to selectionchange when enabled', function() {
sandbox.stub(element, '_handleSelection');
sandbox.stub(element, '_removeActionBox');
element.enabled = true;
document.dispatchEvent(new CustomEvent('selectionchange'));
element.flushDebouncer('selectionChange');
assert.isTrue(element._handleSelection.called);
element.flushDebouncer('removeActionBox');
assert.isTrue(element._removeActionBox.called);
});
suite('comment events', function() {
var builder;
@@ -255,7 +315,7 @@ limitations under the License.
var startContent =
diff.querySelector('.left.lineNum[data-value="138"] ~ .content');
var endContent =
diff.querySelector('.left.lineNum[data-value="149"] ~ .content');
diff.querySelector('.left.lineNum[data-value="141"] ~ .content');
var betweenContent =
diff.querySelector('.left.lineNum[data-value="140"] ~ .content');
var commentThread =
@@ -271,9 +331,9 @@ limitations under the License.
element.enabled = true;
builder.getContentByLine.withArgs(138, 'left').returns(
startContent);
builder.getContentByLine.withArgs(149, 'left').returns(
builder.getContentByLine.withArgs(141, 'left').returns(
endContent);
element._applyRangedHighlight('some', 138, 4, 149, 8, 'left');
element._applyRangedHighlight('some', 138, 4, 141, 8, 'left');
assert.instanceOf(startContent.childNodes[0], Text);
assert.equal(startContent.childNodes[0].textContent, '[14]');
assert.instanceOf(startContent.childNodes[1], Element);
@@ -499,5 +559,237 @@ limitations under the License.
element.fire('show-context');
assert.isFalse(element._applyAllHighlights.called);
});
suite('selection', function() {
var diff;
var builder;
var contentStubs;
var stubContent = function(line, side, opt_child) {
var content = diff.querySelector(
'.' + side + '.lineNum[data-value="' + line + '"] ~ .content');
var lineEl = diff.querySelector(
'.' + side + '.lineNum[data-value="' + line + '"]');
contentStubs.push({
lineEl: lineEl,
content: content,
});
builder.getContentByLineEl.withArgs(lineEl).returns(content);
builder.getLineNumberByChild.withArgs(lineEl).returns(line);
builder.getContentByLine.withArgs(line, side).returns(content);
builder.getSideByLineEl.withArgs(lineEl).returns(side);
return content;
};
var emulateSelection = function(
startNode, startOffset, endNode, endOffset) {
var selection = window.getSelection();
var range = document.createRange();
range.setStart(startNode, startOffset);
range.setEnd(endNode, endOffset);
selection.addRange(range);
element._handleSelection();
};
var getActionRange = function() {
return Polymer.dom(element.root).querySelector(
'gr-selection-action-box').range;
};
var getActionSide = function() {
return Polymer.dom(element.root).querySelector(
'gr-selection-action-box').side;
};
var getLineElByChild = function(node) {
var stubs = contentStubs.find(function(stub) {
return stub.content.contains(node);
});
return stubs && stubs.lineEl;
};
setup(function() {
contentStubs = [];
stub('gr-selection-action-box', {
placeAbove: sandbox.stub(),
});
diff = element.querySelector('#diffTable');
builder = {
getContentByLine: sandbox.stub(),
getContentByLineEl: sandbox.stub(),
getLineElByChild: getLineElByChild,
getLineNumberByChild: sandbox.stub(),
getSideByLineEl: sandbox.stub(),
};
element._cachedDiffBuilder = builder;
element.enabled = true;
});
teardown(function() {
contentStubs = null;
window.getSelection().removeAllRanges();
});
test('single line', function() {
var content = stubContent(138, 'left');
emulateSelection(content.firstChild, 5, content.firstChild, 12);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 138,
startChar: 5,
endLine: 138,
endChar: 12,
});
assert.equal(getActionSide(), 'left');
});
test('multiline', function() {
var startContent = stubContent(119, 'right');
var endContent = stubContent(120, 'right');
emulateSelection(startContent.firstChild, 10, endContent.firstChild, 2);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 119,
startChar: 10,
endLine: 120,
endChar: 2,
});
assert.equal(getActionSide(), 'right');
});
test('collapsed', function() {
var content = stubContent(138, 'left');
emulateSelection(content.firstChild, 5, content.firstChild, 5);
assert.isOk(window.getSelection().getRangeAt(0).startContainer);
assert.isFalse(element.isRangeSelected());
});
test('starts inside hl', function() {
var content = stubContent(140, 'left');
var hl = content.querySelector('.foo');
emulateSelection(hl.firstChild, 2, hl.nextSibling, 7);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 140,
startChar: 8,
endLine: 140,
endChar: 23,
});
assert.equal(getActionSide(), 'left');
});
test('ends inside hl', function() {
var content = stubContent(140, 'left');
var hl = content.querySelector('.bar');
emulateSelection(hl.previousSibling, 2, hl.firstChild, 3);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 140,
startChar: 18,
endLine: 140,
endChar: 27,
});
});
test('multiple hl', function() {
var content = stubContent(140, 'left');
var hl = content.querySelectorAll('hl')[3];
emulateSelection(content.firstChild, 2, hl.firstChild, 2);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 140,
startChar: 2,
endLine: 140,
endChar: 60,
});
assert.equal(getActionSide(), 'left');
});
test('starts outside of diff', function() {
var content = stubContent(140, 'left');
emulateSelection(content.previousElementSibling.firstChild, 2,
content.firstChild, 2);
assert.isFalse(element.isRangeSelected());
});
test('ends outside of diff', function() {
var content = stubContent(140, 'left');
emulateSelection(content.nextElementSibling.firstChild, 2,
content.firstChild, 2);
assert.isFalse(element.isRangeSelected());
});
test('starts and ends on different sides', function() {
var startContent = stubContent(140, 'left');
var endContent = stubContent(130, 'right');
emulateSelection(startContent.firstChild, 2, endContent.firstChild, 2);
assert.isFalse(element.isRangeSelected());
});
test('starts in comment thread element', function() {
var startContent = stubContent(140, 'left');
var comment = startContent.querySelector('gr-diff-comment-thread');
var endContent = stubContent(141, 'left');
emulateSelection(comment.firstChild, 2, endContent.firstChild, 4);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 140,
startChar: 81,
endLine: 141,
endChar: 4,
});
assert.equal(getActionSide(), 'left');
});
test('ends in comment thread element', function() {
var content = stubContent(140, 'left');
var comment = content.querySelector('gr-diff-comment-thread');
emulateSelection(content.firstChild, 4, comment.firstChild, 1);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 140,
startChar: 4,
endLine: 140,
endChar: 81,
});
assert.equal(getActionSide(), 'left');
});
test('starts in context element', function() {
var contextControl = diff.querySelector('.contextControl');
var content = stubContent(146, 'right');
emulateSelection(contextControl, 0, content.firstChild, 7);
// TODO (viktard): Select nearest line.
assert.isFalse(element.isRangeSelected());
});
test('ends in context element', function() {
var contextControl = diff.querySelector('.contextControl');
var content = stubContent(141, 'left');
emulateSelection(content.firstChild, 7, contextControl, 0);
// TODO (viktard): Select nearest line.
assert.isFalse(element.isRangeSelected());
});
test('selection containing context element', function() {
var startContent = stubContent(130, 'right');
var endContent = stubContent(146, 'right');
emulateSelection(startContent.firstChild, 3, endContent.firstChild, 14);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 130,
startChar: 3,
endLine: 146,
endChar: 14,
});
assert.equal(getActionSide(), 'right');
});
// TODO (viktard): Selection starts in line number.
// TODO (viktard): Empty lines in selection start.
// TODO (viktard): Empty lines in selection end.
// TODO (viktard): Only empty lines selected.
// TODO (viktard): Unified mode.
});
});
</script>

View File

@@ -29,6 +29,7 @@ limitations under the License.
cursor: pointer;
padding: .3em;
position: absolute;
white-space: nowrap;
}
.arrow {
background: #fff;
@@ -36,6 +37,7 @@ limitations under the License.
border-width: 0 1px 1px 0;
height: var(--gr-arrow-size);
left: calc(50% - 1em);
margin-top: .05em;
position: absolute;
transform: rotate(45deg);
width: var(--gr-arrow-size);