Automatic merge of gitlab-org/gitlab-ce master

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: {

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() {

spec/frontend/notes/components/discussion_keyboard_navigator_spec.js

@@ -74,4 +74,31 @@ describe('notes/components/discussion_keyboard_navigator', () => {
       expect(wrapper.vm.currentDiscussionId).toEqual(expectedPrevId);
     });
   });
+
+  describe('on destroy', () => {
+    beforeEach(() => {
+      jest.spyOn(Mousetrap, 'unbind');
+
+      createComponent();
+
+      wrapper.destroy();
+    });
+
+    it('unbinds keys', () => {
+      expect(Mousetrap.unbind).toHaveBeenCalledWith('n');
+      expect(Mousetrap.unbind).toHaveBeenCalledWith('p');
+    });
+
+    it('does not call jumpToNextDiscussion when pressing `n`', () => {
+      Mousetrap.trigger('n');
+
+      expect(wrapper.vm.jumpToDiscussion).not.toHaveBeenCalled();
+    });
+
+    it('does not call jumpToNextDiscussion when pressing `p`', () => {
+      Mousetrap.trigger('p');
+
+      expect(wrapper.vm.jumpToDiscussion).not.toHaveBeenCalled();
+    });
+  });
 });