From 3f0eff82592f4a30abb6ffd15ac248a5f773c994 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 1 Jun 2017 21:24:56 +0530 Subject: [PATCH] Update as per review feedback --- .../prometheus_metrics/constants.js | 5 +++ .../javascripts/prometheus_metrics/index.js | 1 - .../prometheus_metrics/prometheus_metrics.js | 45 +++++++++++++------ app/assets/stylesheets/pages/settings.scss | 30 +++++-------- .../services/prometheus/_show.html.haml | 14 +++--- .../prometheus_metrics_spec.js | 35 ++++++++++++++- 6 files changed, 87 insertions(+), 43 deletions(-) create mode 100644 app/assets/javascripts/prometheus_metrics/constants.js 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(`
  • ${metric.group}${metric.active_metrics}
    • `); + this.$monitoredMetricsList.append(`
  • ${metric.group}${metric.active_metrics}
    • `); totalMonitoredMetrics += metric.active_metrics; if (metric.metrics_missing_requirements > 0) { this.$missingEnvVarMetricsList.append(`
  • ${metric.group}
    • `); @@ -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’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', () => { -- GitLab