Merge branch 'fix/detailed-status-groups' into 'master'

Improve icons and style for manual build status

## What does this MR do?

This MR changes the CSS class for manual build status badge to make it a little more different than skipped build status badge. It makes it darker.

## Screenshots

![Screen_Shot_2016-12-17_at_13.37.58](/uploads/9628f0e84fdaad82d8833f546f0192a6/Screen_Shot_2016-12-17_at_13.37.58.png)![Screen_Shot_2016-12-17_at_13.31.48](/uploads/fe8e27df0274b9f9ae840110593a298a/Screen_Shot_2016-12-17_at_13.31.48.png)


See merge request !8102

21 files changed, 102 insertions, 23 deletions

diff --git a/app/assets/stylesheets/framework/icons.scss b/app/assets/stylesheets/framework/icons.scss
index 226bd2ead31..8624a25c052 100644
--- a/app/assets/stylesheets/framework/icons.scss
+++ b/app/assets/stylesheets/framework/icons.scss
@@ -49,3 +49,11 @@
     fill: $gray-darkest;
   }
 }
+
+.ci-status-icon-manual {
+  color: $gl-text-color;
+
+  svg {
+    fill: $gl-text-color;
+  }
+}
diff --git a/app/assets/stylesheets/pages/status.scss b/app/assets/stylesheets/pages/status.scss
index f3b0608e545..055dacd81f4 100644
--- a/app/assets/stylesheets/pages/status.scss
+++ b/app/assets/stylesheets/pages/status.scss
@@ -1,5 +1,6 @@
 .container-fluid {
   .ci-status {
+    display: inline-block;
     padding: 2px 7px;
     margin-right: 10px;
     border: 1px solid $gray-darker;
@@ -15,8 +16,7 @@
       height: 13px;
       width: 13px;
       position: relative;
-      top: 1px;
-      margin-right: 3px;
+      top: 2px;
       overflow: visible;
     }
 
@@ -25,7 +25,7 @@
       border-color: $gl-danger;
 
       &:not(span):hover {
-        background-color: rgba( $gl-danger, .07);
+        background-color: rgba($gl-danger, .07);
       }
 
       svg {
@@ -39,7 +39,7 @@
       border-color: $gl-success;
 
       &:not(span):hover {
-        background-color: rgba( $gl-success, .07);
+        background-color: rgba($gl-success, .07);
       }
 
       svg {
@@ -52,7 +52,7 @@
       border-color: $gl-info;
 
       &:not(span):hover {
-        background-color: rgba( $gl-info, .07);
+        background-color: rgba($gl-info, .07);
       }
 
       svg {
@@ -66,7 +66,7 @@
       border-color: $gl-gray;
 
       &:not(span):hover {
-        background-color: rgba( $gl-gray, .07);
+        background-color: rgba($gl-gray, .07);
       }
 
       svg {
@@ -79,7 +79,7 @@
       border-color: $gl-warning;
 
       &:not(span):hover {
-        background-color: rgba( $gl-warning, .07);
+        background-color: rgba($gl-warning, .07);
       }
 
       svg {
@@ -92,7 +92,7 @@
       border-color: $blue-normal;
 
       &:not(span):hover {
-        background-color: rgba( $blue-normal, .07);
+        background-color: rgba($blue-normal, .07);
       }
 
       svg {
@@ -106,13 +106,26 @@
       border-color: $gl-gray-light;
 
       &:not(span):hover {
-        background-color: rgba( $gl-gray-light, .07);
+        background-color: rgba($gl-gray-light, .07);
       }
 
       svg {
         fill: $gl-gray-light;
       }
     }
+
+    &.ci-manual {
+      color: $gl-text-color;
+      border-color: $gl-text-color;
+
+      &:not(span):hover {
+        background-color: rgba($gl-text-color, .07);
+      }
+
+      svg {
+        fill: $gl-text-color;
+      }
+    }
   }
 }
 
diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml
index f2135af2686..601fb7f0f3f 100644
--- a/app/views/ci/status/_badge.html.haml
+++ b/app/views/ci/status/_badge.html.haml
@@ -1,10 +1,11 @@
 - status = local_assigns.fetch(:status)
+- css_classes = "ci-status ci-#{status.group}"
 
 - if status.has_details?
-  = link_to status.details_path, class: "ci-status ci-#{status}" do
+  = link_to status.details_path, class: css_classes do
     = custom_icon(status.icon)
     = status.text
 - else
-  %span{ class: "ci-status ci-#{status}" }
+  %span{ class: css_classes }
     = custom_icon(status.icon)
     = status.text
diff --git a/app/views/ci/status/_graph_badge.html.haml b/app/views/ci/status/_graph_badge.html.haml
index c7d04ab61e9..9f3a9c0c6b2 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}"
+- klass = "ci-status-icon ci-status-icon-#{status.group}"
 
 - if status.has_details?
   = link_to status.details_path, data: { toggle: 'tooltip', title: "#{subject.name} - #{status.label}" } do
diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb
index 5c506e6d59f..1bf949c96dd 100644
--- a/lib/gitlab/ci/status/build/play.rb
+++ b/lib/gitlab/ci/status/build/play.rb
@@ -17,6 +17,10 @@ module Gitlab
             'icon_status_manual'
           end
 
+          def group
+            'manual'
+          end
+
           def has_action?
             can?(user, :update_build, subject)
           end
diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb
index f8ffa95cde4..e1dfdb76d41 100644
--- a/lib/gitlab/ci/status/build/stop.rb
+++ b/lib/gitlab/ci/status/build/stop.rb
@@ -17,6 +17,10 @@ module Gitlab
             'icon_status_manual'
           end
 
+          def group
+            'manual'
+          end
+
           def has_action?
             can?(user, :update_build, subject)
           end
diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb
index 46fef8262c1..73b6ab5a635 100644
--- a/lib/gitlab/ci/status/core.rb
+++ b/lib/gitlab/ci/status/core.rb
@@ -22,15 +22,8 @@ module Gitlab
           raise NotImplementedError
         end
 
-        # Deprecation warning: this method is here because we need to maintain
-        # backwards compatibility with legacy statuses. We often do something
-        # like "ci-status ci-status-#{status}" to set CSS class.
-        #
-        # `to_s` method should be renamed to `group` at some point, after
-        # phasing legacy satuses out.
-        #
-        def to_s
-          self.class.name.demodulize.downcase.underscore
+        def group
+          self.class.name.demodulize.underscore
         end
 
         def has_details?
diff --git a/lib/gitlab/ci/status/pipeline/success_with_warnings.rb b/lib/gitlab/ci/status/pipeline/success_with_warnings.rb
index a7c98f9e909..24bf8b869e0 100644
--- a/lib/gitlab/ci/status/pipeline/success_with_warnings.rb
+++ b/lib/gitlab/ci/status/pipeline/success_with_warnings.rb
@@ -17,7 +17,7 @@ module Gitlab
             'icon_status_warning'
           end
 
-          def to_s
+          def group
             'success_with_warnings'
           end
 
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 0a77eaa123c..57f1e75ea2c 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -99,7 +99,7 @@ describe "Pipelines", feature: true, js: true do
       context 'when pipeline has manual builds' do
         it 'shows the skipped icon and a play action for the manual build' do
           page.within('a[data-title="manual build - manual play action"]') do
-            expect(page).to have_selector('.ci-status-icon-skipped')
+            expect(page).to have_selector('.ci-status-icon-manual')
             expect(page).to have_content('manual')
           end
 
diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
index 9376bce17a1..b3c07347de1 100644
--- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb
@@ -32,6 +32,14 @@ describe Gitlab::Ci::Status::Build::Cancelable do
     end
   end
 
+  describe '#group' do
+    it 'does not override status group' do
+      expect(status).to receive(:group)
+
+      subject.group
+    end
+  end
+
   describe 'action details' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 4ddf04a8e11..f1b50a59ce6 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -18,6 +18,10 @@ describe Gitlab::Ci::Status::Build::Play do
     it { expect(subject.icon).to eq 'icon_status_manual' }
   end
 
+  describe '#group' do
+    it { expect(subject.group).to eq 'manual' }
+  end
+
   describe 'action details' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
index d61e5bbaa6b..62036f8ec5d 100644
--- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb
@@ -32,6 +32,14 @@ describe Gitlab::Ci::Status::Build::Retryable do
     end
   end
 
+  describe '#group' do
+    it 'does not override status group' do
+      expect(status).to receive(:group)
+
+      subject.group
+    end
+  end
+
   describe 'action details' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb
index 59a85b55f90..597e02e86e4 100644
--- a/spec/lib/gitlab/ci/status/build/stop_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb
@@ -20,6 +20,10 @@ describe Gitlab::Ci::Status::Build::Stop do
     it { expect(subject.icon).to eq 'icon_status_manual' }
   end
 
+  describe '#group' do
+    it { expect(subject.group).to eq 'manual' }
+  end
+
   describe 'action details' do
     let(:user) { create(:user) }
     let(:build) { create(:ci_build) }
diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb
index 4639278ad45..38412fe2e4f 100644
--- a/spec/lib/gitlab/ci/status/canceled_spec.rb
+++ b/spec/lib/gitlab/ci/status/canceled_spec.rb
@@ -16,4 +16,8 @@ describe Gitlab::Ci::Status::Canceled do
   describe '#icon' do
     it { expect(subject.icon).to eq 'icon_status_canceled' }
   end
+
+  describe '#group' do
+    it { expect(subject.group).to eq 'canceled' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb
index 2ce176a29d6..6d847484693 100644
--- a/spec/lib/gitlab/ci/status/created_spec.rb
+++ b/spec/lib/gitlab/ci/status/created_spec.rb
@@ -16,4 +16,8 @@ describe Gitlab::Ci::Status::Created do
   describe '#icon' do
     it { expect(subject.icon).to eq 'icon_status_created' }
   end
+
+  describe '#group' do
+    it { expect(subject.group).to eq 'created' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/failed_spec.rb b/spec/lib/gitlab/ci/status/failed_spec.rb
index 9d527e6a7ef..990d686d22c 100644
--- a/spec/lib/gitlab/ci/status/failed_spec.rb
+++ b/spec/lib/gitlab/ci/status/failed_spec.rb
@@ -16,4 +16,8 @@ describe Gitlab::Ci::Status::Failed do
   describe '#icon' do
     it { expect(subject.icon).to eq 'icon_status_failed' }
   end
+
+  describe '#group' do
+    it { expect(subject.group).to eq 'failed' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb
index d03f595d3c7..7bb6579c317 100644
--- a/spec/lib/gitlab/ci/status/pending_spec.rb
+++ b/spec/lib/gitlab/ci/status/pending_spec.rb
@@ -16,4 +16,8 @@ describe Gitlab::Ci::Status::Pending do
   describe '#icon' do
     it { expect(subject.icon).to eq 'icon_status_pending' }
   end
+
+  describe '#group' do
+    it { expect(subject.group).to eq 'pending' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb
index 7e3383c307f..979160eb9c4 100644
--- a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb
+++ b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb
@@ -17,6 +17,10 @@ describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do
     it { expect(subject.icon).to eq 'icon_status_warning' }
   end
 
+  describe '#group' do
+    it { expect(subject.group).to eq 'success_with_warnings' }
+  end
+
   describe '.matches?' do
     context 'when pipeline is successful' do
       let(:pipeline) do
diff --git a/spec/lib/gitlab/ci/status/running_spec.rb b/spec/lib/gitlab/ci/status/running_spec.rb
index 9f47090d396..852d6c06baf 100644
--- a/spec/lib/gitlab/ci/status/running_spec.rb
+++ b/spec/lib/gitlab/ci/status/running_spec.rb
@@ -16,4 +16,8 @@ describe Gitlab::Ci::Status::Running do
   describe '#icon' do
     it { expect(subject.icon).to eq 'icon_status_running' }
   end
+
+  describe '#group' do
+    it { expect(subject.group).to eq 'running' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb
index 94601648a8d..e00b356a24b 100644
--- a/spec/lib/gitlab/ci/status/skipped_spec.rb
+++ b/spec/lib/gitlab/ci/status/skipped_spec.rb
@@ -16,4 +16,8 @@ describe Gitlab::Ci::Status::Skipped do
   describe '#icon' do
     it { expect(subject.icon).to eq 'icon_status_skipped' }
   end
+
+  describe '#group' do
+    it { expect(subject.group).to eq 'skipped' }
+  end
 end
diff --git a/spec/lib/gitlab/ci/status/success_spec.rb b/spec/lib/gitlab/ci/status/success_spec.rb
index 90f9f615e0d..4a89e1faf40 100644
--- a/spec/lib/gitlab/ci/status/success_spec.rb
+++ b/spec/lib/gitlab/ci/status/success_spec.rb
@@ -16,4 +16,8 @@ describe Gitlab::Ci::Status::Success do
   describe '#icon' do
     it { expect(subject.icon).to eq 'icon_status_success' }
   end
+
+  describe '#group' do
+    it { expect(subject.group).to eq 'success' }
+  end
 end