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 ab12e4c05e3045a96ce5231578caa4a401a63068..c6340e6787a995b8d11dfd69e30bbb1241374e51 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" >
- +
{{ job.name }}
@@ -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/javascripts/vue_shared/components/ci_icon.vue b/app/assets/javascripts/vue_shared/components/ci_icon.vue index 9f4cfa6f690e0d624aca0623d6b87e1ae5515013..a2b6b4642c93c550afdb193b917f5e44fccc3108 100644 --- a/app/assets/javascripts/vue_shared/components/ci_icon.vue +++ b/app/assets/javascripts/vue_shared/components/ci_icon.vue @@ -6,20 +6,11 @@ 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 * } * - * 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 { @@ -35,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: { @@ -62,66 +48,47 @@ 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; + if (this.status.icon) { + return `${this.status.icon}_borderless`; + } + return null; }, - badgeStyles() { + variant() { switch (this.status.icon) { case 'status_success': - return { - textColor: 'gl-text-green-700', - variant: 'success', - }; + return 'success'; case 'status_warning': - return { - textColor: 'gl-text-orange-700', - variant: 'warning', - }; + case 'status_pending': + return 'warning'; case 'status_failed': - return { - textColor: 'gl-text-red-700', - variant: 'danger', - }; + return 'danger'; case 'status_running': - return { - textColor: 'gl-text-blue-700', - 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', - }; + return 'info'; // 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', - }; + return 'neutral'; } }, }, @@ -131,30 +98,18 @@ export default { - - {{ status.text }} + {{ + status.text + }} diff --git a/app/assets/stylesheets/framework/icons.scss b/app/assets/stylesheets/framework/icons.scss index 9d09c160551211ef91513d844695331448855708..bfd55fbb53db6e081caa6dd0edd6700e67f2454f 100644 --- a/app/assets/stylesheets/framework/icons.scss +++ b/app/assets/stylesheets/framework/icons.scss @@ -1,37 +1,18 @@ -@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, @@ -39,18 +20,18 @@ .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 +39,58 @@ .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 { + // .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 { + // fill: currentColor; + 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($white, $blue-500, $blue-600, $blue-50) + } + + &.ci-icon-variant-neutral { + @include ci-icon-style($white, $gray-500) + } } .password-status-icon-success { diff --git a/app/helpers/ci/status_helper.rb b/app/helpers/ci/status_helper.rb index 8bce1ccd7782c8b0ef162efd696d40bb60e5628c..21d982d42bcd268b98cc7d38c12d4e869f7801b3 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,16 +50,12 @@ 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 - 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 @@ -85,16 +81,17 @@ 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 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' } - badge_classes = "ci-icon gl-p-2 #{option_css_classes}" + + 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' }) + 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 @@ -135,16 +132,18 @@ def badge_variant(status) case variant when 'success' :success - when 'success-with-warnings', 'pending' + when 'success-with-warnings' + :warning + when 'pending' + :warning + when 'waiting-for-resource' :warning when 'failed' :danger when 'running' :info - when 'canceled', 'manual' - :neutral else - :muted + :neutral end end end 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 c5a9fa4329431a504ffff59f07eee6b2f4ae25be..0dc1cdd0991947e608b5f90d6092e80269646a0b 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/ee/spec/frontend/vue_shared/dashboards/components/project_pipeline_spec.js b/ee/spec/frontend/vue_shared/dashboards/components/project_pipeline_spec.js index f7ae6853602c4f0ffd7d13f29b9cdb59f8fd3aa2..6228f0825efe3c46b3f6b973e2ff6181aa17fb75 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/qa/qa/page/project/pipeline/show.rb b/qa/qa/page/project/pipeline/show.rb index 16b931c1a568670f025b4a60072cd62bc878fd91..151df85af3df4d050806f66761b0385c466d24ed 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/commits_spec.rb b/spec/features/commits_spec.rb index 1df1b7373d911f84a14b6ec80d87c7b0ec1cb877..d7129c869f297af1439d4f18d55c42a2219cd374 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 42ed3a1959eb0daa5fdf5fe18c4f257ce19972c5..5379dabc713610cf10fff8b9cb793e3dee631213 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/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index 18537d1cddb19a6ef52e6db264d0e5e338e7ea0a..c18b2c97f96df8fdfa6093018b90ae06713de2e9 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 30a81ccc07110c492ef2709c911b9f832e51e96b..36665f2b77dd287690aa540e8f5cf7bbda194eb3 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -941,9 +941,7 @@ 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-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 79e9ca7998e536bb186a8cd17d322597763f910a..7915f446ee0ffa9db4ef2092601c31aafe722f0a 100644 --- a/spec/features/projects/branches_spec.rb +++ b/spec/features/projects/branches_spec.rb @@ -299,13 +299,13 @@ 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 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/commit/mini_pipeline_graph_spec.rb b/spec/features/projects/commit/mini_pipeline_graph_spec.rb index 5bb3d1af92420cc7d28cb390486c0ba4329b8fda..c0a8dabf648ccd28630d792dad4ec38eed5cec51 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 ce3f0541139aca961cf8b1e50e99332896e4326b..24dd673501cab7e2299836de9778cf3c270e6ea0 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 ae0b02dac07d92f40f10df63d1f16ecb746a0595..fccfe00f59346dc1bca0c14aab56d29033d1d1f7 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 @@ -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 @@ -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 @@ -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 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 cda84a9e4d31781e3cc007a1522e5ede678bcbad..10db7f398fef361cc990a5d26876bf61819da5f0 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 ca201aee648df13fdab6e6981086dc5bdb59aec1..1da85ad9f78d498431c7340ff5f9ee59de5c4b43 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 5fe8581e81b5d7a65cec702d42794eb6d17ee453..72be51575d75f8acbdb4135ee8f63d86550330b8 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/ci/pipeline_mini_graph/linked_pipelines_mini_list_spec.js b/spec/frontend/ci/pipeline_mini_graph/linked_pipelines_mini_list_spec.js index 3c9d235bfccdc2f72a2738259dfc2865981fa472..55ce3c79039d7c05e2c9a00413501b6188063c79 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/frontend/ide/components/jobs/detail/description_spec.js b/spec/frontend/ide/components/jobs/detail/description_spec.js index 5465615bf383718228cea711b9807a5bc26e0a0d..909bd1f7c901857f51d4c43ed78b172c8a7302b5 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/ide/components/jobs/item_spec.js b/spec/frontend/ide/components/jobs/item_spec.js index dc0f732d11e1e6be5d07a1ffdfcb196a23b14e00..aa6fc5531dddd5929f14baca5721882c3f99ddc5 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/frontend/vue_shared/components/ci_icon_spec.js b/spec/frontend/vue_shared/components/ci_icon_spec.js index 1dc46538ec495782df955a0e23e7d789062a3e2e..cbb725bf9e63e11c0ed87d7bb3f5cae5b30722d4 100644 --- a/spec/frontend/vue_shared/components/ci_icon_spec.js +++ b/spec/frontend/vue_shared/components/ci_icon_spec.js @@ -2,52 +2,135 @@ 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 findIcon = () => wrapper.findComponent(GlIcon); + + it('should render a span element and an icon', () => { + createComponent(); + + expect(wrapper.attributes('size')).toBe('md'); + 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 | 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(wrapper.find('.ci-icon-wrapper').classes()).toContain(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 18983c832625bfbe5bfc4fbc97df156a789a51f2..502a535e102f907f51afe5d16f1fb49fe47b52dd 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-muted' - 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 diff --git a/spec/views/ci/status/_icon.html.haml_spec.rb b/spec/views/ci/status/_icon.html.haml_spec.rb index 78b19957cf0486be4483218bce10f02ae4de8dda..a3058b2025511dbbff9f3d26cb8c2f49f4070c4b 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 11c398f4e3b27d21156e9340c81b0aa093930099..d37c8c762a3db112c5957daa87751fdbd1010779 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 01e8d23fb9fe899aae88f78edc572de3d312cf84..0ac5efa2e6d4a93a004043328f1a68193f67a756 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