From 35e7e0c37afda66456c61b911ca67694a94e00da Mon Sep 17 00:00:00 2001
From: Becky Siegel <beckysiegel@google.com>
Date: Tue, 20 Dec 2016 11:52:13 -0800
Subject: [PATCH] Expand inline diffs based on user preference

If a user has enabled the expand inline diffs preference, the file list
expands the file inline instead of opening a diff view. The toggle arrow
is also hidden when this preference is selected.

Feature: Issue 5115
Change-Id: I814d7dfff0348a8a315c992c0259810fcbc4704f
---
 .../change/gr-file-list/gr-file-list.html     |  5 ++-
 .../change/gr-file-list/gr-file-list.js       |  9 +++++
 .../gr-file-list/gr-file-list_test.html       | 37 +++++++++++++++----
 3 files changed, 41 insertions(+), 10 deletions(-)

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 86c93c4b76..7ddceacd94 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,7 +232,8 @@ limitations under the License.
           [[_computeFileStatus(file.status)]]
         </div>
         <a class$="[[_computePathClass(file.__expanded)]]"
-            href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]">
+            href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]"
+            on-click="_handleFileClick">
           <div title$="[[_computeFileDisplayName(file.__path)]]"
               class="fullFileName">
             [[_computeFileDisplayName(file.__path)]]
@@ -265,7 +266,7 @@ limitations under the License.
             [[_formatPercentage(file.size, file.size_delta)]]
           </span>
         </div>
-        <div class="show-hide">
+        <div class="show-hide" hidden$="[[_userPrefs.expand_inline_diffs]]">
           <label class="show-hide">
             <input type="checkbox" class="show-hide"
                 checked$="[[!file.__expanded]]" data-path$="[[file.__path]]"
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 8774d83601..3fb2c11f66 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
@@ -312,6 +312,15 @@
           });
     },
 
+    _handleFileClick: function(e) {
+      // If the user prefers to expand inline diffs rather than opening the diff
+      // view, intercept the click event.
+      if (this._userPrefs && this._userPrefs.expand_inline_diffs) {
+        e.preventDefault();
+        this._handleHiddenChange(e);
+      }
+    },
+
     _handleShiftLeftKey: function(e) {
       if (this.shouldSuppressKeyboardShortcut(e)) { return; }
       if (!this._showInlineDiffs) { return; }
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 68bf1104f1..c753060a9e 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
@@ -617,14 +617,6 @@ limitations under the License.
       assert.isNotOk(element.$$('.expanded'));
     });
 
-    test('expanded attribute set on path when expanded', function() {
-      element._files = [
-        {__path: '/COMMIT_MSG', __expanded: true},
-      ];
-      flushAsynchronousOperations();
-      assert.isOk(element.$$('.expanded'));
-    });
-
     test('_getDiffViewMode', function() {
       // No user prefs or diff view mode set.
       assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
@@ -638,5 +630,34 @@ limitations under the License.
       assert.equal(element._getDiffViewMode(
           element.diffViewMode, element._userPrefs), 'SIDE_BY_SIDE');
     });
+    test('expand_inline_diffs user preference', function() {
+      element._files = [
+        {__path: '/COMMIT_MSG', __expanded: false},
+      ];
+      element.changeNum = '42';
+      element.patchRange = {
+        basePatchNum: 'PARENT',
+        patchNum: '2',
+      };
+      flushAsynchronousOperations();
+      var commitMsgFile = Polymer.dom(element.root)
+          .querySelectorAll('.row:not(.header) a')[0];
+
+      // Remove href attribute so the app doesn't route to a diff view
+      commitMsgFile.removeAttribute('href');
+      var hiddenChangeSpy = sandbox.spy(element, '_handleHiddenChange');
+
+      MockInteractions.tap(commitMsgFile);
+      flushAsynchronousOperations();
+      assert(hiddenChangeSpy.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.isOk(element.$$('.expanded'));
+    });
   });
 </script>