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 2490509109..03380e426d 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 b63b9a42bc..4bbf12bc3e 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 @@ -37,6 +37,27 @@ limitations under the License. + + + + + + + + + + + + + + + + + + + + + @@ -253,6 +274,7 @@ limitations under the License. contentStubs = []; stub('gr-selection-action-box', { placeAbove: sandbox.stub(), + placeBelow: sandbox.stub(), }); diff = element.querySelector('#diffTable'); builder = { @@ -270,9 +292,29 @@ limitations under the License. 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 @@ limitations under the License. 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 @@ limitations under the License. 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 47db5f0c52..3993b866e8 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 @@ limitations under the License. + 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 c228235ab4..61a6eb2e0e 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 8c707721a3..7fc634c2b1 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 @@ limitations under the License. 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 @@ limitations under the License. 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 @@ limitations under the License. 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 @@ limitations under the License. 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 c34bc69ba3..1744d2806b 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 @@ limitations under the License. --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 @@ limitations under the License. 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); }
[1] Nam cum ad me in Cumanum salutandi causa uterque venisset,
[1] Nam cum ad me in Cumanum salutandi causa uterque
na💢ti te, inquit, sumus aliquando otiosum, certe a udiam, quid sit, quod Epicurum
nacti , , sumus otiosum, audiam, sit, quod