Changes after Frontend and UX review:

- Moves toggle button to a shared location
- Adds tests for toggle button
- Transforms Clusters class into function
- Improves UX

8 files changed, 155 insertions, 135 deletions

diff --git a/app/assets/javascripts/clusters/clusters_index.js b/app/assets/javascripts/clusters/clusters_index.js
index 6249943da43..879212aad70 100644
--- a/app/assets/javascripts/clusters/clusters_index.js
+++ b/app/assets/javascripts/clusters/clusters_index.js
@@ -3,6 +3,29 @@ import { s__ } from '../locale';
 import ClustersService from './services/clusters_service';
 
 /**
+ * Toggles loading and disabled classes.
+ * @param {HTMLElement} button
+ */
+const toggleLoadingButton = (button) => {
+  if (button.getAttribute('disabled')) {
+    button.removeAttribute('disabled');
+  } else {
+    button.setAttribute('disabled', true);
+  }
+
+  button.classList.toggle('is-disabled');
+  button.classList.toggle('is-loading');
+};
+
+/**
+ * Toggles checked class for the given button
+ * @param {HTMLElement} button
+ */
+const toggleValue = (button) => {
+  button.classList.toggle('is-checked');
+};
+
+/**
  * Handles toggle buttons in the cluster's table.
  *
  * When the user clicks the toggle button for each cluster, it:
@@ -13,56 +36,24 @@ import ClustersService from './services/clusters_service';
  * 1) Show updated status in case of successfull response
  * 2) Show initial status in case of failed response
  */
