Merge branch '229704-aqualls-metrics-modal' into 'master'

Update detailed_metric.vue modal to match Pajamas guidelines

See merge request gitlab-org/gitlab!46183

app/assets/javascripts/performance_bar/components/detailed_metric.vue

 <script>
-import { GlIcon } from '@gitlab/ui';
+import { GlButton, GlIcon, GlModal, GlModalDirective } from '@gitlab/ui';
 import RequestWarning from './request_warning.vue';
 
-import DeprecatedModal2 from '~/vue_shared/components/deprecated_modal_2.vue';
-
 export default {
   components: {
     RequestWarning,
-    GlModal: DeprecatedModal2,
+    GlButton,
+    GlModal,
     GlIcon,
   },
+  directives: {
+    'gl-modal': GlModalDirective,
+  },
   props: {
     currentRequest: {
       type: Object,
@@ -35,7 +37,15 @@ export default {
       required: true,
     },
   },
+  data() {
+    return {
+      openedBacktraces: [],
+    };
+  },
   computed: {
+    modalId() {
+      return `modal-peek-${this.metric}-details`;
+    },
     metricDetails() {
       return this.currentRequest.details[this.metric];
     },
@@ -58,29 +68,35 @@ export default {
       return '';
     },
   },
+  methods: {
+    toggleBacktrace(toggledIndex) {
+      const toggledOpenedIndex = this.openedBacktraces.indexOf(toggledIndex);
+
+      if (toggledOpenedIndex === -1) {
+        this.openedBacktraces = [...this.openedBacktraces, toggledIndex];
+      } else {
+        this.openedBacktraces = this.openedBacktraces.filter(
+          openedIndex => openedIndex !== toggledIndex,
+        );
+      }
+    },
+    itemHasOpenedBacktrace(toggledIndex) {
+      return this.openedBacktraces.find(openedIndex => openedIndex === toggledIndex) >= 0;
+    },
+  },
 };
 </script>
 <template>
   <div
     v-if="currentRequest.details && metricDetails"
     :id="`peek-view-${metric}`"
-    class="view"
+    class="gl-display-flex gl-align-items-center view"
     data-qa-selector="detailed_metric_content"
   >
-    <button
-      :data-target="`#modal-peek-${metric}-details`"
-      class="btn-blank btn-link bold"
-      type="button"
-      data-toggle="modal"
-    >
+    <gl-button v-gl-modal="modalId" class="gl-mr-2" type="button" variant="link">
       {{ metricDetailsLabel }}
-    </button>
-    <gl-modal
-      :id="`modal-peek-${metric}-details`"
-      :header-title-text="header"
-      modal-size="xl"
-      class="performance-bar-modal"
-    >
+    </gl-button>
+    <gl-modal :modal-id="modalId" :title="header" size="lg" modal-class="gl-mt-7" scrollable>
       <table class="table">
         <template v-if="detailsList.length">
           <tr v-for="(item, index) in detailsList" :key="index">
@@ -90,7 +106,7 @@ export default {
               }}</span>
             </td>
             <td>
-              <div class="js-toggle-container">
+              <div>
                 <div
                   v-for="(key, keyIndex) in keys"
                   :key="key"
