Set up basic keyboard next/previous navigation in diff list

Mousetrap is used as the help-tool to listen to keystrokes

Added currentDiffIndex getter to store that holds
the index of currently active diff file in the list

Instead of computing it on the component, we will take advantage of it
being available for all components in DiffsApp

Testing keyboard navigation and jumpToFile()

3 files changed, 227 insertions, 36 deletions

diff --git a/spec/javascripts/diffs/components/app_spec.js b/spec/javascripts/diffs/components/app_spec.js
index 5abdfe695d0..bce6113f75a 100644
--- a/spec/javascripts/diffs/components/app_spec.js
+++ b/spec/javascripts/diffs/components/app_spec.js
@@ -4,12 +4,13 @@ import { TEST_HOST } from 'spec/test_constants';
 import App from '~/diffs/components/app.vue';
 import NoChanges from '~/diffs/components/no_changes.vue';
 import DiffFile from '~/diffs/components/diff_file.vue';
+import Mousetrap from 'mousetrap';
 import createDiffsStore from '../create_diffs_store';
 
 describe('diffs/components/app', () => {
   const oldMrTabs = window.mrTabs;
   let store;
-  let vm;
+  let wrapper;
 
   function createComponent(props = {}, extendStore = () => {}) {
     const localVue = createLocalVue();
@@ -21,7 +22,7 @@ describe('diffs/components/app', () => {
 
     extendStore(store);
 
-    vm = shallowMount(localVue.extend(App), {
+    wrapper = shallowMount(localVue.extend(App), {
       localVue,
       propsData: {
         endpoint: `${TEST_HOST}/diff/endpoint`,
@@ -38,7 +39,6 @@ describe('diffs/components/app', () => {
     // setup globals (needed for component to mount :/)
     window.mrTabs = jasmine.createSpyObj('mrTabs', ['resetViewContainer']);
     window.mrTabs.expandViewContainer = jasmine.createSpy();
-    window.location.hash = 'ABC_123';
   });
 
   afterEach(() => {
@@ -46,25 +46,44 @@ describe('diffs/components/app', () => {
     window.mrTabs = oldMrTabs;
 
     // reset component
-    vm.destroy();
+    wrapper.destroy();
   });
 
   it('does not show commit info', () => {
     createComponent();
 
-    expect(vm.contains('.blob-commit-info')).toBe(false);
+    expect(wrapper.contains('.blob-commit-info')).toBe(false);
   });
 
-  it('sets highlighted row if hash exists in location object', done => {
-    createComponent({
-      shouldShow: true,
+  describe('row highlighting', () => {
+    beforeEach(() => {
+      window.location.hash = 'ABC_123';
     });
 
-    // Component uses $nextTick so we wait until that has finished
-    setTimeout(() => {
-      expect(store.state.diffs.highlightedRow).toBe('ABC_123');
+    it('sets highlighted row if hash exists in location object', done => {
+      createComponent({
+        shouldShow: true,
+      });
 
-      done();
+      // Component uses $nextTick so we wait until that has finished
+      setTimeout(() => {
+        expect(store.state.diffs.highlightedRow).toBe('ABC_123');
+
+        done();
+      });
+    });
+
+    it('marks current diff file based on currently highlighted row', done => {
+      createComponent({
+        shouldShow: true,
+      });
+
+      // Component uses $nextTick so we wait until that has finished
+      setTimeout(() => {
+        expect(store.state.diffs.currentDiffFileId).toBe('ABC');
+
+        done();
+      });
     });
   });
 
@@ -76,7 +95,7 @@ describe('diffs/components/app', () => {
     it('sets initial width when no localStorage has been set', () => {
       createComponent();
 
-      expect(vm.vm.treeWidth).toEqual(320);
+      expect(wrapper.vm.treeWidth).toEqual(320);
     });
 
     it('sets initial width to localStorage size', () => {
@@ -84,13 +103,26 @@ describe('diffs/components/app', () => {
 
       createComponent();
 
-      expect(vm.vm.treeWidth).toEqual(200);
+      expect(wrapper.vm.treeWidth).toEqual(200);
     });
 
     it('sets width of tree list', () => {
       createComponent();
 
-      expect(vm.find('.js-diff-tree-list').element.style.width).toEqual('320px');
+      expect(wrapper.find('.js-diff-tree-list').element.style.width).toEqual('320px');
+    });
+  });
+
+  it('marks current diff file based on currently highlighted row', done => {
+    createComponent({
+      shouldShow: true,
+    });
+
+    // Component uses $nextTick so we wait until that has finished
+    setTimeout(() => {
+      expect(store.state.diffs.currentDiffFileId).toBe('ABC');
+
+      done();
     });
   });
 
@@ -98,7 +130,7 @@ describe('diffs/components/app', () => {
     it('renders empty state when no diff files exist', () => {
       createComponent();
 
-      expect(vm.contains(NoChanges)).toBe(true);
+      expect(wrapper.contains(NoChanges)).toBe(true);
     });
 
     it('does not render empty state when diff files exist', () => {
@@ -108,8 +140,8 @@ describe('diffs/components/app', () => {
         });
       });
 
-      expect(vm.contains(NoChanges)).toBe(false);
-      expect(vm.findAll(DiffFile).length).toBe(1);
+      expect(wrapper.contains(NoChanges)).toBe(false);
+      expect(wrapper.findAll(DiffFile).length).toBe(1);
     });
 
     it('does not render empty state when versions match', () => {
@@ -118,7 +150,161 @@ describe('diffs/components/app', () => {
         store.state.diffs.mergeRequestDiff = { version_index: 1 };
       });
 
-      expect(vm.contains(NoChanges)).toBe(false);
+      expect(wrapper.contains(NoChanges)).toBe(false);
+    });
+  });
+
+  describe('keyboard shortcut navigation', () => {
+    const mappings = {
+      '[': -1,
+      k: -1,
+      ']': +1,
+      j: +1,
+    };
+    let spy;
+
+    describe('visible app', () => {
+      beforeEach(() => {
+        spy = jasmine.createSpy('spy');
+
+        createComponent({
+          shouldShow: true,
+        });
+        wrapper.setMethods({
+          jumpToFile: spy,
+        });
+      });
+
+      it('calls `jumpToFile()` with correct parameter whenever pre-defined key is pressed', done => {
+        wrapper.vm
+          .$nextTick()
+          .then(() => {
+            Object.keys(mappings).forEach(function(key) {
+              Mousetrap.trigger(key);
+
+              expect(spy.calls.mostRecent().args).toEqual([mappings[key]]);
+            });
+
+            expect(spy.calls.count()).toEqual(Object.keys(mappings).length);
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+
+      it('does not call `jumpToFile()` when unknown key is pressed', done => {
+        wrapper.vm
+          .$nextTick()
+          .then(() => {
+            Mousetrap.trigger('d');
+
+            expect(spy).not.toHaveBeenCalled();
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+
+    describe('hideen app', () => {
+      beforeEach(() => {
+        spy = jasmine.createSpy('spy');
+
+        createComponent({
+          shouldShow: false,
+        });
+        wrapper.setMethods({
+          jumpToFile: spy,
+        });
+      });
+
+      it('stops calling `jumpToFile()` when application is hidden', done => {
+        wrapper.vm
+          .$nextTick()
+          .then(() => {
+            Object.keys(mappings).forEach(function(key) {
+              Mousetrap.trigger(key);
+
+              expect(spy).not.toHaveBeenCalled();
+            });
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+  });
+
+  describe('jumpToFile', () => {
+    let spy;
+
+    beforeEach(() => {
+      spy = jasmine.createSpy();
+
+      createComponent({}, () => {
+        store.state.diffs.diffFiles = [
+          { file_hash: '111', file_path: '111.js' },
+          { file_hash: '222', file_path: '222.js' },
+          { file_hash: '333', file_path: '333.js' },
+        ];
+      });
+
+      wrapper.setMethods({
+        scrollToFile: spy,
+      });
+    });
+
+    afterEach(() => {
+      wrapper.destroy();
+    });
+
+    it('jumps to next and previous files in the list', done => {
+      wrapper.vm
+        .$nextTick()
+        .then(() => {
+          wrapper.vm.jumpToFile(+1);
+
+          expect(spy.calls.mostRecent().args).toEqual(['222.js']);
+          store.state.diffs.currentDiffFileId = '222';
+          wrapper.vm.jumpToFile(+1);
+
+          expect(spy.calls.mostRecent().args).toEqual(['333.js']);
+          store.state.diffs.currentDiffFileId = '333';
+          wrapper.vm.jumpToFile(-1);
+
+          expect(spy.calls.mostRecent().args).toEqual(['222.js']);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('does not jump to previous file from the first one', done => {
+      wrapper.vm
+        .$nextTick()
+        .then(() => {
+          store.state.diffs.currentDiffFileId = '333';
+
+          expect(wrapper.vm.currentDiffIndex).toEqual(2);
+
+          wrapper.vm.jumpToFile(+1);
+
+          expect(wrapper.vm.currentDiffIndex).toEqual(2);
+          expect(spy).not.toHaveBeenCalled();
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('does not jump to next file from the last one', done => {
+      wrapper.vm
+        .$nextTick()
+        .then(() => {
+          expect(wrapper.vm.currentDiffIndex).toEqual(0);
+
+          wrapper.vm.jumpToFile(-1);
+
+          expect(wrapper.vm.currentDiffIndex).toEqual(0);
+          expect(spy).not.toHaveBeenCalled();
+        })
+        .then(done)
+        .catch(done.fail);
     });
   });
 });
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index acff80bca62..e47c7906fcb 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -100,9 +100,10 @@ describe('DiffsStoreActions', () => {
   });
 
   describe('setHighlightedRow', () => {
-    it('should set lineHash and fileHash of highlightedRow', () => {
+    it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => {
       testAction(setHighlightedRow, 'ABC_123', {}, [
         { type: types.SET_HIGHLIGHTED_ROW, payload: 'ABC_123' },
+        { type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'ABC' },
       ]);
     });
   });
@@ -713,22 +714,6 @@ describe('DiffsStoreActions', () => {
 
       expect(commit).toHaveBeenCalledWith(types.UPDATE_CURRENT_DIFF_FILE_ID, 'test');
     });
-
-    it('resets currentDiffId after timeout', () => {
-      const state = {
-        treeEntries: {
-          path: {
-            fileHash: 'test',
-          },
-        },
-      };
-
-      scrollToFile({ state, commit }, 'path');
-
-      jasmine.clock().tick(1000);
-
-      expect(commit.calls.argsFor(1)).toEqual([types.UPDATE_CURRENT_DIFF_FILE_ID, '']);
-    });
   });
 
   describe('toggleShowTreeList', () => {
diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js
index 0ab88e6b2aa..eab5703dfb2 100644
--- a/spec/javascripts/diffs/store/getters_spec.js
+++ b/spec/javascripts/diffs/store/getters_spec.js
@@ -270,4 +270,24 @@ describe('Diffs Module Getters', () => {
       expect(getters.diffFilesLength(localState)).toBe(2);
     });
   });
+
+  describe('currentDiffIndex', () => {
+    it('returns index of currently selected diff in diffList', () => {
+      localState.diffFiles = [{ file_hash: '111' }, { file_hash: '222' }, { file_hash: '333' }];
+      localState.currentDiffFileId = '222';
+
+      expect(getters.currentDiffIndex(localState)).toEqual(1);
+
+      localState.currentDiffFileId = '333';
+
+      expect(getters.currentDiffIndex(localState)).toEqual(2);
+    });
+
+    it('returns 0 if no diff is selected yet or diff is not found', () => {
+      localState.diffFiles = [{ file_hash: '111' }, { file_hash: '222' }, { file_hash: '333' }];
+      localState.currentDiffFileId = '';
+
+      expect(getters.currentDiffIndex(localState)).toEqual(0);
+    });
+  });
 });