Merge "Remove event handlers from individual lines in file list"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 0698a0c..74bd9ac 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -232,95 +232,98 @@
</label>
</div>
</header>
- <template is="dom-repeat"
- items="[[_shownFiles]]"
- as="file"
- initial-count="[[_fileListIncrement]]">
- <div class="file-row row">
- <div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
- <input type="checkbox" checked="[[file.isReviewed]]"
- data-path$="[[file.__path]]" on-change="_handleReviewedChange"
- class="reviewed" aria-label="Reviewed checkbox">
- </div>
- <div class$="[[_computeClass('status', file.__path)]]"
- tabindex="0"
- aria-label$="[[_computeFileStatusLabel(file.status)]]">
- [[_computeFileStatus(file.status)]]
- </div>
- <a class$="[[_computePathClass(file.__path, _expandedFilePaths.*)]]"
- href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]"
- on-tap="_handleFileTap">
- <div title$="[[_computeFileDisplayName(file.__path)]]"
- class="fullFileName">
- [[_computeFileDisplayName(file.__path)]]
- </div>
- <div title$="[[_computeFileDisplayName(file.__path)]]"
- class="truncatedFileName">
- [[_computeTruncatedFileDisplayName(file.__path)]]
- </div>
- <div class="oldPath" hidden$="[[!file.old_path]]" hidden
- title$="[[file.old_path]]">
- [[file.old_path]]
- </div>
- </a>
- <div class="comments desktop">
- <span class="drafts">
- [[_computeDraftsString(drafts, patchRange.patchNum, file.__path)]]
- </span>
- [[_computeCommentsString(comments, patchRange.patchNum, file.__path)]]
- [[_computeUnresolvedString(comments, drafts, patchRange.patchNum, file.__path)]]
- </div>
- <div class="comments mobile">
- <span class="drafts">
- [[_computeDraftsStringMobile(drafts, patchRange.patchNum,
- file.__path)]]
- </span>
- [[_computeCommentsStringMobile(comments, patchRange.patchNum,
- file.__path)]]
- </div>
- <div class$="[[_computeClass('stats', file.__path)]]">
- <span
- class="added"
- tabindex="0"
- aria-label$="[[file.lines_inserted]] lines added"
- hidden$=[[file.binary]]>
- +[[file.lines_inserted]]
- </span>
- <span
- class="removed"
- tabindex="0"
- aria-label$="[[file.lines_deleted]] lines removed"
- hidden$=[[file.binary]]>
- -[[file.lines_deleted]]
- </span>
- <span class$="[[_computeBinaryClass(file.size_delta)]]"
- hidden$=[[!file.binary]]>
- [[_formatBytes(file.size_delta)]]
- [[_formatPercentage(file.size, file.size_delta)]]
- </span>
- </div>
- <div class="show-hide" hidden$="[[_userPrefs.expand_inline_diffs]]">
- <label class="show-hide">
- <input type="checkbox" class="show-hide"
- checked$="[[_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
+ <div on-tap="_handleFileListTap">
+ <template is="dom-repeat"
+ items="[[_shownFiles]]"
+ id="files"
+ as="file"
+ initial-count="[[_fileListIncrement]]">
+ <div class="file-row row">
+ <div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
+ <input type="checkbox" checked="[[file.isReviewed]]"
data-path$="[[file.__path]]"
- on-change="_handleHiddenChange">
- [[_computeShowHideText(file.__path, _expandedFilePaths.*)]]
- </label>
+ class="reviewed" aria-label="Reviewed checkbox">
+ </div>
+ <div class$="[[_computeClass('status', file.__path)]]"
+ tabindex="0"
+ aria-label$="[[_computeFileStatusLabel(file.status)]]">
+ [[_computeFileStatus(file.status)]]
+ </div>
+ <a class$="[[_computePathClass(file.__path, _expandedFilePaths.*)]]"
+ href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]"
+ data-path$="[[file.__path]]">
+ <div title$="[[_computeFileDisplayName(file.__path)]]"
+ class="fullFileName">
+ [[_computeFileDisplayName(file.__path)]]
+ </div>
+ <div title$="[[_computeFileDisplayName(file.__path)]]"
+ class="truncatedFileName">
+ [[_computeTruncatedFileDisplayName(file.__path)]]
+ </div>
+ <div class="oldPath" hidden$="[[!file.old_path]]" hidden
+ title$="[[file.old_path]]">
+ [[file.old_path]]
+ </div>
+ </a>
+ <div class="comments desktop">
+ <span class="drafts">
+ [[_computeDraftsString(drafts, patchRange.patchNum, file.__path)]]
+ </span>
+ [[_computeCommentsString(comments, patchRange.patchNum, file.__path)]]
+ [[_computeUnresolvedString(comments, drafts, patchRange.patchNum, file.__path)]]
+ </div>
+ <div class="comments mobile">
+ <span class="drafts">
+ [[_computeDraftsStringMobile(drafts, patchRange.patchNum,
+ file.__path)]]
+ </span>
+ [[_computeCommentsStringMobile(comments, patchRange.patchNum,
+ file.__path)]]
+ </div>
+ <div class$="[[_computeClass('stats', file.__path)]]">
+ <span
+ class="added"
+ tabindex="0"
+ aria-label$="[[file.lines_inserted]] lines added"
+ hidden$=[[file.binary]]>
+ +[[file.lines_inserted]]
+ </span>
+ <span
+ class="removed"
+ tabindex="0"
+ aria-label$="[[file.lines_deleted]] lines removed"
+ hidden$=[[file.binary]]>
+ -[[file.lines_deleted]]
+ </span>
+ <span class$="[[_computeBinaryClass(file.size_delta)]]"
+ hidden$=[[!file.binary]]>
+ [[_formatBytes(file.size_delta)]]
+ [[_formatPercentage(file.size, file.size_delta)]]
+ </span>
+ </div>
+ <div class="show-hide" hidden$="[[_userPrefs.expand_inline_diffs]]">
+ <label class="show-hide" data-path$="[[file.__path]]"
+ data-expand=true>
+ <input type="checkbox" class="show-hide"
+ checked$="[[_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
+ data-path$="[[file.__path]]" data-expand=true>
+ [[_computeShowHideText(file.__path, _expandedFilePaths.*)]]
+ </label>
+ </div>
</div>
- </div>
- <gr-diff
- no-auto-render
- hidden="[[!_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
- project="[[change.project]]"
- commit="[[change.current_revision]]"
- change-num="[[changeNum]]"
- patch-range="[[patchRange]]"
- path="[[file.__path]]"
- prefs="[[_diffPrefs]]"
- project-config="[[projectConfig]]"
- view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"></gr-diff>
- </template>
+ <gr-diff
+ no-auto-render
+ hidden="[[!_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
+ project="[[change.project]]"
+ commit="[[change.current_revision]]"
+ change-num="[[changeNum]]"
+ patch-range="[[patchRange]]"
+ path="[[file.__path]]"
+ prefs="[[_diffPrefs]]"
+ project-config="[[projectConfig]]"
+ view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"></gr-diff>
+ </template>
+ </div>
<div class$="row totalChanges [[_computeExpandInlineClass(_userPrefs)]]">
<div class="total-stats" hidden$="[[_hideChangeTotals]]">
<span
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index a0189f2..54b1099 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -226,10 +226,6 @@
return parseInt(patchNum, 10) >= parseInt(currentPatchNum, 10);
},
- _handleHiddenChange: function(e) {
- this._togglePathExpanded(e.model.file.__path);
- },
-
_togglePathExpanded: function(path) {
// Is the path in the list of expanded diffs? IF so remove it, otherwise
// add it to the list.
@@ -400,15 +396,29 @@
});
},
- _handleFileTap: function(e) {
+ /**
+ * Handle all events from the file list dom-repeat so event handleers don't
+ * have to get registered for potentially very long lists.
+ */
+ _handleFileListTap: function(e) {
+ // Handle checkbox mark as reviewed.
+ if (e.target.classList.contains('reviewed')) {
+ return this._handleReviewedChange(e);
+ }
+
+ // Check to see if the file should be expanded.
+ var path = e.target.dataset.path || e.target.parentElement.dataset.path;
+
// If the user prefers to expand inline diffs rather than opening the diff
// view, intercept the click event.
- if (e.detail.sourceEvent.metaKey || e.detail.sourceEvent.ctrlKey) {
+ if (!path || e.detail.sourceEvent.metaKey ||
+ e.detail.sourceEvent.ctrlKey) {
return;
}
- if (this._userPrefs && this._userPrefs.expand_inline_diffs) {
+ if (e.target.dataset.expand ||
+ this._userPrefs && this._userPrefs.expand_inline_diffs) {
e.preventDefault();
- this._handleHiddenChange(e);
+ this._togglePathExpanded(path);
}
},
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index bd4619f..a582cbb 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -637,10 +637,13 @@
flushAsynchronousOperations();
var fileRows =
Polymer.dom(element.root).querySelectorAll('.row:not(.header)');
+ // Because the label surrounds the input, the tap event is triggered
+ // there first.
+ var showHideLabel = fileRows[0].querySelector('label.show-hide');
var showHideCheck = fileRows[0].querySelector(
'input.show-hide[type="checkbox"]');
assert.isNotOk(showHideCheck.checked);
- MockInteractions.tap(showHideCheck);
+ MockInteractions.tap(showHideLabel);
assert.isOk(showHideCheck.checked);
assert.notEqual(element._expandedFilePaths.indexOf('myfile.txt'), -1);
});
@@ -763,18 +766,18 @@
// Remove href attribute so the app doesn't route to a diff view
commitMsgFile.removeAttribute('href');
- var hiddenChangeSpy = sandbox.spy(element, '_handleHiddenChange');
+ var togglePathSpy = sandbox.spy(element, '_togglePathExpanded');
MockInteractions.tap(commitMsgFile);
flushAsynchronousOperations();
- assert(hiddenChangeSpy.notCalled, 'file is opened as diff view');
+ assert(togglePathSpy.notCalled, 'file is opened as diff view');
assert.isNotOk(element.$$('.expanded'));
element._userPrefs = {expand_inline_diffs: true};
flushAsynchronousOperations();
MockInteractions.tap(commitMsgFile);
flushAsynchronousOperations();
- assert(hiddenChangeSpy.calledOnce, 'file is expanded');
+ assert(togglePathSpy.calledOnce, 'file is expanded');
assert.isOk(element.$$('.expanded'));
});
@@ -812,32 +815,37 @@
element.push('_expandedFilePaths', path);
});
- suite('_handleFileTap', function() {
+ suite('_handleFileListTap', function() {
function testForModifier(modifier) {
var e = {preventDefault: function() {}};
e.detail = {sourceEvent: {}};
+ e.target = {
+ dataset: {path: '/test'},
+ classList: element.classList,
+ };
+
e.detail.sourceEvent[modifier] = true;
- var hiddenChangeStub = sandbox.stub(element, '_handleHiddenChange');
+ var togglePathStub = sandbox.stub(element, '_togglePathExpanded');
element._userPrefs = { expand_inline_diffs: true };
- element._handleFileTap(e);
- assert.isFalse(hiddenChangeStub.called);
+ element._handleFileListTap(e);
+ assert.isFalse(togglePathStub.called);
e.detail.sourceEvent[modifier] = false;
- element._handleFileTap(e);
- assert.equal(hiddenChangeStub.callCount, 1);
+ element._handleFileListTap(e);
+ assert.equal(togglePathStub.callCount, 1);
element._userPrefs = { expand_inline_diffs: false };
- element._handleFileTap(e);
- assert.equal(hiddenChangeStub.callCount, 1);
+ element._handleFileListTap(e);
+ assert.equal(togglePathStub.callCount, 1);
}
- test('_handleFileTap meta', function() {
+ test('_handleFileListTap meta', function() {
testForModifier('metaKey');
});
- test('_handleFileTap ctrl', function() {
+ test('_handleFileListTap ctrl', function() {
testForModifier('ctrlKey');
});
});
