Commit e55c7a9a authored by Paul Slaughter's avatar Paul Slaughter

Add key unbinds to DiscussionKeyboardNavigator

Also adds comment to explain why this works on the diff-tab
when it's only used in `notes-app`.

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/30144#note_204066538
parent 04956155
...@@ -59,6 +59,10 @@ export default () => { ...@@ -59,6 +59,10 @@ export default () => {
render(createElement) { render(createElement) {
const isDiffView = this.activeTab === 'diffs'; 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 } }, [ return createElement(discussionKeyboardNavigator, { props: { isDiffView } }, [
createElement('notes-app', { createElement('notes-app', {
props: { props: {
......
...@@ -25,6 +25,10 @@ export default { ...@@ -25,6 +25,10 @@ export default {
Mousetrap.bind('n', () => this.jumpToNextDiscussion()); Mousetrap.bind('n', () => this.jumpToNextDiscussion());
Mousetrap.bind('p', () => this.jumpToPreviousDiscussion()); Mousetrap.bind('p', () => this.jumpToPreviousDiscussion());
}, },
beforeDestroy() {
Mousetrap.unbind('n');
Mousetrap.unbind('p');
},
methods: { methods: {
...mapActions(['expandDiscussion']), ...mapActions(['expandDiscussion']),
jumpToNextDiscussion() { jumpToNextDiscussion() {
......
...@@ -74,4 +74,31 @@ describe('notes/components/discussion_keyboard_navigator', () => { ...@@ -74,4 +74,31 @@ describe('notes/components/discussion_keyboard_navigator', () => {
expect(wrapper.vm.currentDiscussionId).toEqual(expectedPrevId); 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();
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment