Changes after review

app/assets/javascripts/clusters.js

@@ -39,6 +39,7 @@ export default class Clusters {
     this.errorContainer = document.querySelector('.js-cluster-error');
     this.successContainer = document.querySelector('.js-cluster-success');
     this.creatingContainer = document.querySelector('.js-cluster-creating');
+    this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason');
 
     this.toggleButton.addEventListener('click', this.toggle.bind(this));
 
@@ -47,7 +48,7 @@ export default class Clusters {
     }
 
     if (this.state.statusPath) {
-      this.initPoling();
+      this.initPolling();
     }
   }
 
@@ -56,7 +57,7 @@ export default class Clusters {
     this.toggleInput.setAttribute('value', this.toggleButton.classList.contains('checked').toString());
   }
 
-  initPoling() {
+  initPolling() {
     this.poll = new Poll({
       resource: this.service,
       method: 'fetchData',
@@ -98,7 +99,7 @@ export default class Clusters {
         break;
       case 'errored':
         this.errorContainer.classList.remove('hidden');
-        this.errorContainer.querySelector('.js-error-reason').textContent = error;
+        this.errorReasonContainer.textContent = error;
         break;
       case 'scheduled':
       case 'creating':

app/views/projects/clusters/_header.html.haml

@@ -1,15 +1,14 @@
-.col-lg-8
-  %h4.prepend-top-0
-    = s_('ClusterIntegration|Create new cluster on Google Container Engine')
-  %p
-    = s_('ClusterIntegration|To create a new cluster on Google Container Engine, please sign in with your Google account:')
-  %ul
+%h4.prepend-top-0
+  = s_('ClusterIntegration|Create new cluster on Google Container Engine')
+%p
+  = s_('ClusterIntegration|To create a new cluster on Google Container Engine, please sign in with your Google account:')
+%ul
+  %li
+    - link_to_container_engine = link_to(s_('ClusterIntegration|access to Google Container Engine'), 'https://console.cloud.google.com', target: '_blank', rel: 'noopener noreferrer')
+    = s_('ClusterIntegration|Your account must have %{link_to_container_engine}').html_safe % { link_to_container_engine: link_to_container_engine }
     %li
-      - link_to_container_engine = link_to(s_('ClusterIntegration|access to Google Container Engine'), 'https://console.cloud.google.com', target: '_blank', rel: 'noopener noreferrer')
-      = s_('ClusterIntegration|Your account must have %{link_to_container_engine}').html_safe % { link_to_container_engine: link_to_container_engine }
-      %li
-        - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/container-engine/docs/quickstart', target: '_blank', rel: 'noopener noreferrer')
-        = s_('ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters').html_safe % { link_to_requirements: link_to_requirements }
-      %li
-        - link_to_container_project = link_to(s_('ClusterIntegration|Google Container Engine project'), target: '_blank', rel: 'noopener noreferrer')
-        = s_('ClusterIntegration|A %{link_to_container_project} must have been created under this account').html_safe % { link_to_container_project: link_to_container_project }
+      - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/container-engine/docs/quickstart', target: '_blank', rel: 'noopener noreferrer')
+      = s_('ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters').html_safe % { link_to_requirements: link_to_requirements }
+    %li
+      - link_to_container_project = link_to(s_('ClusterIntegration|Google Container Engine project'), target: '_blank', rel: 'noopener noreferrer')
+      = s_('ClusterIntegration|A %{link_to_container_project} must have been created under this account').html_safe % { link_to_container_project: link_to_container_project }

app/views/projects/clusters/_sidebar.html.haml

@@ -1,8 +1,7 @@
-.col-sm-4
-  %h4.prepend-top-0
-    = s_('ClusterIntegration|Cluster integration')
-  %p
-    = s_('ClusterIntegration|With a cluster associated to this project, you can use review apps, deploy your applications, run your pipelines, and much more in an easy way.')
-  %p
-    - link = link_to(s_('ClusterIntegration|cluster'), help_page_path('TODO'), target: '_blank', rel: 'noopener noreferrer')
-    = s_('ClusterIntegration|Learn more about %{link_to_documentation}').html_safe % { link_to_documentation: link }
+%h4.prepend-top-0
+  = s_('ClusterIntegration|Cluster integration')
+%p
+  = s_('ClusterIntegration|With a cluster associated to this project, you can use review apps, deploy your applications, run your pipelines, and much more in an easy way.')
+%p
+  - link = link_to(s_('ClusterIntegration|cluster'), help_page_path('TODO'), target: '_blank', rel: 'noopener noreferrer')
+  = s_('ClusterIntegration|Learn more about %{link_to_documentation}').html_safe % { link_to_documentation: link }

app/views/projects/clusters/login.html.haml

@@ -1,10 +1,12 @@
 .row.prepend-top-default
-  = render 'sidebar'
-  = render 'header'
+  .col-sm-4
+    = render 'sidebar'
+  .col-sm-8
+    = render 'header'
 .row
   .col-sm-8.col-sm-offset-4
     - if @authorize_url
-      %a{ href: @authorize_url }
+      = link_to @authorize_url do
         = image_tag('auth_buttons/signin_with_google.png')
     - else
       - link = link_to(s_('ClusterIntegration|properly configured'), help_page_path("integration/google"), target: '_blank', rel: 'noopener noreferrer')

app/views/projects/clusters/new.html.haml

@@ -1,4 +1,6 @@
 .row.prepend-top-default
-  = render 'sidebar'
-  = render 'header'
+  .col-sm-4
+    = render 'sidebar'
+  .col-sm-8
+    = render 'header'
 = render 'form'

app/views/projects/clusters/show.html.haml

@@ -3,8 +3,9 @@
   toggle_status: @cluster.enabled? ? 'true': 'false',
   cluster_status: @cluster.status_name,
   cluster_status_reason: @cluster.status_reason }}
-  = render 'sidebar'
-  .col-lg-8
+  .col-sm-4
+    = render 'sidebar'
+  .col-sm-8
     %h4.prepend-top-0
       = s_('ClusterIntegration|Enable cluster integration')
     %p
@@ -24,7 +25,7 @@
 
           %button{ type: 'button',
             class: "js-toggle-cluster project-feature-toggle #{'checked' unless !@cluster.enabled?} #{'disabled' unless can?(current_user, :update_cluster, @cluster)}",
-            'aria-label': 'Toggle',
+            'aria-label': s_('ClusterIntegration|Toggle Cluster'),
             disabled: !can?(current_user, :update_cluster, @cluster),
             data: { 'enabled-text': 'Enabled', 'disabled-text': 'Disabled' } }
 
@@ -50,7 +51,7 @@
         = s_('ClusterIntegration|Cluster was successfully created on Google Container Engine.')
 
     .form_group
-      %label
+      %label{ for: 'cluter_name' }
         = s_('ClusterIntegration|Cluster name')
       .input-group
         %input.form-control{ value: @cluster.gcp_cluster_name, disabled: true}

spec/javascripts/clusters_spec.js

@@ -29,14 +29,14 @@ describe('Clusters', () => {
         cluster.updateContainer('creating');
 
         expect(
-          cluster.creatingContainer.classList,
-        ).not.toContain('hidden');
+          cluster.creatingContainer.classList.contains('hidden'),
+        ).toBeFalsy();
         expect(
-          cluster.successContainer.classList,
-        ).toContain('hidden');
+          cluster.successContainer.classList.contains('hidden'),
+        ).toBeTruthy();
         expect(
-          cluster.errorContainer.classList,
-        ).toContain('hidden');
+          cluster.errorContainer.classList.contains('hidden'),
+        ).toBeTruthy();
       });
     });
 
