diff options
author | Alfredo Sumaran <alfredo@gitlab.com> | 2017-02-15 17:52:01 +0000 |
---|---|---|
committer | Felipe Artur <felipefac@gmail.com> | 2017-02-17 18:01:46 -0200 |
commit | 772b37af122de100ff2db000a3a150ac6ac48107 (patch) | |
tree | 16087bc288c9530775a1778cb1a0b2ebe9ba9eb5 | |
parent | 755745e86029830a3f23154d9e6e5bc9f22251b5 (diff) | |
download | gitlab-ce-772b37af122de100ff2db000a3a150ac6ac48107.tar.gz |
Merge branch '27922-cmd-click-todo-doesn-t-work' into 'master'
Fix regression where cmd-click stopped working for todos and merge request tabs
Closes #27922
See merge request !9115
-rw-r--r-- | app/assets/javascripts/merge_request_tabs.js.es6 | 5 | ||||
-rw-r--r-- | app/assets/javascripts/todos.js.es6 | 17 | ||||
-rw-r--r-- | changelogs/unreleased/27922-cmd-click-todo-doesn-t-work.yml | 5 | ||||
-rw-r--r-- | spec/javascripts/merge_request_tabs_spec.js | 40 | ||||
-rw-r--r-- | spec/javascripts/todos_spec.js | 63 |
5 files changed, 112 insertions, 18 deletions
diff --git a/app/assets/javascripts/merge_request_tabs.js.es6 b/app/assets/javascripts/merge_request_tabs.js.es6 index 317ee658b84..0f79370afa8 100644 --- a/app/assets/javascripts/merge_request_tabs.js.es6 +++ b/app/assets/javascripts/merge_request_tabs.js.es6 @@ -103,9 +103,10 @@ require('./flash'); } clickTab(e) { - if (e.target && gl.utils.isMetaClick(e)) { - const targetLink = e.target.getAttribute('href'); + if (e.currentTarget && gl.utils.isMetaClick(e)) { + const targetLink = e.currentTarget.getAttribute('href'); e.stopImmediatePropagation(); + e.preventDefault(); window.open(targetLink, '_blank'); } } diff --git a/app/assets/javascripts/todos.js.es6 b/app/assets/javascripts/todos.js.es6 index b07e62a8c30..ded683f2ca1 100644 --- a/app/assets/javascripts/todos.js.es6 +++ b/app/assets/javascripts/todos.js.es6 @@ -147,24 +147,21 @@ goToTodoUrl(e) { const todoLink = this.dataset.url; - let targetLink = e.target.getAttribute('href'); - - if (e.target.tagName === 'IMG') { // See if clicked target was Avatar - targetLink = e.target.parentElement.getAttribute('href'); // Parent of Avatar is link - } if (!todoLink) { return; } if (gl.utils.isMetaClick(e)) { + const windowTarget = '_blank'; + const selected = e.target; e.preventDefault(); - // Meta-Click on username leads to different URL than todoLink. - // Turbolinks can resolve that URL, but window.open requires URL manually. - if (targetLink !== todoLink) { - return window.open(targetLink, '_blank'); + + if (selected.tagName === 'IMG') { + const avatarUrl = selected.parentElement.getAttribute('href'); + return window.open(avatarUrl, windowTarget); } else { - return window.open(todoLink, '_blank'); + return window.open(todoLink, windowTarget); } } else { return gl.utils.visitUrl(todoLink); diff --git a/changelogs/unreleased/27922-cmd-click-todo-doesn-t-work.yml b/changelogs/unreleased/27922-cmd-click-todo-doesn-t-work.yml new file mode 100644 index 00000000000..79a54429ee8 --- /dev/null +++ b/changelogs/unreleased/27922-cmd-click-todo-doesn-t-work.yml @@ -0,0 +1,5 @@ +--- +title: Fix regression where cmd-click stopped working for todos and merge request + tabs +merge_request: +author: diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 92a0f1c05f7..3c95fb0dfb6 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -62,19 +62,47 @@ require('vendor/jquery.scrollTo'); }); }); describe('#opensInNewTab', function () { - var commitsLink; var tabUrl; + var windowTarget = '_blank'; beforeEach(function () { - commitsLink = '.commits-tab li a'; - tabUrl = $(commitsLink).attr('href'); + loadFixtures('merge_requests/merge_request_with_task_list.html.raw'); + + tabUrl = $('.commits-tab a').attr('href'); spyOn($.fn, 'attr').and.returnValue(tabUrl); }); + + describe('meta click', () => { + beforeEach(function () { + spyOn(gl.utils, 'isMetaClick').and.returnValue(true); + }); + + it('opens page when commits link is clicked', function () { + spyOn(window, 'open').and.callFake(function (url, name) { + expect(url).toEqual(tabUrl); + expect(name).toEqual(windowTarget); + }); + + this.class.bindEvents(); + document.querySelector('.merge-request-tabs .commits-tab a').click(); + }); + + it('opens page when commits badge is clicked', function () { + spyOn(window, 'open').and.callFake(function (url, name) { + expect(url).toEqual(tabUrl); + expect(name).toEqual(windowTarget); + }); + + this.class.bindEvents(); + document.querySelector('.merge-request-tabs .commits-tab a .badge').click(); + }); + }); + it('opens page tab in a new browser tab with Ctrl+Click - Windows/Linux', function () { spyOn(window, 'open').and.callFake(function (url, name) { expect(url).toEqual(tabUrl); - expect(name).toEqual('_blank'); + expect(name).toEqual(windowTarget); }); this.class.clickTab({ @@ -87,7 +115,7 @@ require('vendor/jquery.scrollTo'); it('opens page tab in a new browser tab with Cmd+Click - Mac', function () { spyOn(window, 'open').and.callFake(function (url, name) { expect(url).toEqual(tabUrl); - expect(name).toEqual('_blank'); + expect(name).toEqual(windowTarget); }); this.class.clickTab({ @@ -100,7 +128,7 @@ require('vendor/jquery.scrollTo'); it('opens page tab in a new browser tab with Middle-click - Mac/PC', function () { spyOn(window, 'open').and.callFake(function (url, name) { expect(url).toEqual(tabUrl); - expect(name).toEqual('_blank'); + expect(name).toEqual(windowTarget); }); this.class.clickTab({ diff --git a/spec/javascripts/todos_spec.js b/spec/javascripts/todos_spec.js new file mode 100644 index 00000000000..66e4fbd6304 --- /dev/null +++ b/spec/javascripts/todos_spec.js @@ -0,0 +1,63 @@ +require('~/todos'); +require('~/lib/utils/common_utils'); + +describe('Todos', () => { + preloadFixtures('todos/todos.html.raw'); + let todoItem; + + beforeEach(() => { + loadFixtures('todos/todos.html.raw'); + todoItem = document.querySelector('.todos-list .todo'); + + return new gl.Todos(); + }); + + describe('goToTodoUrl', () => { + it('opens the todo url', (done) => { + const todoLink = todoItem.dataset.url; + + spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + expect(url).toEqual(todoLink); + done(); + }); + + todoItem.click(); + }); + + describe('meta click', () => { + let visitUrlSpy; + + beforeEach(() => { + spyOn(gl.utils, 'isMetaClick').and.returnValue(true); + visitUrlSpy = spyOn(gl.utils, 'visitUrl').and.callFake(() => {}); + }); + + it('opens the todo url in another tab', (done) => { + const todoLink = todoItem.dataset.url; + + spyOn(window, 'open').and.callFake((url, target) => { + expect(todoLink).toEqual(url); + expect(target).toEqual('_blank'); + done(); + }); + + todoItem.click(); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + + it('opens the avatar\'s url in another tab when the avatar is clicked', (done) => { + const avatarImage = todoItem.querySelector('img'); + const avatarUrl = avatarImage.parentElement.getAttribute('href'); + + spyOn(window, 'open').and.callFake((url, target) => { + expect(avatarUrl).toEqual(url); + expect(target).toEqual('_blank'); + done(); + }); + + avatarImage.click(); + expect(visitUrlSpy).not.toHaveBeenCalled(); + }); + }); + }); +}); |