diff --git a/app/assets/javascripts/merge_request_tabs.js.es6 b/app/assets/javascripts/merge_request_tabs.js.es6 index cc049e004771c07b2cc9fb631209c8b76f4c277a..5ac3da574b5c192336b8a59c58656c0048bbc0bd 100644 --- a/app/assets/javascripts/merge_request_tabs.js.es6 +++ b/app/assets/javascripts/merge_request_tabs.js.es6 @@ -102,9 +102,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 b07e62a8c30330287f3663c6f89fd8a4b8b7aac6..ded683f2ca1d278dd7553c3782e1f365cbc804ad 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 0000000000000000000000000000000000000000..79a54429ee88ea6fb7fec82bef5365a9761d86d2 --- /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 3810991f104632867c1966f3945c80cdd9742fe4..5b0c124962c06ac39f5d4ebd8f6c764b78e36c63 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 0000000000000000000000000000000000000000..66e4fbd6304814421daf965da74563e7ca357122 --- /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(); + }); + }); + }); +});