Merge changes I1a969bb6,Ic674acd3

* changes:
  Move hover handling out of gr-diff-comment
  Fix undesired normalization of internal structure
This commit is contained in:
Ole Rehmsen
2018-11-19 18:58:20 +00:00
committed by Gerrit Code Review
6 changed files with 66 additions and 32 deletions

View File

@@ -232,10 +232,7 @@ limitations under the License.
display: block; display: block;
} }
</style> </style>
<div id="container" <div id="container" class="container">
class="container"
on-mouseenter="_handleMouseEnter"
on-mouseleave="_handleMouseLeave">
<div class="header" id="header" on-tap="_handleToggleCollapsed"> <div class="header" id="header" on-tap="_handleToggleCollapsed">
<div class="headerLeft"> <div class="headerLeft">
<span class="authorName">[[comment.author.name]]</span> <span class="authorName">[[comment.author.name]]</span>

View File

@@ -58,14 +58,6 @@
* @event comment-update * @event comment-update
*/ */
/**
* @event comment-mouse-over
*/
/**
* @event comment-mouse-out
*/
/** /**
* Fired when the comment's timestamp is tapped. * Fired when the comment's timestamp is tapped.
* *
@@ -610,14 +602,6 @@
} }
}, },
_handleMouseEnter(e) {
this.fire('comment-mouse-over', this._getEventPayload());
},
_handleMouseLeave(e) {
this.fire('comment-mouse-out', this._getEventPayload());
},
_handleToggleResolved() { _handleToggleResolved() {
this.$.reporting.recordDraftInteraction(); this.$.reporting.recordDraftInteraction();
this.resolved = !this.resolved; this.resolved = !this.resolved;

View File

@@ -37,8 +37,8 @@
}, },
listeners: { listeners: {
'comment-mouse-out': '_handleCommentMouseOut', 'comment-thread-mouseleave': '_handleCommentThreadMouseleave',
'comment-mouse-over': '_handleCommentMouseOver', 'comment-thread-mouseenter': '_handleCommentThreadMouseenter',
'create-range-comment': '_createRangeComment', 'create-range-comment': '_createRangeComment',
}, },
@@ -74,7 +74,7 @@
this.debounce('selectionChange', this._handleSelection, 200); this.debounce('selectionChange', this._handleSelection, 200);
}, },
_handleCommentMouseOver(e) { _handleCommentThreadMouseenter(e) {
const threadEl = Polymer.dom(e).localTarget; const threadEl = Polymer.dom(e).localTarget;
const index = this._indexForThreadEl(threadEl); const index = this._indexForThreadEl(threadEl);
@@ -83,7 +83,7 @@
} }
}, },
_handleCommentMouseOut(e) { _handleCommentThreadMouseleave(e) {
const threadEl = Polymer.dom(e).localTarget; const threadEl = Polymer.dom(e).localTarget;
const index = this._indexForThreadEl(threadEl); const index = this._indexForThreadEl(threadEl);

View File

@@ -197,22 +197,57 @@ limitations under the License.
element._cachedDiffBuilder = builder; 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'); sandbox.stub(element, 'set');
element.fire('comment-mouse-over', {comment: {}}); threadEl.dispatchEvent(
new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
assert.isFalse(element.set.called); 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, 'set');
sandbox.stub(element, '_indexForThreadEl').returns(0); threadEl.dispatchEvent(
element.fire('comment-mouse-over', {comment: {range: {}}}); new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
assert.isTrue(element.set.called); 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', () => { test('comment-thread-mouseleave from line comments is ignored', () => {
element.fire('comment-mouse-over', {comment: {}}); const threadEl = document.createElement('div');
assert.isFalse(builder.getContentsByLineRange.called); 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', () => { test('on create-range-comment action box is removed', () => {

View File

@@ -250,6 +250,7 @@
// this situation until it occurs. // this situation until it occurs.
this._updateRanges(addedThreadEls); this._updateRanges(addedThreadEls);
this._updateKeyLocations(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 any remaining diff builder rendering work. */
cancel() { cancel() {
this.$.diffBuilder.cancel(); this.$.diffBuilder.cancel();

View File

@@ -167,6 +167,9 @@
const ranges = this.get(['_rangesMap', side, lineNum]) || []; const ranges = this.get(['_rangesMap', side, lineNum]) || [];
return ranges return ranges
.map(range => { .map(range => {
// Make a copy, so that the normalization below does not mess with
// our map.
range = Object.assign({}, range);
range.end = range.end === -1 ? line.text.length : range.end; range.end = range.end === -1 ? line.text.length : range.end;
// Normalize invalid ranges where the start is after the end but the // Normalize invalid ranges where the start is after the end but the