Apply reviewer feedback

- Convert CTA button to link
- Add translation scope
- Set a cookie when clicking the CTA
- Increase cookie expiration in tests
- Add empty value for img tag alt attr
- Change copy
- Flip colored border of dark popover
- Remove popover offset

ee/app/assets/javascripts/projects/sast_entry_points_experiment/components/banner.vue

 <script>
 import { GlBanner } from '@gitlab/ui';
 import { I18N } from '../constants';
-import { isDismissed, dismiss, trackShow, trackCtaClicked } from '../utils';
+import { isDismissed, dismiss, trackShow, trackCtaClicked, trackDismissed } from '../utils';
 
 export default {
   components: {
@@ -27,8 +27,10 @@ export default {
     onDismiss() {
       this.isVisible = false;
       dismiss();
+      trackDismissed();
     },
     onClick() {
+      dismiss();
       trackCtaClicked();
     },
   },

ee/app/assets/javascripts/projects/sast_entry_points_experiment/components/popover_dark.vue

 <script>
-import { GlPopover, GlButton } from '@gitlab/ui';
+import { GlPopover, GlButton, GlLink } from '@gitlab/ui';
 import { I18N, POPOVER_TARGET } from '../constants';
-import { isDismissed, dismiss, trackShow, trackCtaClicked } from '../utils';
+import { isDismissed, dismiss, trackShow, trackCtaClicked, trackDismissed } from '../utils';
 
 export default {
   components: {
     GlPopover,
     GlButton,
+    GlLink,
   },
   props: {
     sastDocumentationPath: {
@@ -29,8 +30,10 @@ export default {
     onDismiss() {
       this.isVisible = false;
       dismiss();
+      trackDismissed();
     },
     onClick() {
+      dismiss();
       trackCtaClicked();
     },
   },
@@ -46,11 +49,10 @@ export default {
     show
     triggers="manual"
     placement="bottomright"
-    offset="93"
-    :css-classes="['marketing-popover', 'gl-border-4', 'gl-border-t-solid']"
+    :css-classes="['marketing-popover', 'gl-border-4']"
   >
     <div class="gl-display-flex gl-mt-n2">
-      <img :src="$options.gitlabLogo" height="24" width="24" class="gl-ml-2 gl-mr-3" />
+      <img :src="$options.gitlabLogo" :alt="''" height="24" width="24" class="gl-ml-2 gl-mr-3" />
       <div>
         <div
           class="gl-font-weight-bold gl-font-lg gl-line-height-20 gl-text-theme-indigo-900 gl-mb-3"
@@ -60,15 +62,14 @@ export default {
         <div class="gl-font-base gl-line-height-20 gl-mb-3">
           {{ $options.i18n.bodyText }}
         </div>
-        <gl-button variant="link" :href="sastDocumentationPath" @click="onClick">
+        <gl-link :href="sastDocumentationPath" @click="onClick">
           {{ $options.i18n.linkText }}
-        </gl-button>
+        </gl-link>
       </div>
       <gl-button
         category="tertiary"
         class="gl-align-self-start gl-mt-n3 gl-mr-n3"
         icon="close"
-        data-testid="close-btn"
         :aria-label="__('Close')"
         @click="onDismiss"
       />

ee/app/assets/javascripts/projects/sast_entry_points_experiment/components/popover_light.vue

 <script>
-import { GlPopover, GlButton } from '@gitlab/ui';
+import { GlPopover, GlButton, GlLink } from '@gitlab/ui';
 import { I18N, POPOVER_TARGET } from '../constants';
-import { isDismissed, dismiss, trackShow, trackCtaClicked } from '../utils';
+import { isDismissed, dismiss, trackShow, trackCtaClicked, trackDismissed } from '../utils';
 
 export default {
   components: {
     GlPopover,
     GlButton,
+    GlLink,
   },
   props: {
     sastDocumentationPath: {
@@ -29,8 +30,10 @@ export default {
     onDismiss() {
       this.isVisible = false;
       dismiss();
+      trackDismissed();
     },
     onClick() {
+      dismiss();
       trackCtaClicked();
     },
   },
@@ -39,14 +42,7 @@ export default {
 </script>
 
 <template>
-  <gl-popover
-    v-if="isVisible"
-    :target="target"
-    show
-    triggers="manual"
-    placement="bottomright"
-    offset="53"
-  >
+  <gl-popover v-if="isVisible" :target="target" show triggers="manual" placement="bottomright">
     <template #title>
       <div class="gl-display-flex">
         <span>
@@ -57,7 +53,6 @@ export default {
           category="tertiary"
           class="gl-align-self-start close gl-opacity-10"
           icon="close"
-          data-testid="close-btn"
           :aria-label="__('Close')"
           @click="onDismiss"
         />
@@ -65,9 +60,9 @@ export default {
     </template>
     {{ $options.i18n.bodyText }}
     <div class="gl-text-right gl-font-weight-bold">
-      <gl-button variant="link" :href="sastDocumentationPath" @click="onClick">
+      <gl-link :href="sastDocumentationPath" @click="onClick">
         {{ $options.i18n.linkText }}
-      </gl-button>
+      </gl-link>
     </div>
   </gl-popover>
 </template>

ee/app/assets/javascripts/projects/sast_entry_points_experiment/constants.js

@@ -11,6 +11,6 @@ export const I18N = {
   bodyText: s__(
     'SastEntryPoints|GitLab can scan your code for security vulnerabilities. Static Application Security Testing (SAST) helps you worry less and build more.',
   ),
-  buttonText: s__('SastEntryPoints|Learn more'),
+  buttonText: s__('SastEntryPoints|Learn more.'),
   linkText: s__('SastEntryPoints|How do I set up SAST?'),
 };

ee/app/assets/javascripts/projects/sast_entry_points_experiment/utils.js

@@ -10,6 +10,9 @@ export const isDismissed = () => {
 
 export const dismiss = () => {
   setCookie(COOKIE_NAME, 'true');
+};
+
+export const trackDismissed = () => {
   tracking.event('dismissed');
 };
 

ee/app/assets/stylesheets/page_bundles/marketing_popover.scss

@@ -2,13 +2,32 @@
 
 .marketing-popover {
   max-width: $grid-size * 45;
-  border-top-color: $theme-indigo-900;
 
-  .arrow {
-    @include gl-mt-n2;
+  &.bs-popover-bottom {
+    @include gl-border-t-solid;
 
-    &::after {
-      border-bottom-color: $theme-indigo-900;
+    border-top-color: $theme-indigo-900;
+
+    .arrow {
+      @include gl-mt-n2;
+
+      &::after {
+        border-bottom-color: $theme-indigo-900;
+      }
+    }
+  }
+
+  &.bs-popover-top {
+    @include gl-border-b-solid;
+
+    border-bottom-color: $theme-indigo-900;
+
+    .arrow {
+      margin-bottom: -$gl-spacing-scale-2;
+
+      &::after {
+        border-top-color: $theme-indigo-900;
+      }
     }
   }
 }

ee/app/presenters/ee/project_presenter.rb

@@ -24,7 +24,7 @@ module EE
 
       ::ProjectPresenter::AnchorData.new(
         false,
-        statistic_icon + _('Add Security Testing'),
+        statistic_icon + s_('SastEntryPoints|Add Security Testing'),
         help_page_path('user/application_security/sast/index'),
         'btn-dashed js-sast-entry-point',
         nil,

ee/spec/frontend/projects/sast_entry_points_experiment/components/__snapshots__/banner_spec.js.snap

@@ -3,7 +3,7 @@
 exports[`When the cookie is not set matches the snapshot 1`] = `
 <gl-banner-stub
   buttonlink="sast_documentation_path"
-  buttontext="Learn more"
+  buttontext="Learn more."
   class="gl-my-5"
   title="Catch your security vulnerabilities ahead of time!"
   variant="promotion"

ee/spec/frontend/projects/sast_entry_points_experiment/components/__snapshots__/popover_dark_spec.js.snap

 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`When the cookie is not set matches the snapshot 1`] = `
-<div
-  class="gl-popover"
-  offset="93"
+<gl-popover-stub
+  cssclasses="marketing-popover,gl-border-4"
+  placement="bottomright"
   show=""
+  triggers="manual"
 >
   <div
     class="gl-display-flex gl-mt-n2"
   >
     <img
+      alt=""
       class="gl-ml-2 gl-mr-3"
       height="24"
       width="24"
@@ -32,45 +34,24 @@ exports[`When the cookie is not set matches the snapshot 1`] = `
       
       </div>
        
-      <a
-        class="btn btn-link btn-md gl-button"
+      <gl-link-stub
         href="sast_documentation_path"
       >
-        <!---->
-         
-        <!---->
-          
-        <span
-          class="gl-button-text"
-        >
-          
+        
         How do I set up SAST?
       
-        </span>
-      </a>
+      </gl-link-stub>
     </div>
      
-    <button
+    <gl-button-stub
       aria-label="Close"
-      class="btn gl-align-self-start gl-mt-n3 gl-mr-n3 btn-default btn-md gl-button btn-default-tertiary btn-icon"
-      data-testid="close-btn"
-      type="button"
-    >
-      <!---->
-       
-      <svg
-        aria-hidden="true"
-        class="gl-button-icon gl-icon s16"
-        data-testid="close-icon"
-        role="img"
-      >
-        <use
-          href="#close"
-        />
-      </svg>
-        
-      <!---->
-    </button>
+      buttontextclasses=""
+      category="tertiary"
+      class="gl-align-self-start gl-mt-n3 gl-mr-n3"
+      icon="close"
+      size="medium"
+      variant="default"
+    />
   </div>
-</div>
+</gl-popover-stub>
 `;

ee/spec/frontend/projects/sast_entry_points_experiment/components/__snapshots__/popover_light_spec.js.snap

@@ -3,7 +3,6 @@
 exports[`When the cookie is not set matches the snapshot 1`] = `
 <div
   class="gl-popover"
-  offset="53"
   show=""
 >
   
@@ -13,20 +12,12 @@ exports[`When the cookie is not set matches the snapshot 1`] = `
     class="gl-text-right gl-font-weight-bold"
   >
     <a
-      class="btn btn-link btn-md gl-button"
+      class="gl-link"
       href="sast_documentation_path"
     >
-      <!---->
-       
-      <!---->
-        
-      <span
-        class="gl-button-text"
-      >
-        
+      
       How do I set up SAST?
     
-      </span>
     </a>
   </div>
   <div
@@ -45,7 +36,6 @@ exports[`When the cookie is not set matches the snapshot 1`] = `
     <button
       aria-label="Close"
       class="btn gl-align-self-start close gl-opacity-10 btn-default btn-md gl-button btn-default-tertiary btn-icon"
-      data-testid="close-btn"
       type="button"
     >
       <!---->

ee/spec/frontend/projects/sast_entry_points_experiment/components/banner_spec.js

@@ -25,7 +25,7 @@ afterEach(() => {
 
 describe('When the cookie is set', () => {
   beforeEach(() => {
-    Cookies.set(COOKIE_NAME, 'true');
+    Cookies.set(COOKIE_NAME, 'true', { expires: 365 });
     createComponent();
   });
 
@@ -63,6 +63,10 @@ describe('When the cookie is not set', () => {
     it('tracks the cta_clicked event', () => {
       expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith('cta_clicked');
     });
+
+    it('sets a cookie', () => {
+      expect(Cookies.get(COOKIE_NAME)).toBe('true');
+    });
   });
 
   describe('When dismissing the component', () => {

ee/spec/frontend/projects/sast_entry_points_experiment/components/popover_dark_spec.js

 import '~/commons';
-import { GlPopover, GlButton } from '@gitlab/ui';
+import { GlPopover, GlButton, GlLink } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
 import Cookies from 'js-cookie';
 import PopoverDark from 'ee/projects/sast_entry_points_experiment/components/popover_dark.vue';
 import { COOKIE_NAME } from 'ee/projects/sast_entry_points_experiment/constants';
-import { mountExtended } from 'helpers/vue_test_utils_helper';
 import ExperimentTracking from '~/experimentation/experiment_tracking';
 
 jest.mock('~/experimentation/experiment_tracking');
@@ -12,11 +12,11 @@ let wrapper;
 
 const sastDocumentationPath = 'sast_documentation_path';
 const findPopover = () => wrapper.findComponent(GlPopover);
-const findCtaButton = () => findPopover().findComponent(GlButton);
-const findCloseButton = () => wrapper.findByTestId('close-btn');
+const findCtaLink = () => findPopover().findComponent(GlLink);
+const findCloseButton = () => findPopover().findComponent(GlButton);
 
 function createComponent() {
-  wrapper = mountExtended(PopoverDark, {
+  wrapper = shallowMount(PopoverDark, {
     propsData: { sastDocumentationPath },
   });
 }
@@ -28,7 +28,7 @@ afterEach(() => {
 
 describe('When the cookie is set', () => {
   beforeEach(() => {
-    Cookies.set(COOKIE_NAME, 'true');
+    Cookies.set(COOKIE_NAME, 'true', { expires: 365 });
     createComponent();
   });
 
@@ -51,7 +51,7 @@ describe('When the cookie is not set', () => {
   });
 
   it('uses the sastDocumentationPath from the props for the button link', () => {
-    expect(findCtaButton().attributes('href')).toBe(sastDocumentationPath);
+    expect(findCtaLink().attributes('href')).toBe(sastDocumentationPath);
   });
 
   it('matches the snapshot', () => {
@@ -60,12 +60,16 @@ describe('When the cookie is not set', () => {
 
   describe('When clicking the CTA button', () => {
     beforeEach(() => {
-      findCtaButton().vm.$emit('click');
+      findCtaLink().vm.$emit('click');
     });
 
     it('tracks the cta_clicked event', () => {
       expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith('cta_clicked');
     });
+
+    it('sets a cookie', () => {
+      expect(Cookies.get(COOKIE_NAME)).toBe('true');
+    });
   });
 
   describe('When dismissing the component', () => {

ee/spec/frontend/projects/sast_entry_points_experiment/components/popover_light_spec.js

 import '~/commons';
-import { GlPopover, GlButton } from '@gitlab/ui';
+import { GlPopover, GlButton, GlLink } from '@gitlab/ui';
+import { mount } from '@vue/test-utils';
 import Cookies from 'js-cookie';
 import PopoverLight from 'ee/projects/sast_entry_points_experiment/components/popover_light.vue';
 import { COOKIE_NAME } from 'ee/projects/sast_entry_points_experiment/constants';
-import { mountExtended } from 'helpers/vue_test_utils_helper';
 import ExperimentTracking from '~/experimentation/experiment_tracking';
 
 jest.mock('~/experimentation/experiment_tracking');
@@ -12,11 +12,11 @@ let wrapper;
 
 const sastDocumentationPath = 'sast_documentation_path';
 const findPopover = () => wrapper.findComponent(GlPopover);
-const findCtaButton = () => findPopover().findComponent(GlButton);
-const findCloseButton = () => wrapper.findByTestId('close-btn');
+const findCtaLink = () => findPopover().findComponent(GlLink);
+const findCloseButton = () => findPopover().findComponent(GlButton);
 
 function createComponent() {
-  wrapper = mountExtended(PopoverLight, {
+  wrapper = mount(PopoverLight, {
     propsData: { sastDocumentationPath },
   });
 }
@@ -28,7 +28,7 @@ afterEach(() => {
 
 describe('When the cookie is set', () => {
   beforeEach(() => {
-    Cookies.set(COOKIE_NAME, 'true');
+    Cookies.set(COOKIE_NAME, 'true', { expires: 365 });
     createComponent();
   });
 
@@ -51,7 +51,7 @@ describe('When the cookie is not set', () => {
   });
 
   it('uses the sastDocumentationPath from the props for the button link', () => {
-    expect(findCtaButton().attributes('href')).toBe(sastDocumentationPath);
+    expect(findCtaLink().attributes('href')).toBe(sastDocumentationPath);
   });
 
   it('matches the snapshot', () => {
@@ -60,12 +60,16 @@ describe('When the cookie is not set', () => {
 
   describe('When clicking the CTA button', () => {
     beforeEach(() => {
-      findCtaButton().vm.$emit('click');
+      findCtaLink().vm.$emit('click');
     });
 
     it('tracks the cta_clicked event', () => {
       expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith('cta_clicked');
     });
+
+    it('sets a cookie', () => {
+      expect(Cookies.get(COOKIE_NAME)).toBe('true');
+    });
   });
 
   describe('When dismissing the component', () => {

locale/gitlab.pot

@@ -1903,9 +1903,6 @@ msgstr ""
 msgid "Add README"
 msgstr ""
 
-msgid "Add Security Testing"
-msgstr ""
-
 msgid "Add Zoom meeting"
 msgstr ""
 
@@ -28546,6 +28543,9 @@ msgstr ""
 msgid "SVG illustration"
 msgstr ""
 
+msgid "SastEntryPoints|Add Security Testing"
+msgstr ""
+
 msgid "SastEntryPoints|Catch your security vulnerabilities ahead of time!"
 msgstr ""
 
@@ -28555,7 +28555,7 @@ msgstr ""
 msgid "SastEntryPoints|How do I set up SAST?"
 msgstr ""
 
-msgid "SastEntryPoints|Learn more"
+msgid "SastEntryPoints|Learn more."
 msgstr ""
 
 msgid "Satisfied"