Merge branch...

Merge branch '334980-fix-compliance-dashboard-drawer-so-it-only-appears-when-a-non-link-is-clicked' into 'master'

Don't show compliance dashboard drawer if link is clicked

See merge request gitlab-org/gitlab!65312

ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/grid.vue

@@ -45,6 +45,14 @@ export default {
     hasBranchDetails(mergeRequest) {
       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: {
     approvalStatusLabel: __('Approval Status'),
@@ -72,12 +80,14 @@ export default {
 
       <!-- TODO: Remove the if/else and duplicate components with https://gitlab.com/gitlab-org/gitlab/-/issues/334682 -->
       <template v-if="drawerEnabled">
-        <a
+        <div
           v-for="mergeRequest in mergeRequests"
           :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"
-          data-testid="merge-request-link"
-          @click="$emit('toggleDrawer', mergeRequest)"
+          data-testid="merge-request-drawer-toggle"
+          tabindex="0"
+          @click="onRowClick($event, mergeRequest)"
+          @keypress.enter="onRowClick($event, mergeRequest)"
         >
           <merge-request
             :key="key(mergeRequest.id, $options.keyTypes.mergeRequest)"
@@ -123,7 +133,7 @@ export default {
               </template>
             </time-ago-tooltip>
           </div>
-        </a>
+        </div>
       </template>
       <template v-else>
         <template v-for="mergeRequest in mergeRequests">

ee/spec/frontend/compliance_dashboard/components/merge_requests/__snapshots__/grid_spec.js.snap

@@ -27,7 +27,12 @@ exports[`MergeRequestsGrid component when drawer enabled is false when initializ
     <div
       data-testid="merge-request"
     >
-      Merge request 0
+      <a
+        data-testid="merge-request-link"
+        href=""
+      >
+        Merge request 0
+      </a>
     </div>
      
     <status-stub
@@ -57,7 +62,12 @@ exports[`MergeRequestsGrid component when drawer enabled is false when initializ
     <div
       data-testid="merge-request"
     >
-      Merge request 1
+      <a
+        data-testid="merge-request-link"
+        href=""
+      >
+        Merge request 1
+      </a>
     </div>
      
     <status-stub
@@ -116,14 +126,20 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize
       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"
-      data-testid="merge-request-link"
+      data-testid="merge-request-drawer-toggle"
+      tabindex="0"
     >
       <div
         data-testid="merge-request"
       >
-        Merge request 0
+        <a
+          data-testid="merge-request-link"
+          href=""
+        >
+          Merge request 0
+        </a>
       </div>
        
       <status-stub
@@ -150,15 +166,21 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize
           tooltipplacement="bottom"
         />
       </div>
-    </a>
-    <a
+    </div>
+    <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"
-      data-testid="merge-request-link"
+      data-testid="merge-request-drawer-toggle"
+      tabindex="0"
     >
       <div
         data-testid="merge-request"
       >
-        Merge request 1
+        <a
+          data-testid="merge-request-link"
+          href=""
+        >
+          Merge request 1
+        </a>
       </div>
        
       <status-stub
@@ -185,7 +207,7 @@ exports[`MergeRequestsGrid component when drawer enabled is true when initialize
           tooltipplacement="bottom"
         />
       </div>
-    </a>
+    </div>
   </div>
    
   <pagination-stub

ee/spec/frontend/compliance_dashboard/components/merge_requests/grid_spec.js

@@ -9,8 +9,10 @@ import { createMergeRequests, mergedAt } from '../../mock_data';
 describe('MergeRequestsGrid component', () => {
   let wrapper;
 
-  const findMergeRequestLinks = () => wrapper.findAllByTestId('merge-request-link');
+  const findMergeRequestDrawerToggles = () =>
+    wrapper.findAllByTestId('merge-request-drawer-toggle');
   const findMergeRequests = () => wrapper.findAllByTestId('merge-request');
+  const findMergeRequestLinks = () => wrapper.findAllByTestId('merge-request-link');
   const findTime = () => wrapper.findComponent(TimeAgoTooltip);
   const findStatuses = () => wrapper.findAllComponents(Status);
   const findApprovers = () => wrapper.findComponent(Approvers);
@@ -26,7 +28,7 @@ describe('MergeRequestsGrid component', () => {
       stubs: {
         MergeRequest: {
           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', () => {
       wrapper = createComponent(mergeRequest, true);
     });
 
-    it('toggles the drawer when a merge request link is clicked', () => {
-      findMergeRequestLinks().at(0).trigger('click');
+    describe.each(['click', 'keypress.enter'])('when the %s event is triggered', (event) => {
+      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]);
+      });
+
+      it('does not toggle the drawer if an inner link is the target', () => {
+        findMergeRequestLinks().at(0).trigger(event);
 
-      expect(wrapper.emitted('toggleDrawer')[0][0]).toStrictEqual(mergeRequests[0]);
+        expect(wrapper.emitted('toggleDrawer')).toBe(undefined);
+      });
     });
   });
 });