Merge "Add params to change-view-integration plugin endpoint"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
index 2490509..03380e4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
@@ -251,6 +251,22 @@
};
},
+ /**
+ * The only line in which add a comment tooltip is cut off is the first
+ * line. Even if there is a collapsed section, The first visible line is
+ * in the position where the second line would have been, if not for the
+ * collapsed section, so don't need to worry about this case for
+ * positioning the tooltip.
+ */
+ _positionActionBox(actionBox, startLine, range) {
+ if (startLine > 1) {
+ actionBox.placeAbove(range);
+ return;
+ }
+ actionBox.positionBelow = true;
+ actionBox.placeBelow(range);
+ },
+
_handleSelection() {
const normalizedRange = this._getNormalizedRange();
if (!normalizedRange) {
@@ -285,17 +301,18 @@
};
actionBox.side = start.side;
if (start.line === end.line) {
- actionBox.placeAbove(domRange);
+ this._positionActionBox(actionBox, start.line, domRange);
} else if (start.node instanceof Text) {
if (start.column) {
- actionBox.placeAbove(start.node.splitText(start.column));
+ this._positionActionBox(actionBox, start.line,
+ 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);
+ start.node.firstChild) {
+ this._positionActionBox(actionBox, start.line, start.node.firstChild);
} else {
- actionBox.placeAbove(start.node);
+ this._positionActionBox(actionBox, start.line, start.node);
}
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
index b63b9a4..4bbf12b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
@@ -38,6 +38,27 @@
<table id="diffTable">
<tbody class="section both">
+ <tr class="diff-row side-by-side" left-type="both" right-type="both">
+ <td class="left lineNum" data-value="1"></td>
+ <td class="content both"><div class="contentText">[1] Nam cum ad me in Cumanum salutandi causa uterque venisset,</div></td>
+ <td class="right lineNum" data-value="1"></td>
+ <td class="content both"><div class="contentText">[1] Nam cum ad me in Cumanum salutandi causa uterque</div></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="2"></td>
+ <!-- Next tag is formatted to eliminate zero-length text nodes. -->
+ <td class="content remove"><div class="contentText">na💢ti <hl class="foo">te, inquit</hl>, sumus <hl class="bar">aliquando</hl> otiosum, <hl>certe</hl> a <hl><span class="tab-indicator" style="tab-size:8;"> </span></hl>udiam, <hl>quid</hl> sit, <span class="tab-indicator" style="tab-size:8;"> </span>quod <hl>Epicurum</hl></div></td>
+ <td class="right lineNum" data-value="2"></td>
+ <!-- Next tag is formatted to eliminate zero-length text nodes. -->
+ <td class="content add"><div class="contentText">nacti , <hl>,</hl> sumus <hl><span class="tab-indicator" style="tab-size:8;"> </span></hl> otiosum, <span class="tab-indicator" style="tab-size:8;"> </span> audiam, sit, quod</div></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="138"></td>
<td class="content both"><div class="contentText">[14] Nam cum ad me in Cumanum salutandi causa uterque venisset,</div></td>
@@ -253,6 +274,7 @@
contentStubs = [];
stub('gr-selection-action-box', {
placeAbove: sandbox.stub(),
+ placeBelow: sandbox.stub(),
});
diff = element.querySelector('#diffTable');
builder = {
@@ -270,9 +292,29 @@
window.getSelection().removeAllRanges();
});
+ test('single first line', () => {
+ const content = stubContent(1, 'right');
+ sandbox.spy(element, '_positionActionBox');
+ emulateSelection(content.firstChild, 5, content.firstChild, 12);
+ const actionBox = element.$$('gr-selection-action-box');
+ assert.isTrue(actionBox.positionBelow);
+ });
+
+ test('multiline starting on first line', () => {
+ const startContent = stubContent(1, 'right');
+ const endContent = stubContent(2, 'right');
+ sandbox.spy(element, '_positionActionBox');
+ emulateSelection(
+ startContent.firstChild, 10, endContent.lastChild, 7);
+ const actionBox = element.$$('gr-selection-action-box');
+ assert.isTrue(actionBox.positionBelow);
+ });
+
test('single line', () => {
const content = stubContent(138, 'left');
+ sandbox.spy(element, '_positionActionBox');
emulateSelection(content.firstChild, 5, content.firstChild, 12);
+ const actionBox = element.$$('gr-selection-action-box');
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 138,
@@ -281,14 +323,18 @@
endChar: 12,
});
assert.equal(getActionSide(), 'left');
+ assert.notOk(actionBox.positionBelow);
});
test('multiline', () => {
const startContent = stubContent(119, 'right');
const endContent = stubContent(120, 'right');
+ sandbox.spy(element, '_positionActionBox');
emulateSelection(
startContent.firstChild, 10, endContent.lastChild, 7);
assert.isTrue(element.isRangeSelected());
+ const actionBox = element.$$('gr-selection-action-box');
+
assert.deepEqual(getActionRange(), {
startLine: 119,
startChar: 10,
@@ -296,6 +342,7 @@
endChar: 36,
});
assert.equal(getActionSide(), 'right');
+ assert.notOk(actionBox.positionBelow);
});
test('multiple ranges aka firefox implementation', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
index 47db5f0..3993b86 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
@@ -17,34 +17,24 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-tooltip/gr-tooltip.html">
<dom-module id="gr-selection-action-box">
<template>
<style include="shared-styles">
:host {
- --gr-arrow-size: .65em;
-
- background-color: rgba(22, 22, 22, .9);
- border-radius: 3px;
- color: #fff;
cursor: pointer;
font-family: var(--font-family);
- padding: .5em .75em;
position: absolute;
white-space: nowrap;
}
- .arrow {
- border: var(--gr-arrow-size) solid transparent;
- border-top: var(--gr-arrow-size) solid rgba(22, 22, 22, 0.9);
- height: 0;
- left: calc(50% - var(--gr-arrow-size));
- margin-top: .5em;
- position: absolute;
- width: 0;
+ #tooltip {
+ --tooltip-background-color: rgba(22, 22, 22, .9);
}
</style>
- Press <strong>c</strong> to comment.
- <div class="arrow"></div>
+ <gr-tooltip id="tooltip"
+ text="press c to comment"
+ position-below="[[positionBelow]]"></gr-tooltip>
</template>
<script src="gr-selection-action-box.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
index c228235..61a6eb2 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
@@ -37,6 +37,7 @@
endChar: NaN,
},
},
+ positionBelow: Boolean,
side: {
type: String,
value: '',
@@ -58,7 +59,7 @@
placeAbove(el) {
Polymer.dom.flush();
const rect = this._getTargetBoundingRect(el);
- const boxRect = this.getBoundingClientRect();
+ const boxRect = this.$.tooltip.getBoundingClientRect();
const parentRect = this.parentElement.getBoundingClientRect();
this.style.top =
rect.top - parentRect.top - boxRect.height - 6 + 'px';
@@ -66,6 +67,17 @@
rect.left - parentRect.left + (rect.width - boxRect.width) / 2 + 'px';
},
+ placeBelow(el) {
+ Polymer.dom.flush();
+ const rect = this._getTargetBoundingRect(el);
+ const boxRect = this.$.tooltip.getBoundingClientRect();
+ const parentRect = this.parentElement.getBoundingClientRect();
+ this.style.top =
+ rect.top - parentRect.top + boxRect.height - 6 + 'px';
+ this.style.left =
+ rect.left - parentRect.left + (rect.width - boxRect.width) / 2 + 'px';
+ },
+
_getTargetBoundingRect(el) {
let rect;
if (el instanceof Text) {
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
index 8c70772..7fc634c 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
@@ -38,15 +38,17 @@
suite('gr-selection-action-box', () => {
let container;
let element;
+ let sandbox;
setup(() => {
container = fixture('basic');
element = container.querySelector('gr-selection-action-box');
- sinon.stub(element, 'fire');
+ sandbox = sinon.sandbox.create();
+ sandbox.stub(element, 'fire');
});
teardown(() => {
- element.fire.restore();
+ sandbox.restore();
});
test('ignores regular keys', () => {
@@ -65,10 +67,10 @@
setup(() => {
e = {
button: 0,
- preventDefault: sinon.stub(),
- stopPropagation: sinon.stub(),
+ preventDefault: sandbox.stub(),
+ stopPropagation: sandbox.stub(),
};
- sinon.stub(element, '_fireCreateComment');
+ sandbox.stub(element, '_fireCreateComment');
});
test('event handled if main button', () => {
@@ -107,20 +109,14 @@
setup(() => {
target = container.querySelector('.target');
- sinon.stub(container, 'getBoundingClientRect').returns(
+ sandbox.stub(container, 'getBoundingClientRect').returns(
{top: 1, bottom: 2, left: 3, right: 4, width: 50, height: 6});
- sinon.stub(element, '_getTargetBoundingRect').returns(
+ sandbox.stub(element, '_getTargetBoundingRect').returns(
{top: 42, bottom: 20, left: 30, right: 40, width: 100, height: 60});
- sinon.stub(element, 'getBoundingClientRect').returns(
+ sandbox.stub(element.$.tooltip, 'getBoundingClientRect').returns(
{width: 10, height: 10});
});
- teardown(() => {
- element.getBoundingClientRect.restore();
- container.getBoundingClientRect.restore();
- element._getTargetBoundingRect.restore();
- });
-
test('placeAbove for Element argument', () => {
element.placeAbove(target);
assert.equal(element.style.top, '25px');
@@ -133,13 +129,24 @@
assert.equal(element.style.left, '72px');
});
+ test('placeBelow for Element argument', () => {
+ element.placeBelow(target);
+ assert.equal(element.style.top, '45px');
+ assert.equal(element.style.left, '72px');
+ });
+
+ test('placeBelow for Text Node argument', () => {
+ element.placeBelow(target.firstChild);
+ assert.equal(element.style.top, '45px');
+ assert.equal(element.style.left, '72px');
+ });
+
test('uses document.createRange', () => {
- sinon.spy(document, 'createRange');
+ sandbox.spy(document, 'createRange');
element._getTargetBoundingRect.restore();
- sinon.spy(element, '_getTargetBoundingRect');
+ sandbox.spy(element, '_getTargetBoundingRect');
element.placeAbove(target.firstChild);
assert.isTrue(document.createRange.called);
- document.createRange.restore();
});
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html
index c34bc69..1744d28 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html
@@ -24,7 +24,7 @@
--gr-tooltip-arrow-size: .5em;
--gr-tooltip-arrow-center-offset: 0;
- background-color: #333;
+ background-color: var(--tooltip-background-color, #333);
box-shadow: 0 1px 3px rgba(0, 0, 0, .3);
color: #fff;
font-size: .75rem;
@@ -52,11 +52,11 @@
width: 0;
}
.arrowPositionAbove {
- border-top: var(--gr-tooltip-arrow-size) solid #333;
+ border-top: var(--gr-tooltip-arrow-size) solid var(--tooltip-background-color, #333);
bottom: -var(--gr-tooltip-arrow-size);
}
.arrowPositionBelow {
- border-bottom: var(--gr-tooltip-arrow-size) solid #333;
+ border-bottom: var(--gr-tooltip-arrow-size) solid var(--tooltip-background-color, #333);
top: -var(--gr-tooltip-arrow-size);
}
</style>
