From 143950ca2ad3cccdbcf6f9863f6f62a307468eda Mon Sep 17 00:00:00 2001
From: Logan Hanks <logan@google.com>
Date: Thu, 11 May 2017 09:55:33 -0700
Subject: [PATCH] Expand (not open) if user prefers inline diffs

Pressing enter or 'o' with no modifier key has three modes of behavior.

1. Show diffs: in this mode this keypress always acts on the diff cursor
2. Expand diffs inline: if user has this preference, expand current file
3. Otherwise: navigate to diff view of current file

Bug: Issue 6067
Change-Id: I64bbe0720ddac54a4dc7fbca6df87aa95321442f
---
 .../change/gr-file-list/gr-file-list.js       |  3 +
 .../gr-file-list/gr-file-list_test.html       | 96 ++++++++++++-------
 2 files changed, 65 insertions(+), 34 deletions(-)

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 705a8f1a51..66d7be9704 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
@@ -519,6 +519,9 @@
       e.preventDefault();
       if (this._showInlineDiffs) {
         this._openCursorFile();
+      } else if (this._userPrefs && this._userPrefs.expand_inline_diffs) {
+        if (this.$.fileCursor.index === -1) { return; }
+        this._togglePathExpandedByIndex(this.$.fileCursor.index);
       } else {
         this._openSelectedFile();
       }
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 a34463bb99..c489da29bd 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
@@ -381,42 +381,70 @@ limitations under the License.
         }
       });
 
-      test('_handleEnterKey navigates', function() {
-        sandbox.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
-        sandbox.stub(element, 'modifierPressed').returns(false);
-        var expandStub = sandbox.stub(element, '_openCursorFile');
-        var navStub = sandbox.stub(element, '_openSelectedFile');
-        var e = new CustomEvent('fake-keyboard-event');
-        sinon.stub(e, 'preventDefault');
-        element._showInlineDiffs = false;
-        element._handleEnterKey(e);
-        assert.isTrue(e.preventDefault.called);
-        assert.isTrue(navStub.called);
-        assert.isFalse(expandStub.called);
-      });
+      suite('_handleEnterKey', function() {
+        var interact;
 
-      test('_handleEnterKey expands', function() {
-        sandbox.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
-        sandbox.stub(element, 'modifierPressed').returns(false);
-        var expandStub = sandbox.stub(element, '_openCursorFile');
-        var navStub = sandbox.stub(element, '_openSelectedFile');
-        var e = new CustomEvent('fake-keyboard-event');
-        sinon.stub(e, 'preventDefault');
-        element._showInlineDiffs = true;
-        element._handleEnterKey(e);
-        assert.isTrue(e.preventDefault.called);
-        assert.isFalse(navStub.called);
-        assert.isTrue(expandStub.called);
-      });
+        setup(function() {
+          sandbox.stub(element, 'shouldSuppressKeyboardShortcut')
+            .returns(false);
+          sandbox.stub(element, 'modifierPressed').returns(false);
+          var openCursorStub = sandbox.stub(element, '_openCursorFile');
+          var openSelectedStub = sandbox.stub(element, '_openSelectedFile');
+          var expandStub = sandbox.stub(element, '_togglePathExpanded');
 
-      test('_handleEnterKey noop when anchor focused', function() {
-        sandbox.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
-        sandbox.stub(element, 'modifierPressed').returns(false);
-        var e = new CustomEvent('fake-keyboard-event',
-            {detail: {keyboardEvent: {target: document.createElement('a')}}});
-        sinon.stub(e, 'preventDefault');
-        element._handleEnterKey(e);
-        assert.isFalse(e.preventDefault.called);
+          interact = function(opt_payload) {
+            openCursorStub.reset();
+            openSelectedStub.reset();
+            expandStub.reset();
+
+            var e = new CustomEvent('fake-keyboard-event', opt_payload);
+            sinon.stub(e, 'preventDefault');
+            element._handleEnterKey(e);
+            assert.isTrue(e.preventDefault.called);
+            var result = {};
+            if (openCursorStub.called) {
+              result.opened_cursor = true;
+            }
+            if (openSelectedStub.called) {
+              result.opened_selected = true;
+            }
+            if (expandStub.called) {
+              result.expanded = true;
+            }
+            return result;
+          };
+        });
+
+        test('open from selected file', function() {
+          element._showInlineDiffs = false;
+          assert.deepEqual(interact(), {opened_selected: true});
+        });
+
+        test('open from diff cursor', function() {
+          element._showInlineDiffs = true;
+          assert.deepEqual(interact(), {opened_cursor: true});
+
+          // "Show diffs" mode overrides userPrefs.expand_inline_diffs
+          element._userPrefs = {expand_inline_diffs: true};
+          assert.deepEqual(interact(), {opened_cursor: true});
+        });
+
+        test('expand when user prefers', function() {
+          element._showInlineDiffs = false;
+          assert.deepEqual(interact(), {opened_selected: true});
+          element._userPrefs = {};
+          assert.deepEqual(interact(), {opened_selected: true});
+          element._userPrefs.expand_inline_diffs = true;
+          assert.deepEqual(interact(), {expanded: true});
+        });
+
+        test('noop when anchor focused', function() {
+          var e = new CustomEvent('fake-keyboard-event',
+              {detail: {keyboardEvent: {target: document.createElement('a')}}});
+          sinon.stub(e, 'preventDefault');
+          element._handleEnterKey(e);
+          assert.isFalse(e.preventDefault.called);
+        });
       });
     });