From fcc4b03ccbb4cbab226c4f4c0af139fdaae5ce6b Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Wed, 6 Sep 2023 13:49:22 +0200
Subject: [PATCH] Updates CiIcon to CiBadge

Updates the appearance of the pipeline icon.

Changelog: changed
---
 .../components/header/pipeline_status.vue     | 11 +++--
 .../components/commit_box_pipeline_status.vue | 17 ++++---
 .../repository/components/last_commit.vue     |  1 -
 .../vue_shared/components/ci_badge_link.vue   |  6 +--
 app/helpers/ci/status_helper.rb               | 32 ++++++++++---
 app/views/ci/status/_icon.html.haml           | 14 ++----
 app/views/projects/branches/_branch.html.haml |  2 +-
 .../merge_requests/_merge_request.html.haml   |  2 +-
 spec/features/projects/branches_spec.rb       |  2 +-
 .../components/header/pipeline_status_spec.js |  4 +-
 .../commit_box_pipeline_status_spec.js        | 16 ++++---
 .../__snapshots__/last_commit_spec.js.snap    |  2 +-
 .../components/ci_badge_link_spec.js          |  4 +-
 spec/helpers/ci/status_helper_spec.rb         | 46 +++++++++++++++++--
 .../projects/tags/index.html.haml_spec.rb     |  8 ++--
 15 files changed, 113 insertions(+), 54 deletions(-)

diff --git a/app/assets/javascripts/ci/pipeline_editor/components/header/pipeline_status.vue b/app/assets/javascripts/ci/pipeline_editor/components/header/pipeline_status.vue
index 58b5c0004e054b..e33ce66ca66b42 100644
--- a/app/assets/javascripts/ci/pipeline_editor/components/header/pipeline_status.vue
+++ b/app/assets/javascripts/ci/pipeline_editor/components/header/pipeline_status.vue
@@ -7,7 +7,7 @@ import getPipelineQuery from '~/ci/pipeline_editor/graphql/queries/pipeline.quer
 import getPipelineEtag from '~/ci/pipeline_editor/graphql/queries/client/pipeline_etag.query.graphql';
 import { getQueryHeaders, toggleQueryPollingByVisibility } from '~/ci/pipeline_details/graph/utils';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
-import CiIcon from '~/vue_shared/components/ci_icon.vue';
+import CiBadgeLink from '~/vue_shared/components/ci_badge_link.vue';
 import PipelineMiniGraph from '~/ci/pipeline_mini_graph/pipeline_mini_graph.vue';
 import PipelineEditorMiniGraph from './pipeline_editor_mini_graph.vue';
 
