Commit 699957ef authored by Sam Bigelow's avatar Sam Bigelow Committed by Phil Hughes

Resolve "Merge Request Popover is not working on the To Do page"

parent 27a96407
......@@ -22,13 +22,10 @@ const handleUserPopoverMouseOut = ({ target }) => {
* Adds a MergeRequestPopover component to the body, hands over as much data as the target element has in data attributes.
* loads based on data-project-path and data-iid more data about an MR from the API and sets it on the popover
*/
const handleMRPopoverMount = apolloProvider => ({ target }) => {
const handleMRPopoverMount = ({ apolloProvider, projectPath, mrTitle, iid }) => ({ target }) => {
// Add listener to actually remove it again
target.addEventListener('mouseleave', handleUserPopoverMouseOut);
const { projectPath, mrTitle, iid } = target.dataset;
const mergeRequest = {};
renderFn = setTimeout(() => {
const MRPopoverComponent = Vue.extend(MRPopover);
renderedPopover = new MRPopoverComponent({
......@@ -36,7 +33,6 @@ const handleMRPopoverMount = apolloProvider => ({ target }) => {
target,
projectPath,
mergeRequestIID: iid,
mergeRequest,
mergeRequestTitle: mrTitle,
},
apolloProvider,
......@@ -57,8 +53,13 @@ export default elements => {
const listenerAddedAttr = 'data-mr-listener-added';
mrLinks.forEach(el => {
if (!el.getAttribute(listenerAddedAttr)) {
el.addEventListener('mouseenter', handleMRPopoverMount(apolloProvider));
const { projectPath, mrTitle, iid } = el.dataset;
if (!el.getAttribute(listenerAddedAttr) && projectPath && mrTitle && iid) {
el.addEventListener(
'mouseenter',
handleMRPopoverMount({ apolloProvider, projectPath, mrTitle, iid }),
);
el.setAttribute(listenerAddedAttr, true);
}
});
......
......@@ -83,7 +83,8 @@ module MarkupHelper
text = sanitize(
text,
tags: tags,
attributes: Rails::Html::WhiteListSanitizer.allowed_attributes + ['style', 'data-src', 'data-name', 'data-unicode-version']
attributes: Rails::Html::WhiteListSanitizer.allowed_attributes +
%w(style data-src data-name data-unicode-version data-iid data-project-path data-mr-title)
)
# since <img> tags are stripped, this can leave empty <a> tags hanging around
......
---
title: Fix MR popover on ToDos page
merge_request: 27382
author:
type: fixed
......@@ -17,6 +17,26 @@ describe 'Dashboard Todos' do
end
end
context 'when the todo references a merge request' do
let(:referenced_mr) { create(:merge_request, source_project: project) }
let(:note) { create(:note, project: project, note: "Check out #{referenced_mr.to_reference}") }
let!(:todo) { create(:todo, :mentioned, user: user, project: project, author: author, note: note) }
before do
sign_in(user)
visit dashboard_todos_path
end
it 'renders the mr link with the extra attributes' do
link = page.find_link(referenced_mr.to_reference)
expect(link).not_to be_nil
expect(link['data-iid']).to eq(referenced_mr.iid.to_s)
expect(link['data-project-path']).to eq(referenced_mr.project.full_path)
expect(link['data-mr-title']).to eq(referenced_mr.title)
end
end
context 'User has a todo', :js do
before do
create(:todo, :mentioned, user: user, project: project, target: issue, author: author)
......
......@@ -7,18 +7,28 @@ createDefaultClient.default = jest.fn();
describe('initMRPopovers', () => {
let mr1;
let mr2;
let mr3;
beforeEach(() => {
setHTMLFixture(`
<div id="one" class="gfm-merge_request">MR1</div>
<div id="two" class="gfm-merge_request">MR2</div>
<div id="one" class="gfm-merge_request" data-mr-title="title" data-iid="1" data-project-path="group/project">
MR1
</div>
<div id="two" class="gfm-merge_request" data-mr-title="title" data-iid="1" data-project-path="group/project">
MR2
</div>
<div id="three" class="gfm-merge_request">
MR3
</div>
`);
mr1 = document.querySelector('#one');
mr2 = document.querySelector('#two');
mr3 = document.querySelector('#three');
mr1.addEventListener = jest.fn();
mr2.addEventListener = jest.fn();
mr3.addEventListener = jest.fn();
});
it('does not add the same event listener twice', () => {
......@@ -27,4 +37,10 @@ describe('initMRPopovers', () => {
expect(mr1.addEventListener).toHaveBeenCalledTimes(1);
expect(mr2.addEventListener).toHaveBeenCalledTimes(1);
});
it('does not add listener if it does not have the necessary data attributes', () => {
initMRPopovers([mr1, mr2, mr3]);
expect(mr3.addEventListener).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