Remove event handlers from individual lines in file list

In order to speed up file list processing for large numbers of files,
event handlers have been removed on a per-line basis and instead
are listened for in a parent container that handles them according to
the click target.

With a sample of 1300 files, the dom-repeat after clicking 'show all'
gets rendered 25% faster.

Bug: Issue 5612
Change-Id: Idc1ba6e0810d5d4c6f36e76667c3748becc1358a

polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html

@@ -232,95 +232,98 @@ limitations under the License.
         </label>
       </div>
     </header>
-    <template is="dom-repeat"
+    <div on-tap="_handleFileListTap">
-        items="[[_shownFiles]]"
+      <template is="dom-repeat"
-        as="file"
+          items="[[_shownFiles]]"
-        initial-count="[[_fileListIncrement]]">
+          id="files"
-      <div class="file-row row">
+          as="file"
-        <div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
+          initial-count="[[_fileListIncrement]]">
-          <input type="checkbox" checked="[[file.isReviewed]]"
+        <div class="file-row row">
-              data-path$="[[file.__path]]" on-change="_handleReviewedChange"
+          <div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
-              class="reviewed" aria-label="Reviewed checkbox">
+            <input type="checkbox" checked="[[file.isReviewed]]"
-        </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.*)]]"
                 data-path$="[[file.__path]]"
-                on-change="_handleHiddenChange">
+                class="reviewed" aria-label="Reviewed checkbox">
-            [[_computeShowHideText(file.__path, _expandedFilePaths.*)]]
+          </div>
-          </label>
+          <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
-      <gr-diff
+            no-auto-render
-          no-auto-render
+            hidden="[[!_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
-          hidden="[[!_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
+            project="[[change.project]]"
-          project="[[change.project]]"
+            commit="[[change.current_revision]]"
-          commit="[[change.current_revision]]"
+            change-num="[[changeNum]]"
-          change-num="[[changeNum]]"
+            patch-range="[[patchRange]]"
-          patch-range="[[patchRange]]"
+            path="[[file.__path]]"
-          path="[[file.__path]]"
+            prefs="[[_diffPrefs]]"
-          prefs="[[_diffPrefs]]"
+            project-config="[[projectConfig]]"
-          project-config="[[projectConfig]]"
+            view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"></gr-diff>
-          view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"></gr-diff>
+      </template>
-    </template>
+    </div>
     <div class$="row totalChanges [[_computeExpandInlineClass(_userPrefs)]]">
       <div class="total-stats" hidden$="[[_hideChangeTotals]]">
         <span

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);
       }
     },
 

polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html

@@ -637,10 +637,13 @@ limitations under the License.
       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 @@ limitations under the License.
 
       // 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 @@ limitations under the License.
       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);
+        element._handleFileListTap(e);
-        assert.isFalse(hiddenChangeStub.called);
+        assert.isFalse(togglePathStub.called);
 
         e.detail.sourceEvent[modifier] = false;
-        element._handleFileTap(e);
+        element._handleFileListTap(e);
-        assert.equal(hiddenChangeStub.callCount, 1);
+        assert.equal(togglePathStub.callCount, 1);
 
         element._userPrefs = { expand_inline_diffs: false };
-        element._handleFileTap(e);
+        element._handleFileListTap(e);
-        assert.equal(hiddenChangeStub.callCount, 1);
+        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');
       });
     });