From 066155704dd46b348688366eae176fdcee641f3c Mon Sep 17 00:00:00 2001 From: dimitrieh Date: Wed, 25 Jan 2017 00:13:28 +0100 Subject: [PATCH 1/7] added icons and fixed mini pipeline action dropdown icons --- app/assets/stylesheets/pages/pipelines.scss | 16 ++++++++++++++-- .../ci/status/_dropdown_graph_badge.html.haml | 2 +- app/views/ci/status/_graph_badge.html.haml | 2 +- .../icons/_icon_action_cancel_borderless.svg | 1 + .../icons/_icon_action_play_borderless.svg | 1 + .../icons/_icon_action_retry_borderless.svg | 1 + .../icons/_icon_action_stop_borderless.svg | 1 + lib/gitlab/ci/status/build/cancelable.rb | 2 +- lib/gitlab/ci/status/build/play.rb | 2 +- lib/gitlab/ci/status/build/retryable.rb | 2 +- lib/gitlab/ci/status/build/stop.rb | 2 +- 11 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 app/views/shared/icons/_icon_action_cancel_borderless.svg create mode 100644 app/views/shared/icons/_icon_action_play_borderless.svg create mode 100644 app/views/shared/icons/_icon_action_retry_borderless.svg create mode 100644 app/views/shared/icons/_icon_action_stop_borderless.svg diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 5d4bd091a6b..fc6a682203a 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -821,11 +821,23 @@ &:hover, &:focus { - text-decoration: none; - color: $gl-text-color; background-color: $stage-hover-bg; border: 1px solid transparent; } + + svg { + width: 22px; + height: 22px; + left: -6px; + position: relative; + top: -3px; + fill: $action-icon-color; + } + + &:hover svg, + &:focus svg { + fill: $gl-text-color; + } } // link to the build diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml index 8dea3479f82..8ed23ac4919 100644 --- a/app/views/ci/status/_dropdown_graph_badge.html.haml +++ b/app/views/ci/status/_dropdown_graph_badge.html.haml @@ -16,4 +16,4 @@ - if status.has_action? = link_to status.action_path, class: 'ci-action-icon-wrapper js-ci-action-icon', method: status.action_method, data: { toggle: 'tooltip', title: status.action_title } do - = icon(status.action_icon, class: status.action_class) + = custom_icon(status.action_icon) diff --git a/app/views/ci/status/_graph_badge.html.haml b/app/views/ci/status/_graph_badge.html.haml index dd2f649de9a..edfb02d21f3 100644 --- a/app/views/ci/status/_graph_badge.html.haml +++ b/app/views/ci/status/_graph_badge.html.haml @@ -17,4 +17,4 @@ - if status.has_action? = link_to status.action_path, class: 'ci-action-icon-container has-tooltip', method: status.action_method, data: { toggle: 'tooltip', title: status.action_title } do %i.ci-action-icon-wrapper - = icon(status.action_icon, class: status.action_class) + = custom_icon(status.action_icon) diff --git a/app/views/shared/icons/_icon_action_cancel_borderless.svg b/app/views/shared/icons/_icon_action_cancel_borderless.svg new file mode 100644 index 00000000000..a1f700eb0ff --- /dev/null +++ b/app/views/shared/icons/_icon_action_cancel_borderless.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_action_play_borderless.svg b/app/views/shared/icons/_icon_action_play_borderless.svg new file mode 100644 index 00000000000..6ac192cd7e9 --- /dev/null +++ b/app/views/shared/icons/_icon_action_play_borderless.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_action_retry_borderless.svg b/app/views/shared/icons/_icon_action_retry_borderless.svg new file mode 100644 index 00000000000..0fa0243f3c0 --- /dev/null +++ b/app/views/shared/icons/_icon_action_retry_borderless.svg @@ -0,0 +1 @@ + diff --git a/app/views/shared/icons/_icon_action_stop_borderless.svg b/app/views/shared/icons/_icon_action_stop_borderless.svg new file mode 100644 index 00000000000..1c8e2fe4156 --- /dev/null +++ b/app/views/shared/icons/_icon_action_stop_borderless.svg @@ -0,0 +1 @@ + diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index a979fe7d573..f9fa58586f2 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -10,7 +10,7 @@ module Gitlab end def action_icon - 'ban' + 'icon_action_cancel_borderless' end def action_path diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 1bf949c96dd..c5c408f0e3e 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -26,7 +26,7 @@ module Gitlab end def action_icon - 'play' + 'icon_action_play_borderless' end def action_title diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 8e38d6a8523..7544df022a4 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -10,7 +10,7 @@ module Gitlab end def action_icon - 'refresh' + 'icon_action_retry_borderless' end def action_title diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index e1dfdb76d41..45d1e80aa35 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -14,7 +14,7 @@ module Gitlab end def icon - 'icon_status_manual' + 'icon_action_stop_borderless' end def group From abc170c40d52bfe03728ff182a876c89b4158c8a Mon Sep 17 00:00:00 2001 From: dimitrieh Date: Wed, 25 Jan 2017 00:32:41 +0100 Subject: [PATCH 2/7] fixed pipeline graph action buttons and counter badge positioning --- app/assets/stylesheets/pages/pipelines.scss | 42 ++++++++++----------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index fc6a682203a..22b37b3ca48 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -490,31 +490,27 @@ // Action Icons in big pipeline-graph nodes > .ci-action-icon-container .ci-action-icon-wrapper { - i { - color: $border-color; - border-radius: 100%; - border: 1px solid $border-color; - padding: 5px 6px; - font-size: 13px; - background: $white-light; - height: 30px; - width: 30px; + height: 30px; + width: 30px; + background: $white-light; + border: 1px solid $border-color; + border-radius: 100%; + display: block; - &::before { - position: relative; - top: 3px; - left: 3px; - } - - &:hover { - color: $gl-text-color; - background-color: $stage-hover-bg; - border: 1px solid $stage-hover-bg; - } + &:hover { + background-color: $stage-hover-bg; + border: 1px solid $stage-hover-bg; } - .ci-play-icon { - padding: 5px 5px 5px 7px; + svg { + fill: $border-color; + position: relative; + left: -1px; + top: -1px; + } + + &:hover svg { + fill: $gl-text-color; } } @@ -653,7 +649,7 @@ font-weight: 100; font-size: 15px; position: absolute; - right: 5px; + right: 13px; top: 8px; } From bf7426a416c7019a124cbe5ec24ce0a8fba9c828 Mon Sep 17 00:00:00 2001 From: dimitrieh Date: Wed, 25 Jan 2017 00:37:03 +0100 Subject: [PATCH 3/7] changelog entry --- ...-manual-action-icons-to-svg-to-propperly-position-them.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/25910-convert-manual-action-icons-to-svg-to-propperly-position-them.yml diff --git a/changelogs/unreleased/25910-convert-manual-action-icons-to-svg-to-propperly-position-them.yml b/changelogs/unreleased/25910-convert-manual-action-icons-to-svg-to-propperly-position-them.yml new file mode 100644 index 00000000000..9506692dd40 --- /dev/null +++ b/changelogs/unreleased/25910-convert-manual-action-icons-to-svg-to-propperly-position-them.yml @@ -0,0 +1,4 @@ +--- +title: Convert pipeline action icons to svg to have them propperly positioned +merge_request: +author: From 4983fbaaf457cf434bca83ecbc178723d90a3dd2 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 25 Jan 2017 09:40:41 +0000 Subject: [PATCH 4/7] Remove unneeded 'borderless' from icons name --- ...n_action_cancel_borderless.svg => _icon_action_cancel.svg} | 0 ..._icon_action_play_borderless.svg => _icon_action_play.svg} | 0 ...con_action_retry_borderless.svg => _icon_action_retry.svg} | 0 ..._icon_action_stop_borderless.svg => _icon_action_stop.svg} | 0 lib/gitlab/ci/status/build/cancelable.rb | 2 +- lib/gitlab/ci/status/build/play.rb | 2 +- lib/gitlab/ci/status/build/retryable.rb | 2 +- lib/gitlab/ci/status/build/stop.rb | 4 ++-- 8 files changed, 5 insertions(+), 5 deletions(-) rename app/views/shared/icons/{_icon_action_cancel_borderless.svg => _icon_action_cancel.svg} (100%) rename app/views/shared/icons/{_icon_action_play_borderless.svg => _icon_action_play.svg} (100%) rename app/views/shared/icons/{_icon_action_retry_borderless.svg => _icon_action_retry.svg} (100%) rename app/views/shared/icons/{_icon_action_stop_borderless.svg => _icon_action_stop.svg} (100%) diff --git a/app/views/shared/icons/_icon_action_cancel_borderless.svg b/app/views/shared/icons/_icon_action_cancel.svg similarity index 100% rename from app/views/shared/icons/_icon_action_cancel_borderless.svg rename to app/views/shared/icons/_icon_action_cancel.svg diff --git a/app/views/shared/icons/_icon_action_play_borderless.svg b/app/views/shared/icons/_icon_action_play.svg similarity index 100% rename from app/views/shared/icons/_icon_action_play_borderless.svg rename to app/views/shared/icons/_icon_action_play.svg diff --git a/app/views/shared/icons/_icon_action_retry_borderless.svg b/app/views/shared/icons/_icon_action_retry.svg similarity index 100% rename from app/views/shared/icons/_icon_action_retry_borderless.svg rename to app/views/shared/icons/_icon_action_retry.svg diff --git a/app/views/shared/icons/_icon_action_stop_borderless.svg b/app/views/shared/icons/_icon_action_stop.svg similarity index 100% rename from app/views/shared/icons/_icon_action_stop_borderless.svg rename to app/views/shared/icons/_icon_action_stop.svg diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index f9fa58586f2..67bbc3c4849 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -10,7 +10,7 @@ module Gitlab end def action_icon - 'icon_action_cancel_borderless' + 'icon_action_cancel' end def action_path diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index c5c408f0e3e..b0549b3473c 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -26,7 +26,7 @@ module Gitlab end def action_icon - 'icon_action_play_borderless' + 'icon_action_play' end def action_title diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 7544df022a4..6b362af7634 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -10,7 +10,7 @@ module Gitlab end def action_icon - 'icon_action_retry_borderless' + 'icon_action_retry' end def action_title diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 45d1e80aa35..90401cad0d2 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -14,7 +14,7 @@ module Gitlab end def icon - 'icon_action_stop_borderless' + 'icon_status_manual' end def group @@ -26,7 +26,7 @@ module Gitlab end def action_icon - 'stop' + 'icon_action_stop' end def action_title From 819ae974d7cf8db98d6400c5ac85ebbd68c36f67 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 25 Jan 2017 11:04:16 +0000 Subject: [PATCH 5/7] Fix broken tests Fix linter error --- app/views/ci/status/_graph_badge.html.haml | 4 ++-- .../projects/pipelines/pipeline_spec.rb | 18 +++++++++--------- .../gitlab/ci/status/build/cancelable_spec.rb | 2 +- spec/lib/gitlab/ci/status/build/play_spec.rb | 2 +- .../gitlab/ci/status/build/retryable_spec.rb | 2 +- spec/lib/gitlab/ci/status/build/stop_spec.rb | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/views/ci/status/_graph_badge.html.haml b/app/views/ci/status/_graph_badge.html.haml index edfb02d21f3..a44adc6205a 100644 --- a/app/views/ci/status/_graph_badge.html.haml +++ b/app/views/ci/status/_graph_badge.html.haml @@ -2,7 +2,7 @@ - subject = local_assigns.fetch(:subject) - status = subject.detailed_status(current_user) -- klass = "ci-status-icon ci-status-icon-#{status.group}" +- klass = "ci-status-icon ci-status-icon-#{status.group} js-ci-status-icon-#{status.group}" - tooltip = "#{subject.name} - #{status.label}" - if status.has_details? @@ -16,5 +16,5 @@ - if status.has_action? = link_to status.action_path, class: 'ci-action-icon-container has-tooltip', method: status.action_method, data: { toggle: 'tooltip', title: status.action_title } do - %i.ci-action-icon-wrapper + %i.ci-action-icon-wrapper{ class: "js-#{status.action_icon}" } = custom_icon(status.action_icon) diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index e673ece37c3..7170beba799 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -66,8 +66,8 @@ describe 'Pipeline', :feature, :js do context 'when pipeline has running builds' do 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('.ci-status-icon-running') - expect(page).to have_selector('.ci-action-icon-container .fa-ban') + expect(page).to have_selector('.js-ci-status-icon-running') + expect(page).to have_selector('.js-icon_action_cancel') expect(page).to have_content('deploy') end end @@ -82,12 +82,12 @@ describe 'Pipeline', :feature, :js do context 'when pipeline has successful builds' do 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('.ci-status-icon-success') + expect(page).to have_selector('.js-ci-status-icon-success') expect(page).to have_content('build') end page.within('#ci-badge-build .ci-action-icon-container') do - expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + expect(page).to have_selector('.js-icon_action_retry') end end @@ -101,12 +101,12 @@ describe 'Pipeline', :feature, :js do context 'when pipeline has failed builds' do 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('.ci-status-icon-failed') + expect(page).to have_selector('.js-ci-status-icon-failed') expect(page).to have_content('test') end page.within('#ci-badge-test .ci-action-icon-container') do - expect(page).to have_selector('.ci-action-icon-container .fa-refresh') + expect(page).to have_selector('.js-icon_action_retry') end end @@ -120,12 +120,12 @@ describe 'Pipeline', :feature, :js do context 'when pipeline has manual builds' do 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('.ci-status-icon-manual') + expect(page).to have_selector('.js-ci-status-icon-manual') expect(page).to have_content('manual') end page.within('#ci-badge-manual-build .ci-action-icon-container') do - expect(page).to have_selector('.ci-action-icon-container .fa-play') + expect(page).to have_selector('.js-icon_action_play') end end @@ -138,7 +138,7 @@ describe 'Pipeline', :feature, :js do context 'when pipeline has external build' do it 'shows the success icon and the generic comit status build' do - expect(page).to have_selector('.ci-status-icon-success') + expect(page).to have_selector('.js-ci-status-icon-success') expect(page).to have_content('jenkins') expect(page).to have_link('jenkins', href: 'http://gitlab.com/status') end diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb index b3c07347de1..8ad9b7cdf07 100644 --- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb @@ -62,7 +62,7 @@ describe Gitlab::Ci::Status::Build::Cancelable do end describe '#action_icon' do - it { expect(subject.action_icon).to eq 'ban' } + it { expect(subject.action_icon).to eq 'icon_action_cancel' } end describe '#action_title' do diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index f1b50a59ce6..f3e72ea1796 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -44,7 +44,7 @@ describe Gitlab::Ci::Status::Build::Play do end describe '#action_icon' do - it { expect(subject.action_icon).to eq 'play' } + it { expect(subject.action_icon).to eq 'icon_action_play' } end describe '#action_title' do diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb index 62036f8ec5d..2db0f8d29bd 100644 --- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb @@ -62,7 +62,7 @@ describe Gitlab::Ci::Status::Build::Retryable do end describe '#action_icon' do - it { expect(subject.action_icon).to eq 'refresh' } + it { expect(subject.action_icon).to eq 'icon_action_retry' } end describe '#action_title' do diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb index 597e02e86e4..41c2b624774 100644 --- a/spec/lib/gitlab/ci/status/build/stop_spec.rb +++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb @@ -46,7 +46,7 @@ describe Gitlab::Ci::Status::Build::Stop do end describe '#action_icon' do - it { expect(subject.action_icon).to eq 'stop' } + it { expect(subject.action_icon).to eq 'icon_action_stop' } end describe '#action_title' do From 7db05c4da120a394dec3b5175720a9f8751eb736 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 26 Jan 2017 19:55:18 +0000 Subject: [PATCH 6/7] Removed unused method --- lib/gitlab/ci/status/build/play.rb | 4 ---- lib/gitlab/ci/status/core.rb | 3 --- 2 files changed, 7 deletions(-) diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index b0549b3473c..0f4b7b24cef 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -33,10 +33,6 @@ module Gitlab 'Play' end - def action_class - 'ci-play-icon' - end - def action_path play_namespace_project_build_path(subject.project.namespace, subject.project, diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 73b6ab5a635..3dd2b9e01f6 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -42,9 +42,6 @@ module Gitlab raise NotImplementedError end - def action_class - end - def action_path raise NotImplementedError end From 10178af1fd30e97dd65d11bba1e640aa7dc5bbbd Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 26 Jan 2017 20:38:21 +0000 Subject: [PATCH 7/7] Fixes after review --- app/views/ci/status/_graph_badge.html.haml | 2 +- spec/features/projects/pipelines/pipeline_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/ci/status/_graph_badge.html.haml b/app/views/ci/status/_graph_badge.html.haml index a44adc6205a..0530d21a7e2 100644 --- a/app/views/ci/status/_graph_badge.html.haml +++ b/app/views/ci/status/_graph_badge.html.haml @@ -16,5 +16,5 @@ - if status.has_action? = link_to status.action_path, class: 'ci-action-icon-container has-tooltip', method: status.action_method, data: { toggle: 'tooltip', title: status.action_title } do - %i.ci-action-icon-wrapper{ class: "js-#{status.action_icon}" } + %i.ci-action-icon-wrapper{ class: "js-#{status.action_icon.dasherize}" } = custom_icon(status.action_icon) diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 7170beba799..917b545e98b 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -67,7 +67,7 @@ describe 'Pipeline', :feature, :js do 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('.js-ci-status-icon-running') - expect(page).to have_selector('.js-icon_action_cancel') + expect(page).to have_selector('.js-icon-action-cancel') expect(page).to have_content('deploy') end end @@ -87,7 +87,7 @@ describe 'Pipeline', :feature, :js do end page.within('#ci-badge-build .ci-action-icon-container') do - expect(page).to have_selector('.js-icon_action_retry') + expect(page).to have_selector('.js-icon-action-retry') end end @@ -106,7 +106,7 @@ describe 'Pipeline', :feature, :js do end page.within('#ci-badge-test .ci-action-icon-container') do - expect(page).to have_selector('.js-icon_action_retry') + expect(page).to have_selector('.js-icon-action-retry') end end @@ -125,7 +125,7 @@ describe 'Pipeline', :feature, :js do end page.within('#ci-badge-manual-build .ci-action-icon-container') do - expect(page).to have_selector('.js-icon_action_play') + expect(page).to have_selector('.js-icon-action-play') end end