Update as per review feedback

6 files changed, 87 insertions, 43 deletions

diff --git a/app/assets/javascripts/prometheus_metrics/constants.js b/app/assets/javascripts/prometheus_metrics/constants.js
new file mode 100644
index 00000000000..50f1248456e
--- /dev/null
+++ b/app/assets/javascripts/prometheus_metrics/constants.js
@@ -0,0 +1,5 @@
+export default {
+  EMPTY: 'empty',
+  LOADING: 'loading',
+  LIST: 'list',
+};
diff --git a/app/assets/javascripts/prometheus_metrics/index.js b/app/assets/javascripts/prometheus_metrics/index.js
index 9f20161783b..a0c43c5abe1 100644
--- a/app/assets/javascripts/prometheus_metrics/index.js
+++ b/app/assets/javascripts/prometheus_metrics/index.js
@@ -2,6 +2,5 @@ import PrometheusMetrics from './prometheus_metrics';
 
 $(() => {
   const prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring');
-  prometheusMetrics.init();
   prometheusMetrics.loadActiveMetrics();
 });
diff --git a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js
index d83e85b2026..ef4d6df5138 100644
--- a/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js
+++ b/app/assets/javascripts/prometheus_metrics/prometheus_metrics.js
@@ -1,3 +1,5 @@
+import PANEL_STATE from './constants';
+
 export default class PrometheusMetrics {
   constructor(wrapperSelector) {
     this.backOffRequestCounter = 0;
@@ -16,31 +18,48 @@ export default class PrometheusMetrics {
     this.$missingEnvVarMetricsList = this.$missingEnvVarPanel.find('.js-missing-var-metrics-list');
 
     this.activeMetricsEndpoint = this.$monitoredMetricsPanel.data('active-metrics');
-  }
 
-  init() {
     this.$panelToggle.on('click', e => this.handlePanelToggle(e));
   }
 
   /* eslint-disable class-methods-use-this */
   handlePanelToggle(e) {
     const $toggleBtn = $(e.currentTarget);
-    const $currentPanelBody = $toggleBtn.parents('.panel').find('.panel-body');
-    if ($currentPanelBody.is(':visible')) {
-      $currentPanelBody.addClass('hidden');
+    const $currentPanelBody = $toggleBtn.closest('.panel').find('.panel-body');
+    $currentPanelBody.toggleClass('hidden');
+    if ($toggleBtn.hasClass('fa-caret-down')) {
       $toggleBtn.removeClass('fa-caret-down').addClass('fa-caret-right');
     } else {
-      $currentPanelBody.removeClass('hidden');
       $toggleBtn.removeClass('fa-caret-right').addClass('fa-caret-down');
     }
   }
 
+  showMonitoringMetricsPanelState(stateName) {
+    switch (stateName) {
+      case PANEL_STATE.LOADING:
+        this.$monitoredMetricsLoading.removeClass('hidden');
+        this.$monitoredMetricsEmpty.addClass('hidden');
+        this.$monitoredMetricsList.addClass('hidden');
+        break;
+      case PANEL_STATE.LIST:
+        this.$monitoredMetricsLoading.addClass('hidden');
+        this.$monitoredMetricsEmpty.addClass('hidden');
+        this.$monitoredMetricsList.removeClass('hidden');
+        break;
+      default:
+        this.$monitoredMetricsLoading.addClass('hidden');
+        this.$monitoredMetricsEmpty.removeClass('hidden');
+        this.$monitoredMetricsList.addClass('hidden');
+        break;
+    }
+  }
+
   populateActiveMetrics(metrics) {
     let totalMonitoredMetrics = 0;
     let totalMissingEnvVarMetrics = 0;
 
     metrics.forEach((metric) => {
-      this.$monitoredMetricsList.append(`<li>${metric.group}<span class="badge-count">${metric.active_metrics}</span></li>`);
+      this.$monitoredMetricsList.append(`<li>${metric.group}<span class="badge">${metric.active_metrics}</span></li>`);
       totalMonitoredMetrics += metric.active_metrics;
       if (metric.metrics_missing_requirements > 0) {
         this.$missingEnvVarMetricsList.append(`<li>${metric.group}</li>`);
@@ -49,17 +68,17 @@ export default class PrometheusMetrics {
     });
 
     this.$monitoredMetricsCount.text(totalMonitoredMetrics);
-    this.$monitoredMetricsLoading.addClass('hidden');
-    this.$monitoredMetricsList.removeClass('hidden');
+    this.showMonitoringMetricsPanelState(PANEL_STATE.LIST);
 
     if (totalMissingEnvVarMetrics > 0) {
       this.$missingEnvVarPanel.removeClass('hidden');
+      this.$missingEnvVarPanel.find('.flash-container').off('click');
       this.$missingEnvVarMetricCount.text(totalMissingEnvVarMetrics);
     }
   }
 
   loadActiveMetrics() {
-    this.$monitoredMetricsLoading.removeClass('hidden');
+    this.showMonitoringMetricsPanelState(PANEL_STATE.LOADING);
     gl.utils.backOff((next, stop) => {
       $.getJSON(this.activeMetricsEndpoint)
         .done((res) => {
@@ -80,13 +99,11 @@ export default class PrometheusMetrics {
       if (res && res.data && res.data.length) {
         this.populateActiveMetrics(res.data);
       } else {
-        this.$monitoredMetricsLoading.addClass('hidden');
-        this.$monitoredMetricsEmpty.removeClass('hidden');
+        this.showMonitoringMetricsPanelState(PANEL_STATE.EMPTY);
       }
     })
     .catch(() => {
-      this.$monitoredMetricsLoading.addClass('hidden');
-      this.$monitoredMetricsEmpty.removeClass('hidden');
+      this.showMonitoringMetricsPanelState(PANEL_STATE.EMPTY);
     });
   }
 }
diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index deed4501399..e2ed1239541 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -39,11 +39,11 @@
 .prometheus-metrics-monitoring {
   .panel {
     .panel-toggle {
-      width: 12px;
+      width: 14px;
+    }
 
-      &.fa-caret-right {
-        padding-left: 4px;
-      }
+    .badge {
+      font-size: inherit;
     }
 
     .panel-heading .badge-count {
@@ -55,14 +55,9 @@
       padding: 0;
     }
 
-    .badge-count {
-      margin-left: 3px;
-      padding: 3px 8px;
-      border-radius: 40%;
-    }
-
     .flash-container {
       margin-bottom: 0;
+      cursor: default;
 
       .flash-notice {
         border-radius: 0;
@@ -80,25 +75,20 @@
       margin-top: 10px;
       margin-bottom: 0;
     }
-
-    p {
-      color: $gl-gray-light;
-    }
   }
 
-  .loading-metrics .fa-spin {
+  .loading-metrics .metrics-load-spinner {
     color: $loading-color;
   }
 
   .metrics-list {
-    list-style-type: none;
-    margin: 0;
-    padding: 0;
+    margin-bottom: 0;
 
     li {
-      padding: 16px;
+      padding: $gl-padding;
 
-      .badge-count {
+      .badge {
+        margin-left: 5px;
         background: $badge-bg;
       }
     }
diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml
index 82d4dd2488f..f0bd8c8bee9 100644
--- a/app/views/projects/services/prometheus/_show.html.haml
+++ b/app/views/projects/services/prometheus/_show.html.haml
@@ -11,28 +11,28 @@
       = link_to 'More information', '#'
 
   .col-lg-9
-    .panel.panel-default.js-panel-monitored-metrics{ data: { "active-metrics" => "#{namespace_project_prometheus_active_metrics_path(@project.namespace, @project)}.json" } }
+    .panel.panel-default.js-panel-monitored-metrics{ data: { "active-metrics" => "#{namespace_project_prometheus_active_metrics_path(@project.namespace, @project, :json)}" } }
       .panel-heading
         %h3.panel-title
           Monitored
-          %span.badge-count.js-monitored-count 0
+          %span.badge.js-monitored-count 0
       .panel-body
         .loading-metrics.js-loading-metrics
-          = icon('spinner spin 3x')
+          = icon('spinner spin 3x', class: 'metrics-load-spinner')
           %p Finding and configuring metrics...
         .empty-metrics.hidden.js-empty-metrics
           = custom_icon('icon_empty_metrics')
           %p No metrics are being monitored. To start monitoring, deploy to an environment.
           = link_to project_environments_path(@project), title: 'View environments', class: 'btn btn-success' do
             View environments
-        %ul.metrics-list.hidden.js-metrics-list
+        %ul.list-unstyled.metrics-list.hidden.js-metrics-list
 
     .panel.panel-default.hidden.js-panel-missing-env-vars
       .panel-heading
         %h3.panel-title
-          = icon('caret-right lg', class: 'panel-toggle js-panel-toggle', 'aria-label' => 'Toggle panel')
+          = icon('caret-right lg fw', class: 'panel-toggle js-panel-toggle', 'aria-label' => 'Toggle panel')
           Missing environment variable
-          %span.badge-count.js-env-var-count 0
+          %span.badge.js-env-var-count 0
       .panel-body.hidden
         .flash-container
           .flash-notice
@@ -42,4 +42,4 @@
                 $CI_ENVIRONMENT_SLUG
               to exporter&rsquo;s queries.
               = link_to 'More information', '#'
-        %ul.metrics-list.js-missing-var-metrics-list
+        %ul.list-unstyled.metrics-list.js-missing-var-metrics-list
diff --git a/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js
index 8b4f386af80..e7187a8a5e0 100644
--- a/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js
+++ b/spec/javascripts/prometheus_metrics/prometheus_metrics_spec.js
@@ -1,4 +1,5 @@
 import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics';
+import PANEL_STATE from '~/prometheus_metrics/constants';
 import { metrics, missingVarMetrics } from './mock_data';
 
 describe('PrometheusMetrics', () => {
@@ -35,6 +36,38 @@ describe('PrometheusMetrics', () => {
     });
   });
 
+  describe('showMonitoringMetricsPanelState', () => {
+    let prometheusMetrics;
+
+    beforeEach(() => {
+      prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring');
+    });
+
+    it('should show loading state when called with `loading`', () => {
+      prometheusMetrics.showMonitoringMetricsPanelState(PANEL_STATE.LOADING);
+
+      expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeFalsy();
+      expect(prometheusMetrics.$monitoredMetricsEmpty.hasClass('hidden')).toBeTruthy();
+      expect(prometheusMetrics.$monitoredMetricsList.hasClass('hidden')).toBeTruthy();
+    });
+
+    it('should show metrics list when called with `list`', () => {
+      prometheusMetrics.showMonitoringMetricsPanelState(PANEL_STATE.LIST);
+
+      expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy();
+      expect(prometheusMetrics.$monitoredMetricsEmpty.hasClass('hidden')).toBeTruthy();
+      expect(prometheusMetrics.$monitoredMetricsList.hasClass('hidden')).toBeFalsy();
+    });
+
+    it('should show empty state when called with `empty`', () => {
+      prometheusMetrics.showMonitoringMetricsPanelState(PANEL_STATE.EMPTY);
+
+      expect(prometheusMetrics.$monitoredMetricsLoading.hasClass('hidden')).toBeTruthy();
+      expect(prometheusMetrics.$monitoredMetricsEmpty.hasClass('hidden')).toBeFalsy();
+      expect(prometheusMetrics.$monitoredMetricsList.hasClass('hidden')).toBeTruthy();
+    });
+  });
+
   describe('populateActiveMetrics', () => {
     let prometheusMetrics;
 
@@ -52,7 +85,7 @@ describe('PrometheusMetrics', () => {
 
       expect(prometheusMetrics.$monitoredMetricsCount.text()).toEqual('12');
       expect($metricsListLi.length).toEqual(metrics.length);
-      expect($metricsListLi.first().find('.badge-count').text()).toEqual(`${metrics[0].active_metrics}`);
+      expect($metricsListLi.first().find('.badge').text()).toEqual(`${metrics[0].active_metrics}`);
     });
 
     it('should show missing environment variables list', () => {