Move hover handling out of gr-diff-comment
The only code that is interested in hover events from gr-comment-diff is
in gr-diff-highlight, which is used only by gr-diff. For these reasons,
it makes more sense for gr-diff (or gr-diff-highlight) to subscribe to
the native hover events of the comment threads associated with it, and
not require gr-comment to expose any custom events for that. This also
has the benefit that it reduces the requirements on thread elements:
They can just fire regular mouseenter and mouseleave events like any
DOMElement would. This makes it easier to use gr-diff with different
comment thread element implementations.
I used this change to also make the unit tests for this more meaningful
and not stub out all the interesting parts.
Change-Id: I1a969bb6a7092bc433039662cd2034dd7141e4ca
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index 72c7285..8633b0e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -232,10 +232,7 @@
display: block;
}
</style>
- <div id="container"
- class="container"
- on-mouseenter="_handleMouseEnter"
- on-mouseleave="_handleMouseLeave">
+ <div id="container" class="container">
<div class="header" id="header" on-tap="_handleToggleCollapsed">
<div class="headerLeft">
<span class="authorName">[[comment.author.name]]</span>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 5165db0..a46a0b2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -59,14 +59,6 @@
*/
/**
- * @event comment-mouse-over
- */
-
- /**
- * @event comment-mouse-out
- */
-
- /**
* Fired when the comment's timestamp is tapped.
*
* @event comment-anchor-tap
@@ -610,14 +602,6 @@
}
},
- _handleMouseEnter(e) {
- this.fire('comment-mouse-over', this._getEventPayload());
- },
-
- _handleMouseLeave(e) {
- this.fire('comment-mouse-out', this._getEventPayload());
- },
-
_handleToggleResolved() {
this.$.reporting.recordDraftInteraction();
this.resolved = !this.resolved;
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 85ba202..75a2b15 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
@@ -37,8 +37,8 @@
},
listeners: {
- 'comment-mouse-out': '_handleCommentMouseOut',
- 'comment-mouse-over': '_handleCommentMouseOver',
+ 'comment-thread-mouseleave': '_handleCommentThreadMouseleave',
+ 'comment-thread-mouseenter': '_handleCommentThreadMouseenter',
'create-range-comment': '_createRangeComment',
},
@@ -74,7 +74,7 @@
this.debounce('selectionChange', this._handleSelection, 200);
},
- _handleCommentMouseOver(e) {
+ _handleCommentThreadMouseenter(e) {
const threadEl = Polymer.dom(e).localTarget;
const index = this._indexForThreadEl(threadEl);
@@ -83,7 +83,7 @@
}
},
- _handleCommentMouseOut(e) {
+ _handleCommentThreadMouseleave(e) {
const threadEl = Polymer.dom(e).localTarget;
const index = this._indexForThreadEl(threadEl);
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 23de407..8c30afa 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
@@ -197,22 +197,57 @@
element._cachedDiffBuilder = builder;
});
- test('comment-mouse-over from line comments is ignored', () => {
+ test('comment-thread-mouseenter from line comments is ignored', () => {
+ const threadEl = document.createElement('div');
+ threadEl.setAttribute('comment-side', 'right');
+ threadEl.setAttribute('line-num', 3);
+ element.appendChild(threadEl);
+ element.commentRanges = [{side: 'right'}];
+
sandbox.stub(element, 'set');
- element.fire('comment-mouse-over', {comment: {}});
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
assert.isFalse(element.set.called);
});
- test('comment-mouse-over from ranged comment causes set', () => {
+ test('comment-thread-mouseenter from ranged comment causes set', () => {
+ const threadEl = document.createElement('div');
+ threadEl.setAttribute('comment-side', 'right');
+ threadEl.setAttribute('line-num', 3);
+ threadEl.setAttribute('range', JSON.stringify({
+ start_line: 3,
+ start_character: 4,
+ end_line: 5,
+ end_character: 6,
+ }));
+ element.appendChild(threadEl);
+ element.commentRanges = [{side: 'right', range: {
+ start_line: 3,
+ start_character: 4,
+ end_line: 5,
+ end_character: 6,
+ }}];
+
sandbox.stub(element, 'set');
- sandbox.stub(element, '_indexForThreadEl').returns(0);
- element.fire('comment-mouse-over', {comment: {range: {}}});
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
assert.isTrue(element.set.called);
+ const args = element.set.lastCall.args;
+ assert.deepEqual(args[0], ['commentRanges', 0, 'hovering']);
+ assert.deepEqual(args[1], true);
});
- test('comment-mouse-out from line comments is ignored', () => {
- element.fire('comment-mouse-over', {comment: {}});
- assert.isFalse(builder.getContentsByLineRange.called);
+ test('comment-thread-mouseleave from line comments is ignored', () => {
+ const threadEl = document.createElement('div');
+ threadEl.setAttribute('comment-side', 'right');
+ threadEl.setAttribute('line-num', 3);
+ element.appendChild(threadEl);
+ element.commentRanges = [{side: 'right'}];
+
+ sandbox.stub(element, 'set');
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseleave', {bubbles: true}));
+ assert.isFalse(element.set.called);
});
test('on create-range-comment action box is removed', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index a8cf320..8b5c6d6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -250,6 +250,7 @@
// this situation until it occurs.
this._updateRanges(addedThreadEls);
this._updateKeyLocations(addedThreadEls);
+ this._redispatchHoverEvents(addedThreadEls);
});
},
@@ -274,6 +275,20 @@
}
},
+ // Dispatch events that are handled by the gr-diff-highlight.
+ _redispatchHoverEvents(addedThreadEls) {
+ for (const threadEl of addedThreadEls) {
+ threadEl.addEventListener('mouseenter', () => {
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
+ });
+ threadEl.addEventListener('mouseleave', () => {
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseleave', {bubbles: true}));
+ });
+ }
+ },
+
/** Cancel any remaining diff builder rendering work. */
cancel() {
this.$.diffBuilder.cancel();