Add discussion_keyboard_navigator component59590-keyboard-shortcut-for-jump-to-next-unresolved-discussion

**Why?**

This should only be applied in the mr view so we
can't put this logic in the `notes_app`, which is
reused by issues. Also, it helps to isolate this
behavior into it's own component for unit testing.

Signed-off-by: jboyson1 <jboyson@gitlab.com>

5 files changed, 153 insertions, 88 deletions

diff --git a/app/assets/javascripts/mr_notes/init_notes.js b/app/assets/javascripts/mr_notes/init_notes.js
index 842a209a545..8caac68e0d4 100644
--- a/app/assets/javascripts/mr_notes/init_notes.js
+++ b/app/assets/javascripts/mr_notes/init_notes.js
@@ -3,6 +3,7 @@ import Vue from 'vue';
 import { mapActions, mapState, mapGetters } from 'vuex';
 import store from 'ee_else_ce/mr_notes/stores';
 import notesApp from '../notes/components/notes_app.vue';
+import discussionKeyboardNavigator from '../notes/components/discussion_keyboard_navigator.vue';
 
 export default () => {
   // eslint-disable-next-line no-new
@@ -56,15 +57,19 @@ export default () => {
       },
     },
     render(createElement) {
-      return createElement('notes-app', {
-        props: {
-          noteableData: this.noteableData,
-          notesData: this.notesData,
-          userData: this.currentUserData,
-          shouldShow: this.activeTab === 'show',
-          helpPagePath: this.helpPagePath,
-        },
-      });
+      const isDiffView = this.activeTab === 'diffs';
+
+      return createElement(discussionKeyboardNavigator, { props: { isDiffView } }, [
+        createElement('notes-app', {
+          props: {
+            noteableData: this.noteableData,
+            notesData: this.notesData,
+            userData: this.currentUserData,
+            shouldShow: this.activeTab === 'show',
+            helpPagePath: this.helpPagePath,
+          },
+        }),
+      ]);
     },
   });
 };