@@ -25,7 +25,7 @@ export const i18n = {
 export default {
   i18n,
   components: {
-    CiIcon,
+    CiBadgeLink,
     GlButton,
     GlIcon,
     GlLink,
@@ -156,7 +156,12 @@ export default {
     <template v-else>
       <div class="gl-text-truncate gl-md-max-w-50p gl-mr-1">
         <a :href="status.detailsPath" class="gl-mr-auto">
-          <ci-icon :status="status" :size="16" data-testid="pipeline-status-icon" class="gl-mr-2" />
+          <ci-badge-link
+            :status="status"
+            badge-size="md"
+            :show-text="false"
+            data-testid="pipeline-status-icon"
+          />
         </a>
         <span class="gl-font-weight-bold">
           <gl-sprintf :message="$options.i18n.pipelineInfo">
diff --git a/app/assets/javascripts/projects/commit_box/info/components/commit_box_pipeline_status.vue b/app/assets/javascripts/projects/commit_box/info/components/commit_box_pipeline_status.vue
index ccecc914cf1789..8072116e2f5445 100644
--- a/app/assets/javascripts/projects/commit_box/info/components/commit_box_pipeline_status.vue
+++ b/app/assets/javascripts/projects/commit_box/info/components/commit_box_pipeline_status.vue
@@ -1,6 +1,6 @@
 <script>
-import { GlLoadingIcon, GlLink } from '@gitlab/ui';
-import CiIcon from '~/vue_shared/components/ci_icon.vue';
+import { GlLoadingIcon } from '@gitlab/ui';
+import CiBadgeLink from '~/vue_shared/components/ci_badge_link.vue';
 import { createAlert } from '~/alert';
 import { getQueryHeaders, toggleQueryPollingByVisibility } from '~/ci/pipeline_details/graph/utils';
 import getLatestPipelineStatusQuery from '../graphql/queries/get_latest_pipeline_status.query.graphql';
@@ -9,9 +9,8 @@ import { COMMIT_BOX_POLL_INTERVAL, PIPELINE_STATUS_FETCH_ERROR } from '../consta
 export default {
   PIPELINE_STATUS_FETCH_ERROR,
   components: {
-    CiIcon,
+    CiBadgeLink,
     GlLoadingIcon,
-    GlLink,
   },
   inject: {
     fullPath: {
@@ -64,8 +63,12 @@ export default {
 <template>
   <div class="gl-display-inline-block gl-vertical-align-middle gl-mr-2">
     <gl-loading-icon v-if="loading" />
-    <gl-link v-else :href="pipelineStatus.detailsPath">
-      <ci-icon :status="pipelineStatus" :size="24" />
-    </gl-link>
+    <ci-badge-link
+      v-else
+      :status="pipelineStatus"
+      :details-path="pipelineStatus.detailsPath"
+      badge-size="md"
+      :show-text="false"
+    />
   </div>
 </template>
diff --git a/app/assets/javascripts/repository/components/last_commit.vue b/app/assets/javascripts/repository/components/last_commit.vue
index ff6acd5239dc4d..05d4d9e1f81a34 100644
--- a/app/assets/javascripts/repository/components/last_commit.vue
+++ b/app/assets/javascripts/repository/components/last_commit.vue
@@ -120,7 +120,6 @@ export default {
           :status="commit.pipeline.detailedStatus"
           :details-path="commit.pipeline.detailedStatus.detailsPath"
           :aria-label="statusTitle"
-          size="lg"
           :show-text="false"
           class="js-commit-pipeline"
         />
diff --git a/app/assets/javascripts/vue_shared/components/ci_badge_link.vue b/app/assets/javascripts/vue_shared/components/ci_badge_link.vue
index 101ebcd1c48cce..036c03a9a1e8bd 100644
--- a/app/assets/javascripts/vue_shared/components/ci_badge_link.vue
+++ b/app/assets/javascripts/vue_shared/components/ci_badge_link.vue
@@ -59,8 +59,8 @@ export default {
     },
   },
   computed: {
-    isSmallBadgeSize() {
-      return this.size === badgeSizeOptions.sm;
+    isNotLargeBadgeSize() {
+      return this.badgeSize !== badgeSizeOptions.lg;
     },
     title() {
       return !this.showText ? this.status?.text : '';
@@ -121,7 +121,7 @@ export default {
 <template>
   <gl-badge
     v-gl-tooltip
-    :class="{ 'gl-pl-2': isSmallBadgeSize, 'gl-p-2!': !showText }"
+    :class="{ 'gl-px-2': !showText && isNotLargeBadgeSize }"
     :title="title"
     :href="detailsPath"
     :size="size"
diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb
index 5d526a6abb674c..b7ada82c5a48db 100644
--- a/app/helpers/ci/status_helper.rb
+++ b/app/helpers/ci/status_helper.rb
@@ -72,20 +72,19 @@ def render_commit_status(commit, status, ref: nil, tooltip_placement: 'left')
         status,
         path,
         tooltip_placement: tooltip_placement,
-        icon_size: 24)
+        icon_size: 16)
     end
 
     def render_status_with_link(status, path = nil, type: _('pipeline'), tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16)
-      klass = "ci-status-link #{ci_icon_class_for_status(status)} d-inline-flex #{cssclass}"
+      variant = badge_variant(status)
+      klass = "ci-status-link #{ci_icon_class_for_status(status)} d-inline-flex gl-line-height-1 #{cssclass}"
       title = "#{type.titleize}: #{ci_label_for_status(status)}"
       data = { toggle: 'tooltip', placement: tooltip_placement, container: container }
+      badge_classes = 'gl-px-2 gl-ml-3'
 
-      if path
-        link_to ci_icon_for_status(status, size: icon_size), path,
-          class: klass, title: title, data: data
-      else
+      gl_badge_tag(variant: variant, size: :md, href: path, class: badge_classes, title: title, data: data) do
         content_tag :span, ci_icon_for_status(status, size: icon_size),
-          class: klass, title: title, data: data
+          class: klass
       end
     end
 
@@ -118,5 +117,24 @@ def ci_label_for_status(status)
       translation = "CiStatusLabel|#{label}"
       s_(translation)
     end
+
+    def badge_variant(status)
+      variant = detailed_status?(status) ? status.group : status.dasherize
+
+      case variant
+      when 'success'
+        :success
+      when 'success-with-warnings', 'pending'
+        :warning
+      when 'failed'
+        :danger
+      when 'running'
+        :info
+      when 'canceled', 'manual'
+        :neutral
+      else
+        :muted
+      end
+    end
   end
 end
diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml
index fdaacb732c77f2..b7fe8a56c2b474 100644
--- a/app/views/ci/status/_icon.html.haml
+++ b/app/views/ci/status/_icon.html.haml
@@ -1,14 +1,10 @@
 - status             = local_assigns.fetch(:status)
-- size               = local_assigns.fetch(:size, 16)
 - tooltip_placement  = local_assigns.fetch(:tooltip_placement, "left")
 - path               = local_assigns.fetch(:path, status.has_details? ? status.details_path : nil)
 - option_css_classes = local_assigns.fetch(:option_css_classes, '')
-- css_classes       = "ci-status-link ci-status-icon ci-status-icon-#{status.group} has-tooltip #{option_css_classes}"
-- title             = s_("PipelineStatusTooltip|Pipeline: %{ci_status}") % {ci_status: status.label}
+- css_classes        = "gl-px-2 #{option_css_classes}"
+- ci_css_classes     = "ci-status-link ci-status-icon ci-status-icon-#{status.group} gl-line-height-1"
+- title              = s_("PipelineStatusTooltip|Pipeline: %{ci_status}") % {ci_status: status.label}
 
-- if path
-  = link_to path, class: css_classes, title: title, data: { placement: tooltip_placement } do
-    = sprite_icon(status.icon, size: size)
-- else
-  %span{ class: css_classes, title: title, data: { placement: tooltip_placement } }
-    = sprite_icon(status.icon, size: size)
+= gl_badge_tag(variant: badge_variant(status), size: :md, href: path, class: css_classes, title: title, data: { toggle: 'tooltip', placement: tooltip_placement }) do
+  = content_tag :span, sprite_icon(status.icon, size: 16), class: ci_css_classes
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index 7c52350f101899..61961172eb28a8 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -28,7 +28,7 @@
 
   .pipeline-status.d-none.d-md-block<
     - if commit_status
-      = render 'ci/status/icon', size: 16, status: commit_status, option_css_classes: 'gl-display-inline-flex gl-vertical-align-middle gl-mr-3'
+      = render 'ci/status/icon', size: 16, status: commit_status
     - elsif show_commit_status
       .gl-display-inline-flex.gl-vertical-align-middle.gl-mr-3
         %svg.s16
diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml
index 4a7aa9a86abc97..21a74d30ba57c8 100644
--- a/app/views/projects/merge_requests/_merge_request.html.haml
+++ b/app/views/projects/merge_requests/_merge_request.html.haml
@@ -43,7 +43,7 @@
               = link_to_label(label, type: :merge_request, small: true)
 
     .issuable-meta
-      %ul.controls.d-flex.align-items-end
+      %ul.controls.d-flex.align-items-center
         - if merge_request.merged?
           - merged_at = merge_request.merged_at ? l(merge_request.merged_at.to_time) : _("Merge date & time could not be determined")
           %li.d-none.d-sm-flex
diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb
index 50df7bb7ca5bf4..de805820de6607 100644
--- a/spec/features/projects/branches_spec.rb
+++ b/spec/features/projects/branches_spec.rb
@@ -299,7 +299,7 @@
 
     it 'shows pipeline status when available' do
       page.within first('.all-branches li') do
-        expect(page).to have_css 'a.ci-status-icon-success'
+        expect(page).to have_css 'a.gl-badge .ci-status-icon-success'
       end
     end
 
diff --git a/spec/frontend/ci/pipeline_editor/components/header/pipeline_status_spec.js b/spec/frontend/ci/pipeline_editor/components/header/pipeline_status_spec.js
index 1a2ed60a6f4b2d..9bb0618b758c9d 100644
--- a/spec/frontend/ci/pipeline_editor/components/header/pipeline_status_spec.js
+++ b/spec/frontend/ci/pipeline_editor/components/header/pipeline_status_spec.js
@@ -1,4 +1,4 @@
-import { GlIcon, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
+import { GlIcon, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
@@ -43,7 +43,7 @@ describe('Pipeline Status', () => {
         },
         projectFullPath: mockProjectFullPath,
       },
-      stubs: { GlLink, GlSprintf },
+      stubs: { GlSprintf },
     });
   };
 
diff --git a/spec/frontend/commit/components/commit_box_pipeline_status_spec.js b/spec/frontend/commit/components/commit_box_pipeline_status_spec.js
index 80b75a0a65e19d..844a2d81832648 100644
--- a/spec/frontend/commit/components/commit_box_pipeline_status_spec.js
+++ b/spec/frontend/commit/components/commit_box_pipeline_status_spec.js
@@ -1,11 +1,11 @@
-import { GlLoadingIcon, GlLink } from '@gitlab/ui';
+import { GlLoadingIcon } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import { createAlert } from '~/alert';
-import CiIcon from '~/vue_shared/components/ci_icon.vue';
+import CiBadgeLink from '~/vue_shared/components/ci_badge_link.vue';
 import CommitBoxPipelineStatus from '~/projects/commit_box/info/components/commit_box_pipeline_status.vue';
 import {
   COMMIT_BOX_POLL_INTERVAL,
@@ -32,8 +32,7 @@ describe('Commit box pipeline status', () => {
   const failedHandler = jest.fn().mockRejectedValue(new Error('GraphQL error'));
 
   const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
-  const findStatusIcon = () => wrapper.findComponent(CiIcon);
-  const findPipelineLink = () => wrapper.findComponent(GlLink);
+  const findCiBadgeLink = () => wrapper.findComponent(CiBadgeLink);
 
   const advanceToNextFetch = () => {
     jest.advanceTimersByTime(COMMIT_BOX_POLL_INTERVAL);
@@ -50,6 +49,9 @@ describe('Commit box pipeline status', () => {
       provide: {
         ...mockProvide,
       },
+      stubs: {
+        CiBadgeLink,
+      },
       apolloProvider: createMockApolloProvider(handler),
     });
   };
@@ -59,7 +61,7 @@ describe('Commit box pipeline status', () => {
       createComponent();
 
       expect(findLoadingIcon().exists()).toBe(true);
-      expect(findStatusIcon().exists()).toBe(false);
+      expect(findCiBadgeLink().exists()).toBe(false);
     });
   });
 
@@ -71,7 +73,7 @@ describe('Commit box pipeline status', () => {
     });
 
     it('should display pipeline status after the query is resolved successfully', () => {
-      expect(findStatusIcon().exists()).toBe(true);
+      expect(findCiBadgeLink().exists()).toBe(true);
 
       expect(findLoadingIcon().exists()).toBe(false);
       expect(createAlert).toHaveBeenCalledTimes(0);
@@ -88,7 +90,7 @@ describe('Commit box pipeline status', () => {
         },
       } = mockPipelineStatusResponse;
 
-      expect(findPipelineLink().attributes('href')).toBe(detailsPath);
+      expect(findCiBadgeLink().attributes('href')).toBe(detailsPath);
     });
   });
 
diff --git a/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap b/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap
index e48041c1032c32..208af4ba24a749 100644
--- a/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap
+++ b/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap
@@ -14,7 +14,7 @@ exports[`Repository last commit component renders commit widget 1`] = `
         aria-label="Pipeline: failed"
         class="js-commit-pipeline"
         details-path="https://test.com/pipeline"
-        size="lg"
+        size="md"
         status="[object Object]"
       />
     </div>
diff --git a/spec/frontend/vue_shared/components/ci_badge_link_spec.js b/spec/frontend/vue_shared/components/ci_badge_link_spec.js
index fa42e87ec70548..46fe5a0a74e757 100644
--- a/spec/frontend/vue_shared/components/ci_badge_link_spec.js
+++ b/spec/frontend/vue_shared/components/ci_badge_link_spec.js
@@ -150,9 +150,9 @@ describe('CI Badge Link Component', () => {
     expect(findBadge().props('size')).toBe('lg');
   });
 
-  it('should have class `gl-p-2!` when `showText` is false', () => {
+  it('should have class `gl-px-2` when `showText` is false', () => {
     createComponent({ status: statuses.success, size: 'lg', showText: false });
 
-    expect(findBadge().classes()).toContain('gl-p-2!');
+    expect(findBadge().classes()).toContain('gl-px-2');
   });
 });
diff --git a/spec/helpers/ci/status_helper_spec.rb b/spec/helpers/ci/status_helper_spec.rb
index 66c821df8f1d60..17fe474b360052 100644
--- a/spec/helpers/ci/status_helper_spec.rb
+++ b/spec/helpers/ci/status_helper_spec.rb
@@ -55,10 +55,6 @@
         is_expected.to include("href=\"/commit-path\"")
       end
 
-      it "does not contain a span element" do
-        is_expected.not_to include("<span")
-      end
-
       it "has 'Pipeline' as the status type in the title" do
         is_expected.to include("title=\"Pipeline: passed\"")
       end
@@ -88,7 +84,8 @@
       subject { helper.render_status_with_link("success", cssclass: "extra-class") }
 
       it "has appended extra class to icon classes" do
-        is_expected.to include("class=\"ci-status-link ci-status-icon-success d-inline-flex extra-class\"")
+        is_expected.to include('class="ci-status-link ci-status-icon-success d-inline-flex ' \
+                               'gl-line-height-1 extra-class"')
       end
     end
 
@@ -107,5 +104,44 @@
         is_expected.to include("<svg class=\"s24\"")
       end
     end
+
+    context "when status is success-with-warnings" do
+      subject { helper.render_status_with_link("success-with-warnings") }
+
+      it "renders warning variant of gl-badge" do
+        is_expected.to include('gl-badge badge badge-pill badge-warning')
+      end
+    end
+
+    context "when status is manual" do
+      subject { helper.render_status_with_link("manual") }
+
+      it "renders neutral variant of gl-badge" do
+        is_expected.to include('gl-badge badge badge-pill badge-neutral')
+      end
+    end
+  end
+
+  describe '#badge_variant' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:status, :expected_badge_variant_class) do
+      'success'               | 'badge-success'
+      'success-with-warnings' | 'badge-warning'
+      'pending'               | 'badge-warning'
+      'failed'                | 'badge-danger'
+      'running'               | 'badge-info'
+      'canceled'              | 'badge-neutral'
+      'manual'                | 'badge-neutral'
+      'other-status'          | 'badge-muted'
+    end
+
+    with_them do
+      subject { helper.render_status_with_link(status) }
+
+      it 'uses the correct badge variant classes for gl-badge' do
+        is_expected.to include("gl-badge badge badge-pill #{expected_badge_variant_class}")
+      end
+    end
   end
 end
diff --git a/spec/views/projects/tags/index.html.haml_spec.rb b/spec/views/projects/tags/index.html.haml_spec.rb
index dfa27afb72f1ce..01e8d23fb9fe89 100644
--- a/spec/views/projects/tags/index.html.haml_spec.rb
+++ b/spec/views/projects/tags/index.html.haml_spec.rb
@@ -91,14 +91,14 @@
 
       render
 
-      expect(page.find('.tags .content-list li', text: tag)).to have_css 'a.ci-status-icon-success'
-      expect(page.all('.tags .content-list li')).to all(have_css('svg.s24'))
+      expect(page.find('.tags .content-list li', text: tag)).to have_css '.gl-badge .ci-status-icon-success'
+      expect(page.all('.tags .content-list li')).to all(have_css('svg.s16'))
     end
 
     it 'shows no build status or placeholder when no pipelines present' do
       render
 
-      expect(page.all('.tags .content-list li')).not_to have_css 'svg.s24'
+      expect(page.all('.tags .content-list li')).not_to have_css 'svg.s16'
     end
 
     it 'shows no build status or placeholder when pipelines are private' do
@@ -107,7 +107,7 @@
 
       render
 
-      expect(page.all('.tags .content-list li')).not_to have_css 'svg.s24'
+      expect(page.all('.tags .content-list li')).not_to have_css 'svg.s16'
     end
   end
 
-- 
GitLab