Merge branch 'pipeline-minigraph-dropdown-migration-pt2' into 'master'

Pipeline MiniGraph: Migrate dropdown to GlDisclosureDropdown (Pt. 2)

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144606



Merged-by: Payton Burdette <pburdette@gitlab.com>
Approved-by: Julia Miocene <jmiocene@gitlab.com>
Approved-by: Stan Hu <stanhu@gmail.com>
Approved-by: Payton Burdette <pburdette@gitlab.com>
Reviewed-by: Sascha Eggenberger <seggenberger@gitlab.com>
Co-authored-by: Sascha Eggenberger <seggenberger@gitlab.com>
Co-authored-by: Phil Hughes <me@iamphill.com>

app/assets/javascripts/ci/common/private/job_action_component.vue

@@ -80,7 +80,9 @@ export default {
      * different apps it avoids repetition & complexity.
      *
      */
-    onClickAction() {
+    onClickAction(e) {
+      e.preventDefault();
+
       if (this.withConfirmationModal) {
         this.$emit('showActionConfirmationModal');
       } else {

app/assets/javascripts/ci/pipeline_mini_graph/legacy_job_item.vue

 <script>
-import { GlTooltipDirective, GlLink } from '@gitlab/ui';
+import { GlDisclosureDropdownItem, GlTooltipDirective } from '@gitlab/ui';
 import ActionComponent from '~/ci/common/private/job_action_component.vue';
 import JobNameComponent from '~/ci/common/private/job_name_component.vue';
 import { ICONS } from '~/ci/constants';
@@ -44,7 +44,7 @@ export default {
   components: {
     ActionComponent,
     JobNameComponent,
-    GlLink,
+    GlDisclosureDropdownItem,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -72,8 +72,14 @@ export default {
     },
   },
   computed: {
-    boundary() {
-      return this.dropdownLength === 1 ? 'viewport' : 'scrollParent';
+    alternativeTooltipConfig() {
+      const boundary = this.dropdownLength === 1 ? 'viewport' : 'scrollParent';
+
+      return {
+        boundary,
+        placement: 'bottom',
+        customClass: 'gl-pointer-events-none',
+      };
     },
     detailsPath() {
       return this.status?.details_path;
@@ -81,9 +87,18 @@ export default {
     hasDetails() {
       return this.status?.has_details;
     },
+    item() {
+      return {
+        text: this.job.name,
+        href: this.hasDetails ? this.detailsPath : '',
+      };
+    },
     status() {
       return this.job?.status ? this.job.status : {};
     },
+    tooltipConfig() {
+      return this.hasDetails ? this.$options.tooltipConfig : this.alternativeTooltipConfig;
+    },
     tooltipText() {
       const textBuilder = [];
       const { name: jobName } = this.job;
@@ -123,6 +138,9 @@ export default {
         ? this.$options.i18n.runAgainTooltipText
         : title;
     },
+    testid() {
+      return this.hasDetails ? 'job-with-link' : 'job-without-link';
+    },
   },
   errorCaptured(err, _vm, info) {
     reportToSentry('pipelines_job_item', `pipelines_job_item error: ${err}, info: ${info}`);
@@ -130,37 +148,38 @@ export default {
 };
 </script>
 <template>
-  <div
-    class="ci-job-component gl-display-flex gl-align-items-center gl-justify-content-space-between"
+  <gl-disclosure-dropdown-item
+    :item="item"
+    class="ci-job-component"
+    :class="[
+      cssClassJobName,
+      {
+        'js-pipeline-graph-job-link gl-text-gray-900 gl-active-text-decoration-none gl-focus-text-decoration-none gl-hover-text-decoration-none': hasDetails,
+        'js-job-component-tooltip non-details-job-component': !hasDetails,
+      },
+    ]"
+    :data-testid="testid"
   >
-    <gl-link
-      v-if="hasDetails"
-      v-gl-tooltip="$options.tooltipConfig"
-      :href="detailsPath"
-      :title="tooltipText"
-      :class="cssClassJobName"
-      class="js-pipeline-graph-job-link menu-item gl-text-gray-900 gl-active-text-decoration-none gl-focus-text-decoration-none gl-hover-text-decoration-none"
-      data-testid="job-with-link"
-    >
-      <job-name-component :name="job.name" :status="job.status" />
-    </gl-link>
-
-    <div
-      v-else
-      v-gl-tooltip="{ boundary, placement: 'bottom', customClass: 'gl-pointer-events-none' }"
-      :title="tooltipText"
-      :class="cssClassJobName"
-      class="js-job-component-tooltip non-details-job-component menu-item"
-      data-testid="job-without-link"
-    >
-      <job-name-component :name="job.name" :status="job.status" />
-    </div>
+    <template #list-item>
+      <div
+        class="gl-display-flex gl-align-items-center gl-justify-content-space-between gl-mt-n2 gl-mb-n2 gl-ml-n2"
+      >
+        <job-name-component
+          v-gl-tooltip="tooltipConfig"
+          :title="tooltipText"
+          :name="job.name"
+          :status="job.status"
+          data-testid="job-name"
+        />
 
-    <action-component
-      v-if="hasJobAction"
-      :tooltip-text="jobActionTooltipText"
-      :link="status.action.path"
-      :action-icon="status.action.icon"
-    />
-  </div>
+        <action-component
+          v-if="hasJobAction"
+          :tooltip-text="jobActionTooltipText"
+          :link="status.action.path"
+          :action-icon="status.action.icon"
+          class="gl-mt-n2 gl-mr-n2"
+        />
+      </div>
+    </template>
+  </gl-disclosure-dropdown-item>
 </template>

app/assets/javascripts/ci/pipeline_mini_graph/legacy_pipeline_stage.vue

@@ -12,7 +12,7 @@
  * 4. Commit widget
  */
 
-import { GlDropdown, GlLoadingIcon, GlTooltipDirective } from '@gitlab/ui';
+import { GlDisclosureDropdown, GlButton, GlLoadingIcon, GlTooltipDirective } from '@gitlab/ui';
 import CiIcon from '~/vue_shared/components/ci_icon/ci_icon.vue';
 import { createAlert } from '~/alert';
 import eventHub from '~/ci/event_hub';
@@ -28,14 +28,11 @@ export default {
     stage: __('Stage:'),
     viewStageLabel: __('View Stage: %{title}'),
   },
-  dropdownPopperOpts: {
-    placement: 'bottom',
-    positionFixed: true,
-  },
   components: {
     CiIcon,
     GlLoadingIcon,
-    GlDropdown,
+    GlDisclosureDropdown,
+    GlButton,
     LegacyJobItem,
   },
   directives: {
@@ -95,7 +92,7 @@ export default {
           this.isLoading = false;
         })
         .catch(() => {
-          this.$refs.dropdown.hide();
+          this.$refs.dropdown.close();
           this.isLoading = false;
 
           createAlert({
@@ -111,58 +108,66 @@ export default {
 </script>
 
 <template>
-  <gl-dropdown
+  <gl-disclosure-dropdown
     ref="dropdown"
-    v-gl-tooltip.hover.ds0
-    v-gl-tooltip="stage.title"
     data-testid="mini-pipeline-graph-dropdown"
+    class="mini-pipeline-graph-dropdown"
     variant="link"
     :aria-label="stageAriaLabel(stage.title)"
-    :lazy="true"
-    :popper-opts="$options.dropdownPopperOpts"
-    :toggle-class="['gl-rounded-full!']"
-    menu-class="mini-pipeline-graph-dropdown-menu"
-    @hide="onHideDropdown"
-    @show="onShowDropdown"
+    no-caret
+    @hidden="onHideDropdown"
+    @shown="onShowDropdown"
   >
-    <template #button-content>
-      <ci-icon :status="stage.status" :show-tooltip="false" :use-link="false" class="gl-mb-0!" />
+    <template #toggle>
+      <gl-button
+        v-gl-tooltip.ds0="isDropdownOpen ? '' : stage.title"
+        variant="link"
+        class="gl-rounded-full!"
+        data-testid="mini-pipeline-graph-dropdown-toggle"
+      >
+        <ci-icon :status="stage.status" :show-tooltip="false" :use-link="false" class="gl-mb-0!" />
+      </gl-button>
     </template>
-    <div v-if="isLoading" class="gl--flex-center gl-p-2" data-testid="pipeline-stage-loading-state">
+
+    <template #header>
+      <div
+        class="gl-display-flex gl-align-items-center gl-p-4! gl-min-h-8 gl-border-b-1 gl-border-b-solid gl-border-b-gray-200 gl-font-sm gl-font-weight-bold gl-line-height-1"
+      >
+        <span class="gl-mr-1">{{ $options.i18n.stage }}</span>
+        <span data-testid="pipeline-stage-dropdown-menu-title">{{ stageName }}</span>
+      </div>
+    </template>
+
+    <div
+      v-if="isLoading"
+      class="gl-display-flex gl-py-3 gl-px-4"
+      data-testid="pipeline-stage-loading-state"
+    >
       <gl-loading-icon size="sm" class="gl-mr-3" />
       <p class="gl-line-height-normal gl-mb-0">{{ $options.i18n.loadingText }}</p>
     </div>
     <ul
       v-else
-      class="js-builds-dropdown-list scrollable-menu"
+      class="mini-pipeline-graph-dropdown-menu gl-overflow-y-auto gl-m-0 gl-p-0"
       data-testid="mini-pipeline-graph-dropdown-menu-list"
     >
-      <div class="gl--flex-center gl-border-b gl-font-weight-bold gl-mb-3 gl-pb-3">
-        <span class="gl-mr-1">{{ $options.i18n.stage }}</span>
-        <span data-testid="pipeline-stage-dropdown-menu-title">{{ stageName }}</span>
-      </div>
-      <li v-for="job in dropdownContent" :key="job.id">
-        <legacy-job-item
-          :dropdown-length="dropdownContent.length"
-          :job="job"
-          css-class-job-name="pipeline-job-item"
-        />
-      </li>
-      <template v-if="isMergeTrain">
-        <li class="gl-dropdown-divider" role="presentation">
-          <hr role="separator" aria-orientation="horizontal" class="dropdown-divider" />
-        </li>
-        <li>
-          <div
-            class="gl-display-flex gl-align-items-center"
-            data-testid="warning-message-merge-trains"
-          >
-            <div class="menu-item gl-font-sm gl-text-gray-300!">
-              {{ $options.i18n.mergeTrainMessage }}
-            </div>
-          </div>
-        </li>
-      </template>
+      <legacy-job-item
+        v-for="job in dropdownContent"
+        :key="job.id"
+        :dropdown-length="dropdownContent.length"
+        :job="job"
+        css-class-job-name="pipeline-job-item"
+      />
     </ul>
-  </gl-dropdown>
+
+    <template #footer>
+      <div
+        v-if="!isLoading && isMergeTrain"
+        class="gl-font-sm gl-text-secondary gl-py-3 gl-px-4 gl-border-t"
+        data-testid="warning-message-merge-trains"
+      >
+        {{ $options.i18n.mergeTrainMessage }}
+      </div>
+    </template>
+  </gl-disclosure-dropdown>
 </template>

app/assets/stylesheets/page_bundles/_pipeline_mixins.scss

@@ -21,15 +21,7 @@
  - mini graph in Commit widget pipeline
 */
 @mixin pipeline-graph-dropdown-menu() {
-  width: auto;
-  max-width: 400px;
-
-  // override dropdown.scss
-  &.dropdown-menu li button,
-  &.dropdown-menu li a.ci-action-icon-container {
-    padding: 0;
-    text-align: center;
-  }
+  max-height: $gl-max-dropdown-max-height;
 
   .ci-action-icon-container {
     position: absolute;

app/assets/stylesheets/page_bundles/pipelines.scss

@@ -78,7 +78,7 @@
       border-bottom: 2px solid $gray-200;
       position: absolute;
       right: -4px;
-      top: 11px;
+      top: 12px;
       width: 4px;
     }
   }

ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb

@@ -75,8 +75,8 @@
         expect(page).to have_selector('[data-testid="mini-pipeline-graph-dropdown"]')
       end
 
-      it 'does not allow retry for merge train pipeline' do
-        find('[data-testid="mini-pipeline-graph-dropdown"] .dropdown-toggle').click
+      it 'does not allow retry for merge train pipeline', :js do
+        find_by_testid('mini-pipeline-graph-dropdown-toggle').click
         page.within '.ci-job-component' do
           expect(page).to have_selector('[data-testid="ci-icon"]')
           expect(page).not_to have_selector('.retry')

spec/features/projects/commit/mini_pipeline_graph_spec.rb

@@ -31,7 +31,7 @@
     end
   end
 
-  context 'when commit has pipelines and feature flag is disabled' do
+  context 'when commit has pipelines and feature flag is disabled', :js do
     let(:pipeline) do
       create(
         :ci_pipeline,
@@ -58,11 +58,11 @@
     it 'displays a mini pipeline graph' do
       expect(page).to have_selector('[data-testid="commit-box-pipeline-mini-graph"]')
 
-      first('[data-testid="mini-pipeline-graph-dropdown"]').click
+      find_by_testid('mini-pipeline-graph-dropdown-toggle').click
 
       wait_for_requests
 
-      page.within '.js-builds-dropdown-list' do
+      within_testid('mini-pipeline-graph-dropdown') do
         expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
         expect(page).to have_content(build.stage_name)
       end

spec/features/projects/pipelines/pipelines_spec.rb

@@ -269,7 +269,7 @@
         end
       end
 
-      context 'with manual actions' do
+      context 'with manual actions', :js do
         let!(:manual) do
           create(:ci_build, :manual,
             pipeline: pipeline,
@@ -286,7 +286,7 @@
         end
 
         it 'has link to the manual action' do
-          find('[data-testid="pipelines-manual-actions-dropdown"]').click
+          find_by_testid('pipelines-manual-actions-dropdown').click
 
           wait_for_requests
 
@@ -295,11 +295,13 @@
 
         context 'when manual action was played' do
           before do
-            find('[data-testid="pipelines-manual-actions-dropdown"] button').click
+            find_by_testid('pipelines-manual-actions-dropdown').click
 
             wait_for_requests
 
             click_button('manual build')
+
+            wait_for_all_requests
           end
 
           it 'enqueues manual action job', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/409984' do
@@ -325,8 +327,8 @@
           expect(page).to have_selector('[data-testid="pipelines-manual-actions-dropdown"] [data-testid="play-icon"]')
         end
 
-        it "has link to the delayed job's action" do
-          find('[data-testid="pipelines-manual-actions-dropdown"] button').click
+        it "has link to the delayed job's action", :js do
+          find_by_testid('pipelines-manual-actions-dropdown').click
 
           wait_for_requests
 
@@ -344,8 +346,8 @@
               stage: 'test')
           end
 
-          it "shows 00:00:00 as the remaining time" do
-            find('[data-testid="pipelines-manual-actions-dropdown"] button').click
+          it "shows 00:00:00 as the remaining time", :js do
+            find_by_testid('pipelines-manual-actions-dropdown').click
 
             wait_for_requests
 
@@ -535,32 +537,60 @@
           expect(page).to have_selector(dropdown_selector)
         end
 
-        context 'when clicking a stage badge' do
+        context 'when clicking a stage badge', :js do
           it 'opens a dropdown' do
-            find(dropdown_selector).click
+            find_by_testid('mini-pipeline-graph-dropdown-toggle').click
+
+            wait_for_requests
 
             expect(page).to have_link build.name
           end
 
           it 'is possible to cancel pending build' do
-            find(dropdown_selector).click
-            find('.js-ci-action').click
+            find_by_testid('mini-pipeline-graph-dropdown-toggle').click
+
+            wait_for_requests
+
+            find_by_testid('ci-action-button').click
             wait_for_requests
 
             expect(build.reload).to be_canceled
           end
+
+          context 'manual job', :js do
+            let!(:build) do
+              create(:ci_build, :manual, pipeline: pipeline, stage: 'build', name: 'manual-build')
+            end
+
+            it 'is possible to play manual build' do
+              find_by_testid('mini-pipeline-graph-dropdown-toggle').click
+
+              wait_for_requests
+
+              within first('[data-testid="job-with-link"]') do
+                expect(find_by_testid('play-icon')).to be_visible
+              end
+
+              find_by_testid('ci-action-button').click
+              wait_for_requests
+
+              expect(find('[data-testid="mini-pipeline-graph-dropdown-toggle"][aria-expanded="true"]')).to be_visible
+            end
+          end
         end
 
-        context 'for a failed pipeline' do
+        context 'for a failed pipeline', :js do
           let!(:build) do
             create(:ci_build, :failed, pipeline: pipeline, stage: 'build', name: 'build')
           end
 
           it 'displays the failure reason' do
-            find(dropdown_selector).click
+            find_by_testid('mini-pipeline-graph-dropdown-toggle').click
+
+            wait_for_requests
 
-            within('.js-builds-dropdown-list') do
-              build_element = page.find('.pipeline-job-item')
+            within_testid('mini-pipeline-graph-dropdown') do
+              build_element = page.find('.pipeline-job-item [data-testid="job-name"]')
               expect(build_element['title']).to eq('build - failed - (unknown failure)')
             end
           end

spec/frontend/ci/pipeline_mini_graph/legacy_pipeline_stage_spec.js

-import { GlDropdown } from '@gitlab/ui';
+import { GlDisclosureDropdown } from '@gitlab/ui';
 import { nextTick } from 'vue';
 import { mount } from '@vue/test-utils';
 import MockAdapter from 'axios-mock-adapter';
@@ -53,8 +53,9 @@ describe('Pipelines stage component', () => {
 
   const findCiActionBtn = () => wrapper.find('.js-ci-action');
   const findCiIcon = () => wrapper.findComponent(CiIcon);
-  const findDropdown = () => wrapper.findComponent(GlDropdown);
-  const findDropdownToggle = () => wrapper.find('button.dropdown-toggle');
+  const findDropdown = () => wrapper.findComponent(GlDisclosureDropdown);
+  const findDropdownToggle = () =>
+    wrapper.find('[data-testid="mini-pipeline-graph-dropdown-toggle"]');
   const findDropdownMenu = () =>
     wrapper.find('[data-testid="mini-pipeline-graph-dropdown-menu-list"]');
   const findDropdownMenuTitle = () =>
@@ -78,8 +79,9 @@ describe('Pipelines stage component', () => {
     });
 
     it('displays loading state while jobs are being fetched', async () => {
-      jest.runOnlyPendingTimers();
-      await nextTick();
+      // eslint-disable-next-line no-restricted-syntax
+      wrapper.setData({ isLoading: true });
+      await waitForPromises();
 
       expect(findLoadingState().exists()).toBe(true);
       expect(findLoadingState().text()).toBe(LegacyPipelineStage.i18n.loadingText);
@@ -144,7 +146,7 @@ describe('Pipelines stage component', () => {
       await axios.waitForAll();
       await waitForPromises();
 
-      expect(findDropdown().classes('show')).toBe(false);
+      expect(findDropdownToggle().attributes('aria-expanded')).toBe('false');
     });
   });
 
@@ -197,7 +199,7 @@ describe('Pipelines stage component', () => {
       await clickCiAction();
       await waitForPromises();
 
-      expect(findDropdown().classes('show')).toBe(true);
+      expect(findDropdownToggle().attributes('aria-expanded')).toBe('true');
     });
   });
 

spec/frontend/ci/pipelines_page/pipelines_spec.js

@@ -96,7 +96,7 @@ describe('Pipelines', () => {
   const findCiLintButton = () => wrapper.findByTestId('ci-lint-button');
   const findCleanCacheButton = () => wrapper.findByTestId('clear-cache-button');
   const findStagesDropdownToggle = () =>
-    wrapper.find('[data-testid="mini-pipeline-graph-dropdown"] .dropdown-toggle');
+    wrapper.find('.mini-pipeline-graph-dropdown [data-testid="base-dropdown-toggle"]');
   const findPipelineUrlLinks = () => wrapper.findAll('[data-testid="pipeline-url-link"]');
 
   const createComponent = ({ props = {}, withPermissions = true } = {}) => {
@@ -769,6 +769,11 @@ describe('Pipelines', () => {
           .onGet(mockPipelineWithStages.details.stages[0].dropdown_path)
           .reply(HTTP_STATUS_OK, stageReply);
 
+        // cancelMock is getting overwritten in pipelines_service.js#L29
+        // so we have to spy on it again here
+        cancelMock = { cancel: jest.fn() };
+        jest.spyOn(axios.CancelToken, 'source').mockReturnValue(cancelMock);
+
         createComponent();
 
         stopMock = jest.spyOn(window, 'clearTimeout');
@@ -789,13 +794,9 @@ describe('Pipelines', () => {
           await findStagesDropdownToggle().trigger('click');
           jest.runOnlyPendingTimers();
 
-          // cancelMock is getting overwritten in pipelines_service.js#L29
-          // so we have to spy on it again here
-          cancelMock = jest.spyOn(axios.CancelToken, 'source');
-
           await waitForPromises();
 
-          expect(cancelMock).toHaveBeenCalled();
+          expect(cancelMock.cancel).toHaveBeenCalled();
           expect(stopMock).toHaveBeenCalled();
           expect(restartMock).toHaveBeenCalledWith(
             `${mockPipelinesResponse.pipelines[0].path}/stage.json?stage=build`,
@@ -807,7 +808,7 @@ describe('Pipelines', () => {
           jest.runOnlyPendingTimers();
           await waitForPromises();
 
-          expect(cancelMock).not.toHaveBeenCalled();
+          expect(cancelMock.cancel).not.toHaveBeenCalled();
           expect(stopMock).toHaveBeenCalled();
           expect(restartMock).toHaveBeenCalledWith(
             `${mockPipelinesResponse.pipelines[0].path}/stage.json?stage=build`,

spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js

@@ -38,6 +38,7 @@ import StatusIcon from '~/vue_merge_request_widget/components/extensions/status_
 import getStateQuery from '~/vue_merge_request_widget/queries/get_state.query.graphql';
 import getStateSubscription from '~/vue_merge_request_widget/queries/get_state.subscription.graphql';
 import readyToMergeSubscription from '~/vue_merge_request_widget/queries/states/ready_to_merge.subscription.graphql';
+import securityReportMergeRequestDownloadPathsQuery from '~/vue_merge_request_widget/extensions/security_reports/graphql/security_report_merge_request_download_paths.query.graphql';
 import readyToMergeQuery from 'ee_else_ce/vue_merge_request_widget/queries/states/ready_to_merge.query.graphql';
 import approvalsQuery from 'ee_else_ce/vue_merge_request_widget/components/approvals/queries/approvals.query.graphql';
 import approvedBySubscription from 'ee_else_ce/vue_merge_request_widget/components/approvals/queries/approvals.subscription.graphql';
@@ -123,6 +124,8 @@ describe('MrWidgetOptions', () => {
         conflictsStateQuery,
         jest.fn().mockResolvedValue({ data: { project: { mergeRequest: {} } } }),
       ],
+      [securityReportMergeRequestDownloadPathsQuery, jest.fn().mockResolvedValue(null)],
+      ...(options.apolloMock || []),
     ];
     const subscriptionHandlers = [
       [approvedBySubscription, () => mockedApprovalsSubscription],