Commit f37e00da authored by Robert Hunt's avatar Robert Hunt Committed by Miguel Rincon

Fix the drawer so it only toggles on the non-links or the row link

Added a check to make sure that the element being clicked is either not
a link or is the row link. If it isn't one of these, then don't toggle
the drawer as it either means we didn't click on the drawer or the link
is going to redirect to a different page
parent e20ad592
...@@ -45,6 +45,14 @@ export default { ...@@ -45,6 +45,14 @@ export default {
hasBranchDetails(mergeRequest) { hasBranchDetails(mergeRequest) {
return mergeRequest.target_branch && mergeRequest.source_branch; return mergeRequest.target_branch && mergeRequest.source_branch;
}, },
onRowClick(e, mergeRequest) {
const link = e.target.closest('a');
// Only toggle the drawer if the element isn't a link
if (!link) {
this.$emit('toggleDrawer', mergeRequest);
}
},
}, },
strings: { strings: {
approvalStatusLabel: __('Approval Status'), approvalStatusLabel: __('Approval Status'),
...@@ -72,12 +80,14 @@ export default { ...@@ -72,12 +80,14 @@ export default {
<!-- TODO: Remove the if/else and duplicate components with https://gitlab.com/gitlab-org/gitlab/-/issues/334682 --> <!-- TODO: Remove the if/else and duplicate components with https://gitlab.com/gitlab-org/gitlab/-/issues/334682 -->
<template v-if="drawerEnabled"> <template v-if="drawerEnabled">
<a <div
v-for="mergeRequest in mergeRequests" v-for="mergeRequest in mergeRequests"
:key="mergeRequest.id" :key="mergeRequest.id"
class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer" class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer"
data-testid="merge-request-link" data-testid="merge-request-drawer-toggle"
@click="$emit('toggleDrawer', mergeRequest)" tabindex="0"
@click="onRowClick($event, mergeRequest)"
@keypress.enter="onRowClick($event, mergeRequest)"
> >
<merge-request <merge-request
:key="key(mergeRequest.id, $options.keyTypes.mergeRequest)" :key="key(mergeRequest.id, $options.keyTypes.mergeRequest)"
...@@ -123,7 +133,7 @@ export default { ...@@ -123,7 +133,7 @@ export default {
</template> </template>
</time-ago-tooltip> </time-ago-tooltip>
</div> </div>
</a> </div>
</template> </template>
<template v-else> <template v-else>
<template v-for="mergeRequest in mergeRequests"> <template v-for="mergeRequest in mergeRequests">
......
...@@ -26,8 +26,13 @@ exports[`MergeRequestsGrid component when drawer enabled is false when initializ ...@@ -26,8 +26,13 @@ exports[`MergeRequestsGrid component when drawer enabled is false when initializ
<div <div
data-testid="merge-request" data-testid="merge-request"
>
<a
data-testid="merge-request-link"
href=""
> >
Merge request 0 Merge request 0
</a>
</div> </div>
<status-stub <status-stub
...@@ -56,8 +61,13 @@ exports[`MergeRequestsGrid component when drawer enabled is false when initializ ...@@ -56,8 +61,13 @@ exports[`MergeRequestsGrid component when drawer enabled is false when initializ
</div> </div>
<div <div
data-testid="merge-request" data-testid="merge-request"
>
<a
data-testid="merge-request-link"
href=""
> >
Merge request 1 Merge request 1
</a>
</div> </div>
<status-stub <status-stub
...@@ -116,14 +126,20 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize ...@@ -116,14 +126,20 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize
heading="Updates" heading="Updates"
/> />
<a <div
class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer" class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer"
data-testid="merge-request-link" data-testid="merge-request-drawer-toggle"
tabindex="0"
> >
<div <div
data-testid="merge-request" data-testid="merge-request"
>
<a
data-testid="merge-request-link"
href=""
> >
Merge request 0 Merge request 0
</a>
</div> </div>
<status-stub <status-stub
...@@ -150,15 +166,21 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize ...@@ -150,15 +166,21 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize
tooltipplacement="bottom" tooltipplacement="bottom"
/> />
</div> </div>
</a> </div>
<a <div
class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer" class="dashboard-merge-request dashboard-grid gl-display-grid gl-grid-tpl-rows-auto gl-hover-bg-blue-50 gl-hover-text-decoration-none gl-hover-cursor-pointer"
data-testid="merge-request-link" data-testid="merge-request-drawer-toggle"
tabindex="0"
> >
<div <div
data-testid="merge-request" data-testid="merge-request"
>
<a
data-testid="merge-request-link"
href=""
> >
Merge request 1 Merge request 1
</a>
</div> </div>
<status-stub <status-stub
...@@ -185,7 +207,7 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize ...@@ -185,7 +207,7 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize
tooltipplacement="bottom" tooltipplacement="bottom"
/> />
</div> </div>
</a> </div>
</div> </div>
<pagination-stub <pagination-stub
......
...@@ -9,8 +9,10 @@ import { createMergeRequests, mergedAt } from '../../mock_data'; ...@@ -9,8 +9,10 @@ import { createMergeRequests, mergedAt } from '../../mock_data';
describe('MergeRequestsGrid component', () => { describe('MergeRequestsGrid component', () => {
let wrapper; let wrapper;
const findMergeRequestLinks = () => wrapper.findAllByTestId('merge-request-link'); const findMergeRequestDrawerToggles = () =>
wrapper.findAllByTestId('merge-request-drawer-toggle');
const findMergeRequests = () => wrapper.findAllByTestId('merge-request'); const findMergeRequests = () => wrapper.findAllByTestId('merge-request');
const findMergeRequestLinks = () => wrapper.findAllByTestId('merge-request-link');
const findTime = () => wrapper.findComponent(TimeAgoTooltip); const findTime = () => wrapper.findComponent(TimeAgoTooltip);
const findStatuses = () => wrapper.findAllComponents(Status); const findStatuses = () => wrapper.findAllComponents(Status);
const findApprovers = () => wrapper.findComponent(Approvers); const findApprovers = () => wrapper.findComponent(Approvers);
...@@ -26,7 +28,7 @@ describe('MergeRequestsGrid component', () => { ...@@ -26,7 +28,7 @@ describe('MergeRequestsGrid component', () => {
stubs: { stubs: {
MergeRequest: { MergeRequest: {
props: { mergeRequest: Object }, props: { mergeRequest: Object },
template: `<div data-testid="merge-request">{{ mergeRequest.title }}</div>`, template: `<div data-testid="merge-request"><a href="" data-testid="merge-request-link">{{ mergeRequest.title }}</a></div>`,
}, },
}, },
}); });
...@@ -116,10 +118,18 @@ describe('MergeRequestsGrid component', () => { ...@@ -116,10 +118,18 @@ describe('MergeRequestsGrid component', () => {
wrapper = createComponent(mergeRequest, true); wrapper = createComponent(mergeRequest, true);
}); });
it('toggles the drawer when a merge request link is clicked', () => { describe.each(['click', 'keypress.enter'])('when the %s event is triggered', (event) => {
findMergeRequestLinks().at(0).trigger('click'); it('toggles the drawer when a merge request drawer toggle is the target', () => {
findMergeRequestDrawerToggles().at(0).trigger(event);
expect(wrapper.emitted('toggleDrawer')[0][0]).toStrictEqual(mergeRequests[0]); expect(wrapper.emitted('toggleDrawer')[0][0]).toStrictEqual(mergeRequests[0]);
}); });
it('does not toggle the drawer if an inner link is the target', () => {
findMergeRequestLinks().at(0).trigger(event);
expect(wrapper.emitted('toggleDrawer')).toBe(undefined);
});
});
}); });
}); });
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