diff --git a/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue b/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
new file mode 100644
index 00000000000..5fc2b6ba04c
--- /dev/null
+++ b/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
@@ -0,0 +1,47 @@
+<script>
+/* global Mousetrap */
+import 'mousetrap';
+import { mapGetters, mapActions } from 'vuex';
+import discussionNavigation from '~/notes/mixins/discussion_navigation';
+
+export default {
+  mixins: [discussionNavigation],
+  props: {
+    isDiffView: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  data() {
+    return {
+      currentDiscussionId: null,
+    };
+  },
+  computed: {
+    ...mapGetters(['nextUnresolvedDiscussionId', 'previousUnresolvedDiscussionId']),
+  },
+  mounted() {
+    Mousetrap.bind('n', () => this.jumpToNextDiscussion());
+    Mousetrap.bind('p', () => this.jumpToPreviousDiscussion());
+  },
+  methods: {
+    ...mapActions(['expandDiscussion']),
+    jumpToNextDiscussion() {
+      const nextId = this.nextUnresolvedDiscussionId(this.currentDiscussionId, this.isDiffView);
+
+      this.jumpToDiscussion(nextId);
+      this.currentDiscussionId = nextId;
+    },
+    jumpToPreviousDiscussion() {
+      const prevId = this.previousUnresolvedDiscussionId(this.currentDiscussionId, this.isDiffView);
+
+      this.jumpToDiscussion(prevId);
+      this.currentDiscussionId = prevId;
+    },
+  },
+  render() {
+    return this.$slots.default;
+  },
+};
+</script>
diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index ec0aa191513..a0695f9e191 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -1,6 +1,4 @@
 <script>
-/* global Mousetrap */
-import 'mousetrap';
 import { __ } from '~/locale';
 import { mapGetters, mapActions } from 'vuex';
 import { getLocationHash } from '../../lib/utils/url_utility';
@@ -17,7 +15,6 @@ import placeholderSystemNote from '../../vue_shared/components/notes/placeholder
 import skeletonLoadingContainer from '../../vue_shared/components/notes/skeleton_note.vue';
 import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user';
 import initUserPopovers from '../../user_popovers';
-import discussionNavigation from '../mixins/discussion_navigation';
 
 export default {
   name: 'NotesApp',
@@ -31,7 +28,6 @@ export default {
     skeletonLoadingContainer,
     discussionFilterNote,
   },
-  mixins: [discussionNavigation],
   props: {
     noteableData: {
       type: Object,
@@ -61,7 +57,6 @@ export default {
     return {
       isFetching: false,
       currentFilter: null,
-      currentDiscussionId: null,
     };
   },
   computed: {
@@ -74,8 +69,6 @@ export default {
       'commentsDisabled',
       'getNoteableData',
       'userCanReply',
-      'nextUnresolvedDiscussionId',
-      'previousUnresolvedDiscussionId',
     ]),
     noteableType() {
       return this.noteableData.noteableType;
@@ -117,9 +110,6 @@ export default {
     eventHub.$once('fetchNotesData', this.fetchNotes);
   },
   mounted() {
-    Mousetrap.bind('n', () => this.jumpToNextDiscussion());
-    Mousetrap.bind('p', () => this.jumpToPreviousDiscussion());
-
     if (this.shouldShow) {
       this.fetchNotes();
     }
@@ -211,21 +201,6 @@ export default {
         .then(() => this.$nextTick())
         .then(() => eventHub.$emit('startReplying', discussionId));
     },
-    jumpToNextDiscussion() {
-      const nextId = this.nextUnresolvedDiscussionId(this.currentDiscussionId, !this.shouldShow);
-
-      this.jumpToDiscussion(nextId);
-      this.currentDiscussionId = nextId;
-    },
-    jumpToPreviousDiscussion() {
-      const prevId = this.previousUnresolvedDiscussionId(
-        this.currentDiscussionId,
-        !this.shouldShow,
-      );
-
-      this.jumpToDiscussion(prevId);
-      this.currentDiscussionId = prevId;
-    },
   },
   systemNote: constants.SYSTEM_NOTE,
 };
diff --git a/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js b/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js
new file mode 100644
index 00000000000..6d50713999d
--- /dev/null
+++ b/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js
@@ -0,0 +1,77 @@
+/* global Mousetrap */
+import 'mousetrap';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import Vuex from 'vuex';
+import DiscussionKeyboardNavigator from '~/notes/components/discussion_keyboard_navigator.vue';
+import notesModule from '~/notes/stores/modules';
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+const NEXT_ID = 'abc123';
+const PREV_ID = 'def456';
+const NEXT_DIFF_ID = 'abc123_diff';
+const PREV_DIFF_ID = 'def456_diff';
+
+describe('notes/components/discussion_keyboard_navigator', () => {
+  let storeOptions;
+  let wrapper;
+  let store;
+
+  const createComponent = (options = {}) => {
+    store = new Vuex.Store(storeOptions);
+
+    wrapper = shallowMount(DiscussionKeyboardNavigator, {
+      localVue,
+      store,
+      ...options,
+    });
+
+    wrapper.vm.jumpToDiscussion = jest.fn();
+  };
+
+  beforeEach(() => {
+    const notes = notesModule();
+
+    notes.getters.nextUnresolvedDiscussionId = () => (currId, isDiff) =>
+      isDiff ? NEXT_DIFF_ID : NEXT_ID;
+    notes.getters.previousUnresolvedDiscussionId = () => (currId, isDiff) =>
+      isDiff ? PREV_DIFF_ID : PREV_ID;
+
+    storeOptions = {
+      modules: {
+        notes,
+      },
+    };
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+    storeOptions = null;
+    store = null;
+  });
+
+  describe.each`
+    isDiffView | expectedNextId  | expectedPrevId
+    ${true}    | ${NEXT_DIFF_ID} | ${PREV_DIFF_ID}
+    ${false}   | ${NEXT_ID}      | ${PREV_ID}
+  `('when isDiffView is $isDiffView', ({ isDiffView, expectedNextId, expectedPrevId }) => {
+    beforeEach(() => {
+      createComponent({ propsData: { isDiffView } });
+    });
+
+    it('calls jumpToNextDiscussion when pressing `n`', () => {
+      Mousetrap.trigger('n');
+
+      expect(wrapper.vm.jumpToDiscussion).toHaveBeenCalledWith(expectedNextId);
+      expect(wrapper.vm.currentDiscussionId).toEqual(expectedNextId);
+    });
+
+    it('calls jumpToPreviousDiscussion when pressing `p`', () => {
+      Mousetrap.trigger('p');
+
+      expect(wrapper.vm.jumpToDiscussion).toHaveBeenCalledWith(expectedPrevId);
+      expect(wrapper.vm.currentDiscussionId).toEqual(expectedPrevId);
+    });
+  });
+});
diff --git a/spec/frontend/notes/components/note_app_spec.js b/spec/frontend/notes/components/note_app_spec.js
index 0ba2861f1f8..ff833d2c899 100644
--- a/spec/frontend/notes/components/note_app_spec.js
+++ b/spec/frontend/notes/components/note_app_spec.js
@@ -1,20 +1,15 @@
-/* global Mousetrap */
-import 'mousetrap';
 import $ from 'helpers/jquery';
-import Vuex from 'vuex';
+import Vue from 'vue';
 import { mount, createLocalVue } from '@vue/test-utils';
 import NotesApp from '~/notes/components/notes_app.vue';
 import service from '~/notes/services/notes_service';
-import notesModule from '~/notes/stores/modules';
+import createStore from '~/notes/stores';
 import '~/behaviors/markdown/render_gfm';
 import { setTestTimeout } from 'helpers/timeout';
 // TODO: use generated fixture (https://gitlab.com/gitlab-org/gitlab-ce/issues/62491)
 import * as mockData from '../../../javascripts/notes/mock_data';
 
-const localVue = createLocalVue();
-localVue.use(Vuex);
-
-const originalInterceptors = [...localVue.http.interceptors];
+const originalInterceptors = [...Vue.http.interceptors];
 
 const emptyResponseInterceptor = (request, next) => {
   next(
@@ -30,8 +25,6 @@ describe('note_app', () => {
   let mountComponent;
   let wrapper;
   let store;
-  const NEXT_ID = 'abc123';
-  const PREV_ID = 'def456';
 
   /**
    * waits for fetchNotes() to complete
@@ -52,16 +45,14 @@ describe('note_app', () => {
   beforeEach(() => {
     $('body').attr('data-page', 'projects:merge_requests:show');
 
-    const notesOptions = notesModule();
-    notesOptions.getters.nextUnresolvedDiscussionId = jest.fn(() => () => NEXT_ID);
-    notesOptions.getters.previousUnresolvedDiscussionId = jest.fn(() => () => PREV_ID);
-    store = new Vuex.Store(notesOptions);
+    store = createStore();
     mountComponent = data => {
       const propsData = data || {
         noteableData: mockData.noteableDataMock,
         notesData: mockData.notesDataMock,
         userData: mockData.userDataMock,
       };
+      const localVue = createLocalVue();
 
       return mount(
         {
@@ -83,12 +74,12 @@ describe('note_app', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    localVue.http.interceptors = [...originalInterceptors];
+    Vue.http.interceptors = [...originalInterceptors];
   });
 
   describe('set data', () => {
     beforeEach(() => {
-      localVue.http.interceptors.push(emptyResponseInterceptor);
+      Vue.http.interceptors.push(emptyResponseInterceptor);
       wrapper = mountComponent();
       return waitForDiscussionsRequest();
     });
@@ -114,7 +105,7 @@ describe('note_app', () => {
     beforeEach(() => {
       setFixtures('<div class="js-discussions-count"></div>');
 
-      localVue.http.interceptors.push(mockData.individualNoteInterceptor);
+      Vue.http.interceptors.push(mockData.individualNoteInterceptor);
       wrapper = mountComponent();
       return waitForDiscussionsRequest();
     });
@@ -173,7 +164,7 @@ describe('note_app', () => {
 
   describe('while fetching data', () => {
     beforeEach(() => {
-      localVue.http.interceptors.push(emptyResponseInterceptor);
+      Vue.http.interceptors.push(emptyResponseInterceptor);
       wrapper = mountComponent();
     });
 
@@ -194,7 +185,7 @@ describe('note_app', () => {
   describe('update note', () => {
     describe('individual note', () => {
       beforeEach(() => {
-        localVue.http.interceptors.push(mockData.individualNoteInterceptor);
+        Vue.http.interceptors.push(mockData.individualNoteInterceptor);
         jest.spyOn(service, 'updateNote');
         wrapper = mountComponent();
         return waitForDiscussionsRequest().then(() => {
@@ -216,7 +207,7 @@ describe('note_app', () => {
 
     describe('discussion note', () => {
       beforeEach(() => {
-        localVue.http.interceptors.push(mockData.discussionNoteInterceptor);
+        Vue.http.interceptors.push(mockData.discussionNoteInterceptor);
         jest.spyOn(service, 'updateNote');
         wrapper = mountComponent();
         return waitForDiscussionsRequest().then(() => {
@@ -239,7 +230,7 @@ describe('note_app', () => {
 
   describe('new note form', () => {
     beforeEach(() => {
-      localVue.http.interceptors.push(mockData.individualNoteInterceptor);
+      Vue.http.interceptors.push(mockData.individualNoteInterceptor);
       wrapper = mountComponent();
       return waitForDiscussionsRequest();
     });
@@ -269,7 +260,7 @@ describe('note_app', () => {
 
   describe('edit form', () => {
     beforeEach(() => {
-      localVue.http.interceptors.push(mockData.individualNoteInterceptor);
+      Vue.http.interceptors.push(mockData.individualNoteInterceptor);
       wrapper = mountComponent();
       return waitForDiscussionsRequest();
     });
@@ -278,7 +269,7 @@ describe('note_app', () => {
       wrapper.find('.js-note-edit').trigger('click');
       const { markdownDocsPath } = mockData.notesDataMock;
 
-      return localVue.nextTick().then(() => {
+      return Vue.nextTick().then(() => {
         expect(
           wrapper
             .find(`.edit-note a[href="${markdownDocsPath}"]`)
@@ -297,7 +288,7 @@ describe('note_app', () => {
 
   describe('emoji awards', () => {
     beforeEach(() => {
-      localVue.http.interceptors.push(emptyResponseInterceptor);
+      Vue.http.interceptors.push(emptyResponseInterceptor);
       wrapper = mountComponent();
       return waitForDiscussionsRequest();
     });
@@ -310,14 +301,10 @@ describe('note_app', () => {
         },
       });
       const toggleAwardAction = jest.fn().mockName('toggleAward');
-      const setLoadingState = jest.fn().mockName('setLoadingState');
-      const setNotesFetchedState = jest.fn().mockName('setNotesFetchedState');
       wrapper.vm.$store.hotUpdate({
         actions: {
           toggleAward: toggleAwardAction,
           stopPolling() {},
-          setLoadingState,
-          setNotesFetchedState,
         },
       });
 
@@ -332,30 +319,4 @@ describe('note_app', () => {
       });
     });
   });
-
-  describe('keyboard shortcuts', () => {
-    let vm;
-
-    beforeEach(() => {
-      localVue.http.interceptors.push(mockData.discussionNoteInterceptor);
-      wrapper = mountComponent();
-
-      // eslint-disable-next-line prefer-destructuring
-      vm = wrapper.find(NotesApp).vm;
-      vm.jumpToDiscussion = jest.fn();
-      return waitForDiscussionsRequest();
-    });
-
-    it('calls jumpToNextDiscussion when pressing `n`', () => {
-      Mousetrap.trigger('n');
-      expect(vm.jumpToDiscussion).toHaveBeenCalledWith(NEXT_ID);
-      expect(vm.currentDiscussionId).toEqual(NEXT_ID);
-    });
-
-    it('calls jumpToPreviousDiscussion when pressing `p`', () => {
-      Mousetrap.trigger('p');
-      expect(vm.jumpToDiscussion).toHaveBeenCalledWith(PREV_ID);
-      expect(vm.currentDiscussionId).toEqual(PREV_ID);
-    });
-  });
 });