Implement keyboard shortcuts for the change view file list

+ This introduces an app-level viewState that is used to persist
  the position of the carets across DOM restamps.
+ Additionally, the current view is placed in app.params in order
  for the view itself to see whether changes to the params should
  affect it.

Bug: Issue 3775
Change-Id: Idee167fee4ddac5354908a61b9e8138525924907

polygerrit-ui/app/elements/gr-app.html

@@ -113,7 +113,7 @@ limitations under the License.
       </template>
       <template is="dom-if" if="{{_showChangeView}}" restamp="true">
         <gr-change-view server-config="[[config]]"
-            params="[[params]]"></gr-change-view>
+            params="[[params]]" view-state="{{_viewState.changeView}}"></gr-change-view>
       </template>
       <template is="dom-if" if="{{_showDiffView}}" restamp="true">
         <gr-diff-view prefs="{{_diffPreferences}}" params="[[params]]"></gr-diff-view>
@@ -180,27 +180,38 @@ limitations under the License.
           reflectToAttribute: true,
         },
         params: Object,
-        route: {
-          type: String,
-          observer: '_routeChanged',
-        },
 
         _diffPreferences: Object,
-
         _showChangeListView: Boolean,
         _showDashboardView: Boolean,
         _showChangeView: Boolean,
         _showDiffView: Boolean,
+        _viewState: Object,
       },
 
       listeners: {
         'title-change': '_handleTitleChange',
       },
 
+      observers: [ '_viewChanged(params.view)' ],
+
       get loggedIn() {
         return !!(this.account && Object.keys(this.account).length > 0);
       },
 
+      ready: function() {
+        this._viewState = {
+          changeView: {
+            changeNum: null,
+            patchNum: null,
+            selectedFileIndex: 0,
+          },
+          changeListView: {
+            selectedChangeIndex: 0,
+          },
+        }
+      },
+
       _accountChanged: function() {
         this._resolveAccountReady();
         this.$.accountContainer.classList.toggle('loggedIn', this.loggedIn);
@@ -221,12 +232,12 @@ limitations under the License.
         this._resolveConfigReady(config);
       },
 
