From 53fe9ea5ecd6cf24a0ec254b3d6334c7d5f2204d Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Thu, 5 Oct 2023 13:39:54 +0200
Subject: [PATCH 1/7] New CiIcons

Changes CiIcon styles to a solid color. Adds darkmode
improvements.

Changelog: changed
---
 .../graph/components/job_item.vue             |  2 +-
 .../vue_shared/components/ci_icon.vue         | 28 ++----
 app/assets/stylesheets/framework/icons.scss   | 96 +++++++++++++------
 app/helpers/ci/status_helper.rb               | 33 +++++--
 .../components/project_pipeline_spec.js       | 26 +++--
 .../features/projects/blobs/blob_show_spec.rb |  2 +-
 spec/features/projects/branches_spec.rb       |  4 +-
 .../projects/pipelines/pipeline_spec.rb       | 18 ++--
 .../frontend/ide/components/jobs/item_spec.js |  2 +-
 spec/helpers/ci/status_helper_spec.rb         |  2 +-
 10 files changed, 138 insertions(+), 75 deletions(-)

diff --git a/app/assets/javascripts/ci/pipeline_details/graph/components/job_item.vue b/app/assets/javascripts/ci/pipeline_details/graph/components/job_item.vue
index ab12e4c05e3045..c6340e6787a995 100644
--- a/app/assets/javascripts/ci/pipeline_details/graph/components/job_item.vue
+++ b/app/assets/javascripts/ci/pipeline_details/graph/components/job_item.vue
@@ -329,7 +329,7 @@ export default {
       @mouseout="hideTooltips"
     >
       <div class="gl-display-flex gl-align-items-center gl-flex-grow-1">
-        <ci-icon :status="job.status" size="md" :use-link="false" />
+        <ci-icon :status="job.status" :use-link="false" />
         <div class="gl-pl-3 gl-pr-3 gl-display-flex gl-flex-direction-column gl-pipeline-job-width">
           <div class="gl-text-truncate gl-pr-9 gl-line-height-normal">{{ job.name }}</div>
           <div
diff --git a/app/assets/javascripts/vue_shared/components/ci_icon.vue b/app/assets/javascripts/vue_shared/components/ci_icon.vue
index 9f4cfa6f690e0d..991c4da8579b9b 100644
--- a/app/assets/javascripts/vue_shared/components/ci_icon.vue
+++ b/app/assets/javascripts/vue_shared/components/ci_icon.vue
@@ -76,51 +76,42 @@ export default {
       return `ci-status-icon ci-status-icon-${status} gl-rounded-full gl-justify-content-center gl-line-height-0`;
     },
     icon() {
-      return this.status.icon;
+      return this.status.icon === undefined ? this.status.icon : `${this.status.icon}_borderless`;
     },
     badgeStyles() {
       switch (this.status.icon) {
         case 'status_success':
           return {
             textColor: 'gl-text-green-700',
+            bgColor: 'gl-bg-green-500',
             variant: 'success',
           };
         case 'status_warning':
+        case 'status_pending':
           return {
             textColor: 'gl-text-orange-700',
+            bgColor: 'gl-bg-orange-500',
             variant: 'warning',
           };
         case 'status_failed':
           return {
             textColor: 'gl-text-red-700',
+            bgColor: 'gl-bg-red-500',
             variant: 'danger',
           };
         case 'status_running':
           return {
             textColor: 'gl-text-blue-700',
+            bgColor: 'gl-bg-white',
             variant: 'info',
           };
-        case 'status_pending':
-          return {
-            textColor: 'gl-text-orange-700',
-            variant: 'warning',
-          };
-        case 'status_canceled':
-          return {
-            textColor: 'gl-text-gray-700',
-            variant: 'neutral',
-          };
-        case 'status_manual':
-          return {
-            textColor: 'gl-text-gray-700',
-            variant: 'neutral',
-          };
         // default covers the styles for the remainder of CI
         // statuses that are not explicitly stated here
         default:
           return {
-            textColor: 'gl-text-gray-600',
-            variant: 'muted',
+            textColor: 'gl-text-gray-700',
+            bgColor: 'gl-bg-white',
+            variant: 'neutral',
           };
       }
     },
@@ -143,6 +134,7 @@ export default {
       class="ci-icon-wrapper"
       :class="[
         wrapperStyleClasses,
+        badgeStyles.bgColor,
         {
           'gl-display-inline-block gl-vertical-align-top': showStatusText,
         },
diff --git a/app/assets/stylesheets/framework/icons.scss b/app/assets/stylesheets/framework/icons.scss
index 9d09c160551211..1652065c973d68 100644
--- a/app/assets/stylesheets/framework/icons.scss
+++ b/app/assets/stylesheets/framework/icons.scss
@@ -1,56 +1,36 @@
-@mixin icon-styles($primary-color, $svg-color) {
+@mixin icon-styles($color) {
   svg,
   .gl-icon {
-    fill: $primary-color;
-  }
-
-  // For the pipeline mini graph, we pass a custom 'gl-border' so that we can enforce
-  // a border of 1px instead of the thicker svg borders to adhere to design standards.
-  // If we implement the component with 'isBorderless' and also pass that border,
-  // this css is to dynamically apply the correct border color for those specific icons.
-  &.borderless {
-    border-color: $primary-color;
-  }
-
-  &.interactive {
-    &:hover {
-      background: $svg-color;
-    }
-
-    &:hover,
-    &.active {
-      box-shadow: 0 0 0 1px $primary-color;
-    }
+    fill: $color;
   }
 }
 
 .ci-status-icon-success,
 .ci-status-icon-passed {
-  @include icon-styles($green-500, $green-100);
+  @include icon-styles($green-500);
 }
 
 .ci-status-icon-error,
 .ci-status-icon-failed {
-  @include icon-styles($red-500, $red-100);
+  @include icon-styles($red-500);
 }
 
-.ci-status-icon-pending,
 .ci-status-icon-waiting-for-resource,
 .ci-status-icon-waiting-for-callback,
 .ci-status-icon-failed-with-warnings,
 .ci-status-icon-success-with-warnings {
-  @include icon-styles($orange-500, $orange-100);
+  @include icon-styles($orange-500);
 }
 
 .ci-status-icon-running {
-  @include icon-styles($blue-500, $blue-100);
+  @include icon-styles($blue-500);
 }
 
 .ci-status-icon-canceled,
 .ci-status-icon-disabled,
 .ci-status-icon-scheduled,
 .ci-status-icon-manual {
-  @include icon-styles($gray-900, $gray-100);
+  @include icon-styles($gray-900);
 }
 
 .ci-status-icon-notification,
@@ -58,7 +38,67 @@
 .ci-status-icon-created,
 .ci-status-icon-skipped,
 .ci-status-icon-notfound {
-  @include icon-styles($gray-500, $gray-100);
+  @include icon-styles($gray-500);
+}
+
+.ci-icon {
+  .gl-icon {
+    width: 20px;
+    height: 20px;
+    margin: -2px;
+    vertical-align: -2px;
+  }
+
+  .ci-status-icon-success,
+  .ci-status-icon-passed {
+    @include icon-styles($white);
+
+    .gl-dark & {
+      @include icon-styles($green-50);
+      background-color: $green-600;
+    }
+  }
+
+  .ci-status-icon-error,
+  .ci-status-icon-failed {
+    @include icon-styles($white);
+
+    .gl-dark & {
+      @include icon-styles($red-50);
+      background-color: $red-600;
+    }
+  }
+
+  .ci-status-icon-pending,
+  .ci-status-icon-waiting-for-resource,
+  .ci-status-icon-failed-with-warnings,
+  .ci-status-icon-success-with-warnings {
+    @include icon-styles($white);
+
+    .gl-dark & {
+      @include icon-styles($orange-50);
+      background-color: $orange-600;
+    }
+  }
+
+  .ci-status-icon-running {
+    .gl-dark & {
+      @include icon-styles($blue-50);
+      background-color: $blue-600;
+    }
+  }
+
+  .ci-status-icon-canceled,
+  .ci-status-icon-disabled,
+  .ci-status-icon-scheduled,
+  .ci-status-icon-manual,
+  .ci-status-icon-notification,
+  .ci-status-icon-preparing,
+  .ci-status-icon-created,
+  .ci-status-icon-skipped,
+  .ci-status-icon-notfound {
+    @include icon-styles($gray-600);
+  }
 }
 
 .password-status-icon-success {
diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb
index 8bce1ccd7782c8..6b6772a4cb6147 100644
--- a/app/helpers/ci/status_helper.rb
+++ b/app/helpers/ci/status_helper.rb
@@ -15,7 +15,7 @@ def ci_status_for_statuseable(subject)
     end
 
     # rubocop:disable Metrics/CyclomaticComplexity
-    def ci_icon_for_status(status, size: 16)
+    def ci_icon_for_status(status, size: 24)
       icon_name =
         if detailed_status?(status)
           status.icon
@@ -50,6 +50,8 @@ def ci_icon_for_status(status, size: 16)
           end
         end
 
+      icon_name = icon_name == 'play' ? icon_name : "#{icon_name}_borderless"
+
       sprite_icon(icon_name, size: size, css_class: 'gl-icon')
     end
     # rubocop:enable Metrics/CyclomaticComplexity
@@ -85,10 +87,10 @@ def render_ci_icon(
       show_status_text: false
     )
       variant = badge_variant(status)
-      klass = "js-ci-status-badge-legacy ci-status-icon #{ci_icon_class_for_status(status)} gl-rounded-full gl-justify-content-center gl-line-height-0"
+      badge_classes = "ci-icon gl-p-2 #{option_css_classes}"
+      klass = "js-ci-status-badge-legacy ci-status-icon #{ci_icon_class_for_status(status)} gl-rounded-full gl-justify-content-center gl-line-height-0 #{ci_icon_background_color(status)}"
       title = "#{_('Pipeline')}: #{ci_label_for_status(status)}"
       data = { toggle: 'tooltip', placement: tooltip_placement, container: container, testid: 'ci-icon' }
-      badge_classes = "ci-icon gl-p-2 #{option_css_classes}"
 
       gl_badge_tag(variant: variant, size: :md, href: path, class: badge_classes, title: title, data: data) do
         if show_status_text
@@ -129,22 +131,39 @@ def ci_label_for_status(status)
       s_(translation)
     end
 
+    def ci_icon_background_color(status)
+      variant = detailed_status?(status) ? status.group : status.dasherize
+
+      case variant
+      when 'success'
+        'gl-bg-green-500'
+      when 'success-with-warnings'
+        'gl-bg-orange-500'
+      when 'pending'
+        'gl-bg-orange-500'
+      when 'failed'
+        'gl-bg-red-500'
+      else
+        'gl-bg-white'
+      end
+    end
+
     def badge_variant(status)
       variant = detailed_status?(status) ? status.group : status.dasherize
 
       case variant
       when 'success'
         :success
-      when 'success-with-warnings', 'pending'
+      when 'success-with-warnings'
+        :warning
+      when 'pending'
         :warning
       when 'failed'
         :danger
       when 'running'
         :info
-      when 'canceled', 'manual'
-        :neutral
       else
-        :muted
+        :neutral
       end
     end
   end
diff --git a/ee/spec/frontend/vue_shared/dashboards/components/project_pipeline_spec.js b/ee/spec/frontend/vue_shared/dashboards/components/project_pipeline_spec.js
index f7ae6853602c4f..6228f0825efe3c 100644
--- a/ee/spec/frontend/vue_shared/dashboards/components/project_pipeline_spec.js
+++ b/ee/spec/frontend/vue_shared/dashboards/components/project_pipeline_spec.js
@@ -1,6 +1,7 @@
 import { mountExtended } from 'helpers/vue_test_utils_helper';
 import ProjectPipeline from 'ee/vue_shared/dashboards/components/project_pipeline.vue';
 import { mockPipelineData } from 'ee_jest/vue_shared/dashboards/mock_data';
+import CiIcon from '~/vue_shared/components/ci_icon.vue';
 
 describe('project pipeline component', () => {
   let wrapper;
@@ -8,6 +9,9 @@ describe('project pipeline component', () => {
   const mountComponent = (propsData = {}) =>
     mountExtended(ProjectPipeline, {
       propsData,
+      stubs: {
+        CiIcon,
+      },
     });
 
   describe('current pipeline only', () => {
@@ -17,7 +21,7 @@ describe('project pipeline component', () => {
         hasPipelineFailed: false,
       });
 
-      expect(wrapper.findByTestId('status_success-icon').exists()).toBe(true);
+      expect(wrapper.findByTestId('status_success_borderless-icon').exists()).toBe(true);
     });
 
     it('should render failed badge', () => {
@@ -26,7 +30,7 @@ describe('project pipeline component', () => {
         hasPipelineFailed: true,
       });
 
-      expect(wrapper.findByTestId('status_failed-icon').exists()).toBe(true);
+      expect(wrapper.findByTestId('status_failed_borderless-icon').exists()).toBe(true);
     });
 
     it('should render running badge', () => {
@@ -35,7 +39,7 @@ describe('project pipeline component', () => {
         hasPipelineFailed: false,
       });
 
-      expect(wrapper.findByTestId('status_running-icon').exists()).toBe(true);
+      expect(wrapper.findByTestId('status_running_borderless-icon').exists()).toBe(true);
     });
   });
 
@@ -49,7 +53,9 @@ describe('project pipeline component', () => {
       });
 
       expect(
-        wrapper.find('.js-upstream-pipeline-status [data-testid="status_success-icon"]').exists(),
+        wrapper
+          .find('.js-upstream-pipeline-status [data-testid="status_success_borderless-icon"]')
+          .exists(),
       ).toBe(true);
     });
   });
@@ -64,7 +70,9 @@ describe('project pipeline component', () => {
       });
 
       expect(
-        wrapper.find('.js-downstream-pipeline-status [data-testid="status_success-icon"]').exists(),
+        wrapper
+          .find('.js-downstream-pipeline-status [data-testid="status_success_borderless-icon"]')
+          .exists(),
       ).toBe(true);
     });
 
