Add kbd shortcuts for discussion navigation

Add keyboard shortcuts `p` and `n` to navigate duscussions.

5 files changed, 80 insertions, 11 deletions

diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 8434ef5cc7a..6695d9fe96c 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -109,7 +109,7 @@ export const toggleLineDiscussions = ({ commit }, options) => {
 export const renderFileForDiscussionId = ({ commit, rootState, state }, discussionId) => {
   const discussion = rootState.notes.discussions.find(d => d.id === discussionId);
 
-  if (discussion) {
+  if (discussion && discussion.diff_file) {
     const file = state.diffFiles.find(f => f.file_hash === discussion.diff_file.file_hash);
 
     if (file) {
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/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js
index 8aa8f5037b3..52410f18d4a 100644
--- a/app/assets/javascripts/notes/stores/getters.js
+++ b/app/assets/javascripts/notes/stores/getters.js
@@ -183,6 +183,15 @@ export const nextUnresolvedDiscussionId = (state, getters) => (discussionId, dif
   return slicedIds.length ? idsOrdered.slice(currentIndex + 1, currentIndex + 2)[0] : idsOrdered[0];
 };
 
+export const previousUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => {
+  const idsOrdered = getters.unresolvedDiscussionsIdsOrdered(diffOrder);
+  const currentIndex = idsOrdered.indexOf(discussionId);
+  const slicedIds = idsOrdered.slice(currentIndex - 1, currentIndex);
+
+  // Get the last ID if there is none after the currentIndex
+  return slicedIds.length ? slicedIds[0] : idsOrdered[idsOrdered.length - 1];
+};
+
 // @param {Boolean} diffOrder - is ordered by diff?
 export const firstUnresolvedDiscussionId = (state, getters) => diffOrder => {
   if (diffOrder) {
diff --git a/app/views/help/_shortcuts.html.haml b/app/views/help/_shortcuts.html.haml
index 46d7c367aa7..0624f66b3a5 100644
--- a/app/views/help/_shortcuts.html.haml
+++ b/app/views/help/_shortcuts.html.haml
@@ -332,7 +332,7 @@
                   %td.shortcut
                     %kbd l
                   %td Change Label
-              %tbody.hidden-shortcut{ style: 'display:none' }
+              %tbody.hidden-shortcut.merge_requests{ style: 'display:none' }
                 %tr
                   %th
                   %th Merge Requests
@@ -368,6 +368,14 @@
                     \/
                     %kbd k
                   %td Move to previous file
+                %tr
+                  %td.shortcut
+                    %kbd n
+                  %td= _("Move to next unresolved discussion")
+                %tr
+                  %td.shortcut
+                    %kbd p
+                  %td= _("Move to previous unresolved discussion")
               %tbody.hidden-shortcut{ style: 'display:none' }
                 %tr
                   %th