opendev/gerrit
Code Issues Proposed changes

Make gr-diff-selection work with native Shadow DOM

Browse Source 
With native Shadow DOM, the selections returned by document.getSelection()
cannot reference the elements actually making up the diff because those
live in the Shadow DOM of gr-diff. Instead, a shadowRoot.getSelection()
method is offered that works as the pre-Shadow DOM Selection used to,
but that one is only available from gr-diff.

This change moves listening for the selectionchange event and retrieving
the selection to gr-diff, which then delegates the extraction and
normalization of the ranges to gr-diff-highlight (it's current
location).

This change also fixes an incompatibility with native Shadow DOM in
gr-selection-action-box, where a call to parentElement cannot cross the
Shadow DOM boundary and needs special handling to retrieve the Shadow
Host.

Together these changes fix creating range comments for native Shadow
DOM.

Bug: Issue 6372
Change-Id: I4bce21025ab5a4c06d77db096d6aa56e7840987b
This commit is contained in:
Ole Rehmsen
2018-12-18 16:56:10 +01:00
parent 1b0c4f0843
commit 201610028d
5 changed files with 87 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
View File

@@ -33,7 +33,6 @@
* @type {?HTMLElement}
* */
_cachedDiffBuilder: Object,
isAttached: Boolean,
},
listeners: {
@@ -42,10 +41,6 @@
'create-range-comment': '_createRangeComment',
},
observers: [
'_enableSelectionObserver(loggedIn, isAttached)',
],
get diffBuilder() {
if (!this._cachedDiffBuilder) {
this._cachedDiffBuilder =
@@ -54,24 +49,30 @@
return this._cachedDiffBuilder;
},
_enableSelectionObserver(loggedIn, isAttached) {
if (loggedIn && isAttached) {
this.listen(document, 'selectionchange', '_handleSelectionChange');
} else {
this.unlisten(document, 'selectionchange', '_handleSelectionChange');
}
},
isRangeSelected() {
return !!this.$$('gr-selection-action-box');
},
_handleSelectionChange() {
/**
* Determines side/line/range for a DOM selection and shows a tooltip.
*
* With native shadow DOM, gr-diff-highlight cannot access a selection that
* references the DOM elements making up the diff because they are in the
* shadow DOM the gr-diff element. For this reason, we listen to the
* selectionchange event and retrieve the selection in gr-diff, and then
* call this method to process the Selection.
*
* @param {Selection} selection A DOM Selection living in the shadow DOM of
* the diff element.
*/
handleSelectionChange(selection) {
// 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);
this.debounce(
'selectionChange', () => this._handleSelection(selection), 200);
},
_getThreadEl(e) {
@@ -128,6 +129,7 @@
* Merges multiple ranges, accounts for triple click, accounts for
* syntax highligh, convert native DOM Range objects to Gerrit concepts
* (line, side, etc).
* @param {Selection} selection
* @return {({
* start: {
* node: Node,
@@ -143,8 +145,7 @@
* }
* })|null|!Object}
*/
_getNormalizedRange() {
const selection = window.getSelection();
_getNormalizedRange(selection) {
const rangeCount = selection.rangeCount;
if (rangeCount === 0) {
return null;
@@ -289,12 +290,12 @@
actionBox.placeBelow(range);
},
_handleSelection() {
const normalizedRange = this._getNormalizedRange();
_handleSelection(selection) {
const normalizedRange = this._getNormalizedRange(selection);
if (!normalizedRange) {
return;
}
const domRange = window.getSelection().getRangeAt(0);
const domRange = selection.getRangeAt(0);
/** @type {?} */
const start = normalizedRange.start;
if (!start) {

35
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
View File

@@ -158,33 +158,6 @@ limitations under the License.
sandbox.restore();
});
suite('selectionchange event handling', () => {
const emulateSelection = function() {
document.dispatchEvent(new CustomEvent('selectionchange'));
element.flushDebouncer('selectionChange');
element.flushDebouncer('removeActionBox');
};
setup(() => {
sandbox.stub(element, '_handleSelection');
sandbox.stub(element, '_removeActionBox');
});
test('enabled if logged in', () => {
element.loggedIn = true;
emulateSelection();
assert.isTrue(element._handleSelection.called);
assert.isTrue(element._removeActionBox.called);
});
test('ignored if logged out', () => {
element.loggedIn = false;
emulateSelection();
assert.isFalse(element._handleSelection.called);
assert.isFalse(element._removeActionBox.called);
});
});
suite('comment events', () => {
let builder;
@@ -293,7 +266,7 @@ limitations under the License.
range.setStart(startNode, startOffset);
range.setEnd(endNode, endOffset);
selection.addRange(range);
element._handleSelection();
element._handleSelection(selection);
};
const getActionRange = () =>
@@ -400,12 +373,12 @@ limitations under the License.
getRangeAtStub
.onFirstCall().returns(startRange)
.onSecondCall().returns(endRange);
sandbox.stub(window, 'getSelection').returns({
const selection = {
rangeCount: 2,
getRangeAt: getRangeAtStub,
removeAllRanges: sandbox.stub(),
});
element._handleSelection();
};
element._handleSelection(selection);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
start_line: 119,

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

@@ -252,6 +252,9 @@
* @type {?PolymerDomApi.ObserveHandle}
*/
_nodeObserver: Object,
/** Set by Polymer. */
isAttached: Boolean,
},
behaviors: [
@@ -263,6 +266,10 @@
'render-content': '_handleRenderContent',
},
observers: [
'_enableSelectionObserver(loggedIn, isAttached)',
],
attached() {
this._observeNodes();
},
@@ -272,6 +279,27 @@
this._unobserveNodes();
},
_enableSelectionObserver(loggedIn, isAttached) {
if (loggedIn && isAttached) {
this.listen(document, 'selectionchange', '_handleSelectionChange');
} else {
this.unlisten(document, 'selectionchange', '_handleSelectionChange');
}
},
_handleSelectionChange() {
// When using native shadow DOM, the selection returned by
// document.getSelection() cannot reference the actual DOM elements making
// up the diff, because they are in the shadow DOM of the gr-diff element.
// For this reason, we handle the selectionchange here, and pass the
// shadow DOM selection into gr-diff-highlight, where the corresponding
// range is determined and normalized.
const selection = this.root.getSelection ?
this.root.getSelection() :
document.getSelection();
this.$.highlights.handleSelectionChange(selection);
},
_observeNodes() {
this._nodeObserver = Polymer.dom(this).observeNodes(info => {
const addedThreadEls = info.addedNodes.filter(isThreadEl);

24
polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
View File

@@ -48,6 +48,30 @@ limitations under the License.
sandbox.restore();
});
suite('selectionchange event handling', () => {
const emulateSelection = function() {
document.dispatchEvent(new CustomEvent('selectionchange'));
};
setup(() => {
element = fixture('basic');
sandbox.stub(element.$.highlights, 'handleSelectionChange');
});
test('enabled if logged in', () => {
element.loggedIn = true;
emulateSelection();
assert.isTrue(element.$.highlights.handleSelectionChange.called);
});
test('ignored if logged out', () => {
element.loggedIn = false;
emulateSelection();
assert.isFalse(element.$.highlights.handleSelectionChange.called);
});
});
test('cancel', () => {
element = fixture('basic');
const cancelStub = sandbox.stub(element.$.diffBuilder, 'cancel');

15
polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
View File

@@ -63,7 +63,7 @@
Polymer.dom.flush();
const rect = this._getTargetBoundingRect(el);
const boxRect = this.$.tooltip.getBoundingClientRect();
const parentRect = this.parentElement.getBoundingClientRect();
const parentRect = this._getParentBoundingClientRect();
this.style.top =
rect.top - parentRect.top - boxRect.height - 6 + 'px';
this.style.left =
@@ -74,11 +74,18 @@
Polymer.dom.flush();
const rect = this._getTargetBoundingRect(el);
const boxRect = this.$.tooltip.getBoundingClientRect();
const parentRect = this.parentElement.getBoundingClientRect();
const parentRect = this._getParentBoundingClientRect();
this.style.top =
rect.top - parentRect.top + boxRect.height - 6 + 'px';
rect.top - parentRect.top + boxRect.height - 6 + 'px';
this.style.left =
rect.left - parentRect.left + (rect.width - boxRect.width) / 2 + 'px';
rect.left - parentRect.left + (rect.width - boxRect.width) / 2 + 'px';
},
_getParentBoundingClientRect() {
// With native shadow DOM, the parent is the shadow root, not the gr-diff
// element
const parent = this.parentElement || this.parentNode.host;
return parent.getBoundingClientRect();
},
_getTargetBoundingRect(el) {