Merge branch 'fc-refactor-pipeline-card-to-use-flexbox' into 'master'

Refactor child/parent pipeline cards to use flexbox

See merge request gitlab-org/gitlab!80104

app/assets/javascripts/pipelines/components/graph/linked_pipeline.vue

 <script>
-import { GlTooltipDirective, GlButton, GlLink, GlLoadingIcon, GlBadge } from '@gitlab/ui';
+import { GlBadge, GlButton, GlLink, GlLoadingIcon, GlTooltipDirective } from '@gitlab/ui';
 import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants';
 import { __, sprintf } from '~/locale';
 import CiStatus from '~/vue_shared/components/ci_icon.vue';
@@ -12,10 +12,10 @@ export default {
   },
   components: {
     CiStatus,
+    GlBadge,
     GlButton,
     GlLink,
     GlLoadingIcon,
-    GlBadge,
   },
   props: {
     columnTitle: {
@@ -26,6 +26,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    isLoading: {
+      type: Boolean,
+      required: true,
+    },
     pipeline: {
       type: Object,
       required: true,
@@ -34,33 +38,40 @@ export default {
       type: String,
       required: true,
     },
-    isLoading: {
-      type: Boolean,
-      required: true,
-    },
   },
   computed: {
-    tooltipText() {
-      return `${this.downstreamTitle} #${this.pipeline.id} - ${this.pipelineStatus.label} -
-      ${this.sourceJobInfo}`;
+    buttonBorderClass() {
+      return this.isUpstream ? 'gl-border-r-1!' : 'gl-border-l-1!';
     },
     buttonId() {
       return `js-linked-pipeline-${this.pipeline.id}`;
     },
-    pipelineStatus() {
-      return this.pipeline.status;
+    cardSpacingClass() {
+      return this.isDownstream ? 'gl-pr-0' : '';
     },
-    projectName() {
-      return this.pipeline.project.name;
+    expandedIcon() {
+      if (this.isUpstream) {
+        return this.expanded ? 'angle-right' : 'angle-left';
+      }
+      return this.expanded ? 'angle-left' : 'angle-right';
+    },
+    childPipeline() {
+      return this.isDownstream && this.isSameProject;
     },
     downstreamTitle() {
       return this.childPipeline ? this.sourceJobName : this.pipeline.project.name;
     },
-    parentPipeline() {
-      return this.isUpstream && this.isSameProject;
+    flexDirection() {
+      return this.isUpstream ? 'gl-flex-direction-row-reverse' : 'gl-flex-direction-row';
     },
-    childPipeline() {
-      return this.isDownstream && this.isSameProject;
+    isDownstream() {
+      return this.type === DOWNSTREAM;
+    },
+    isSameProject() {
+      return !this.pipeline.multiproject;
+    },
+    isUpstream() {
+      return this.type === UPSTREAM;
     },
     label() {
       if (this.parentPipeline) {
@@ -70,17 +81,17 @@ export default {
       }
       return __('Multi-project');
     },
+    parentPipeline() {
+      return this.isUpstream && this.isSameProject;
+    },
     pipelineIsLoading() {
       return Boolean(this.isLoading || this.pipeline.isLoading);
     },
-    isDownstream() {
-      return this.type === DOWNSTREAM;
-    },
-    isUpstream() {
-      return this.type === UPSTREAM;
+    pipelineStatus() {
+      return this.pipeline.status;
     },
-    isSameProject() {
-      return !this.pipeline.multiproject;
+    projectName() {
+      return this.pipeline.project.name;
     },
     sourceJobName() {
       return this.pipeline.sourceJob?.name ?? '';
@@ -88,28 +99,23 @@ export default {
     sourceJobInfo() {
       return this.isDownstream ? sprintf(__('Created by %{job}'), { job: this.sourceJobName }) : '';
     },
-    expandedIcon() {
-      if (this.isUpstream) {
-        return this.expanded ? 'angle-right' : 'angle-left';
-      }
-      return this.expanded ? 'angle-left' : 'angle-right';
-    },
-    expandButtonPosition() {
-      return this.isUpstream ? 'gl-left-0 gl-border-r-1!' : 'gl-right-0 gl-border-l-1!';
+    tooltipText() {
+      return `${this.downstreamTitle} #${this.pipeline.id} - ${this.pipelineStatus.label} -
+      ${this.sourceJobInfo}`;
     },
   },
   errorCaptured(err, _vm, info) {
     reportToSentry('linked_pipeline', `error: ${err}, info: ${info}`);
   },
   methods: {
+    hideTooltips() {
+      this.$root.$emit(BV_HIDE_TOOLTIP);
+    },
     onClickLinkedPipeline() {
       this.hideTooltips();
       this.$emit('pipelineClicked', this.$refs.linkedPipeline);
       this.$emit('pipelineExpandToggle', this.sourceJobName, !this.expanded);
     },
-    hideTooltips() {
-      this.$root.$emit(BV_HIDE_TOOLTIP);
-    },
     onDownstreamHovered() {
       this.$emit('downstreamHovered', this.sourceJobName);
     },
@@ -124,27 +130,23 @@ export default {
   <div
     ref="linkedPipeline"
     v-gl-tooltip
-    class="gl-downstream-pipeline-job-width"
+    class="gl-h-full gl-display-flex! gl-border-solid gl-border-gray-100 gl-border-1"
+    :class="flexDirection"
     :title="tooltipText"
     data-qa-selector="child_pipeline"
     @mouseover="onDownstreamHovered"
     @mouseleave="onDownstreamHoverLeave"
   >
-    <div
-      class="gl-relative gl-bg-white gl-p-3 gl-border-solid gl-border-gray-100 gl-border-1"
-      :class="{ 'gl-pl-9': isUpstream }"
-    >
-      <div class="gl-display-flex gl-pr-7 gl-pipeline-job-width">
+    <div class="gl-w-full gl-bg-white gl-p-3" :class="cardSpacingClass">
+      <div class="gl-display-flex gl-pr-3">
         <ci-status
           v-if="!pipelineIsLoading"
           :status="pipelineStatus"
           :size="24"
           css-classes="gl-top-0 gl-pr-2"
         />
-        <div v-else class="gl-pr-2"><gl-loading-icon size="sm" inline /></div>
-        <div
-          class="gl-display-flex gl-flex-direction-column gl-pipeline-job-width gl-text-truncate"
-        >
+        <div v-else class="gl-pr-3"><gl-loading-icon size="sm" inline /></div>
+        <div class="gl-display-flex gl-flex-direction-column gl-downstream-pipeline-job-width">
           <span class="gl-text-truncate" data-testid="downstream-title">
             {{ downstreamTitle }}
           </span>
@@ -160,10 +162,12 @@ export default {
           {{ label }}
         </gl-badge>
       </div>
+    </div>
+    <div class="gl-display-flex">
       <gl-button
         :id="buttonId"
-        class="gl-absolute gl-top-0 gl-bottom-0 gl-shadow-none! gl-rounded-0!"
-        :class="`js-pipeline-expand-${pipeline.id} ${expandButtonPosition}`"
+        class="gl-shadow-none! gl-rounded-0!"
+        :class="`js-pipeline-expand-${pipeline.id} ${buttonBorderClass}`"
         :icon="expandedIcon"
         :aria-label="__('Expand pipeline')"
         data-testid="expand-pipeline-button"

app/assets/stylesheets/page_bundles/pipeline.scss

@@ -139,7 +139,7 @@
 }
 
 .gl-downstream-pipeline-job-width {
-  width: 240px;
+  width: 170px;
 }
 
 .gl-linked-pipeline-padding {

spec/frontend/pipelines/graph/linked_pipeline_spec.js

@@ -9,6 +9,23 @@ import mockPipeline from './linked_pipelines_mock_data';
 describe('Linked pipeline', () => {
   let wrapper;
 
+  const downstreamProps = {
+    pipeline: {
+      ...mockPipeline,
+      multiproject: false,
+    },
+    columnTitle: 'Downstream',
+    type: DOWNSTREAM,
+    expanded: false,
+    isLoading: false,
+  };
+
+  const upstreamProps = {
+    ...downstreamProps,
+    columnTitle: 'Upstream',
+    type: UPSTREAM,
+  };
+
   const findButton = () => wrapper.find(GlButton);
   const findDownstreamPipelineTitle = () => wrapper.find('[data-testid="downstream-title"]');
   const findPipelineLabel = () => wrapper.find('[data-testid="downstream-pipeline-label"]');
@@ -86,91 +103,65 @@ describe('Linked pipeline', () => {
     });
   });
 
-  describe('parent/child', () => {
-    const downstreamProps = {
-      pipeline: {
-        ...mockPipeline,
-        multiproject: false,
-      },
-      columnTitle: 'Downstream',
-      type: DOWNSTREAM,
-      expanded: false,
-      isLoading: false,
-    };
+  describe('upstream pipelines', () => {
+    beforeEach(() => {
+      createWrapper(upstreamProps);
+    });
 
-    const upstreamProps = {
-      ...downstreamProps,
-      columnTitle: 'Upstream',
-      type: UPSTREAM,
-    };
+    it('should display parent label when pipeline project id is the same as triggered_by pipeline project id', () => {
+      expect(findPipelineLabel().exists()).toBe(true);
+    });
 
-    it('parent/child label container should exist', () => {
+    it('upstream pipeline should contain the correct link', () => {
+      expect(findPipelineLink().attributes('href')).toBe(upstreamProps.pipeline.path);
+    });
+
+    it('applies the reverse-row css class to the card', () => {
+      expect(findLinkedPipeline().classes()).toContain('gl-flex-direction-row-reverse');
+      expect(findLinkedPipeline().classes()).not.toContain('gl-flex-direction-row');
+    });
+  });
+
+  describe('downstream pipelines', () => {
+    beforeEach(() => {
       createWrapper(downstreamProps);
+    });
+
+    it('parent/child label container should exist', () => {
       expect(findPipelineLabel().exists()).toBe(true);
     });
 
     it('should display child label when pipeline project id is the same as triggered pipeline project id', () => {
-      createWrapper(downstreamProps);
       expect(findPipelineLabel().exists()).toBe(true);
     });
 
     it('should have the name of the trigger job on the card when it is a child pipeline', () => {
-      createWrapper(downstreamProps);
       expect(findDownstreamPipelineTitle().text()).toBe(mockPipeline.sourceJob.name);
     });
 
-    it('should display parent label when pipeline project id is the same as triggered_by pipeline project id', () => {
-      createWrapper(upstreamProps);
-      expect(findPipelineLabel().exists()).toBe(true);
-    });
-
     it('downstream pipeline should contain the correct link', () => {
-      createWrapper(downstreamProps);
       expect(findPipelineLink().attributes('href')).toBe(downstreamProps.pipeline.path);
     });
 
-    it('upstream pipeline should contain the correct link', () => {
-      createWrapper(upstreamProps);
-      expect(findPipelineLink().attributes('href')).toBe(upstreamProps.pipeline.path);
+    it('applies the flex-row css class to the card', () => {
+      expect(findLinkedPipeline().classes()).toContain('gl-flex-direction-row');
+      expect(findLinkedPipeline().classes()).not.toContain('gl-flex-direction-row-reverse');
+    });
   });
 
+  describe('expand button', () => {
     it.each`
-      presentClass        | missingClass
-      ${'gl-right-0'}     | ${'gl-left-0'}
-      ${'gl-border-l-1!'} | ${'gl-border-r-1!'}
-    `(
-      'pipeline expand button should be postioned right when child pipeline',
-      ({ presentClass, missingClass }) => {
-        createWrapper(downstreamProps);
-        expect(findExpandButton().classes()).toContain(presentClass);
-        expect(findExpandButton().classes()).not.toContain(missingClass);
-      },
-    );
-
-    it.each`
-      presentClass        | missingClass
-      ${'gl-left-0'}      | ${'gl-right-0'}
-      ${'gl-border-r-1!'} | ${'gl-border-l-1!'}
-    `(
-      'pipeline expand button should be postioned left when parent pipeline',
-      ({ presentClass, missingClass }) => {
-        createWrapper(upstreamProps);
-        expect(findExpandButton().classes()).toContain(presentClass);
-        expect(findExpandButton().classes()).not.toContain(missingClass);
-      },
-    );
-
-    it.each`
-      pipelineType       | anglePosition    | expanded
-      ${downstreamProps} | ${'angle-right'} | ${false}
-      ${downstreamProps} | ${'angle-left'}  | ${true}
-      ${upstreamProps}   | ${'angle-left'}  | ${false}
-      ${upstreamProps}   | ${'angle-right'} | ${true}
+      pipelineType       | anglePosition    | borderClass         | expanded
+      ${downstreamProps} | ${'angle-right'} | ${'gl-border-l-1!'} | ${false}
+      ${downstreamProps} | ${'angle-left'}  | ${'gl-border-l-1!'} | ${true}
+      ${upstreamProps}   | ${'angle-left'}  | ${'gl-border-r-1!'} | ${false}
+      ${upstreamProps}   | ${'angle-right'} | ${'gl-border-r-1!'} | ${true}
     `(
-      '$pipelineType.columnTitle pipeline button icon should be $anglePosition if expanded state is $expanded',
-      ({ pipelineType, anglePosition, expanded }) => {
+      '$pipelineType.columnTitle pipeline button icon should be $anglePosition with $borderClass if expanded state is $expanded',
+      ({ pipelineType, anglePosition, borderClass, expanded }) => {
         createWrapper({ ...pipelineType, expanded });
         expect(findExpandButton().props('icon')).toBe(anglePosition);
+        expect(findExpandButton().classes()).toContain(borderClass);
       },
     );
   });