@@ -77,7 +85,9 @@ describe('project pipeline component', () => {
       });
 
       expect(
-        wrapper.find('.js-downstream-pipeline-status [data-testid="status_failed-icon"]').exists(),
+        wrapper
+          .find('.js-downstream-pipeline-status [data-testid="status_failed_borderless-icon"]')
+          .exists(),
       ).toBe(true);
     });
 
@@ -90,7 +100,9 @@ describe('project pipeline component', () => {
       });
 
       expect(
-        wrapper.find('.js-downstream-pipeline-status [data-testid="status_running-icon"]').exists(),
+        wrapper
+          .find('.js-downstream-pipeline-status [data-testid="status_running_borderless-icon"]')
+          .exists(),
       ).toBe(true);
     });
 
diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb
index 30a81ccc07110c..838de64cff9cce 100644
--- a/spec/features/projects/blobs/blob_show_spec.rb
+++ b/spec/features/projects/blobs/blob_show_spec.rb
@@ -943,7 +943,7 @@ module example.com/mymodule
       page.within('.commit-actions') do
         expect(page).to have_css('.ci-status-icon')
         expect(page).to have_css('.ci-status-icon-running')
-        expect(page).to have_selector('[data-testid="status_running-icon"]')
+        expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
       end
     end
   end
diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb
index 79e9ca7998e536..153cba33b27191 100644
--- a/spec/features/projects/branches_spec.rb
+++ b/spec/features/projects/branches_spec.rb
@@ -305,7 +305,7 @@
 
     it 'displays a placeholder when not available' do
       page.all('.all-branches li') do |li|
-        expect(li).to have_css '.pipeline-status svg.s16'
+        expect(li).to have_css '.pipeline-status svg.s24'
       end
     end
   end
@@ -317,7 +317,7 @@
 
     it 'does not show placeholder or pipeline status' do
       page.all('.all-branches') do |branches|
-        expect(branches).not_to have_css '.pipeline-status svg.s16'
+        expect(branches).not_to have_css '.pipeline-status svg.s24'
       end
     end
   end
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index ae0b02dac07d92..40af868a6b6c64 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -165,7 +165,7 @@
 
         it 'shows a running icon and a cancel action for the running build' do
           page.within('#ci-badge-deploy') do
-            expect(page).to have_selector('[data-testid="status_running-icon"]')
+            expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
             expect(page).to have_selector('.js-icon-cancel')
             expect(page).to have_content('deploy')
           end
@@ -187,7 +187,7 @@
 
         it 'shows a preparing icon and a cancel action' do
           page.within('#ci-badge-prepare') do
-            expect(page).to have_selector('[data-testid="status_preparing-icon"]')
+            expect(page).to have_selector('[data-testid="status_preparing_borderless-icon"]')
             expect(page).to have_selector('.js-icon-cancel')
             expect(page).to have_content('prepare')
           end
@@ -209,7 +209,7 @@
 
         it 'shows the success icon and a retry action for the successful build' do
           page.within('#ci-badge-build') do
-            expect(page).to have_selector('[data-testid="status_success-icon"]')
+            expect(page).to have_selector('[data-testid="status_success_borderless-icon"]')
             expect(page).to have_content('build')
           end
 
@@ -238,7 +238,7 @@
 
         it 'shows the scheduled icon and an unschedule action for the delayed job' do
           page.within('#ci-badge-delayed-job') do
-            expect(page).to have_selector('[data-testid="status_scheduled-icon"]')
+            expect(page).to have_selector('[data-testid="status_scheduled_borderless-icon"]')
             expect(page).to have_content('delayed-job')
           end
 
@@ -263,7 +263,7 @@
 
         it 'shows the failed icon and a retry action for the failed build' do
           page.within('#ci-badge-test') do
-            expect(page).to have_selector('[data-testid="status_failed-icon"]')
+            expect(page).to have_selector('[data-testid="status_failed_borderless-icon"]')
             expect(page).to have_content('test')
           end
 
@@ -297,7 +297,7 @@
 
         it 'shows the skipped icon and a play action for the manual build' do
           page.within('#ci-badge-manual-build') do
-            expect(page).to have_selector('[data-testid="status_manual-icon"]')
+            expect(page).to have_selector('[data-testid="status_manual_borderless-icon"]')
             expect(page).to have_content('manual')
           end
 
@@ -323,7 +323,7 @@
         end
 
         it 'shows the success icon and the generic comit status build' do
-          expect(page).to have_selector('[data-testid="status_success-icon"]')
+          expect(page).to have_selector('[data-testid="status_success_borderless-icon"]')
           expect(page).to have_content('jenkins')
           expect(page).to have_link('jenkins', href: 'http://gitlab.com/status')
         end
@@ -782,8 +782,8 @@
         expect(page).to have_content('Cancel pipeline')
       end
 
-      it 'does not link to job', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/408215' do
-        expect(page).not_to have_selector('.js-pipeline-graph-job-link')
+      it 'does link to job' do
+        expect(page).to have_selector('.js-pipeline-graph-job-link')
       end
     end
   end
