Fixes as per reviewmr-widget-redesign-review-memgraph

5 files changed, 36 insertions, 22 deletions

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js
index 11b6e3514d0..45c29fde6b9 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js
@@ -47,7 +47,7 @@ export default {
   template: `
     <div class="mr-widget-heading">
       <div v-for="deployment in mr.deployments">
-        <div class="ci_widget">
+        <div class="ci-widget">
           <div class="ci-status-icon ci-status-icon-success">
             <span class="js-icon-link icon-link">
               <span v-html="svg" aria-hidden="true"></span>
@@ -58,27 +58,34 @@ export default {
             <a
               v-if="hasDeploymentMeta(deployment)"
               :href="deployment.url"
-              target="_blank" rel="noopener noreferrer nofollow" class="js-deploy-meta">
+              target="_blank"
+              rel="noopener noreferrer nofollow"
+              class="js-deploy-meta">
               {{deployment.name}}
             </a>
             <span v-if="hasExternalUrls(deployment)">on</span>
             <a
               v-if="hasExternalUrls(deployment)"
               :href="deployment.external_url"
-              target="_blank" rel="noopener noreferrer nofollow" class="js-deploy-url">
+              target="_blank"
+              rel="noopener noreferrer nofollow"
+              class="js-deploy-url">
               <i class="fa fa-external-link" aria-hidden="true"></i>
               {{deployment.external_url_formatted}}
             </a>
             <span
               v-if="hasDeploymentTime(deployment)"
               :data-title="deployment.deployed_at_formatted"
-              class="js-deploy-time" data-toggle="tooltip" data-placement="top">
+              class="js-deploy-time"
+              data-toggle="tooltip"
+              data-placement="top">
               {{formatDate(deployment.deployed_at)}}
             </span>
             <button
+              type="button"
               v-if="deployment.stop_url"
               @click="stopEnvironment(deployment)"
-              class="btn btn-default btn-xs" type="button">
+              class="btn btn-default btn-xs">
               Stop environment
             </button>
           </span>
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js
index 1228fcf195a..225e039b754 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js
@@ -14,6 +14,7 @@ export default {
       // memoryTo: 0,
       memoryMetrics: [],
       hasMetrics: false,
+      loadFailed: false,
       loadingMetrics: true,
       backOffRequestCounter: 0,
     };
@@ -67,17 +68,21 @@ export default {
     .then((res) => {
       this.computeGraphData(res.metrics);
       return res;
+    })
+    .catch(() => {
+      this.$props.loadFailed = true;
     });
   },
   template: `
     <div class="mr-info-list mr-memory-usage">
       <div class="legend"></div>
       <p class="usage-info usage-info-loading" v-if="loadingMetrics">
-        <i class="fa fa-spinner fa-spin" aria-hidden="true"></i>Loading deployment statistics.
+        <i class="fa fa-spinner fa-spin usage-info-load-spinner" aria-hidden="true"></i>Loading deployment statistics.
       </p>
       <p class="usage-info usage-info-loading" v-if="!hasMetrics && !loadingMetrics">Deployment statistics are not available currently.</p>
       <p class="usage-info" v-if="hasMetrics">Deployment memory usage:</p>
-      <mr-memory-graph v-if="hasMetrics" :height=25 :width=100 :metrics="memoryMetrics"></mr-memory-graph>
+      <p class="usage-info" v-if="loadFailed">Failed to load deployment statistics.</p>
+      <mr-memory-graph v-if="hasMetrics" height=25 width=100 :metrics="memoryMetrics"></mr-memory-graph>
     </div>
   `,
 };
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
index fa024030f4b..e809fc8099d 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
@@ -26,7 +26,7 @@ export default {
   },
   template: `
     <div class="mr-widget-heading">
-      <div class="ci_widget">
+      <div class="ci-widget">
         <template v-if="hasCIError">
           <div class="ci-status-icon ci-status-icon-failed js-ci-error">
             <span class="js-icon-link icon-link">
diff --git a/app/assets/javascripts/vue_shared/components/memory_graph.js b/app/assets/javascripts/vue_shared/components/memory_graph.js
index 50033e8408b..2a605b24339 100644
--- a/app/assets/javascripts/vue_shared/components/memory_graph.js
+++ b/app/assets/javascripts/vue_shared/components/memory_graph.js
@@ -2,8 +2,16 @@ export default {
   name: 'MemoryGraph',
   props: {
     metrics: { type: Array, required: true },
-    width: { type: Number, required: true },
-    height: { type: Number, required: true },
+    width: { type: String, required: true },
+    height: { type: String, required: true },
+  },
+  data() {
+    return {
+      pathD: '',
+      pathViewBox: '',
+      // dotX: '',
+      // dotY: '',
+    };
   },
   mounted() {
     const renderData = this.$props.metrics.map(v => v[1]);
@@ -12,22 +20,16 @@ export default {
     const diff = maxMemory - minMemory;
     // const cx = 0;
     // const cy = 0;
-
-    const svgEl = this.$el.querySelector('svg');
-    const pathEl = svgEl.querySelector('path');
-    // const circleEl = svgEl.querySelector('circle');
     const lineWidth = renderData.length;
     const linePath = renderData.map((y, x) => `${x} ${maxMemory - y}`);
-    pathEl.setAttribute('d', `M ${linePath}`);
-    // circleEl.setAttribute('cx', cx);
-    // circleEl.setAttribute('cy', cy);
-    svgEl.setAttribute('viewBox', `0 0 ${lineWidth} ${diff}`);
+    this.pathD = `M ${linePath}`;
+    this.pathViewBox = `0 0 ${lineWidth} ${diff}`;
   },
   template: `
     <div class="memory-graph-container">
       <svg :width="width" :height="height" xmlns="http://www.w3.org/2000/svg">
-        <path />
-        <!--<circle r="0.8" tranform="translate(0 -1)" /> -->
+        <path :d="pathD" :viewBox="pathViewBox" />
+        <!--<circle r="0.8" :cx="dotX" :cy="dotY" tranform="translate(0 -1)" /> -->
       </svg>
     </div>
   `,
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index e7f34fb71e4..0411b43910a 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -82,7 +82,7 @@
     }
   }
 
-  .ci_widget {
+  .ci-widget {
     color: $gl-text-color;
     display: -webkit-flex;
     display: flex;
@@ -713,7 +713,7 @@
   p.usage-info-loading {
     margin-bottom: 6px;
 
-    .fa {
+    .usage-info-load-spinner {
       margin-right: 10px;
       font-size: 16px;
     }