Merge branch...

Merge branch '344212-project-variables-fields-desperately-need-to-be-wider-for-a-good-ux-experience' into 'master'

Add tooltip to project variables in CI/CD settings

See merge request gitlab-org/gitlab!79778

app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue

@@ -3,6 +3,7 @@ import { GlTable, GlButton, GlModalDirective, GlIcon, GlTooltipDirective } from
 import { mapState, mapActions } from 'vuex';
 import { s__, __ } from '~/locale';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue';
 import { ADD_CI_VARIABLE_MODAL_ID } from '../constants';
 import CiVariablePopover from './ci_variable_popover.vue';
 
@@ -52,10 +53,11 @@ export default {
     },
   ],
   components: {
-    GlTable,
+    CiVariablePopover,
     GlButton,
     GlIcon,
-    CiVariablePopover,
+    GlTable,
+    TooltipOnTruncate,
   },
   directives: {
     GlModalDirective,
@@ -67,8 +69,8 @@ export default {
     valuesButtonText() {
       return this.valuesHidden ? __('Reveal values') : __('Hide values');
     },
-    tableIsNotEmpty() {
-      return this.variables && this.variables.length > 0;
+    isTableEmpty() {
+      return !this.variables || this.variables.length === 0;
     },
     fields() {
       return this.$options.fields;
@@ -103,12 +105,14 @@ export default {
         <col v-for="field in scope.fields" :key="field.key" :style="field.customStyle" />
       </template>
       <template #cell(key)="{ item }">
-        <div class="gl-display-flex truncated-container gl-align-items-center">
+        <div class="gl-display-flex gl-align-items-center">
+          <tooltip-on-truncate :title="item.key" truncate-target="child">
             <span
               :id="`ci-variable-key-${item.id}`"
               class="gl-display-inline-block gl-max-w-full gl-text-truncate"
               >{{ item.key }}</span
             >
+          </tooltip-on-truncate>
           <gl-button
             v-gl-tooltip
             category="tertiary"
@@ -120,7 +124,7 @@ export default {
         </div>
       </template>
       <template #cell(value)="{ item }">
-        <div class="gl-display-flex gl-align-items-center truncated-container">
+        <div class="gl-display-flex gl-align-items-center">
           <span v-if="valuesHidden">*********************</span>
           <span
             v-else
@@ -147,10 +151,12 @@ export default {
         <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" />
       </template>
       <template #cell(environment_scope)="{ item }">
-        <div class="d-flex truncated-container">
-          <span :id="`ci-variable-env-${item.id}`" class="d-inline-block mw-100 text-truncate">{{
-            item.environment_scope
-          }}</span>
+        <div class="gl-display-flex">
+          <span
+            :id="`ci-variable-env-${item.id}`"
+            class="gl-display-inline-block gl-max-w-full gl-text-truncate"
+            >{{ item.environment_scope }}</span
+          >
           <ci-variable-popover
             :target="`ci-variable-env-${item.id}`"
             :value="item.environment_scope"
@@ -160,7 +166,6 @@ export default {
       </template>
       <template #cell(actions)="{ item }">
         <gl-button
-          ref="edit-ci-variable"
           v-gl-modal-directive="$options.modalId"
           icon="pencil"
           :aria-label="__('Edit')"
@@ -169,17 +174,16 @@ export default {
         />
       </template>
       <template #empty>
-        <p ref="empty-variables" class="text-center empty-variables text-plain">
+        <p class="gl-text-center gl-py-6 gl-text-black-normal gl-mb-0">
           {{ __('There are no variables yet.') }}
         </p>
       </template>
     </gl-table>
     <div
       class="ci-variable-actions gl-display-flex"
-      :class="{ 'justify-content-center': !tableIsNotEmpty }"
+      :class="{ 'gl-justify-content-center': isTableEmpty }"
     >
       <gl-button
-        ref="add-ci-variable"
         v-gl-modal-directive="$options.modalId"
         class="gl-mr-3"
         data-qa-selector="add_ci_variable_button"
@@ -188,8 +192,7 @@ export default {
         >{{ __('Add variable') }}</gl-button
       >
       <gl-button
-        v-if="tableIsNotEmpty"
-        ref="secret-value-reveal-button"
+        v-if="!isTableEmpty"
         data-qa-selector="reveal_ci_variable_value_button"
         @click="toggleValues(!valuesHidden)"
         >{{ valuesButtonText }}</gl-button

app/assets/stylesheets/pages/settings.scss

@@ -130,10 +130,6 @@
   border-radius: $border-radius-base;
 }
 
-.empty-variables {
-  padding: 20px 0;
-}
-
 .warning-title {
   color: $gray-900;
 }

spec/frontend/ci_variable_list/components/ci_variable_table_spec.js

-import { mount } from '@vue/test-utils';
-import Vue, { nextTick } from 'vue';
+import Vue from 'vue';
 import Vuex from 'vuex';
+import { mountExtended } from 'helpers/vue_test_utils_helper';
 import CiVariableTable from '~/ci_variable_list/components/ci_variable_table.vue';
 import createStore from '~/ci_variable_list/store';
 import mockData from '../services/mock_data';
@@ -14,15 +14,15 @@ describe('Ci variable table', () => {
   const createComponent = () => {
     store = createStore();
     jest.spyOn(store, 'dispatch').mockImplementation();
-    wrapper = mount(CiVariableTable, {
+    wrapper = mountExtended(CiVariableTable, {
       attachTo: document.body,
       store,
     });
   };
 
-  const findRevealButton = () => wrapper.find({ ref: 'secret-value-reveal-button' });
-  const findEditButton = () => wrapper.find({ ref: 'edit-ci-variable' });
-  const findEmptyVariablesPlaceholder = () => wrapper.find({ ref: 'empty-variables' });
+  const findRevealButton = () => wrapper.findByText('Reveal values');
+  const findEditButton = () => wrapper.findByLabelText('Edit');
+  const findEmptyVariablesPlaceholder = () => wrapper.findByText('There are no variables yet.');
 
   beforeEach(() => {
     createComponent();
@@ -36,18 +36,36 @@ describe('Ci variable table', () => {
     expect(store.dispatch).toHaveBeenCalledWith('fetchVariables');
   });
 
-  describe('Renders correct data', () => {
-    it('displays empty message when variables are not present', () => {
+  describe('When table is empty', () => {
+    beforeEach(() => {
+      store.state.variables = [];
+    });
+
+    it('displays empty message', () => {
       expect(findEmptyVariablesPlaceholder().exists()).toBe(true);
     });
 
-    it('displays correct amount of variables present and no empty message', async () => {
+    it('hides the reveal button', () => {
+      expect(findRevealButton().exists()).toBe(false);
+    });
+  });
+
+  describe('When table has variables', () => {
+    beforeEach(() => {
       store.state.variables = mockData.mockVariables;
+    });
 
-      await nextTick();
-      expect(wrapper.findAll('.js-ci-variable-row').length).toBe(1);
+    it('does not display the empty message', () => {
       expect(findEmptyVariablesPlaceholder().exists()).toBe(false);
     });
+
+    it('displays the reveal button', () => {
+      expect(findRevealButton().exists()).toBe(true);
+    });
+
+    it('displays the correct amount of variables', async () => {
+      expect(wrapper.findAll('.js-ci-variable-row')).toHaveLength(1);
+    });
   });
 
   describe('Table click actions', () => {