Commit 6df388ea authored by Alfredo Sumaran's avatar Alfredo Sumaran

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
parents 798e03f0 25fec0f8
...@@ -103,9 +103,10 @@ require('./flash'); ...@@ -103,9 +103,10 @@ require('./flash');
} }
clickTab(e) { clickTab(e) {
if (e.target && gl.utils.isMetaClick(e)) { if (e.currentTarget && gl.utils.isMetaClick(e)) {
const targetLink = e.target.getAttribute('href'); const targetLink = e.currentTarget.getAttribute('href');
e.stopImmediatePropagation(); e.stopImmediatePropagation();
e.preventDefault();
window.open(targetLink, '_blank'); window.open(targetLink, '_blank');
} }
} }
......
...@@ -147,24 +147,21 @@ ...@@ -147,24 +147,21 @@
goToTodoUrl(e) { goToTodoUrl(e) {
const todoLink = this.dataset.url; 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) { if (!todoLink) {
return; return;
} }
if (gl.utils.isMetaClick(e)) { if (gl.utils.isMetaClick(e)) {
const windowTarget = '_blank';
const selected = e.target;
e.preventDefault(); e.preventDefault();
// Meta-Click on username leads to different URL than todoLink.
// Turbolinks can resolve that URL, but window.open requires URL manually. if (selected.tagName === 'IMG') {
if (targetLink !== todoLink) { const avatarUrl = selected.parentElement.getAttribute('href');
return window.open(targetLink, '_blank'); return window.open(avatarUrl, windowTarget);
} else { } else {
return window.open(todoLink, '_blank'); return window.open(todoLink, windowTarget);
} }
} else { } else {
return gl.utils.visitUrl(todoLink); return gl.utils.visitUrl(todoLink);
......
---
title: Fix regression where cmd-click stopped working for todos and merge request
tabs
merge_request:
author:
...@@ -62,19 +62,47 @@ require('vendor/jquery.scrollTo'); ...@@ -62,19 +62,47 @@ require('vendor/jquery.scrollTo');
}); });
}); });
describe('#opensInNewTab', function () { describe('#opensInNewTab', function () {
var commitsLink;
var tabUrl; var tabUrl;
var windowTarget = '_blank';
beforeEach(function () { beforeEach(function () {
commitsLink = '.commits-tab li a'; loadFixtures('merge_requests/merge_request_with_task_list.html.raw');
tabUrl = $(commitsLink).attr('href');
tabUrl = $('.commits-tab a').attr('href');
spyOn($.fn, 'attr').and.returnValue(tabUrl); 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 () { it('opens page tab in a new browser tab with Ctrl+Click - Windows/Linux', function () {
spyOn(window, 'open').and.callFake(function (url, name) { spyOn(window, 'open').and.callFake(function (url, name) {
expect(url).toEqual(tabUrl); expect(url).toEqual(tabUrl);
expect(name).toEqual('_blank'); expect(name).toEqual(windowTarget);
}); });
this.class.clickTab({ this.class.clickTab({
...@@ -87,7 +115,7 @@ require('vendor/jquery.scrollTo'); ...@@ -87,7 +115,7 @@ require('vendor/jquery.scrollTo');
it('opens page tab in a new browser tab with Cmd+Click - Mac', function () { it('opens page tab in a new browser tab with Cmd+Click - Mac', function () {
spyOn(window, 'open').and.callFake(function (url, name) { spyOn(window, 'open').and.callFake(function (url, name) {
expect(url).toEqual(tabUrl); expect(url).toEqual(tabUrl);
expect(name).toEqual('_blank'); expect(name).toEqual(windowTarget);
}); });
this.class.clickTab({ this.class.clickTab({
...@@ -100,7 +128,7 @@ require('vendor/jquery.scrollTo'); ...@@ -100,7 +128,7 @@ require('vendor/jquery.scrollTo');
it('opens page tab in a new browser tab with Middle-click - Mac/PC', function () { it('opens page tab in a new browser tab with Middle-click - Mac/PC', function () {
spyOn(window, 'open').and.callFake(function (url, name) { spyOn(window, 'open').and.callFake(function (url, name) {
expect(url).toEqual(tabUrl); expect(url).toEqual(tabUrl);
expect(name).toEqual('_blank'); expect(name).toEqual(windowTarget);
}); });
this.class.clickTab({ this.class.clickTab({
......
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();
});
});
});
});
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