diff --git a/spec/frontend/ide/components/jobs/item_spec.js b/spec/frontend/ide/components/jobs/item_spec.js
index dc0f732d11e1e6..aa6fc5531dddd5 100644
--- a/spec/frontend/ide/components/jobs/item_spec.js
+++ b/spec/frontend/ide/components/jobs/item_spec.js
@@ -18,7 +18,7 @@ describe('IDE jobs item', () => {
   });
 
   it('renders CI icon', () => {
-    expect(wrapper.find('[data-testid="status_success-icon"]').exists()).toBe(true);
+    expect(wrapper.find('[data-testid="ci-icon"]').exists()).toBe(true);
   });
 
   it('does not render view logs button if not started', async () => {
diff --git a/spec/helpers/ci/status_helper_spec.rb b/spec/helpers/ci/status_helper_spec.rb
index 18983c832625bf..ff6cc79fa36970 100644
--- a/spec/helpers/ci/status_helper_spec.rb
+++ b/spec/helpers/ci/status_helper_spec.rb
@@ -92,7 +92,7 @@
       'running'               | 'badge-info'
       'canceled'              | 'badge-neutral'
       'manual'                | 'badge-neutral'
-      'other-status'          | 'badge-muted'
+      'other-status'          | 'badge-neutral'
     end
 
     with_them do
-- 
GitLab


From bf3cee1ba9717e563af053de09bc57f2f7cb487b Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Wed, 25 Oct 2023 11:46:48 +0200
Subject: [PATCH 2/7] Update aria-label for the GlIcon to a readable name of
 the actual status.

---
 .../vue_shared/components/ci_icon.vue         |  73 ++++++----
 .../vue_shared/components/ci_icon_spec.js     | 137 +++++++++++++++---
 2 files changed, 154 insertions(+), 56 deletions(-)

diff --git a/app/assets/javascripts/vue_shared/components/ci_icon.vue b/app/assets/javascripts/vue_shared/components/ci_icon.vue
index 991c4da8579b9b..cdf424007167df 100644
--- a/app/assets/javascripts/vue_shared/components/ci_icon.vue
+++ b/app/assets/javascripts/vue_shared/components/ci_icon.vue
@@ -8,18 +8,10 @@ import { GlBadge, GlTooltipDirective, GlIcon } from '@gitlab/ui';
  * status: {
  *   group:"running" // used for CSS class
  *   icon: "icon_status_running" // used to render the icon
+ *   text: "Running",
+ *   detailsPath: '/project1/jobs/1' // can also be details_path
  * }
  *
- * Used in:
- * - Extended MR Popover
- * - Jobs show view header
- * - Jobs show view sidebar
- * - Jobs table
- * - Linked pipelines
- * - Pipeline graph
- * - Pipeline mini graph
- * - Pipeline show view badge
- * - Pipelines table Badge
  */
 
 export default {
@@ -62,18 +54,25 @@ export default {
   },
   computed: {
     title() {
-      return this.showTooltip && !this.showStatusText ? this.status?.text : '';
+      if (this.showTooltip) {
+        // show tooltip only when not showing text already
+        return !this.showStatusText ? this.status?.text : null;
+      }
+      return null;
     },
-    detailsPath() {
-      // For now, this can either come from graphQL with camelCase or REST API in snake_case
-      if (!this.useLink) {
-        return null;
+    ariaLabel() {
+      // show aria-label only when text is not rendered
+      if (!this.showStatusText) {
+        return this.status?.text;
       }
-      return this.status.detailsPath || this.status.details_path;
+      return null;
     },
-    wrapperStyleClasses() {
-      const status = this.status.group;
-      return `ci-status-icon ci-status-icon-${status} gl-rounded-full gl-justify-content-center gl-line-height-0`;
+    href() {
+      // href can come from GraphQL (camelCase) or REST API (snake_case)
+      if (this.useLink) {
+        return this.status.detailsPath || this.status.details_path;
+      }
+      return null;
     },
     icon() {
       return this.status.icon === undefined ? this.status.icon : `${this.status.icon}_borderless`;
@@ -115,6 +114,26 @@ export default {
           };
       }
     },
+    wrapperSpanClasses() {
+      const classes = [
+        'gl-rounded-full',
+        'gl-justify-content-center',
+        'gl-line-height-0',
+        'ci-status-icon',
+      ];
+
+      if (this.status?.group) {
+        classes.push(`ci-status-icon-${this.status?.group}`);
+      }
+
+      if (this.showStatusText) {
+        classes.push('gl-display-inline-block', 'gl-vertical-align-top');
+      }
+
+      classes.push(this.badgeStyles.bgColor);
+
+      return classes;
+    },
   },
 };
 </script>
@@ -123,24 +142,14 @@ export default {
     v-gl-tooltip
     class="ci-icon gl-p-2"
     :title="title"
-    :aria-label="title"
-    :href="detailsPath"
+    :aria-label="ariaLabel"
+    :href="href"
     size="md"
     :variant="badgeStyles.variant"
     data-testid="ci-icon"
     @click="$emit('ciStatusBadgeClick')"
   >
-    <span
-      class="ci-icon-wrapper"
-      :class="[
-        wrapperStyleClasses,
-        badgeStyles.bgColor,
-        {
-          'gl-display-inline-block gl-vertical-align-top': showStatusText,
-        },
-      ]"
-    >
-      <gl-icon :name="icon" :aria-label="status.icon" /> </span
+    <span :class="wrapperSpanClasses"><gl-icon :name="icon" /></span
     ><span
       v-if="showStatusText"
       class="gl-mx-2 gl-white-space-nowrap"
diff --git a/spec/frontend/vue_shared/components/ci_icon_spec.js b/spec/frontend/vue_shared/components/ci_icon_spec.js
index 1dc46538ec4957..9352a534d5b654 100644
--- a/spec/frontend/vue_shared/components/ci_icon_spec.js
+++ b/spec/frontend/vue_shared/components/ci_icon_spec.js
@@ -2,52 +2,141 @@ import { GlIcon } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import CiIcon from '~/vue_shared/components/ci_icon.vue';
 
+const mockStatus = {
+  group: 'success',
+  icon: 'status_success',
+  text: 'Success',
+};
+
 describe('CI Icon component', () => {
   let wrapper;
 
-  const createComponent = (props) => {
+  const createComponent = ({ props = {} } = {}) => {
     wrapper = shallowMount(CiIcon, {
       propsData: {
+        status: mockStatus,
         ...props,
       },
     });
   };
 
-  it('should render a span element with an svg', () => {
-    createComponent({
-      status: {
-        group: 'success',
-        icon: 'status_success',
-      },
+  const findSpanWrapper = () => wrapper.find('span');
+  const findIcon = () => wrapper.findComponent(GlIcon);
+  const findText = () => wrapper.find('[data-testid="ci-icon-text"]');
+
+  it('should render a span element and an icon', () => {
+    createComponent();
+
+    expect(wrapper.attributes('size')).toBe('md');
+
+    expect(findSpanWrapper().exists()).toBe(true);
+    expect(findIcon().exists()).toBe(true);
+  });
+
+  describe.each`
+    showStatusText | showTooltip | expectedText | expectedTooltip | expectedAriaLabel
+    ${true}        | ${true}     | ${'Success'} | ${undefined}    | ${undefined}
+    ${true}        | ${false}    | ${'Success'} | ${undefined}    | ${undefined}
+    ${false}       | ${true}     | ${''}        | ${'Success'}    | ${'Success'}
+    ${false}       | ${false}    | ${''}        | ${undefined}    | ${'Success'}
+  `(
+    'when showStatusText is %{showStatusText} and showTooltip is %{showTooltip}',
+    ({ showStatusText, showTooltip, expectedText, expectedTooltip, expectedAriaLabel }) => {
+      beforeEach(() => {
+        createComponent({
+          props: {
+            showStatusText,
+            showTooltip,
+          },
+        });
+      });
+
+      it(`aria-label is ${expectedAriaLabel}`, () => {
+        expect(wrapper.attributes('aria-label')).toBe(expectedAriaLabel);
+      });
+
+      it(`text shown is ${expectedAriaLabel}`, () => {
+        expect(wrapper.text()).toBe(expectedText);
+      });
+
+      it(`tooltip shown is ${expectedAriaLabel}`, () => {
+        expect(wrapper.attributes('title')).toBe(expectedTooltip);
+      });
+    },
+  );
+
+  describe('when appearing as a link', () => {
+    it('shows a GraphQL path', () => {
+      createComponent({
+        props: {
+          status: {
+            ...mockStatus,
+            detailsPath: '/path',
+          },
+          useLink: true,
+        },
+      });
+
+      expect(wrapper.attributes('href')).toBe('/path');
+    });
+
+    it('shows a REST API path', () => {
+      createComponent({
+        props: {
+          status: {
+            ...mockStatus,
+            details_path: '/path',
+          },
+          useLink: true,
+        },
+      });
+
+      expect(wrapper.attributes('href')).toBe('/path');
     });
 
-    expect(wrapper.find('span').exists()).toBe(true);
-    expect(wrapper.findComponent(GlIcon).exists()).toBe(true);
+    it('shows no path', () => {
+      createComponent({
+        status: {
+          detailsPath: '/path',
+          details_path: '/path',
+        },
+        props: {
+          useLink: false,
+        },
+      });
+
+      expect(wrapper.attributes('href')).toBe(undefined);
+    });
   });
 
-  describe('rendering a status', () => {
+  describe('rendering a status icon and class', () => {
     it.each`
-      icon                 | group         | cssClass
-      ${'status_success'}  | ${'success'}  | ${'ci-status-icon-success'}
-      ${'status_failed'}   | ${'failed'}   | ${'ci-status-icon-failed'}
-      ${'status_warning'}  | ${'warning'}  | ${'ci-status-icon-warning'}
-      ${'status_pending'}  | ${'pending'}  | ${'ci-status-icon-pending'}
-      ${'status_running'}  | ${'running'}  | ${'ci-status-icon-running'}
-      ${'status_created'}  | ${'created'}  | ${'ci-status-icon-created'}
-      ${'status_skipped'}  | ${'skipped'}  | ${'ci-status-icon-skipped'}
-      ${'status_canceled'} | ${'canceled'} | ${'ci-status-icon-canceled'}
-      ${'status_manual'}   | ${'manual'}   | ${'ci-status-icon-manual'}
-    `('should render a $group status', ({ icon, group, cssClass }) => {
-      wrapper = shallowMount(CiIcon, {
-        propsData: {
+      icon                 | group         | cssClass                     | textClass               | bgClass               | variant
+      ${'status_success'}  | ${'success'}  | ${'ci-status-icon-success'}  | ${'gl-text-green-700'}  | ${'gl-bg-green-500'}  | ${'success'}
+      ${'status_warning'}  | ${'warning'}  | ${'ci-status-icon-warning'}  | ${'gl-text-orange-700'} | ${'gl-bg-orange-500'} | ${'warning'}
+      ${'status_pending'}  | ${'pending'}  | ${'ci-status-icon-pending'}  | ${'gl-text-orange-700'} | ${'gl-bg-orange-500'} | ${'warning'}
+      ${'status_failed'}   | ${'failed'}   | ${'ci-status-icon-failed'}   | ${'gl-text-red-700'}    | ${'gl-bg-red-500'}    | ${'danger'}
+      ${'status_running'}  | ${'running'}  | ${'ci-status-icon-running'}  | ${'gl-text-blue-700'}   | ${'gl-bg-white'}      | ${'info'}
+      ${'status_created'}  | ${'created'}  | ${'ci-status-icon-created'}  | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
+      ${'status_skipped'}  | ${'skipped'}  | ${'ci-status-icon-skipped'}  | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
+      ${'status_canceled'} | ${'canceled'} | ${'ci-status-icon-canceled'} | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
+      ${'status_manual'}   | ${'manual'}   | ${'ci-status-icon-manual'}   | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
+    `('should render a $group status', ({ icon, group, cssClass, textClass, bgClass, variant }) => {
+      createComponent({
+        props: {
           status: {
+            ...mockStatus,
             icon,
             group,
           },
+          showStatusText: true,
         },
       });
+      expect(wrapper.attributes('variant')).toBe(variant);
 
-      expect(wrapper.find('.ci-icon-wrapper').classes()).toContain(cssClass);
+      expect(findSpanWrapper().classes()).toEqual(expect.arrayContaining([cssClass, bgClass]));
+      expect(findText().classes()).toContain(textClass);
+      expect(findIcon().props('name')).toBe(`${icon}_borderless`);
     });
   });
 });
-- 
GitLab


From e9fa30ffceca689043bdcb5fbb62bc74c4da9e82 Mon Sep 17 00:00:00 2001
From: Miguel Rincon <mrincon@gitlab.com>
Date: Mon, 30 Oct 2023 18:14:11 +0100
Subject: [PATCH 3/7] Refactor ci_icon.vue to use utility classes

- Remove references to `status.group` as the source of truth for the
appearance of the icon. Now only using `status.icon`.
- Remove `fill: $color` usages, as icons suport `color` CSS rules.
- Remove `ci-status-icon-*` classes in favor of utility CSS classes for
background and text color.
- Add support for dark mode variations in colors
- Remove a few extra classes that were apparently unused
---
 .../vue_shared/components/ci_icon.vue         | 89 ++++++++---------
 app/assets/stylesheets/framework/icons.scss   | 97 +------------------
 app/assets/stylesheets/utilities.scss         | 12 +++
 .../graph/components/job_item_spec.js         |  8 +-
 .../components/job_name_component_spec.js     |  2 +-
 .../graph/components/linked_pipeline_spec.js  |  2 +-
 .../jobs/detail/description_spec.js           |  3 +-
 .../vue_shared/components/ci_icon_spec.js     | 30 +++---
 8 files changed, 79 insertions(+), 164 deletions(-)

diff --git a/app/assets/javascripts/vue_shared/components/ci_icon.vue b/app/assets/javascripts/vue_shared/components/ci_icon.vue
index cdf424007167df..dea44e35a28468 100644
--- a/app/assets/javascripts/vue_shared/components/ci_icon.vue
+++ b/app/assets/javascripts/vue_shared/components/ci_icon.vue
@@ -6,8 +6,7 @@ import { GlBadge, GlTooltipDirective, GlIcon } from '@gitlab/ui';
  *
  * Receives status object containing:
  * status: {
- *   group:"running" // used for CSS class
- *   icon: "icon_status_running" // used to render the icon
+ *   icon: "status_running" // used to render the icon and CSS class
  *   text: "Running",
  *   detailsPath: '/project1/jobs/1' // can also be details_path
  * }
@@ -27,13 +26,8 @@ export default {
       type: Object,
       required: true,
       validator(status) {
-        const { group, icon } = status;
-        return (
-          typeof group === 'string' &&
-          group.length &&
-          typeof icon === 'string' &&
-          icon.startsWith('status_')
-        );
+        const { icon } = status;
+        return typeof icon === 'string' && icon.startsWith('status_');
       },
     },
     showStatusText: {
@@ -75,65 +69,63 @@ export default {
       return null;
     },
     icon() {
-      return this.status.icon === undefined ? this.status.icon : `${this.status.icon}_borderless`;
+      if (this.status.icon) {
+        return `${this.status.icon}_borderless`;
+      }
+      return null;
     },
-    badgeStyles() {
+    styles() {
       switch (this.status.icon) {
         case 'status_success':
           return {
-            textColor: 'gl-text-green-700',
-            bgColor: 'gl-bg-green-500',
             variant: 'success',
+            iconClasses: [
+              'gl-text-white',
+              'gl-bg-green-500',
+              'gl-dark--text-green-50',
+              'gl-dark--bg-green-600',
+            ],
           };
         case 'status_warning':
         case 'status_pending':
           return {
-            textColor: 'gl-text-orange-700',
-            bgColor: 'gl-bg-orange-500',
             variant: 'warning',
+            iconClasses: [
+              'gl-text-white',
+              'gl-bg-orange-500',
+              'gl-dark--text-orange-50',
+              'gl-dark--bg-orange-600',
+            ],
           };
         case 'status_failed':
           return {
-            textColor: 'gl-text-red-700',
-            bgColor: 'gl-bg-red-500',
             variant: 'danger',
+            iconClasses: [
+              'gl-text-white',
+              'gl-bg-red-500',
+              'gl-dark--text-red-50',
+              'gl-dark--bg-red-600',
+            ],
           };
         case 'status_running':
           return {
-            textColor: 'gl-text-blue-700',
-            bgColor: 'gl-bg-white',
             variant: 'info',
+            iconClasses: [
+              'gl-text-blue-500',
+              'gl-bg-white',
+              'gl-dark--text-blue-50',
+              'gl-dark--bg-blue-600',
+            ],
           };
         // default covers the styles for the remainder of CI
         // statuses that are not explicitly stated here
         default:
           return {
-            textColor: 'gl-text-gray-700',
-            bgColor: 'gl-bg-white',
             variant: 'neutral',
+            iconClasses: ['gl-text-gray-500', 'gl-bg-white'],
           };
       }
     },
-    wrapperSpanClasses() {
-      const classes = [
-        'gl-rounded-full',
-        'gl-justify-content-center',
-        'gl-line-height-0',
-        'ci-status-icon',
-      ];
-
-      if (this.status?.group) {
-        classes.push(`ci-status-icon-${this.status?.group}`);
-      }
-
-      if (this.showStatusText) {
-        classes.push('gl-display-inline-block', 'gl-vertical-align-top');
-      }
-
-      classes.push(this.badgeStyles.bgColor);
-
-      return classes;
-    },
   },
 };
 </script>
@@ -145,17 +137,14 @@ export default {
     :aria-label="ariaLabel"
     :href="href"
     size="md"
-    :variant="badgeStyles.variant"
+    :variant="styles.variant"
     data-testid="ci-icon"
     @click="$emit('ciStatusBadgeClick')"
   >
-    <span :class="wrapperSpanClasses"><gl-icon :name="icon" /></span
-    ><span
-      v-if="showStatusText"
-      class="gl-mx-2 gl-white-space-nowrap"
-      :class="badgeStyles.textColor"
-      data-testid="ci-icon-text"
-      >{{ status.text }}</span
-    >
+    <span class="gl-rounded-full gl-line-height-0" :class="styles.iconClasses"
+      ><gl-icon :name="icon" /></span
+    ><span v-if="showStatusText" class="gl-mx-2 gl-white-space-nowrap" data-testid="ci-icon-text">{{
+      status.text
+    }}</span>
   </gl-badge>
 </template>
diff --git a/app/assets/stylesheets/framework/icons.scss b/app/assets/stylesheets/framework/icons.scss
index 1652065c973d68..c28d9b798db651 100644
--- a/app/assets/stylesheets/framework/icons.scss
+++ b/app/assets/stylesheets/framework/icons.scss
@@ -1,103 +1,10 @@
-@mixin icon-styles($color) {
-  svg,
-  .gl-icon {
-    fill: $color;
-  }
-}
-
-.ci-status-icon-success,
-.ci-status-icon-passed {
-  @include icon-styles($green-500);
-}
-
-.ci-status-icon-error,
-.ci-status-icon-failed {
-  @include icon-styles($red-500);
-}
-
-.ci-status-icon-waiting-for-resource,
-.ci-status-icon-waiting-for-callback,
-.ci-status-icon-failed-with-warnings,
-.ci-status-icon-success-with-warnings {
-  @include icon-styles($orange-500);
-}
-
-.ci-status-icon-running {
-  @include icon-styles($blue-500);
-}
-
-.ci-status-icon-canceled,
-.ci-status-icon-disabled,
-.ci-status-icon-scheduled,
-.ci-status-icon-manual {
-  @include icon-styles($gray-900);
-}
-
-.ci-status-icon-notification,
-.ci-status-icon-preparing,
-.ci-status-icon-created,
-.ci-status-icon-skipped,
-.ci-status-icon-notfound {
-  @include icon-styles($gray-500);
-}
-
 .ci-icon {
+  // Make the borderless CI icons appear slightly bigger than the default 16px
+  // could be fixed by making the SVG bigger in a follow up issue.
   .gl-icon {
     width: 20px;
     height: 20px;
     margin: -2px;
-    vertical-align: -2px;
-  }
-
-  .ci-status-icon-success,
-  .ci-status-icon-passed {
-    @include icon-styles($white);
-
-    .gl-dark & {
-      @include icon-styles($green-50);
-      background-color: $green-600;
-    }
-  }
-
-  .ci-status-icon-error,
-  .ci-status-icon-failed {
-    @include icon-styles($white);
-
-    .gl-dark & {
-      @include icon-styles($red-50);
-      background-color: $red-600;
-    }
-  }
-
-  .ci-status-icon-pending,
-  .ci-status-icon-waiting-for-resource,
-  .ci-status-icon-failed-with-warnings,
-  .ci-status-icon-success-with-warnings {
-    @include icon-styles($white);
-
-    .gl-dark & {
-      @include icon-styles($orange-50);
-      background-color: $orange-600;
-    }
-  }
-
-  .ci-status-icon-running {
-    .gl-dark & {
-      @include icon-styles($blue-50);
-      background-color: $blue-600;
-    }
-  }
-
-  .ci-status-icon-canceled,
-  .ci-status-icon-disabled,
-  .ci-status-icon-scheduled,
-  .ci-status-icon-manual,
-  .ci-status-icon-notification,
-  .ci-status-icon-preparing,
-  .ci-status-icon-created,
-  .ci-status-icon-skipped,
-  .ci-status-icon-notfound {
-    @include icon-styles($gray-600);
   }
 }
 
diff --git a/app/assets/stylesheets/utilities.scss b/app/assets/stylesheets/utilities.scss
index 347b8e20ab419e..d444db68dbe6eb 100644
--- a/app/assets/stylesheets/utilities.scss
+++ b/app/assets/stylesheets/utilities.scss
@@ -136,3 +136,15 @@
 .gl-last-of-type-border-b-0:last-of-type {
   @include gl-border-b-0;
 }
+
+// Experimental .gl-dark--* classes, in some cases we want to apply specific colors in dark mode
+// Currently only being used in implementations of ci-icon to display status.
+.gl-dark--text-green-50  { .gl-dark & { color: $green-50 } }
+.gl-dark--text-red-50 { .gl-dark & { color: $red-50 } }
+.gl-dark--text-orange-50 { .gl-dark & { color: $orange-50 } }
+.gl-dark--text-blue-50 { .gl-dark & { color: $blue-50 } }
+
+.gl-dark--bg-green-600 { .gl-dark & { background-color: $green-600; } }
+.gl-dark--bg-red-600 { .gl-dark & { background-color: $red-600; } }
+.gl-dark--bg-orange-600 { .gl-dark & { background-color: $orange-600; } }
+.gl-dark--bg-blue-600 { .gl-dark & { background-color: $blue-600; } }
diff --git a/spec/frontend/ci/pipeline_details/graph/components/job_item_spec.js b/spec/frontend/ci/pipeline_details/graph/components/job_item_spec.js
index cda84a9e4d3178..10db7f398fef36 100644
--- a/spec/frontend/ci/pipeline_details/graph/components/job_item_spec.js
+++ b/spec/frontend/ci/pipeline_details/graph/components/job_item_spec.js
@@ -87,7 +87,9 @@ describe('pipeline graph job item', () => {
       expect(link.attributes('title')).toBe(`${mockJob.name} - ${mockJob.status.label}`);
 
       expect(findJobCiIcon().exists()).toBe(true);
-      expect(findJobCiIcon().find('.ci-status-icon-success').exists()).toBe(true);
+      expect(findJobCiIcon().find('[data-testid="status_success_borderless-icon"]').exists()).toBe(
+        true,
+      );
 
       expect(wrapper.text()).toBe(mockJob.name);
     });
@@ -106,7 +108,9 @@ describe('pipeline graph job item', () => {
 
     it('should render status and name', () => {
       expect(findJobCiIcon().exists()).toBe(true);
-      expect(findJobCiIcon().find('.ci-status-icon-success').exists()).toBe(true);
+      expect(findJobCiIcon().find('[data-testid="status_success_borderless-icon"]').exists()).toBe(
+        true,
+      );
       expect(findJobLink().exists()).toBe(false);
 
       expect(wrapper.text()).toBe(mockJobWithoutDetails.name);
diff --git a/spec/frontend/ci/pipeline_details/graph/components/job_name_component_spec.js b/spec/frontend/ci/pipeline_details/graph/components/job_name_component_spec.js
index ca201aee648df1..1da85ad9f78d49 100644
--- a/spec/frontend/ci/pipeline_details/graph/components/job_name_component_spec.js
+++ b/spec/frontend/ci/pipeline_details/graph/components/job_name_component_spec.js
@@ -25,6 +25,6 @@ describe('job name component', () => {
 
   it('should render an icon with the provided status', () => {
     expect(wrapper.findComponent(CiIcon).exists()).toBe(true);
-    expect(wrapper.find('.ci-status-icon-success').exists()).toBe(true);
+    expect(wrapper.find('[data-testid="status_success_borderless-icon"]').exists()).toBe(true);
   });
 });
diff --git a/spec/frontend/ci/pipeline_details/graph/components/linked_pipeline_spec.js b/spec/frontend/ci/pipeline_details/graph/components/linked_pipeline_spec.js
index 5fe8581e81b5d7..72be51575d75f8 100644
--- a/spec/frontend/ci/pipeline_details/graph/components/linked_pipeline_spec.js
+++ b/spec/frontend/ci/pipeline_details/graph/components/linked_pipeline_spec.js
@@ -93,7 +93,7 @@ describe('Linked pipeline', () => {
     });
 
     it('should render the pipeline status icon svg', () => {
-      expect(wrapper.find('.ci-status-icon-success svg').exists()).toBe(true);
+      expect(wrapper.findByTestId('status_success_borderless-icon').exists()).toBe(true);
     });
 
     it('should have a ci-status child component', () => {
diff --git a/spec/frontend/ide/components/jobs/detail/description_spec.js b/spec/frontend/ide/components/jobs/detail/description_spec.js
index 5465615bf38371..909bd1f7c90185 100644
--- a/spec/frontend/ide/components/jobs/detail/description_spec.js
+++ b/spec/frontend/ide/components/jobs/detail/description_spec.js
@@ -20,7 +20,8 @@ describe('IDE job description', () => {
   });
 
   it('renders CI icon', () => {
-    expect(wrapper.find('.ci-status-icon').findComponent(GlIcon).exists()).toBe(true);
+    expect(wrapper.find('[data-testid="ci-icon"]').findComponent(GlIcon).exists()).toBe(true);
+    expect(wrapper.find('[data-testid="status_success_borderless-icon"]').exists()).toBe(true);
   });
 
   it('renders bridge job details without the job link', () => {
diff --git a/spec/frontend/vue_shared/components/ci_icon_spec.js b/spec/frontend/vue_shared/components/ci_icon_spec.js
index 9352a534d5b654..28d025aa4ae0bf 100644
--- a/spec/frontend/vue_shared/components/ci_icon_spec.js
+++ b/spec/frontend/vue_shared/components/ci_icon_spec.js
@@ -22,7 +22,6 @@ describe('CI Icon component', () => {
 
   const findSpanWrapper = () => wrapper.find('span');
   const findIcon = () => wrapper.findComponent(GlIcon);
-  const findText = () => wrapper.find('[data-testid="ci-icon-text"]');
 
   it('should render a span element and an icon', () => {
     createComponent();
@@ -111,17 +110,17 @@ describe('CI Icon component', () => {
 
   describe('rendering a status icon and class', () => {
     it.each`
-      icon                 | group         | cssClass                     | textClass               | bgClass               | variant
-      ${'status_success'}  | ${'success'}  | ${'ci-status-icon-success'}  | ${'gl-text-green-700'}  | ${'gl-bg-green-500'}  | ${'success'}
-      ${'status_warning'}  | ${'warning'}  | ${'ci-status-icon-warning'}  | ${'gl-text-orange-700'} | ${'gl-bg-orange-500'} | ${'warning'}
-      ${'status_pending'}  | ${'pending'}  | ${'ci-status-icon-pending'}  | ${'gl-text-orange-700'} | ${'gl-bg-orange-500'} | ${'warning'}
-      ${'status_failed'}   | ${'failed'}   | ${'ci-status-icon-failed'}   | ${'gl-text-red-700'}    | ${'gl-bg-red-500'}    | ${'danger'}
-      ${'status_running'}  | ${'running'}  | ${'ci-status-icon-running'}  | ${'gl-text-blue-700'}   | ${'gl-bg-white'}      | ${'info'}
-      ${'status_created'}  | ${'created'}  | ${'ci-status-icon-created'}  | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
-      ${'status_skipped'}  | ${'skipped'}  | ${'ci-status-icon-skipped'}  | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
-      ${'status_canceled'} | ${'canceled'} | ${'ci-status-icon-canceled'} | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
-      ${'status_manual'}   | ${'manual'}   | ${'ci-status-icon-manual'}   | ${'gl-text-gray-700'}   | ${'gl-bg-white'}      | ${'neutral'}
-    `('should render a $group status', ({ icon, group, cssClass, textClass, bgClass, variant }) => {
+      icon                 | group         | cssClass                                                                                      | variant
+      ${'status_success'}  | ${'success'}  | ${['gl-text-white', 'gl-bg-green-500', 'gl-dark--text-green-50', 'gl-dark--bg-green-600']}    | ${'success'}
+      ${'status_warning'}  | ${'warning'}  | ${['gl-text-white', 'gl-bg-orange-500', 'gl-dark--text-orange-50', 'gl-dark--bg-orange-600']} | ${'warning'}
+      ${'status_pending'}  | ${'pending'}  | ${['gl-text-white', 'gl-bg-orange-500', 'gl-dark--text-orange-50', 'gl-dark--bg-orange-600']} | ${'warning'}
+      ${'status_failed'}   | ${'failed'}   | ${['gl-text-white', 'gl-bg-red-500', 'gl-dark--text-red-50', 'gl-dark--bg-red-600']}          | ${'danger'}
+      ${'status_running'}  | ${'running'}  | ${['gl-text-blue-500', 'gl-bg-white', 'gl-dark--text-blue-50', 'gl-dark--bg-blue-600']}       | ${'info'}
+      ${'status_created'}  | ${'created'}  | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
+      ${'status_skipped'}  | ${'skipped'}  | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
+      ${'status_canceled'} | ${'canceled'} | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
+      ${'status_manual'}   | ${'manual'}   | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
+    `('should render a $group status', ({ icon, group, cssClass, variant }) => {
       createComponent({
         props: {
           status: {
@@ -134,8 +133,11 @@ describe('CI Icon component', () => {
       });
       expect(wrapper.attributes('variant')).toBe(variant);
 
-      expect(findSpanWrapper().classes()).toEqual(expect.arrayContaining([cssClass, bgClass]));
-      expect(findText().classes()).toContain(textClass);
+      expect(findSpanWrapper().classes()).toEqual([
+        'gl-rounded-full',
+        'gl-line-height-0',
+        ...cssClass,
+      ]);
       expect(findIcon().props('name')).toBe(`${icon}_borderless`);
     });
   });
-- 
GitLab


From f543395efcc3beb6677f2354b807d829417140c3 Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Mon, 30 Oct 2023 20:19:11 +0100
Subject: [PATCH 4/7] Change HAML to match updates to Vue

---
 app/helpers/ci/status_helper.rb | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb
index 6b6772a4cb6147..9fe57f31d9a365 100644
--- a/app/helpers/ci/status_helper.rb
+++ b/app/helpers/ci/status_helper.rb
@@ -136,15 +136,17 @@ def ci_icon_background_color(status)
 
       case variant
       when 'success'
-        'gl-bg-green-500'
+        'gl-text-white gl-bg-green-500 gl-dark--text-green-50 gl-dark--bg-green-600'
       when 'success-with-warnings'
-        'gl-bg-orange-500'
+        'gl-text-white gl-bg-orange-500 gl-dark--text-orange-50 gl-dark--bg-orange-600'
       when 'pending'
-        'gl-bg-orange-500'
+        'gl-text-white gl-bg-orange-500 gl-dark--text-orange-50 gl-dark--bg-orange-600'
       when 'failed'
-        'gl-bg-red-500'
+        'gl-text-white gl-bg-red-500 gl-dark--text-red-50 gl-dark--bg-red-600'
+      when 'running'
+        'gl-text-blue-500 gl-bg-white gl-dark--text-blue-50 gl-dark--bg-blue-600'
       else
-        'gl-bg-white'
+        'gl-text-gray-500 gl-bg-white'
       end
     end
 
-- 
GitLab


From 2922644d410695c2a5eaa031e1b7f585775e2436 Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Tue, 31 Oct 2023 09:34:05 +0100
Subject: [PATCH 5/7] Fixing specs

---
 app/helpers/ci/status_helper.rb               | 12 +++-------
 ..._adds_merge_request_to_merge_train_spec.rb |  2 +-
 qa/qa/page/project/pipeline/show.rb           |  2 +-
 .../user_sees_merge_widget_spec.rb            |  2 +-
 .../features/projects/blobs/blob_show_spec.rb |  2 --
 .../commit/mini_pipeline_graph_spec.rb        |  4 ++--
 .../files/user_reads_pipeline_status_spec.rb  |  2 +-
 .../projects/pipelines/pipeline_spec.rb       | 24 +++++++++----------
 8 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb
index 9fe57f31d9a365..2b01d8c03a412f 100644
--- a/app/helpers/ci/status_helper.rb
+++ b/app/helpers/ci/status_helper.rb
@@ -56,12 +56,6 @@ def ci_icon_for_status(status, size: 24)
     end
     # rubocop:enable Metrics/CyclomaticComplexity
 
-    def ci_icon_class_for_status(status)
-      group = detailed_status?(status) ? status.group : status.dasherize
-
-      "ci-status-icon-#{group}"
-    end
-
     def pipeline_status_cache_key(pipeline_status)
       "pipeline-status/#{pipeline_status.sha}-#{pipeline_status.status}"
     end
@@ -88,13 +82,13 @@ def render_ci_icon(
     )
       variant = badge_variant(status)
       badge_classes = "ci-icon gl-p-2 #{option_css_classes}"
-      klass = "js-ci-status-badge-legacy ci-status-icon #{ci_icon_class_for_status(status)} gl-rounded-full gl-justify-content-center gl-line-height-0 #{ci_icon_background_color(status)}"
+      klass = "js-ci-status-badge-legacy gl-rounded-full gl-line-height-0 #{ci_icon_style_classes(status)}"
       title = "#{_('Pipeline')}: #{ci_label_for_status(status)}"
       data = { toggle: 'tooltip', placement: tooltip_placement, container: container, testid: 'ci-icon' }
 
       gl_badge_tag(variant: variant, size: :md, href: path, class: badge_classes, title: title, data: data) do
         if show_status_text
-          content_tag(:span, ci_icon_for_status(status), { class: klass }) + content_tag(:span, status.label, { class: 'gl-mx-2 gl-white-space-nowrap' })
+          content_tag(:span, ci_icon_for_status(status), { class: klass }) + content_tag(:span, status.label, { class: 'gl-mx-2 gl-white-space-nowrap', data: { testid: 'ci-icon-text' } })
         else
           content_tag(:span, ci_icon_for_status(status), { class: klass })
         end
@@ -131,7 +125,7 @@ def ci_label_for_status(status)
       s_(translation)
     end
 
-    def ci_icon_background_color(status)
+    def ci_icon_style_classes(status)
       variant = detailed_status?(status) ? status.group : status.dasherize
 
       case variant
diff --git a/ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb b/ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb
index c5a9fa4329431a..0dc1cdd0991947 100644
--- a/ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb
+++ b/ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb
@@ -78,7 +78,7 @@
       it 'does not allow retry for merge train pipeline' do
         find('[data-testid="mini-pipeline-graph-dropdown"] .dropdown-toggle').click
         page.within '.ci-job-component' do
-          expect(page).to have_selector('.ci-status-icon')
+          expect(page).to have_selector('[data-testid="ci-icon"]')
           expect(page).not_to have_selector('.retry')
         end
       end
diff --git a/qa/qa/page/project/pipeline/show.rb b/qa/qa/page/project/pipeline/show.rb
index 16b931c1a56867..151df85af3df4d 100644
--- a/qa/qa/page/project/pipeline/show.rb
+++ b/qa/qa/page/project/pipeline/show.rb
@@ -43,7 +43,7 @@ def running?(wait: 0)
           def has_build?(name, status: :success, wait: nil)
             if status
               within_element('job-item-container', text: name) do
-                has_selector?(".ci-status-icon-#{status}", **{ wait: wait }.compact)
+                has_selector?("[data-testid='status_#{status}_borderless-icon']", **{ wait: wait }.compact)
               end
             else
               has_element?('job-item-container', text: name)
diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb
index 18537d1cddb19a..c18b2c97f96df8 100644
--- a/spec/features/merge_request/user_sees_merge_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb
@@ -176,7 +176,7 @@ def click_expand_button
       expect(page).to have_content("Merge blocked")
       expect(page).to have_content(
         "pipeline must succeed. It's waiting for a manual action to continue.")
-      expect(page).to have_css('.ci-status-icon-manual')
+      expect(page).to have_css('[data-testid="status_manual_borderless-icon"]')
     end
   end
 
diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb
index 838de64cff9cce..36665f2b77dd28 100644
--- a/spec/features/projects/blobs/blob_show_spec.rb
+++ b/spec/features/projects/blobs/blob_show_spec.rb
@@ -941,8 +941,6 @@ module example.com/mymodule
 
     it 'shows the realtime pipeline status' do
       page.within('.commit-actions') do
-        expect(page).to have_css('.ci-status-icon')
-        expect(page).to have_css('.ci-status-icon-running')
         expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
       end
     end
diff --git a/spec/features/projects/commit/mini_pipeline_graph_spec.rb b/spec/features/projects/commit/mini_pipeline_graph_spec.rb
index 5bb3d1af92420c..c0a8dabf648ccd 100644
--- a/spec/features/projects/commit/mini_pipeline_graph_spec.rb
+++ b/spec/features/projects/commit/mini_pipeline_graph_spec.rb
@@ -52,7 +52,7 @@
     end
 
     it 'display icon with status' do
-      expect(page).to have_selector('.ci-status-icon-running')
+      expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
     end
 
     it 'displays a mini pipeline graph' do
@@ -63,7 +63,7 @@
       wait_for_requests
 
       page.within '.js-builds-dropdown-list' do
-        expect(page).to have_selector('.ci-status-icon-running')
+        expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
         expect(page).to have_content(build.stage_name)
       end
 
diff --git a/spec/features/projects/files/user_reads_pipeline_status_spec.rb b/spec/features/projects/files/user_reads_pipeline_status_spec.rb
index ce3f0541139aca..24dd673501cab7 100644
--- a/spec/features/projects/files/user_reads_pipeline_status_spec.rb
+++ b/spec/features/projects/files/user_reads_pipeline_status_spec.rb
@@ -25,7 +25,7 @@
 
       page.within('.commit-detail') do
         expect(page).to have_link('', href: project_pipeline_path(project, expected_pipeline))
-        expect(page).to have_selector(".ci-status-icon-#{expected_pipeline.status}")
+        expect(page).to have_selector("[data-testid='status_#{expected_pipeline.status}_borderless-icon']")
       end
     end
   end
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 40af868a6b6c64..fccfe00f59346d 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -358,7 +358,7 @@
             let(:status) { :success }
 
             it 'does not show the cancel or retry action' do
-              expect(page).to have_selector('.ci-status-icon-success')
+              expect(page).to have_selector('[data-testid="status_success_borderless-icon"]')
               expect(page).not_to have_selector('button[aria-label="Retry downstream pipeline"]')
               expect(page).not_to have_selector('button[aria-label="Cancel downstream pipeline"]')
             end
@@ -379,7 +379,7 @@
 
               it 'shows the pipeline as canceled with the retry action' do
                 expect(page).to have_selector('button[aria-label="Retry downstream pipeline"]')
-                expect(page).to have_selector('.ci-status-icon-canceled')
+                expect(page).to have_selector('[data-testid="status_canceled_borderless-icon"]')
               end
             end
           end
@@ -398,7 +398,7 @@
               end
 
               it 'shows running pipeline with the cancel action' do
-                expect(page).to have_selector('.ci-status-icon-running')
+                expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
                 expect(page).to have_selector('button[aria-label="Cancel downstream pipeline"]')
               end
             end
@@ -418,7 +418,7 @@
               end
 
               it 'shows running pipeline with the cancel action' do
-                expect(page).to have_selector('.ci-status-icon-running')
+                expect(page).to have_selector('[data-testid="status_running_borderless-icon"]')
                 expect(page).to have_selector('button[aria-label="Cancel downstream pipeline"]')
               end
             end
@@ -438,7 +438,7 @@
           end
 
           it 'does not show the retry button' do
-            expect(page).to have_selector('.ci-status-icon-failed')
+            expect(page).to have_selector('[data-testid="status_failed_borderless-icon"]')
             expect(page).not_to have_selector('button[aria-label="Retry downstream pipeline"]')
           end
         end
@@ -906,12 +906,12 @@
         within('.js-pipeline-graph') do
           within(all('[data-testid="stage-column"]')[0]) do
             expect(page).to have_content('test')
-            expect(page).to have_css('.ci-status-icon-pending')
+            expect(page).to have_css('[data-testid="status_pending_borderless-icon"]')
           end
 
           within(all('[data-testid="stage-column"]')[1]) do
             expect(page).to have_content('deploy')
-            expect(page).to have_css('.ci-status-icon-created')
+            expect(page).to have_css('[data-testid="status_created_borderless-icon"]')
           end
         end
       end
@@ -931,12 +931,12 @@
           within('.js-pipeline-graph') do
             within(all('[data-testid="stage-column"]')[0]) do
               expect(page).to have_content('test')
-              expect(page).to have_css('.ci-status-icon-success')
+              expect(page).to have_css('[data-testid="status_success_borderless-icon"]')
             end
 
             within(all('[data-testid="stage-column"]')[1]) do
               expect(page).to have_content('deploy')
-              expect(page).to have_css('.ci-status-icon-pending')
+              expect(page).to have_css('[data-testid="status_pending_borderless-icon"]')
             end
           end
         end
@@ -960,7 +960,7 @@
           within('.js-pipeline-graph') do
             within(all('[data-testid="stage-column"]')[1]) do
               expect(page).to have_content('deploy')
-              expect(page).to have_css('.ci-status-icon-waiting-for-resource')
+              expect(page).to have_css('[data-testid="status_pending_borderless-icon"]')
             end
           end
         end
@@ -980,7 +980,7 @@
             within('.js-pipeline-graph') do
               within(all('[data-testid="stage-column"]')[1]) do
                 expect(page).to have_content('deploy')
-                expect(page).to have_css('.ci-status-icon-pending')
+                expect(page).to have_css('[data-testid="status_pending_borderless-icon"]')
               end
             end
           end
@@ -1008,7 +1008,7 @@
             within('.js-pipeline-graph') do
               within(all('[data-testid="stage-column"]')[1]) do
                 expect(page).to have_content('deploy')
-                expect(page).to have_css('.ci-status-icon-waiting-for-resource')
+                expect(page).to have_css('[data-testid="status_pending_borderless-icon"]')
               end
             end
           end
-- 
GitLab


From 4004c1557834aadc2d2663c158e6979c1ecce4e2 Mon Sep 17 00:00:00 2001
From: Miguel Rincon <mrincon@gitlab.com>
Date: Wed, 1 Nov 2023 09:30:05 +0100
Subject: [PATCH 6/7] Remove status utility classes in favor of css

---
 .../vue_shared/components/ci_icon.vue         | 53 ++---------
 app/assets/stylesheets/framework/icons.scss   | 88 ++++++++++++++++++-
 app/assets/stylesheets/utilities.scss         | 12 ---
 app/helpers/ci/status_helper.rb               | 30 ++-----
 .../vue_shared/components/ci_icon_spec.js     | 32 +++----
 spec/helpers/ci/status_helper_spec.rb         | 87 ++++++++----------
 6 files changed, 153 insertions(+), 149 deletions(-)

diff --git a/app/assets/javascripts/vue_shared/components/ci_icon.vue b/app/assets/javascripts/vue_shared/components/ci_icon.vue
index dea44e35a28468..a2b6b4642c93c5 100644
--- a/app/assets/javascripts/vue_shared/components/ci_icon.vue
+++ b/app/assets/javascripts/vue_shared/components/ci_icon.vue
@@ -74,56 +74,21 @@ export default {
       }
       return null;
     },
-    styles() {
+    variant() {
       switch (this.status.icon) {
         case 'status_success':
-          return {
-            variant: 'success',
-            iconClasses: [
-              'gl-text-white',
-              'gl-bg-green-500',
-              'gl-dark--text-green-50',
-              'gl-dark--bg-green-600',
-            ],
-          };
+          return 'success';
         case 'status_warning':
         case 'status_pending':
-          return {
-            variant: 'warning',
-            iconClasses: [
-              'gl-text-white',
-              'gl-bg-orange-500',
-              'gl-dark--text-orange-50',
-              'gl-dark--bg-orange-600',
-            ],
-          };
+          return 'warning';
         case 'status_failed':
-          return {
-            variant: 'danger',
-            iconClasses: [
-              'gl-text-white',
-              'gl-bg-red-500',
-              'gl-dark--text-red-50',
-              'gl-dark--bg-red-600',
-            ],
-          };
+          return 'danger';
         case 'status_running':
-          return {
-            variant: 'info',
-            iconClasses: [
-              'gl-text-blue-500',
-              'gl-bg-white',
-              'gl-dark--text-blue-50',
-              'gl-dark--bg-blue-600',
-            ],
-          };
+          return 'info';
         // default covers the styles for the remainder of CI
         // statuses that are not explicitly stated here
         default:
-          return {
-            variant: 'neutral',
-            iconClasses: ['gl-text-gray-500', 'gl-bg-white'],
-          };
+          return 'neutral';
       }
     },
   },
@@ -133,16 +98,16 @@ export default {
   <gl-badge
     v-gl-tooltip
     class="ci-icon gl-p-2"
+    :class="`ci-icon-variant-${variant}`"
+    :variant="variant"
     :title="title"
     :aria-label="ariaLabel"
     :href="href"
     size="md"
-    :variant="styles.variant"
     data-testid="ci-icon"
     @click="$emit('ciStatusBadgeClick')"
   >
-    <span class="gl-rounded-full gl-line-height-0" :class="styles.iconClasses"
-      ><gl-icon :name="icon" /></span
+    <span class="ci-icon-gl-icon-wrapper"><gl-icon :name="icon" /></span
     ><span v-if="showStatusText" class="gl-mx-2 gl-white-space-nowrap" data-testid="ci-icon-text">{{
       status.text
     }}</span>
diff --git a/app/assets/stylesheets/framework/icons.scss b/app/assets/stylesheets/framework/icons.scss
index c28d9b798db651..3d95622d2f8267 100644
--- a/app/assets/stylesheets/framework/icons.scss
+++ b/app/assets/stylesheets/framework/icons.scss
@@ -1,11 +1,95 @@
+@mixin icon-styles($color) {
+  svg,
+  .gl-icon {
+    fill: $color;
+  }
+}
+
+.ci-status-icon-success,
+.ci-status-icon-passed {
+  @include icon-styles($green-500);
+}
+
+.ci-status-icon-error,
+.ci-status-icon-failed {
+  @include icon-styles($red-500);
+}
+
+.ci-status-icon-pending,
+.ci-status-icon-waiting-for-resource,
+.ci-status-icon-waiting-for-callback,
+.ci-status-icon-failed-with-warnings,
+.ci-status-icon-success-with-warnings {
+  @include icon-styles($orange-500);
+}
+
+.ci-status-icon-running {
+  @include icon-styles($blue-500);
+}
+
+.ci-status-icon-canceled,
+.ci-status-icon-disabled,
+.ci-status-icon-scheduled,
+.ci-status-icon-manual {
+  @include icon-styles($gray-900);
+}
+
+.ci-status-icon-notification,
+.ci-status-icon-preparing,
+.ci-status-icon-created,
+.ci-status-icon-skipped,
+.ci-status-icon-notfound {
+  @include icon-styles($gray-500);
+}
+
 .ci-icon {
-  // Make the borderless CI icons appear slightly bigger than the default 16px
-  // could be fixed by making the SVG bigger in a follow up issue.
+  // .ci-icon class is used at
+  // - app/assets/javascripts/vue_shared/components/ci_icon.vue
+  // - app/helpers/ci/status_helper.rb
+  .ci-icon-gl-icon-wrapper {
+    @include gl-rounded-full;
+    @include gl-line-height-0;
+  }
+
+  // Makes the borderless CI icons appear slightly bigger than the default 16px.
+  // Could be fixed by making the SVG fill up the canvas in a follow up issue.
   .gl-icon {
     width: 20px;
     height: 20px;
     margin: -2px;
   }
+
+  @mixin ci-icon-style($bg-color, $color, $gl-dark-bg-color: null, $gl-dark-color: null) {
+    .ci-icon-gl-icon-wrapper {
+      background-color: $bg-color;
+      color: $color;
+
+      .gl-dark & {
+        background-color: $gl-dark-bg-color;
+        color: $gl-dark-color;
+      }
+    }
+  }
+
+  &.ci-icon-variant-success {
+    @include ci-icon-style($green-500, $white, $green-600, $green-50)
+  }
+
+  &.ci-icon-variant-warning {
+    @include ci-icon-style($orange-500, $white, $orange-600, $orange-50)
+  }
+
+  &.ci-icon-variant-danger {
+    @include ci-icon-style($red-500, $white, $red-600, $red-50)
+  }
+
+  &.ci-icon-variant-info {
+    @include ci-icon-style($blue-500, $white, $blue-600, $blue-50)
+  }
+
+  &.ci-icon-variant-neutral {
+    @include ci-icon-style($white, $gray-500)
+  }
 }
 
 .password-status-icon-success {
diff --git a/app/assets/stylesheets/utilities.scss b/app/assets/stylesheets/utilities.scss
index d444db68dbe6eb..347b8e20ab419e 100644
--- a/app/assets/stylesheets/utilities.scss
+++ b/app/assets/stylesheets/utilities.scss
@@ -136,15 +136,3 @@
 .gl-last-of-type-border-b-0:last-of-type {
   @include gl-border-b-0;
 }
-
-// Experimental .gl-dark--* classes, in some cases we want to apply specific colors in dark mode
-// Currently only being used in implementations of ci-icon to display status.
-.gl-dark--text-green-50  { .gl-dark & { color: $green-50 } }
-.gl-dark--text-red-50 { .gl-dark & { color: $red-50 } }
-.gl-dark--text-orange-50 { .gl-dark & { color: $orange-50 } }
-.gl-dark--text-blue-50 { .gl-dark & { color: $blue-50 } }
-
-.gl-dark--bg-green-600 { .gl-dark & { background-color: $green-600; } }
-.gl-dark--bg-red-600 { .gl-dark & { background-color: $red-600; } }
-.gl-dark--bg-orange-600 { .gl-dark & { background-color: $orange-600; } }
-.gl-dark--bg-blue-600 { .gl-dark & { background-color: $blue-600; } }
diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb
index 2b01d8c03a412f..78cd7ccdaa1eb3 100644
--- a/app/helpers/ci/status_helper.rb
+++ b/app/helpers/ci/status_helper.rb
@@ -81,16 +81,17 @@ def render_ci_icon(
       show_status_text: false
     )
       variant = badge_variant(status)
-      badge_classes = "ci-icon gl-p-2 #{option_css_classes}"
-      klass = "js-ci-status-badge-legacy gl-rounded-full gl-line-height-0 #{ci_icon_style_classes(status)}"
+      badge_classes = "ci-icon gl-p-2 ci-icon-variant-#{variant} #{option_css_classes}"
       title = "#{_('Pipeline')}: #{ci_label_for_status(status)}"
       data = { toggle: 'tooltip', placement: tooltip_placement, container: container, testid: 'ci-icon' }
 
+      icon_wrapper_class = "js-ci-status-badge-legacy ci-icon-gl-icon-wrapper"
+
       gl_badge_tag(variant: variant, size: :md, href: path, class: badge_classes, title: title, data: data) do
         if show_status_text
-          content_tag(:span, ci_icon_for_status(status), { class: klass }) + content_tag(:span, status.label, { class: 'gl-mx-2 gl-white-space-nowrap', data: { testid: 'ci-icon-text' } })
+          content_tag(:span, ci_icon_for_status(status), { class: icon_wrapper_class }) + content_tag(:span, status.label, { class: 'gl-mx-2 gl-white-space-nowrap', data: { testid: 'ci-icon-text' } })
         else
-          content_tag(:span, ci_icon_for_status(status), { class: klass })
+          content_tag(:span, ci_icon_for_status(status), { class: icon_wrapper_class })
         end
       end
     end
@@ -125,25 +126,6 @@ def ci_label_for_status(status)
       s_(translation)
     end
 
-    def ci_icon_style_classes(status)
-      variant = detailed_status?(status) ? status.group : status.dasherize
-
-      case variant
-      when 'success'
-        'gl-text-white gl-bg-green-500 gl-dark--text-green-50 gl-dark--bg-green-600'
-      when 'success-with-warnings'
-        'gl-text-white gl-bg-orange-500 gl-dark--text-orange-50 gl-dark--bg-orange-600'
-      when 'pending'
-        'gl-text-white gl-bg-orange-500 gl-dark--text-orange-50 gl-dark--bg-orange-600'
-      when 'failed'
-        'gl-text-white gl-bg-red-500 gl-dark--text-red-50 gl-dark--bg-red-600'
-      when 'running'
-        'gl-text-blue-500 gl-bg-white gl-dark--text-blue-50 gl-dark--bg-blue-600'
-      else
-        'gl-text-gray-500 gl-bg-white'
-      end
-    end
-
     def badge_variant(status)
       variant = detailed_status?(status) ? status.group : status.dasherize
 
@@ -154,6 +136,8 @@ def badge_variant(status)
         :warning
       when 'pending'
         :warning
+      when 'waiting-for-resource'
+        :warning
       when 'failed'
         :danger
       when 'running'
diff --git a/spec/frontend/vue_shared/components/ci_icon_spec.js b/spec/frontend/vue_shared/components/ci_icon_spec.js
index 28d025aa4ae0bf..cbb725bf9e63e1 100644
--- a/spec/frontend/vue_shared/components/ci_icon_spec.js
+++ b/spec/frontend/vue_shared/components/ci_icon_spec.js
@@ -20,15 +20,12 @@ describe('CI Icon component', () => {
     });
   };
 
-  const findSpanWrapper = () => wrapper.find('span');
   const findIcon = () => wrapper.findComponent(GlIcon);
 
   it('should render a span element and an icon', () => {
     createComponent();
 
     expect(wrapper.attributes('size')).toBe('md');
-
-    expect(findSpanWrapper().exists()).toBe(true);
     expect(findIcon().exists()).toBe(true);
   });
 
@@ -110,34 +107,29 @@ describe('CI Icon component', () => {
 
   describe('rendering a status icon and class', () => {
     it.each`
-      icon                 | group         | cssClass                                                                                      | variant
-      ${'status_success'}  | ${'success'}  | ${['gl-text-white', 'gl-bg-green-500', 'gl-dark--text-green-50', 'gl-dark--bg-green-600']}    | ${'success'}
-      ${'status_warning'}  | ${'warning'}  | ${['gl-text-white', 'gl-bg-orange-500', 'gl-dark--text-orange-50', 'gl-dark--bg-orange-600']} | ${'warning'}
-      ${'status_pending'}  | ${'pending'}  | ${['gl-text-white', 'gl-bg-orange-500', 'gl-dark--text-orange-50', 'gl-dark--bg-orange-600']} | ${'warning'}
-      ${'status_failed'}   | ${'failed'}   | ${['gl-text-white', 'gl-bg-red-500', 'gl-dark--text-red-50', 'gl-dark--bg-red-600']}          | ${'danger'}
-      ${'status_running'}  | ${'running'}  | ${['gl-text-blue-500', 'gl-bg-white', 'gl-dark--text-blue-50', 'gl-dark--bg-blue-600']}       | ${'info'}
-      ${'status_created'}  | ${'created'}  | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
-      ${'status_skipped'}  | ${'skipped'}  | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
-      ${'status_canceled'} | ${'canceled'} | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
-      ${'status_manual'}   | ${'manual'}   | ${['gl-text-gray-500', 'gl-bg-white']}                                                        | ${'neutral'}
-    `('should render a $group status', ({ icon, group, cssClass, variant }) => {
+      icon                 | variant
+      ${'status_success'}  | ${'success'}
+      ${'status_warning'}  | ${'warning'}
+      ${'status_pending'}  | ${'warning'}
+      ${'status_failed'}   | ${'danger'}
+      ${'status_running'}  | ${'info'}
+      ${'status_created'}  | ${'neutral'}
+      ${'status_skipped'}  | ${'neutral'}
+      ${'status_canceled'} | ${'neutral'}
+      ${'status_manual'}   | ${'neutral'}
+    `('should render a $group status', ({ icon, variant }) => {
       createComponent({
         props: {
           status: {
             ...mockStatus,
             icon,
-            group,
           },
           showStatusText: true,
         },
       });
       expect(wrapper.attributes('variant')).toBe(variant);
+      expect(wrapper.classes(`ci-icon-variant-${variant}`)).toBe(true);
 
-      expect(findSpanWrapper().classes()).toEqual([
-        'gl-rounded-full',
-        'gl-line-height-0',
-        ...cssClass,
-      ]);
       expect(findIcon().props('name')).toBe(`${icon}_borderless`);
     });
   });
diff --git a/spec/helpers/ci/status_helper_spec.rb b/spec/helpers/ci/status_helper_spec.rb
index ff6cc79fa36970..502a535e102f90 100644
--- a/spec/helpers/ci/status_helper_spec.rb
+++ b/spec/helpers/ci/status_helper_spec.rb
@@ -29,7 +29,7 @@
     end
 
     it "has the success status icon" do
-      is_expected.to include("ci-status-icon-success")
+      is_expected.to include("ci-icon-variant-success")
     end
 
     context "when pipeline has commit path" do
@@ -44,7 +44,19 @@
       end
 
       it "has the correct status icon" do
-        is_expected.to include("ci-status-icon-success")
+        is_expected.to include("ci-icon-variant-success")
+      end
+    end
+
+    context "when showing status text" do
+      subject do
+        detailed_status = Gitlab::Ci::Status::Success.new(build(:ci_build, :success), build(:user))
+        helper.render_ci_icon(detailed_status, show_status_text: true)
+      end
+
+      it "contains status text" do
+        is_expected.to include("data-testid=\"ci-icon-text\"")
+        is_expected.to include("passed")
       end
     end
 
@@ -79,56 +91,35 @@
         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-neutral'
-    end
-
-    with_them do
-      subject { helper.render_ci_icon(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}")
+    describe 'badge and icon appearance' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:status, :icon, :badge_variant) do
+        'success'               | 'status_success_borderless'   | 'success'
+        'success-with-warnings' | 'status_warning_borderless'   | 'warning'
+        'pending'               | 'status_pending_borderless'   | 'warning'
+        'waiting-for-resource'  | 'status_pending_borderless'   | 'warning'
+        'failed'                | 'status_failed_borderless'    | 'danger'
+        'running'               | 'status_running_borderless'   | 'info'
+        'preparing'             | 'status_preparing_borderless' | 'neutral'
+        'canceled'              | 'status_canceled_borderless'  | 'neutral'
+        'created'               | 'status_created_borderless'   | 'neutral'
+        'scheduled'             | 'status_scheduled_borderless' | 'neutral'
+        'play'                  | 'play'                        | 'neutral'
+        'skipped'               | 'status_skipped_borderless'   | 'neutral'
+        'manual'                | 'status_manual_borderless'    | 'neutral'
+        'other-status'          | 'status_canceled_borderless'  | 'neutral'
       end
-    end
-  end
-
-  describe '#ci_icon_for_status' do
-    using RSpec::Parameterized::TableSyntax
-
-    where(:status, :icon_variant) do
-      'success'               | 'status_success'
-      'success-with-warnings' | 'status_warning'
-      'preparing'             | 'status_preparing'
-      'pending'               | 'status_pending'
-      'waiting-for-resource'  | 'status_pending'
-      'failed'                | 'status_failed'
-      'running'               | 'status_running'
-      'canceled'              | 'status_canceled'
-      'created'               | 'status_created'
-      'scheduled'             | 'status_scheduled'
-      'play'                  | 'play'
-      'skipped'               | 'status_skipped'
-      'manual'                | 'status_manual'
-    end
 
-    with_them do
-      subject { helper.render_ci_icon(status).to_s }
+      with_them do
+        subject { helper.render_ci_icon(status) }
 
-      it 'uses the correct icon variant for status' do
-        is_expected.to include("ci-status-icon-#{status}")
-        is_expected.to include(icon_variant)
+        it 'uses the correct variant and icon for status' do
+          is_expected.to include("gl-badge badge badge-pill badge-#{badge_variant}")
+          is_expected.to include("ci-icon-variant-#{badge_variant}")
+          is_expected.to include("data-testid=\"#{icon}-icon\"")
+        end
       end
     end
   end
-- 
GitLab


From 3f8246169cc3df291206b906c784cc124515dab0 Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Wed, 1 Nov 2023 12:43:20 +0100
Subject: [PATCH 7/7] Fixing specs

---
 .../ci/pipeline_mini_graph/linked_pipelines_mini_list.vue   | 6 ------
 app/assets/stylesheets/framework/icons.scss                 | 3 ++-
 app/helpers/ci/status_helper.rb                             | 2 +-
 spec/features/commits_spec.rb                               | 2 +-
 spec/features/dashboard/projects_spec.rb                    | 4 ++--
 spec/features/projects/branches_spec.rb                     | 2 +-
 .../pipeline_mini_graph/linked_pipelines_mini_list_spec.js  | 4 ++--
 spec/views/ci/status/_icon.html.haml_spec.rb                | 6 +++---
 .../projects/issues/_related_branches.html.haml_spec.rb     | 2 +-
 spec/views/projects/tags/index.html.haml_spec.rb            | 2 +-
 10 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/app/assets/javascripts/ci/pipeline_mini_graph/linked_pipelines_mini_list.vue b/app/assets/javascripts/ci/pipeline_mini_graph/linked_pipelines_mini_list.vue
index 4dace6b501aeec..f6a375ab94c128 100644
--- a/app/assets/javascripts/ci/pipeline_mini_graph/linked_pipelines_mini_list.vue
+++ b/app/assets/javascripts/ci/pipeline_mini_graph/linked_pipelines_mini_list.vue
@@ -81,11 +81,6 @@ export default {
       // detailedStatus is graphQL, details.status is REST
       return pipeline?.detailedStatus || pipeline?.details?.status;
     },
-    triggerButtonClass(pipeline) {
-      const { group } = accessValue(pipeline, this.dataMethod, 'detailedStatus');
-
-      return `ci-status-icon-${group}`;
-    },
   },
 };
 </script>
@@ -105,7 +100,6 @@ export default {
       v-gl-tooltip="{ title: pipelineTooltipText(pipeline) }"
       :status="pipelineStatus(pipeline)"
       :show-tooltip="false"
-      :class="triggerButtonClass(pipeline)"
       class="linked-pipeline-mini-item gl-mb-0!"
       data-testid="linked-pipeline-mini-item"
     />
diff --git a/app/assets/stylesheets/framework/icons.scss b/app/assets/stylesheets/framework/icons.scss
index 3d95622d2f8267..bfd55fbb53db6e 100644
--- a/app/assets/stylesheets/framework/icons.scss
+++ b/app/assets/stylesheets/framework/icons.scss
@@ -54,6 +54,7 @@
   // Makes the borderless CI icons appear slightly bigger than the default 16px.
   // Could be fixed by making the SVG fill up the canvas in a follow up issue.
   .gl-icon {
+    // fill: currentColor;
     width: 20px;
     height: 20px;
     margin: -2px;
@@ -84,7 +85,7 @@
   }
 
   &.ci-icon-variant-info {
-    @include ci-icon-style($blue-500, $white, $blue-600, $blue-50)
+    @include ci-icon-style($white, $blue-500, $blue-600, $blue-50)
   }
 
   &.ci-icon-variant-neutral {
diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb
index 78cd7ccdaa1eb3..21d982d42bcd26 100644
--- a/app/helpers/ci/status_helper.rb
+++ b/app/helpers/ci/status_helper.rb
@@ -81,7 +81,7 @@ def render_ci_icon(
       show_status_text: false
     )
       variant = badge_variant(status)
-      badge_classes = "ci-icon gl-p-2 ci-icon-variant-#{variant} #{option_css_classes}"
+      badge_classes = "ci-icon ci-icon-variant-#{variant} gl-p-2 #{option_css_classes}"
       title = "#{_('Pipeline')}: #{ci_label_for_status(status)}"
       data = { toggle: 'tooltip', placement: tooltip_placement, container: container, testid: 'ci-icon' }
 
diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb
index 1df1b7373d911f..d7129c869f297a 100644
--- a/spec/features/commits_spec.rb
+++ b/spec/features/commits_spec.rb
@@ -82,7 +82,7 @@
           it 'shows correct build status from default branch' do
             page.within("//li[@id='commit-#{pipeline.short_sha}']") do
               expect(page).to have_css("[data-testid='ci-icon']")
-              expect(page).to have_css('.ci-status-icon-success')
+              expect(page).to have_css('[data-testid="status_success_borderless-icon"]')
             end
           end
         end
diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb
index 42ed3a1959eb0d..5379dabc713610 100644
--- a/spec/features/dashboard/projects_spec.rb
+++ b/spec/features/dashboard/projects_spec.rb
@@ -154,7 +154,7 @@
       page.within('[data-testid="project_controls"]') do
         expect(page).to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']")
         expect(page).to have_css("[data-testid='ci-icon']")
-        expect(page).to have_css('.ci-status-icon-success')
+        expect(page).to have_css('[data-testid="status_success_borderless-icon"]')
         expect(page).to have_link('Pipeline: passed')
       end
     end
@@ -166,7 +166,7 @@
         page.within('[data-testid="project_controls"]') do
           expect(page).not_to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']")
           expect(page).not_to have_css("[data-testid='ci-icon']")
-          expect(page).not_to have_css('.ci-status-icon-success')
+          expect(page).not_to have_css('[data-testid="status_success_borderless-icon"]')
           expect(page).not_to have_link('Pipeline: passed')
         end
       end
diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb
index 153cba33b27191..7915f446ee0ffa 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.gl-badge .ci-status-icon-success'
+        expect(page).to have_css '[data-testid="status_success_borderless-icon"]'
       end
     end
 
diff --git a/spec/frontend/ci/pipeline_mini_graph/linked_pipelines_mini_list_spec.js b/spec/frontend/ci/pipeline_mini_graph/linked_pipelines_mini_list_spec.js
index 3c9d235bfccdc2..55ce3c79039d7c 100644
--- a/spec/frontend/ci/pipeline_mini_graph/linked_pipelines_mini_list_spec.js
+++ b/spec/frontend/ci/pipeline_mini_graph/linked_pipelines_mini_list_spec.js
@@ -51,7 +51,7 @@ describe('Linked pipeline mini list', () => {
     });
 
     it('should render the correct ci status icon', () => {
-      expect(findCiIcon().classes('ci-status-icon-running')).toBe(true);
+      expect(wrapper.find('[data-testid="status_running_borderless-icon"]').exists()).toBe(true);
     });
 
     it('should have an activated tooltip', () => {
@@ -95,7 +95,7 @@ describe('Linked pipeline mini list', () => {
     });
 
     it('should render the correct ci status icon', () => {
-      expect(findCiIcon().classes('ci-status-icon-running')).toBe(true);
+      expect(wrapper.find('[data-testid="status_running_borderless-icon"]').exists()).toBe(true);
     });
 
     it('should have an activated tooltip', () => {
diff --git a/spec/views/ci/status/_icon.html.haml_spec.rb b/spec/views/ci/status/_icon.html.haml_spec.rb
index 78b19957cf0486..a3058b2025511d 100644
--- a/spec/views/ci/status/_icon.html.haml_spec.rb
+++ b/spec/views/ci/status/_icon.html.haml_spec.rb
@@ -31,7 +31,7 @@
       end
 
       it 'contains build status text' do
-        expect(rendered).to have_css('.ci-status-icon.ci-status-icon-success')
+        expect(rendered).to have_css('[data-testid="status_success_borderless-icon"]')
       end
 
       it 'does not contain links' do
@@ -59,7 +59,7 @@
         end
 
         it 'contains valid commit status text' do
-          expect(rendered).to have_css('.ci-status-icon.ci-status-icon-running')
+          expect(rendered).to have_css('[data-testid="status_running_borderless-icon"]')
         end
 
         it 'has link to external status page' do
@@ -75,7 +75,7 @@
         end
 
         it 'contains valid commit status text' do
-          expect(rendered).to have_css('.ci-status-icon.ci-status-icon-canceled')
+          expect(rendered).to have_css('[data-testid="status_canceled_borderless-icon"]')
         end
 
         it 'has link to external status page' do
diff --git a/spec/views/projects/issues/_related_branches.html.haml_spec.rb b/spec/views/projects/issues/_related_branches.html.haml_spec.rb
index 11c398f4e3b27d..d37c8c762a3db1 100644
--- a/spec/views/projects/issues/_related_branches.html.haml_spec.rb
+++ b/spec/views/projects/issues/_related_branches.html.haml_spec.rb
@@ -25,7 +25,7 @@
     expect(rendered).to have_text('other')
     expect(rendered).to have_link(href: 'link-to-feature')
     expect(rendered).to have_link(href: 'link-to-other')
-    expect(rendered).to have_css('.ci-status-icon')
+    expect(rendered).to have_css('[data-testid="ci-icon"]')
     expect(rendered).to have_css('.related-branch-info')
   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 01e8d23fb9fe89..0ac5efa2e6d4a9 100644
--- a/spec/views/projects/tags/index.html.haml_spec.rb
+++ b/spec/views/projects/tags/index.html.haml_spec.rb
@@ -91,7 +91,7 @@
 
       render
 
-      expect(page.find('.tags .content-list li', text: tag)).to have_css '.gl-badge .ci-status-icon-success'
+      expect(page.find('.tags .content-list li', text: tag)).to have_css '[data-testid="status_success_borderless-icon"]'
       expect(page.all('.tags .content-list li')).to all(have_css('svg.s16'))
     end
 
-- 
GitLab