Merge branch '223107-remove-suggest-popup' into 'master'

Replace suggest pipeline popup

See merge request gitlab-org/gitlab!35069

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_suggest_pipeline.vue

 <script>
-import { GlLink, GlSprintf } from '@gitlab/ui';
+import { GlLink, GlSprintf, GlButton } from '@gitlab/ui';
 import MrWidgetIcon from './mr_widget_icon.vue';
-import PipelineTourState from './states/mr_widget_pipeline_tour.vue';
+import Tracking from '~/tracking';
+import { s__ } from '~/locale';
+
+const trackingMixin = Tracking.mixin();
+const TRACK_LABEL = 'no_pipeline_noticed';
 
 export default {
   name: 'MRWidgetSuggestPipeline',
   iconName: 'status_notfound',
-  popoverTarget: 'suggest-popover',
-  popoverContainer: 'suggest-pipeline',
-  trackLabel: 'no_pipeline_noticed',
+  trackLabel: TRACK_LABEL,
   linkTrackValue: 30,
   linkTrackEvent: 'click_link',
+  showTrackValue: 10,
+  showTrackEvent: 'click_button',
+  helpContent: s__(
+    `mrWidget|Use %{linkStart}CI pipelines to test your code%{linkEnd} by simply adding a GitLab CI configuration file to your project. It only takes a minute to make your code more secure and robust.`,
+  ),
+  helpURL: 'https://about.gitlab.com/blog/2019/07/12/guide-to-ci-cd-pipelines/',
   components: {
     GlLink,
     GlSprintf,
+    GlButton,
     MrWidgetIcon,
-    PipelineTourState,
   },
+  mixins: [trackingMixin],
   props: {
     pipelinePath: {
       type: String,
@@ -31,45 +40,89 @@ export default {
       required: true,
     },
   },
+  computed: {
+    tracking() {
+      return {
+        label: TRACK_LABEL,
+        property: this.humanAccess,
+      };
+    },
+  },
+  mounted() {
+    this.track();
+  },
 };
 </script>
 <template>
-  <div :id="$options.popoverContainer" class="d-flex mr-pipeline-suggest gl-mb-3">
-    <mr-widget-icon :name="$options.iconName" />
-    <div :id="$options.popoverTarget">
-      <gl-sprintf
-        :message="
-          s__(`mrWidget|%{prefixToLinkStart}No pipeline%{prefixToLinkEnd}
+  <div class="mr-widget-body mr-pipeline-suggest gl-mb-3">
+    <div class="gl-display-flex gl-align-items-center">
+      <mr-widget-icon :name="$options.iconName" />
+      <div>
+        <gl-sprintf
+          :message="
+            s__(`mrWidget|%{prefixToLinkStart}No pipeline%{prefixToLinkEnd}
           %{addPipelineLinkStart}Add the .gitlab-ci.yml file%{addPipelineLinkEnd}
           to create one.`)
-        "
-      >
-        <template #prefixToLink="{content}">
+          "
+        >
+          <template #prefixToLink="{content}">
+            <strong>
+              {{ content }}
+            </strong>
+          </template>
+          <template #addPipelineLink="{content}">
+            <gl-link
+              :href="pipelinePath"
+              class="gl-ml-1"
+              data-testid="add-pipeline-link"
+              :data-track-property="humanAccess"
+              :data-track-value="$options.linkTrackValue"
+              :data-track-event="$options.linkTrackEvent"
+              :data-track-label="$options.trackLabel"
+            >
+              {{ content }}
+            </gl-link>
+          </template>
+        </gl-sprintf>
+      </div>
+    </div>
+    <div class="row">
+      <div class="col-md-5 order-md-last col-12 gl-mt-5 mt-md-n3 svg-content svg-225">
+        <img data-testid="pipeline-image" :src="pipelineSvgPath" />
+      </div>
+      <div class="col-md-7 order-md-first col-12">
+        <div class="ml-6 gl-pt-5">
           <strong>
-            {{ content }}
+            {{ s__('mrWidget|Are you adding technical debt or code vulnerabilities?') }}
           </strong>
-        </template>
-        <template #addPipelineLink="{content}">
-          <gl-link
+          <p class="gl-mt-2">
+            <gl-sprintf :message="$options.helpContent">
+              <template #link="{ content }">
+                <gl-link
+                  data-testid="help"
+                  :href="$options.helpURL"
+                  target="_blank"
+                  class="font-size-inherit"
+                  >{{ content }}
+                </gl-link>
+              </template>
+            </gl-sprintf>
+          </p>
+          <gl-button
+            data-testid="ok"
+            category="primary"
+            class="gl-mt-2"
+            variant="info"
             :href="pipelinePath"
-            class="ml-2 js-add-pipeline-path"
             :data-track-property="humanAccess"
-            :data-track-value="$options.linkTrackValue"
-            :data-track-event="$options.linkTrackEvent"
+            :data-track-value="$options.showTrackValue"
+            :data-track-event="$options.showTrackEvent"
             :data-track-label="$options.trackLabel"
           >
-            {{ content }}
-          </gl-link>
-        </template>
-      </gl-sprintf>
-      <pipeline-tour-state
-        :pipeline-path="pipelinePath"
-        :pipeline-svg-path="pipelineSvgPath"
-        :human-access="humanAccess"
-        :popover-target="$options.popoverTarget"
-        :popover-container="$options.popoverContainer"
-        :track-label="$options.trackLabel"
-      />
+            {{ __('Show me how to add a pipeline') }}
+          </gl-button>
+        </div>
+      </div>
     </div>
   </div>
 </template>

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_pipeline_tour.vue

-<script>
-import { GlPopover, GlDeprecatedButton, GlSprintf, GlLink } from '@gitlab/ui';
-import Icon from '~/vue_shared/components/icon.vue';
-import Cookies from 'js-cookie';
-import { parseBoolean } from '~/lib/utils/common_utils';
-import Tracking from '~/tracking';
-import { s__ } from '~/locale';
-
-const trackingMixin = Tracking.mixin();
-
-const cookieKey = 'suggest_pipeline_dismissed';
-
-export default {
-  name: 'MRWidgetPipelineTour',
-  dismissTrackValue: 20,
-  showTrackValue: 10,
-  trackEvent: 'click_button',
-  helpContent: s__(
-    `mrWidget|Use %{linkStart}CI pipelines to test your code%{linkEnd}, simply add a GitLab CI configuration file to your project. It only takes a minute to make your code more secure and robust.`,
-  ),
-  helpURL: 'https://about.gitlab.com/blog/2019/07/12/guide-to-ci-cd-pipelines/',
-  components: {
-    GlPopover,
-    GlDeprecatedButton,
-    Icon,
-    GlSprintf,
-    GlLink,
-  },
-  mixins: [trackingMixin],
-  props: {
-    pipelinePath: {
-      type: String,
-      required: true,
-    },
-    pipelineSvgPath: {
-      type: String,
-      required: true,
-    },
-    humanAccess: {
-      type: String,
-      required: true,
-    },
-    popoverTarget: {
-      type: String,
-      required: true,
-    },
-    popoverContainer: {
-      type: String,
-      required: true,
-    },
-    trackLabel: {
-      type: String,
-      required: true,
-    },
-  },
-  data() {
-    return {
-      popoverDismissed: parseBoolean(Cookies.get(cookieKey)),
-      tracking: {
-        label: this.trackLabel,
-        property: this.humanAccess,
-      },
-    };
-  },
-  mounted() {
-    this.trackOnShow();
-  },
-  methods: {
-    trackOnShow() {
-      if (!this.popoverDismissed) {
-        this.track();
-      }
-    },
-    dismissPopover() {
-      this.popoverDismissed = true;
-      Cookies.set(cookieKey, this.popoverDismissed, { expires: 365 });
-    },
-  },
-};
-</script>
-<template>
-  <gl-popover
-    v-if="!popoverDismissed"
-    show
-    :target="popoverTarget"
-    :container="popoverContainer"
-    placement="rightbottom"
-  >
-    <template #title>
-      <button
-        class="btn-blank float-right mt-1"
-        type="button"
-        :aria-label="__('Close')"
-        :data-track-property="humanAccess"
-        :data-track-value="$options.dismissTrackValue"
-        :data-track-event="$options.trackEvent"
-        :data-track-label="trackLabel"
-        @click="dismissPopover"
-      >
-        <icon name="close" aria-hidden="true" />
-      </button>
-      {{ s__('mrWidget|Are you adding technical debt or code vulnerabilities?') }}
-    </template>
-    <div class="svg-content svg-150 pt-1">
-      <img :src="pipelineSvgPath" />
-    </div>
-    <gl-sprintf :message="$options.helpContent">
-      <template #link="{ content }">
-        <gl-link :href="$options.helpURL" target="_blank" class="font-size-inherit">{{
-          content
-        }}</gl-link>
-      </template>
-    </gl-sprintf>
-    <gl-deprecated-button
-      ref="ok"
-      category="primary"
-      class="mt-2 mb-0"
-      variant="info"
-      block
-      :href="pipelinePath"
-      :data-track-property="humanAccess"
-      :data-track-value="$options.showTrackValue"
-      :data-track-event="$options.trackEvent"
-      :data-track-label="trackLabel"
-    >
-      {{ __('Show me how to add a pipeline') }}
-    </gl-deprecated-button>
-    <gl-deprecated-button
-      ref="no-thanks"
-      category="secondary"
-      class="mt-2 mb-0"
-      variant="info"
-      block
-      :data-track-property="humanAccess"
-      :data-track-value="$options.dismissTrackValue"
-      :data-track-event="$options.trackEvent"
-      :data-track-label="trackLabel"
-      @click="dismissPopover"
-    >
-      {{ __('No thanks') }}
-    </gl-deprecated-button>
-  </gl-popover>
-</template>

app/assets/stylesheets/framework/images.scss

@@ -20,7 +20,7 @@
     width: 100%;
   }
 
-  $image-widths: 80 130 150 250 306 394 430;
+  $image-widths: 80 130 150 225 250 306 394 430;
   @each $width in $image-widths {
     &.svg-#{$width} {
       img,

app/assets/stylesheets/pages/merge_requests.scss

@@ -397,6 +397,16 @@ $mr-widget-min-height: 69px;
         }
       }
     }
+
+    &.mr-pipeline-suggest {
+      border-radius: $border-radius-default;
+      line-height: 20px;
+      border: 1px solid $border-color;
+
+      .circle-icon-container {
+        color: $gl-text-color-quaternary;
+      }
+    }
   }
 
   .mr-widget-help {
@@ -596,26 +606,6 @@ $mr-widget-min-height: 69px;
   }
 }
 
-.mr-pipeline-suggest {
-  flex-wrap: wrap;
-  border-radius: $border-radius-default;
-  padding: $gl-padding;
-  border: 1px solid $border-color;
-  min-height: $mr-widget-min-height;
-
-  @include media-breakpoint-up(md) {
-    align-items: center;
-  }
-
-  .circle-icon-container {
-    color: $gl-text-color-quaternary;
-  }
-
-  .popover {
-    z-index: 240;
-  }
-}
-
 .card-new-merge-request {
   .card-header {
     padding: 5px 10px;

locale/gitlab.pot

@@ -15311,9 +15311,6 @@ msgstr ""
 msgid "No test coverage"
 msgstr ""
 
-msgid "No thanks"
-msgstr ""
-
 msgid "No vulnerabilities present"
 msgstr ""
 
@@ -27554,7 +27551,7 @@ msgstr ""
 msgid "mrWidget|To approve this merge request, please enter your password. This project requires all approvals to be authenticated."
 msgstr ""
 
-msgid "mrWidget|Use %{linkStart}CI pipelines to test your code%{linkEnd}, simply add a GitLab CI configuration file to your project. It only takes a minute to make your code more secure and robust."
+msgid "mrWidget|Use %{linkStart}CI pipelines to test your code%{linkEnd} by simply adding a GitLab CI configuration file to your project. It only takes a minute to make your code more secure and robust."
 msgstr ""
 
 msgid "mrWidget|When this merge request is ready, remove the WIP: prefix from the title to allow it to be merged"

spec/frontend/vue_mr_widget/components/mr_widget_suggest_pipeline_spec.js

 import { mount } from '@vue/test-utils';
 import { GlLink, GlSprintf } from '@gitlab/ui';
 import suggestPipelineComponent from '~/vue_merge_request_widget/components/mr_widget_suggest_pipeline.vue';
-import stubChildren from 'helpers/stub_children';
-import PipelineTourState from '~/vue_merge_request_widget/components/states/mr_widget_pipeline_tour.vue';
 import MrWidgetIcon from '~/vue_merge_request_widget/components/mr_widget_icon.vue';
 import { mockTracking, triggerEvent, unmockTracking } from 'helpers/tracking_helper';
+import { popoverProps, iconName } from './pipeline_tour_mock_data';
 
-describe('MRWidgetHeader', () => {
+describe('MRWidgetSuggestPipeline', () => {
   let wrapper;
-  const pipelinePath = '/foo/bar/add/pipeline/path';
-  const pipelineSvgPath = '/foo/bar/pipeline/svg/path';
-  const humanAccess = 'maintainer';
-  const iconName = 'status_notfound';
+  let trackingSpy;
+
+  const mockTrackingOnWrapper = () => {
+    trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
+  };
 
   beforeEach(() => {
+    document.body.dataset.page = 'projects:merge_requests:show';
+    trackingSpy = mockTracking('_category_', undefined, jest.spyOn);
+
     wrapper = mount(suggestPipelineComponent, {
-      propsData: { pipelinePath, pipelineSvgPath, humanAccess },
+      propsData: popoverProps,
       stubs: {
-        ...stubChildren(PipelineTourState),
         GlSprintf,
       },
     });
@@ -25,14 +27,17 @@ describe('MRWidgetHeader', () => {
 
   afterEach(() => {
     wrapper.destroy();
+    unmockTracking();
   });
 
   describe('template', () => {
+    const findOkBtn = () => wrapper.find('[data-testid="ok"]');
+
     it('renders add pipeline file link', () => {
       const link = wrapper.find(GlLink);
 
       expect(link.exists()).toBe(true);
-      expect(link.attributes().href).toBe(pipelinePath);
+      expect(link.attributes().href).toBe(popoverProps.pipelinePath);
     });
 
     it('renders the expected text', () => {
@@ -52,25 +57,60 @@ describe('MRWidgetHeader', () => {
       );
     });
 
+    it('renders the show me how button', () => {
+      const button = findOkBtn();
+
+      expect(button.exists()).toBe(true);
+      expect(button.classes('btn-info')).toEqual(true);
+      expect(button.attributes('href')).toBe(popoverProps.pipelinePath);
+    });
+
+    it('renders the help link', () => {
+      const link = wrapper.find('[data-testid="help"]');
+
+      expect(link.exists()).toBe(true);
+      expect(link.attributes('href')).toBe(wrapper.vm.$options.helpURL);
+    });
+
+    it('renders the empty pipelines image', () => {
+      const image = wrapper.find('[data-testid="pipeline-image"]');
+
+      expect(image.exists()).toBe(true);
+      expect(image.attributes().src).toBe(popoverProps.pipelineSvgPath);
+    });
+
     describe('tracking', () => {
-      let spy;
+      it('send event for basic view of the suggest pipeline widget', () => {
+        const expectedCategory = undefined;
+        const expectedAction = undefined;
 
-      beforeEach(() => {
-        spy = mockTracking('_category_', wrapper.element, jest.spyOn);
+        expect(trackingSpy).toHaveBeenCalledWith(expectedCategory, expectedAction, {
+          label: wrapper.vm.$options.trackLabel,
+          property: popoverProps.humanAccess,
+        });
       });
 
-      afterEach(() => {
-        unmockTracking();
+      it('send an event when add pipeline link is clicked', () => {
+        mockTrackingOnWrapper();
+        const link = wrapper.find('[data-testid="add-pipeline-link"]');
+        triggerEvent(link.element);
+
+        expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_link', {
+          label: wrapper.vm.$options.trackLabel,
+          property: popoverProps.humanAccess,
+          value: '30',
+        });
       });
 
       it('send an event when ok button is clicked', () => {
-        const link = wrapper.find(GlLink);
-        triggerEvent(link.element);
+        mockTrackingOnWrapper();
+        const okBtn = findOkBtn();
+        triggerEvent(okBtn.element);
 
-        expect(spy).toHaveBeenCalledWith('_category_', 'click_link', {
-          label: 'no_pipeline_noticed',
-          property: humanAccess,
-          value: '30',
+        expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
+          label: wrapper.vm.$options.trackLabel,
+          property: popoverProps.humanAccess,
+          value: '10',
         });
       });
     });

spec/frontend/vue_mr_widget/components/states/pipeline_tour_mock_data.js → spec/frontend/vue_mr_widget/components/pipeline_tour_mock_data.js

@@ -2,9 +2,6 @@ export const popoverProps = {
   pipelinePath: '/foo/bar/add/pipeline/path',
   pipelineSvgPath: 'assets/illustrations/something.svg',
   humanAccess: 'maintainer',
-  popoverTarget: 'suggest-popover',
-  popoverContainer: 'suggest-pipeline',
-  trackLabel: 'some_tracking_label',
 };
 
-export const cookieKey = 'suggest_pipeline_dismissed';
+export const iconName = 'status_notfound';

spec/frontend/vue_mr_widget/components/states/mr_widget_pipeline_tour_spec.js

-import { shallowMount } from '@vue/test-utils';
-import { GlPopover, GlLink, GlSprintf } from '@gitlab/ui';
-import Cookies from 'js-cookie';
-import { mockTracking, triggerEvent, unmockTracking } from 'helpers/tracking_helper';
-import pipelineTourState from '~/vue_merge_request_widget/components/states/mr_widget_pipeline_tour.vue';
-import { popoverProps, cookieKey } from './pipeline_tour_mock_data';
-
-describe('MRWidgetPipelineTour', () => {
-  let wrapper;
-
-  afterEach(() => {
-    wrapper.destroy();
-  });
-
-  describe('template', () => {
-    describe(`when ${cookieKey} cookie is set`, () => {
-      beforeEach(() => {
-        Cookies.set(cookieKey, true);
-        wrapper = shallowMount(pipelineTourState, {
-          propsData: popoverProps,
-        });
-      });
-
-      it('does not render the popover', () => {
-        const popover = wrapper.find(GlPopover);
-
-        expect(popover.exists()).toBe(false);
-      });
-
-      describe('tracking', () => {
-        let trackingSpy;
-
-        beforeEach(() => {
-          trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
-        });
-
-        afterEach(() => {
-          unmockTracking();
-        });
-        it('does not call tracking', () => {
-          expect(trackingSpy).not.toHaveBeenCalled();
-        });
-      });
-    });
-
-    describe(`when ${cookieKey} cookie is not set`, () => {
-      const findOkBtn = () => wrapper.find({ ref: 'ok' });
-      const findDismissBtn = () => wrapper.find({ ref: 'no-thanks' });
-
-      beforeEach(() => {
-        Cookies.remove(cookieKey);
-        wrapper = shallowMount(pipelineTourState, {
-          propsData: popoverProps,
-          stubs: {
-            GlSprintf,
-          },
-        });
-      });
-
-      it('renders the popover', () => {
-        const popover = wrapper.find(GlPopover);
-
-        expect(popover.exists()).toBe(true);
-      });
-
-      it('renders the help link', () => {
-        const link = wrapper.find(GlLink);
-
-        expect(link.exists()).toBe(true);
-        expect(link.attributes('href')).toBe(wrapper.vm.$options.helpURL);
-      });
-
-      it('renders the show me how button', () => {
-        const button = findOkBtn();
-
-        expect(button.exists()).toBe(true);
-        expect(button.attributes().category).toBe('primary');
-      });
-
-      it('renders the dismiss button', () => {
-        const button = findDismissBtn();
-
-        expect(button.exists()).toBe(true);
-        expect(button.attributes().category).toBe('secondary');
-      });
-
-      it('renders the empty pipelines image', () => {
-        const image = wrapper.find('img');
-
-        expect(image.exists()).toBe(true);
-        expect(image.attributes().src).toBe(popoverProps.pipelineSvgPath);
-      });
-
-      describe('tracking', () => {
-        let trackingSpy;
-
-        beforeEach(() => {
-          trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
-        });
-
-        afterEach(() => {
-          unmockTracking();
-        });
-
-        it('send event for basic view of popover', () => {
-          document.body.dataset.page = 'projects:merge_requests:show';
-
-          wrapper.vm.trackOnShow();
-
-          expect(trackingSpy).toHaveBeenCalledWith(undefined, undefined, {
-            label: popoverProps.trackLabel,
-            property: popoverProps.humanAccess,
-          });
-        });
-
-        it('send an event when ok button is clicked', () => {
-          const okBtn = findOkBtn();
-          triggerEvent(okBtn.element);
-
-          expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
-            label: popoverProps.trackLabel,
-            property: popoverProps.humanAccess,
-            value: '10',
-          });
-        });
-
-        it('send an event when dismiss button is clicked', () => {
-          const dismissBtn = findDismissBtn();
-          triggerEvent(dismissBtn.element);
-
-          expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_button', {
-            label: popoverProps.trackLabel,
-            property: popoverProps.humanAccess,
-            value: '20',
-          });
-        });
-      });
-
-      describe('dismissPopover', () => {
-        it('updates popoverDismissed', () => {
-          const button = findDismissBtn();
-          const popover = wrapper.find(GlPopover);
-          button.vm.$emit('click');
-
-          return wrapper.vm.$nextTick().then(() => {
-            expect(Cookies.get(cookieKey)).toBe('true');
-            expect(popover.exists()).toBe(false);
-          });
-        });
-      });
-    });
-  });
-});