diff options
author | Kushal Pandya <kushalspandya@gmail.com> | 2019-08-16 06:14:34 +0000 |
---|---|---|
committer | Kushal Pandya <kushalspandya@gmail.com> | 2019-08-16 06:14:34 +0000 |
commit | 2be32287f56c333734d6bc1c2fdd7498ea70d1e6 (patch) | |
tree | cb60aa88f03940ff4edec50ecdeebde5764f5279 | |
parent | 41f22c6b0f8a6525b015357cf869c1d4ae91144a (diff) | |
parent | e55c7a9a3572ad37a8b69cd2d3db9ab24b5ba5ab (diff) | |
download | gitlab-ce-2be32287f56c333734d6bc1c2fdd7498ea70d1e6.tar.gz |
Merge branch 'fe-add-unbinds-to-discussion-keyboard-navigator' into 'master'
Add key unbinds to DiscussionKeyboardNavigator
See merge request gitlab-org/gitlab-ce!31857
3 files changed, 35 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() { diff --git a/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js b/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js index 6d50713999d..8881bedf3cc 100644 --- a/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js +++ b/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(); + }); + }); }); |