Minor review comments13076-stage-card-ui-component-ce

Address maintainer feedback

Remove unused `canEdit` property

Set `canEdit` required to false

Replace button with gl-button

Fix missing i18n string

Use .toContain to check rendered text

5 files changed, 19 insertions, 23 deletions

diff --git a/app/assets/javascripts/cycle_analytics/components/stage_card_list_item.vue b/app/assets/javascripts/cycle_analytics/components/stage_card_list_item.vue
index 52f7cb91b71..d946594a069 100644
--- a/app/assets/javascripts/cycle_analytics/components/stage_card_list_item.vue
+++ b/app/assets/javascripts/cycle_analytics/components/stage_card_list_item.vue
@@ -1,10 +1,12 @@
 <script>
 import Icon from '~/vue_shared/components/icon.vue';
+import { GlButton } from '@gitlab/ui';
 
 export default {
   name: 'StageCardListItem',
   components: {
     Icon,
+    GlButton,
   },
   props: {
     isActive: {
@@ -14,6 +16,7 @@ export default {
     canEdit: {
       type: Boolean,
       default: false,
+      required: false,
     },
   },
 };
@@ -23,14 +26,13 @@ export default {
   <div :class="{ active: isActive }" class="stage-nav-item d-flex pl-4 pr-4 m-0 mb-1 ml-2 rounded">
     <slot></slot>
     <div v-if="canEdit" class="dropdown">
-      <button
-        type="button"
-        title="More actions"
+      <gl-button
+        :title="__('More actions')"
         class="more-actions-toggle btn btn-transparent p-0"
         data-toggle="dropdown"
       >
         <icon css-classes="icon" name="ellipsis_v" />
-      </button>
+      </gl-button>
       <ul class="more-actions-dropdown dropdown-menu dropdown-open-left">
         <slot name="dropdown-options"></slot>
       </ul>
diff --git a/app/assets/javascripts/cycle_analytics/components/stage_nav_item.vue b/app/assets/javascripts/cycle_analytics/components/stage_nav_item.vue
index 952fa25975b..004d335f572 100644
--- a/app/assets/javascripts/cycle_analytics/components/stage_nav_item.vue
+++ b/app/assets/javascripts/cycle_analytics/components/stage_nav_item.vue
@@ -10,10 +10,12 @@ export default {
     isDefaultStage: {
       type: Boolean,
       default: false,
+      required: false,
     },
     isActive: {
       type: Boolean,
       default: false,
+      required: false,
     },
     isUserAllowed: {
       type: Boolean,
@@ -26,17 +28,14 @@ export default {
     value: {
       type: String,
       default: '',
+      required: false,
     },
     canEdit: {
       type: Boolean,
       default: false,
+      required: false,
     },
   },
-  data() {
-    return {
-      displayMenu: true,
-    };
-  },
   computed: {
     hasValue() {
       return this.value && this.value.length > 0;
@@ -45,15 +44,12 @@ export default {
       return this.isUserAllowed && this.canEdit;
     },
   },
-  mounted() {
-    // console.log('StageNavItem::props', this.propsData);
-  },
 };
 </script>
 
 <template>
   <li @click="$emit('select')">
-    <stage-card-list-item :is-active="isActive" :display-menu="true" :can-edit="editable">
+    <stage-card-list-item :is-active="isActive" :can-edit="editable">
       <div class="stage-nav-item-cell stage-name p-0" :class="{ 'font-weight-bold': isActive }">
         {{ title }}
       </div>
diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss
index 31df8ef75dd..d80155a416d 100644
--- a/app/assets/stylesheets/pages/cycle_analytics.scss
+++ b/app/assets/stylesheets/pages/cycle_analytics.scss
@@ -146,7 +146,7 @@
 
     .stage-nav-item {
       line-height: 65px;
-      border: $border-color solid 1px;
+      border: 1px solid $border-color;
 
       &.active {
         background: $blue-50;
diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml
index b982dda7549..2b594c125f4 100644
--- a/app/views/projects/cycle_analytics/show.html.haml
+++ b/app/views/projects/cycle_analytics/show.html.haml
@@ -56,8 +56,7 @@
         .stage-panel-body
           %nav.stage-nav
             %ul
-              -# canEdit will eventually be a License.feature_available? check, default to false
-              %stage-nav-item{ "v-for" => "stage in state.stages", ":key" => '`ca-stage-title-${stage.title}`', '@select' => 'selectStage(stage)', ":title" => "stage.title", ":is-user-allowed" => "stage.isUserAllowed", ":value" => "stage.value", ":is-active" => "stage.active", ":can-edit" => false }
+              %stage-nav-item{ "v-for" => "stage in state.stages", ":key" => '`ca-stage-title-${stage.title}`', '@select' => 'selectStage(stage)', ":title" => "stage.title", ":is-user-allowed" => "stage.isUserAllowed", ":value" => "stage.value", ":is-active" => "stage.active" }
           .section.stage-events
             %template{ "v-if" => "isLoadingStage" }
               = icon("spinner spin")
diff --git a/spec/frontend/cycle_analytics/stage_nav_item_spec.js b/spec/frontend/cycle_analytics/stage_nav_item_spec.js
index 1c1c2877e22..ff079082ca7 100644
--- a/spec/frontend/cycle_analytics/stage_nav_item_spec.js
+++ b/spec/frontend/cycle_analytics/stage_nav_item_spec.js
@@ -1,4 +1,3 @@
-// import Vue from 'vue';
 import { mount, shallowMount } from '@vue/test-utils';
 import StageNavItem from '~/cycle_analytics/components/stage_nav_item.vue';
 
@@ -146,13 +145,13 @@ describe('StageNavItem', () => {
         );
       });
       it('can hide the stage', () => {
-        expect(wrapper.html().indexOf('Hide stage') > 0).toBe(true);
+        expect(wrapper.text()).toContain('Hide stage');
       });
       it('can not edit the stage', () => {
-        expect(wrapper.html().indexOf('Edit stage') > 0).toBe(false);
+        expect(wrapper.text()).not.toContain('Edit stage');
       });
       it('can not remove the stage', () => {
-        expect(wrapper.html().indexOf('Remove stage') > 0).toBe(false);
+        expect(wrapper.text()).not.toContain('Remove stage');
       });
     });
 
@@ -164,14 +163,14 @@ describe('StageNavItem', () => {
         );
       });
       it('can edit the stage', () => {
-        expect(wrapper.html().indexOf('Edit stage') > 0).toBe(true);
+        expect(wrapper.text()).toContain('Edit stage');
       });
       it('can remove the stage', () => {
-        expect(wrapper.html().indexOf('Remove stage') > 0).toBe(true);
+        expect(wrapper.text()).toContain('Remove stage');
       });
 
       it('can not hide the stage', () => {
-        expect(wrapper.html().indexOf('Hide stage') > 0).toBe(false);
+        expect(wrapper.text()).not.toContain('Hide stage');
       });
     });
   });