@@ -45,14 +45,14 @@ describe('Clusters', () => {
         cluster.updateContainer('created');
 
         expect(
-          cluster.creatingContainer.classList,
-        ).toContain('hidden');
+          cluster.creatingContainer.classList.contains('hidden'),
+        ).toBeTruthy();
         expect(
-          cluster.successContainer.classList,
-        ).not.toContain('hidden');
+          cluster.successContainer.classList.contains('hidden'),
+        ).toBeFalsy();
         expect(
-          cluster.errorContainer.classList,
-        ).toContain('hidden');
+          cluster.errorContainer.classList.contains('hidden'),
+        ).toBeTruthy();
       });
     });
 
@@ -61,17 +61,17 @@ describe('Clusters', () => {
         cluster.updateContainer('errored', 'this is an error');
 
         expect(
-          cluster.creatingContainer.classList,
-        ).toContain('hidden');
+          cluster.creatingContainer.classList.contains('hidden'),
+        ).toBeTruthy();
         expect(
-          cluster.successContainer.classList,
-        ).toContain('hidden');
+          cluster.successContainer.classList.contains('hidden'),
+        ).toBeTruthy();
         expect(
-          cluster.errorContainer.classList,
-        ).not.toContain('hidden');
+          cluster.errorContainer.classList.contains('hidden'),
+        ).toBeFalsy();
 
         expect(
-          cluster.errorContainer.querySelector('.js-error-reason').textContent,
+          cluster.errorReasonContainer.textContent,
         ).toContain('this is an error');
       });
     });