Merge branch 'fe-add-unbinds-to-discussion-keyboard-navigator' into 'master'

Add key unbinds to DiscussionKeyboardNavigator

See merge request gitlab-org/gitlab-ce!31857

2 files changed, 8 insertions, 0 deletions

diff --git a/app/assets/javascripts/mr_notes/init_notes.js b/app/assets/javascripts/mr_notes/init_notes.js
index 8caac68e0d4..622db360d1f 100644
--- a/app/assets/javascripts/mr_notes/init_notes.js
+++ b/app/assets/javascripts/mr_notes/init_notes.js
@@ -59,6 +59,10 @@ export default () => {
     render(createElement) {
       const isDiffView = this.activeTab === 'diffs';
 
+      // NOTE: Even though `discussionKeyboardNavigator` is added to the `notes-app`,
+      // it adds a global key listener so it works on the diffs tab as well.
+      // If we create a single Vue app for all of the MR tabs, we should move this
+      // up the tree, to the root.
       return createElement(discussionKeyboardNavigator, { props: { isDiffView } }, [
         createElement('notes-app', {
           props: {
diff --git a/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue b/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
index 5fc2b6ba04c..7fbfe8eebb2 100644
--- a/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
+++ b/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
@@ -25,6 +25,10 @@ export default {
     Mousetrap.bind('n', () => this.jumpToNextDiscussion());
     Mousetrap.bind('p', () => this.jumpToPreviousDiscussion());
   },
+  beforeDestroy() {
+    Mousetrap.unbind('n');
+    Mousetrap.unbind('p');
+  },
   methods: {
     ...mapActions(['expandDiscussion']),
     jumpToNextDiscussion() {