-      _routeChanged: function(route) {
+      _viewChanged: function(view) {
-        this.set('_showChangeListView', route == 'gr-change-list-view');
+        this.set('_showChangeListView', view == 'gr-change-list-view');
-        this.set('_showDashboardView', route == 'gr-dashboard-view');
+        this.set('_showDashboardView', view == 'gr-dashboard-view');
-        this.set('_showChangeView', route == 'gr-change-view');
+        this.set('_showChangeView', view == 'gr-change-view');
-        this.set('_showDiffView', route == 'gr-diff-view');
+        this.set('_showDiffView', view == 'gr-diff-view');
-        this.constrained = route == 'gr-change-view';
+        this.constrained = view == 'gr-change-view';
       },
 
       _loginTapHandler: function(e) {

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

@@ -129,6 +129,8 @@ limitations under the License.
       },
 
       _paramsChanged: function(value) {
+        if (value.view != this.tagName.toLowerCase()) { return; }
+
         this.query = value.query;
         this.offset = value.offset || 0;
 

polygerrit-ui/app/elements/gr-change-view.html

@@ -252,7 +252,8 @@ limitations under the License.
       <gr-file-list id="fileList"
           change-num="[[_changeNum]]"
           patch-num="[[_patchNum]]"
-          comments="[[_comments]]"></gr-file-list>
+          comments="[[_comments]]"
+          selected-index="{{viewState.selectedFileIndex}}"></gr-file-list>
       <gr-messages-list id="messageList"
           change-num="[[_changeNum]]"
           messages="[[_change.messages]]"
@@ -281,6 +282,10 @@ limitations under the License.
           type: Object,
           observer: '_paramsChanged',
         },
+        viewState: {
+          type: Object,
+          notify: true,
+        },
         serverConfig: Object,
 
         _comments: Object,
@@ -369,8 +374,16 @@ limitations under the License.
       },
 
       _paramsChanged: function(value) {
+        if (value.view != this.tagName.toLowerCase()) { return; }
+
         this._changeNum = value.changeNum;
         this._patchNum = value.patchNum;
+        if (this.viewState.changeNum != this._changeNum ||
+            this.viewState.patchNum != this._patchNum) {
+          this.set('viewState.selectedFileIndex', 0);
+          this.set('viewState.changeNum', this._changeNum);
+          this.set('viewState.patchNum', this._patchNum);
+        }
         if (!this._changeNum) {
           return;
         }

polygerrit-ui/app/elements/gr-diff-view.html

@@ -223,6 +223,8 @@ limitations under the License.
       },
 
       _paramsChanged: function(value) {
+        if (value.view != this.tagName.toLowerCase()) { return; }
+
         this._changeNum = value.changeNum;
         this._patchRange = {
           patchNum: value.patchNum,

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

@@ -40,6 +40,16 @@ limitations under the License.
       th {
         text-align: left;
       }
+      .positionIndicator {
+        padding-left: .25em;
+        visibility: hidden;
+      }
+      tr[selected] {
+        background-color: #ebf5fb;
+      }
+      tr[selected] .positionIndicator {
+        visibility: visible;
+      }
       .status {
         width: 20px;
       }
@@ -65,8 +75,8 @@ limitations under the License.
           <th>Stats</th>
         </tr>
         <template is="dom-repeat" items="{{files}}" as="file">
-          <tr>
+          <tr class="fileRow" selected$="[[_computeFileSelected(index, selectedIndex)]]">
-            <td></td>
+            <td class="positionIndicator">&#x25b6;</td>
             <td class="status">[[file.status]]</td>
             <td class="path">
               <a class="file"
@@ -97,8 +107,24 @@ limitations under the License.
         changeNum: String,
         comments: Object,
         files: Array,
+        selectedIndex: {
+          type: Number,
+          notify: true,
+        },
 
         _drafts: Object,
+        _boundKeyHandler: {
+          type: Function,
+          value: function() { return this._handleKey.bind(this); },
+        }
+      },
+
+      attached: function() {
+        document.body.addEventListener('keydown', this._boundKeyHandler);
+      },
+
+      detached: function() {
+        document.body.removeEventListener('keydown', this._boundKeyHandler);
       },
 
       reload: function() {
@@ -153,6 +179,32 @@ limitations under the License.
         this.files = files;
       },
 
+      _handleKey: function(e) {
+        if (util.shouldSupressKeyboardShortcut(e)) { return; }
+
+        switch(e.keyCode) {
+          case 74:  // 'j'
+            e.preventDefault();
+            this.selectedIndex =
+                Math.min(this.files.length - 1, this.selectedIndex + 1);
+            break;
+          case 75:  // 'k'
+            e.preventDefault();
+            this.selectedIndex = Math.max(0, this.selectedIndex - 1);
+            break;
+          case 13:  // <enter>
+          case 79:  // 'o'
+            e.preventDefault();
+            page(this._computeDiffURL(this.changeNum, this.patchNum,
+                this.files[this.selectedIndex].__path));
+            break;
+        }
+      },
+
+      _computeFileSelected: function(index, selectedIndex) {
+        return index == selectedIndex;
+      },
+
       _computeDiffURL: function(changeNum, patchNum, path) {
         return '/c/' + changeNum + '/' + patchNum + '/' + path;
       },

polygerrit-ui/app/scripts/app.js

@@ -47,7 +47,7 @@ window.addEventListener('WebComponentsReady', function() {
 
   page('/dashboard/(.*)', loadUser, function(data) {
     if (app.loggedIn) {
-      app.route = 'gr-dashboard-view';
+      data.params.view = 'gr-dashboard-view';
       app.params = data.params;
     } else {
       page.redirect('/login/' + encodeURIComponent(data.canonicalPath));
@@ -55,7 +55,7 @@ window.addEventListener('WebComponentsReady', function() {
   });
 
   function queryHandler(data) {
-    app.route = 'gr-change-list-view';
+    data.params.view = 'gr-change-list-view';
     app.params = data.params;
   }
 
@@ -67,17 +67,17 @@ window.addEventListener('WebComponentsReady', function() {
   });
 
   page('/c/:changeNum/:patchNum?', function(data) {
-    app.route = 'gr-change-view';
+    data.params.view = 'gr-change-view';
     app.params = data.params;
   });
 
   page(/^\/c\/(\d+)\/((\d+)(\.\.(\d+))?)\/(.+)/, function(ctx) {
-    app.route = 'gr-diff-view';
     var params = {
       changeNum: ctx.params[0],
       basePatchNum: ctx.params[2],
       patchNum: ctx.params[4],
-      path: ctx.params[5]
+      path: ctx.params[5],
+      view: 'gr-diff-view',
     };
     // Don't allow diffing the same patch number against itself because WHY?
     if (params.basePatchNum == params.patchNum) {

polygerrit-ui/app/test/gr-file-list-test.html

@@ -123,6 +123,46 @@ limitations under the License.
       }, 1);
     });
 
+    test('keyboard shortcuts', function(done) {
+      element.changeNum = '42';
+      element.patchNum = '2';
+      element.selectedIndex = 0;
+      element.reload();
+      server.respond();
+
+      element.async(function() {
+        var elementItems = Polymer.dom(element.root).querySelectorAll(
+            '.fileRow');
+        assert.equal(elementItems.length, 3);
+        assert.isTrue(elementItems[0].hasAttribute('selected'));
+        assert.isFalse(elementItems[1].hasAttribute('selected'));
+        assert.isFalse(elementItems[2].hasAttribute('selected'));
+        MockInteractions.pressAndReleaseKeyOn(element, 74);  // 'j'
+        assert.equal(element.selectedIndex, 1);
+        MockInteractions.pressAndReleaseKeyOn(element, 74);  // 'j'
+
+        var showStub = sinon.stub(page, 'show');
+        assert.equal(element.selectedIndex, 2);
+        MockInteractions.pressAndReleaseKeyOn(element, 13);  // 'enter'
+        assert(showStub.lastCall.calledWith('/c/42/2/myfile.txt'),
+            'Should navigate to /c/42/2/myfile.txt');
+
+        MockInteractions.pressAndReleaseKeyOn(element, 75);  // 'k'
+        assert.equal(element.selectedIndex, 1);
+        MockInteractions.pressAndReleaseKeyOn(element, 79);  // 'o'
+        assert(showStub.lastCall.calledWith('/c/42/2/file_added_in_rev2.txt'),
+            'Should navigate to /c/42/2/file_added_in_rev2.txt');
+
+        MockInteractions.pressAndReleaseKeyOn(element, 75);  // 'k'
+        MockInteractions.pressAndReleaseKeyOn(element, 75);  // 'k'
+        MockInteractions.pressAndReleaseKeyOn(element, 75);  // 'k'
+        assert.equal(element.selectedIndex, 0);
+
+        showStub.restore();
+        done();
+      }, 1);
+    });
+
     test('comment filtering', function() {
       var comments = {
         '/COMMIT_MSG': [