@@ -98,16 +114,18 @@ export default {
                   :class="{ 'mb-3 bold': keyIndex == 0 }"
                 >
                   {{ item[key] }}
-                  <button
+                  <gl-button
                     v-if="keyIndex == 0 && item.backtrace"
-                    class="text-expander js-toggle-button"
+                    class="gl-ml-3"
+                    data-testid="backtrace-expand-btn"
                     type="button"
                     :aria-label="__('Toggle backtrace')"
+                    @click="toggleBacktrace(index)"
                   >
                     <gl-icon :size="12" name="ellipsis_h" />
-                  </button>
+                  </gl-button>
                 </div>
-                <pre v-if="item.backtrace" class="backtrace-row js-toggle-content mt-2">{{
+                <pre v-if="itemHasOpenedBacktrace(index)" class="backtrace-row mt-2">{{
                   item.backtrace
                 }}</pre>
               </div>

app/assets/stylesheets/performance_bar.scss

@@ -6,7 +6,7 @@
   left: 0;
   top: 0;
   width: 100%;
-  z-index: #{$zindex-modal-backdrop + 1};
+  z-index: #{$zindex-modal-backdrop - 1};
 
   height: $performance-bar-height;
   background: $black;

changelogs/unreleased/229704-aqualls-metrics-modal.yml

+---
+title: Update detailed_metric.vue modal to match Pajamas guidelines
+merge_request: 46183
+author:
+type: changed

spec/frontend/performance_bar/components/detailed_metric_spec.js

+import { nextTick } from 'vue';
 import { shallowMount } from '@vue/test-utils';
 import { trimText } from 'helpers/text_helper';
 import DetailedMetric from '~/performance_bar/components/detailed_metric.vue';
@@ -14,6 +15,11 @@ describe('detailedMetric', () => {
     });
   };
 
+  const findAllTraceBlocks = () => wrapper.findAll('pre');
+  const findTraceBlockAtIndex = index => findAllTraceBlocks().at(index);
+  const findExpandBacktraceBtns = () => wrapper.findAll('[data-testid="backtrace-expand-btn"]');
+  const findExpandedBacktraceBtnAtIndex = index => findExpandBacktraceBtns().at(index);
+
   afterEach(() => {
     wrapper.destroy();
   });
@@ -37,7 +43,12 @@ describe('detailedMetric', () => {
   describe('when the current request has details', () => {
     const requestDetails = [
       { duration: '100', feature: 'find_commit', request: 'abcdef', backtrace: ['hello', 'world'] },
-      { duration: '23', feature: 'rebase_in_progress', request: '', backtrace: ['world', 'hello'] },
+      {
+        duration: '23',
+        feature: 'rebase_in_progress',
+        request: '',
+        backtrace: ['other', 'example'],
+      },
     ];
 
     describe('with a default metric name', () => {
@@ -82,7 +93,7 @@ describe('detailedMetric', () => {
             expect(request.text()).toContain(requestDetails[index].request);
           });
 
-        expect(wrapper.find('.text-expander.js-toggle-button')).not.toBeNull();
+        expect(wrapper.find('.js-toggle-button')).not.toBeNull();
 
         wrapper.findAll('.performance-bar-modal td:nth-child(2)').wrappers.forEach(request => {
           expect(request.text()).toContain('world');
@@ -96,6 +107,30 @@ describe('detailedMetric', () => {
       it('displays request warnings', () => {
         expect(wrapper.find(RequestWarning).exists()).toBe(true);
       });
+
+      it('can open and close traces', async () => {
+        expect(findAllTraceBlocks()).toHaveLength(0);
+
+        // Each block click on a new trace and assert that the correct
+        // count is open and that the content is what we expect to ensure
+        // we opened or closed the right one
+        const secondExpandButton = findExpandedBacktraceBtnAtIndex(1);
+
+        findExpandedBacktraceBtnAtIndex(0).vm.$emit('click');
+        await nextTick();
+        expect(findAllTraceBlocks()).toHaveLength(1);
+        expect(findTraceBlockAtIndex(0).text()).toContain(requestDetails[0].backtrace[0]);
+
+        secondExpandButton.vm.$emit('click');
+        await nextTick();
+        expect(findAllTraceBlocks()).toHaveLength(2);
+        expect(findTraceBlockAtIndex(1).text()).toContain(requestDetails[1].backtrace[0]);
+
+        secondExpandButton.vm.$emit('click');
+        await nextTick();
+        expect(findAllTraceBlocks()).toHaveLength(1);
+        expect(findTraceBlockAtIndex(0).text()).toContain(requestDetails[0].backtrace[0]);
+      });
     });
 
     describe('when using a custom metric title', () => {
@@ -140,7 +175,11 @@ describe('detailedMetric', () => {
       });
     });
 
-    it('renders only the number of calls', () => {
+    it('renders only the number of calls', async () => {
+      expect(trimText(wrapper.text())).toEqual('456 notification bullet');
+
+      findExpandedBacktraceBtnAtIndex(0).vm.$emit('click');
+      await nextTick();
       expect(trimText(wrapper.text())).toEqual('456 notification backtrace bullet');
     });
   });