-export default class ClusterTable {
-  constructor() {
-    this.container = '.js-clusters-list';
-    document.querySelectorAll(`${this.container} .js-toggle-cluster-list`).forEach(button => button.addEventListener('click', e => ClusterTable.updateCluster(e)));
-  }
-  /**
-   * When the toggle button is clicked,
-   * updates the status and makes a request to the API to update the cluster
-   * @param {Event} e
-   */
-  static updateCluster(e) {
-    const toggleButton = e.currentTarget;
-    const value = toggleButton.classList.contains('checked').toString();
-    const endpoint = toggleButton.getAttribute('data-endpoint');
-
-    ClusterTable.toggleValue(toggleButton);
-    ClusterTable.toggleLoadingButton(toggleButton);
-
-    ClustersService.updateCluster(endpoint, { cluster: { enabled: value } })
-      .then(() => {
-        ClusterTable.toggleLoadingButton(toggleButton);
-      })
-      .catch(() => {
-        ClusterTable.toggleLoadingButton(toggleButton);
-        ClusterTable.toggleValue(toggleButton);
-        Flash(s__('ClusterIntegration|Something went wrong on our end.'));
-      });
-  }
+export default function setClusterTableToggles() {
+  document.querySelectorAll('.js-toggle-cluster-list')
+    .forEach(button => button.addEventListener('click', (e) => {
+      const toggleButton = e.currentTarget;
+      const value = toggleButton.classList.contains('checked').toString();
+      const endpoint = toggleButton.getAttribute('data-endpoint');
 
-  /**
-   * Toggles loading and disabled classes.
-   * @param {HTMLElement} button
-   */
-  static toggleLoadingButton(button) {
-    if (button.getAttribute('disabled')) {
-      button.removeAttribute('disabled');
-    } else {
-      button.setAttribute('disabled', true);
-    }
+      toggleValue(toggleButton);
+      toggleLoadingButton(toggleButton);
 
-    button.classList.toggle('disabled');
-    button.classList.toggle('is-loading');
-    button.querySelector('.loading-icon').classList.toggle('hidden');
-  }
-
-  /**
-   * Toggles checked class for the given button
-   * @param {HTMLElement} button
-   */
-  static toggleValue(button) {
-    button.classList.toggle('checked');
-  }
+      ClustersService.updateCluster(endpoint, { cluster: { enabled: value } })
+        .then(() => {
+          toggleLoadingButton(toggleButton);
+        })
+        .catch(() => {
+          toggleLoadingButton(toggleButton);
+          toggleValue(toggleButton);
+          Flash(s__('ClusterIntegration|Something went wrong on our end.'));
+        });
+    }));
 }
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 49459f01bea..493d8726a13 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/app/assets/javascripts/dispatcher.js
@@ -556,7 +556,7 @@ import ProjectVariables from './project_variables';
           break;
         case 'projects:clusters:index':
           import(/* webpackChunkName: "clusters_index" */ './clusters/clusters_index')
-            .then(clusterIndex => new clusterIndex.default()) // eslint-disable-line new-cap
+            .then(clusterIndex => clusterIndex.default())
             .catch((err) => {
               Flash(s__('ClusterIntegration|Problem setting up the clusters list'));
               throw err;
diff --git a/app/assets/javascripts/projects/permissions/components/project_feature_setting.vue b/app/assets/javascripts/projects/permissions/components/project_feature_setting.vue
index 80c5d39f736..8fce4c63872 100644
--- a/app/assets/javascripts/projects/permissions/components/project_feature_setting.vue
+++ b/app/assets/javascripts/projects/permissions/components/project_feature_setting.vue
@@ -1,5 +1,5 @@
 <script>
-import projectFeatureToggle from './project_feature_toggle.vue';
+import projectFeatureToggle from '../../../vue_shared/components/toggle_button.vue';
 
 export default {
   props: {
diff --git a/app/assets/javascripts/projects/permissions/components/project_feature_toggle.vue b/app/assets/javascripts/projects/permissions/components/project_feature_toggle.vue
deleted file mode 100644
index 2403c60186a..00000000000
--- a/app/assets/javascripts/projects/permissions/components/project_feature_toggle.vue
+++ /dev/null
@@ -1,51 +0,0 @@
-<script>
-export default {
-  props: {
-    name: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    value: {
-      type: Boolean,
-      required: true,
-    },
-    disabledInput: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
-  },
-
-  model: {
-    prop: 'value',
-    event: 'change',
-  },
-
-  methods: {
-    toggleFeature() {
-      if (!this.disabledInput) this.$emit('change', !this.value);
-    },
-  },
-};
-</script>
-
-<template>
-  <label class="toggle-wrapper">
-    <input
-      v-if="name"
-      type="hidden"
-      :name="name"
-      :value="value"
-    />
-    <button
-      type="button"
-      aria-label="Toggle"
-      class="project-feature-toggle"
-      data-enabled-text="Enabled"
-      data-disabled-text="Disabled"
-      :class="{ checked: value, disabled: disabledInput }"
-      @click="toggleFeature"
-    />
-  </label>
-</template>
diff --git a/app/assets/javascripts/projects/permissions/components/settings_panel.vue b/app/assets/javascripts/projects/permissions/components/settings_panel.vue
index 326d9105666..639429baf26 100644
--- a/app/assets/javascripts/projects/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/projects/permissions/components/settings_panel.vue
@@ -1,6 +1,6 @@
 <script>
 import projectFeatureSetting from './project_feature_setting.vue';
-import projectFeatureToggle from './project_feature_toggle.vue';
+import projectFeatureToggle from '../../../vue_shared/components/toggle_button.vue';
 import projectSettingRow from './project_setting_row.vue';
 import { visibilityOptions, visibilityLevelDescriptions } from '../constants';
 import { toggleHiddenClassBySelector } from '../external';
diff --git a/app/assets/javascripts/vue_shared/components/toggle_button.vue b/app/assets/javascripts/vue_shared/components/toggle_button.vue
new file mode 100644
index 00000000000..ddc9ddbc3a3
--- /dev/null
+++ b/app/assets/javascripts/vue_shared/components/toggle_button.vue
@@ -0,0 +1,77 @@
+<script>
+  import loadingIcon from './loading_icon.vue';
+
+  export default {
+    props: {
+      name: {
+        type: String,
+        required: false,
+        default: '',
+      },
+      value: {
+        type: Boolean,
+        required: true,
+      },
+      disabledInput: {
+        type: Boolean,
+        required: false,
+        default: false,
+      },
+      isLoading: {
+        type: Boolean,
+        required: false,
+        default: false,
+      },
+      enabledText: {
+        type: String,
+        required: false,
+        default: 'Enabled',
+      },
+      disabledText: {
+        type: String,
+        required: false,
+        default: 'Disabled',
+      },
+    },
+
+    components: {
+      loadingIcon,
+    },
+
+    model: {
+      prop: 'value',
+      event: 'change',
+    },
+
+    methods: {
+      toggleFeature() {
+        if (!this.disabledInput) this.$emit('change', !this.value);
+      },
+    },
+  };
+</script>
+
+<template>
+  <label class="toggle-wrapper">
+    <input
+      type="hidden"
+      :name="name"
+      :value="value"
+    />
+    <button
+      type="button"
+      aria-label="Toggle"
+      class="project-feature-toggle"
+      :data-enabled-text="enabledText"
+      :data-disabled-text="disabledText"
+      :class="{
+        'is-checked': value,
+        'is-disabled': disabledInput,
+        'is-loading': isLoading
+      }"
+      @click="toggleFeature"
+    >
+      <loadingIcon class="loading-icon" />
+    </button>
+  </label>
+</template>
diff --git a/app/assets/stylesheets/framework/toggle.scss b/app/assets/stylesheets/framework/toggle.scss
index e5ab604dab6..71765da3908 100644
--- a/app/assets/stylesheets/framework/toggle.scss
+++ b/app/assets/stylesheets/framework/toggle.scss
@@ -2,22 +2,22 @@
 * Toggle button
 *
 * @usage
-*  ### Active text
-*  <button type="button" class="project-feature-toggle checked" data-enabled-text="Enabled" data-disabled-text="Disabled">
+*  ### Active and Inactive text should be provided as data attributes:
+*  <button type="button" class="project-feature-toggle" data-enabled-text="Enabled" data-disabled-text="Disabled">
 *  <i class="fa fa-spinner fa-spin loading-icon hidden"></i>
 *  </button>
 
-*  ### Disabled text
-*  <button type="button" class="project-feature-toggle" data-enabled-text="Enabled" data-disabled-text="Disabled">
+*  ### Checked should have `is-checked` class
+*  <button type="button" class="project-feature-toggle is-checked" data-enabled-text="Enabled" data-disabled-text="Disabled">
 *  <i class="fa fa-spinner fa-spin loading-icon hidden"></i>
 *  </button>
 
-*  ### Disabled button
-*  <button type="button" class="project-feature-toggle disabled" data-enabled-text="Enabled" data-disabled-text="Disabled" disabled="true">
+*  ### Disabled should have `is-disabled` class
+*  <button type="button" class="project-feature-toggle is-disabled" data-enabled-text="Enabled" data-disabled-text="Disabled" disabled="true">
 *  <i class="fa fa-spinner fa-spin loading-icon hidden"></i>
 *  </button>
 
-*  ### Loading
+*  ### Loading should have `is-loading` and an icon with `loading-icon` class
 *  <button type="button" class="project-feature-toggle is-loading" data-enabled-text="Enabled" data-disabled-text="Disabled">
 *  <i class="fa fa-spinner fa-spin loading-icon"></i>
 *  </button>
@@ -69,37 +69,36 @@
     transition: all .2s ease;
   }
 
+  .loading-icon {
+    display: none;
+    font-size: 12px;
+    color: $white-light;
+    position: absolute;
+    top: 50%;
+    left: 50%;
+    transform: translate(-50%, -50%);
+
+  }
+
   &.is-loading {
     &::before {
-      left: 38px;
-      right: 5px;
+      display: none;
     }
 
     .loading-icon {
-      position: absolute;
-      left: 28px;
-      font-size: $tooltip-font-size;
-      color: $white-light;
-      top: 6px;
-    }
-  }
-
-  &.checked {
-    background: $feature-toggle-color-enabled;
+      display: block;
 
-    &.is-loading {
       &::before {
-        left: 10px;
-        right: 42px;
-        animation: animate-enabled .2s ease-in;
-        content: attr(data-enabled-text);
-      }
-
-      .loading-icon {
-        left: 60px;
-        top: 6px;
+        position: absolute;
+        top: 50%;
+        left: 50%;
+        transform: translate(-50%, -50%);
       }
     }
+  }
+
+  &.is-checked {
+    background: $feature-toggle-color-enabled;
 
     &::before {
       left: 5px;
@@ -113,7 +112,7 @@
     }
   }
 
-  &.disabled {
+  &.is-disabled {
     opacity: 0.4;
     cursor: not-allowed;
   }
@@ -122,7 +121,7 @@
     width: 50px;
 
     &::before,
-    &.checked::before {
+    &.is-checked::before {
       display: none;
     }
   }
diff --git a/app/assets/stylesheets/pages/clusters.scss b/app/assets/stylesheets/pages/clusters.scss
index 88395325fd2..1d9a5150065 100644
--- a/app/assets/stylesheets/pages/clusters.scss
+++ b/app/assets/stylesheets/pages/clusters.scss
@@ -13,4 +13,8 @@
   .nav-bar-right {
     padding: $gl-padding-top $gl-padding;
   }
+
+  .empty-state .svg-content img {
+    width: 145px;
+  }
 }