Commit 18967d68 authored by Filipa Lacerda's avatar Filipa Lacerda

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
parent 8796e727
...@@ -2,6 +2,29 @@ import Flash from '../flash'; ...@@ -2,6 +2,29 @@ import Flash from '../flash';
import { s__ } from '../locale'; import { s__ } from '../locale';
import ClustersService from './services/clusters_service'; 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. * Handles toggle buttons in the cluster's table.
* *
...@@ -13,56 +36,24 @@ import ClustersService from './services/clusters_service'; ...@@ -13,56 +36,24 @@ import ClustersService from './services/clusters_service';
* 1) Show updated status in case of successfull response * 1) Show updated status in case of successfull response
* 2) Show initial status in case of failed response * 2) Show initial status in case of failed response
*/ */
export default class ClusterTable { export default function setClusterTableToggles() {
constructor() { document.querySelectorAll('.js-toggle-cluster-list')
this.container = '.js-clusters-list'; .forEach(button => button.addEventListener('click', (e) => {
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 toggleButton = e.currentTarget;
const value = toggleButton.classList.contains('checked').toString(); const value = toggleButton.classList.contains('checked').toString();
const endpoint = toggleButton.getAttribute('data-endpoint'); const endpoint = toggleButton.getAttribute('data-endpoint');
ClusterTable.toggleValue(toggleButton); toggleValue(toggleButton);
ClusterTable.toggleLoadingButton(toggleButton); toggleLoadingButton(toggleButton);
ClustersService.updateCluster(endpoint, { cluster: { enabled: value } }) ClustersService.updateCluster(endpoint, { cluster: { enabled: value } })
.then(() => { .then(() => {
ClusterTable.toggleLoadingButton(toggleButton); toggleLoadingButton(toggleButton);
}) })
.catch(() => { .catch(() => {
ClusterTable.toggleLoadingButton(toggleButton); toggleLoadingButton(toggleButton);
ClusterTable.toggleValue(toggleButton); toggleValue(toggleButton);
Flash(s__('ClusterIntegration|Something went wrong on our end.')); Flash(s__('ClusterIntegration|Something went wrong on our end.'));
}); });
} }));
/**
* Toggles loading and disabled classes.
* @param {HTMLElement} button
*/
static toggleLoadingButton(button) {
if (button.getAttribute('disabled')) {
button.removeAttribute('disabled');
} else {
button.setAttribute('disabled', true);
}
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');
}
} }
...@@ -556,7 +556,7 @@ import ProjectVariables from './project_variables'; ...@@ -556,7 +556,7 @@ import ProjectVariables from './project_variables';
break; break;
case 'projects:clusters:index': case 'projects:clusters:index':
import(/* webpackChunkName: "clusters_index" */ './clusters/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) => { .catch((err) => {
Flash(s__('ClusterIntegration|Problem setting up the clusters list')); Flash(s__('ClusterIntegration|Problem setting up the clusters list'));
throw err; throw err;
......
<script> <script>
import projectFeatureToggle from './project_feature_toggle.vue'; import projectFeatureToggle from '../../../vue_shared/components/toggle_button.vue';
export default { export default {
props: { props: {
......
<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>
<script> <script>
import projectFeatureSetting from './project_feature_setting.vue'; 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 projectSettingRow from './project_setting_row.vue';
import { visibilityOptions, visibilityLevelDescriptions } from '../constants'; import { visibilityOptions, visibilityLevelDescriptions } from '../constants';
import { toggleHiddenClassBySelector } from '../external'; import { toggleHiddenClassBySelector } from '../external';
......
<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>
...@@ -2,22 +2,22 @@ ...@@ -2,22 +2,22 @@
* Toggle button * Toggle button
* *
* @usage * @usage
* ### Active text * ### Active and Inactive text should be provided as data attributes:
* <button type="button" class="project-feature-toggle checked" data-enabled-text="Enabled" data-disabled-text="Disabled"> * <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> * <i class="fa fa-spinner fa-spin loading-icon hidden"></i>
* </button> * </button>
* ### Disabled text * ### Checked should have `is-checked` class
* <button type="button" class="project-feature-toggle" data-enabled-text="Enabled" data-disabled-text="Disabled"> * <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> * <i class="fa fa-spinner fa-spin loading-icon hidden"></i>
* </button> * </button>
* ### Disabled button * ### Disabled should have `is-disabled` class
* <button type="button" class="project-feature-toggle disabled" data-enabled-text="Enabled" data-disabled-text="Disabled" disabled="true"> * <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> * <i class="fa fa-spinner fa-spin loading-icon hidden"></i>
* </button> * </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"> * <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> * <i class="fa fa-spinner fa-spin loading-icon"></i>
* </button> * </button>
...@@ -69,37 +69,36 @@ ...@@ -69,37 +69,36 @@
transition: all .2s ease; transition: all .2s ease;
} }
&.is-loading {
&::before {
left: 38px;
right: 5px;
}
.loading-icon { .loading-icon {
position: absolute; display: none;
left: 28px; font-size: 12px;
font-size: $tooltip-font-size;
color: $white-light; color: $white-light;
top: 6px; position: absolute;
} top: 50%;
} left: 50%;
transform: translate(-50%, -50%);
&.checked { }
background: $feature-toggle-color-enabled;
&.is-loading { &.is-loading {
&::before { &::before {
left: 10px; display: none;
right: 42px;
animation: animate-enabled .2s ease-in;
content: attr(data-enabled-text);
} }
.loading-icon { .loading-icon {
left: 60px; display: block;
top: 6px;
&::before {
position: absolute;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
} }
} }
}
&.is-checked {
background: $feature-toggle-color-enabled;
&::before { &::before {
left: 5px; left: 5px;
...@@ -113,7 +112,7 @@ ...@@ -113,7 +112,7 @@
} }
} }
&.disabled { &.is-disabled {
opacity: 0.4; opacity: 0.4;
cursor: not-allowed; cursor: not-allowed;
} }
...@@ -122,7 +121,7 @@ ...@@ -122,7 +121,7 @@
width: 50px; width: 50px;
&::before, &::before,
&.checked::before { &.is-checked::before {
display: none; display: none;
} }
} }
......
...@@ -13,4 +13,8 @@ ...@@ -13,4 +13,8 @@
.nav-bar-right { .nav-bar-right {
padding: $gl-padding-top $gl-padding; padding: $gl-padding-top $gl-padding;
} }
.empty-state .svg-content img {
width: 145px;
}
} }
...@@ -102,7 +102,8 @@ class Projects::ClustersController < Projects::ApplicationController ...@@ -102,7 +102,8 @@ class Projects::ClustersController < Projects::ApplicationController
end end
def clusters def clusters
scope = params[:scope]&.to_sym || :all @scope = params[:scope] || 'all'
scope = @scope&.to_sym
@clusters = ClustersFinder.new(project, current_user, scope).execute @clusters = ClustersFinder.new(project, current_user, scope).execute
end end
......
.gl-responsive-table-row .gl-responsive-table-row
.table-section.section-30 .table-section.section-30
.table-mobile-header{ role: 'rowheader' }= s_('ClusterIntegration|Cluster') .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Cluster")
.table-mobile-content .table-mobile-content
= link_to cluster.name, namespace_project_cluster_path(@project.namespace, @project, cluster) = link_to cluster.name, namespace_project_cluster_path(@project.namespace, @project, cluster)
.table-section.section-30 .table-section.section-30
.table-mobile-header{ role: 'rowheader' }= s_('ClusterIntegration|Environment pattern') .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Environment pattern")
.table-mobile-content= cluster.environment_scope .table-mobile-content= cluster.environment_scope
.table-section.section-30 .table-section.section-30
.table-mobile-header{ role: 'rowheader' }= s_('ClusterIntegration|Project namespace') .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Project namespace")
.table-mobile-content= cluster.platform_kubernetes&.namespace .table-mobile-content= cluster.platform_kubernetes&.namespace
.table-section.section-10 .table-section.section-10
.table-mobile-header{ role: 'rowheader' } .table-mobile-header{ role: "rowheader" }
.table-mobile-content .table-mobile-content
%button{ type: 'button', %button{ type: "button",
class: "js-toggle-cluster-list project-feature-toggle #{'checked' unless !cluster.enabled?} #{'disabled' unless can?(current_user, :update_cluster, cluster)}", class: "js-toggle-cluster-list project-feature-toggle #{'is-checked' unless !cluster.enabled?} #{'is-disabled' unless can?(current_user, :update_cluster, cluster) && cluster.provider_gcp&.created?}",
'aria-label': s_('ClusterIntegration|Toggle Cluster'), "aria-label": s_("ClusterIntegration|Toggle Cluster"),
disabled: !can?(current_user, :update_cluster, cluster), disabled: !can?(current_user, :update_cluster, cluster),
data: { 'enabled-text': 'Enabled', data: { "enabled-text": s_("ClusterIntegration|Active"),
'disabled-text': 'Disabled', "disabled-text": s_("ClusterIntegration|Inactive"),
endpoint: namespace_project_cluster_path(@project.namespace, @project, cluster, format: :json) } } endpoint: namespace_project_cluster_path(@project.namespace, @project, cluster, format: :json) } }
= icon('spinner spin', class: 'hidden loading-icon') = icon("spinner spin", class: "loading-icon")
...@@ -8,5 +8,5 @@ ...@@ -8,5 +8,5 @@
%p= s_('ClusterIntegration|Clusters allow you to use review apps, deploy your applications, run your pipelines, and much more in an easy way. %{link_to_help_page}').html_safe % { link_to_help_page: link_to_help_page} %p= s_('ClusterIntegration|Clusters allow you to use review apps, deploy your applications, run your pipelines, and much more in an easy way. %{link_to_help_page}').html_safe % { link_to_help_page: link_to_help_page}
%p %p
= link_to s_('ClusterIntegration|Add cluster'), new_project_cluster_path(@project), class: 'btn btn-success', title: 'Add cluster' = link_to s_('ClusterIntegration|Add cluster'), new_project_cluster_path(@project), class: 'btn btn-success'
...@@ -2,8 +2,12 @@ ...@@ -2,8 +2,12 @@
- page_title "Clusters" - page_title "Clusters"
.clusters-container .clusters-container
- if @clusters.empty? - if @clusters.empty? && @scope == 'all'
= render "empty_state" = render "empty_state"
- elsif @clusters.empty? && @scope != 'all'
= render "tabs"
.prepend-top-20.text-center
= s_("ClusterIntegration|There are no clusters to show")
- else - else
= render "tabs" = render "tabs"
.ci-table.js-clusters-list .ci-table.js-clusters-list
...@@ -17,4 +21,4 @@ ...@@ -17,4 +21,4 @@
.table-section.section-10{ role: "rowheader" } .table-section.section-10{ role: "rowheader" }
- @clusters.each do |cluster| - @clusters.each do |cluster|
= render "cluster", cluster: cluster = render "cluster", cluster: cluster
= paginate @clusters, theme: 'gitlab' = paginate @clusters, theme: "gitlab"
import MockAdapter from 'axios-mock-adapter'; import MockAdapter from 'axios-mock-adapter';
import axios from '~/lib/utils/axios_utils'; import axios from '~/lib/utils/axios_utils';
import ClusterTable from '~/clusters/clusters_index'; import setClusterTableToggles from '~/clusters/clusters_index';
import { setTimeout } from 'core-js/library/web/timers'; import { setTimeout } from 'core-js/library/web/timers';
describe('Clusters table', () => { describe('Clusters table', () => {
...@@ -10,21 +10,17 @@ describe('Clusters table', () => { ...@@ -10,21 +10,17 @@ describe('Clusters table', () => {
beforeEach(() => { beforeEach(() => {
loadFixtures('clusters/index_cluster.html.raw'); loadFixtures('clusters/index_cluster.html.raw');
mock = new MockAdapter(axios); mock = new MockAdapter(axios);
return new ClusterTable(); setClusterTableToggles();
}); });
describe('update cluster', () => { describe('update cluster', () => {
it('renders a toggle button', () => {
expect(document.querySelector('.js-toggle-cluster-list')).not.toBeNull();
});
it('renders loading state while request is made', () => { it('renders loading state while request is made', () => {
const button = document.querySelector('.js-toggle-cluster-list'); const button = document.querySelector('.js-toggle-cluster-list');
button.click(); button.click();
expect(button.classList).toContain('is-loading'); expect(button.classList).toContain('is-loading');
expect(button.classList).toContain('disabled'); expect(button.classList).toContain('is-disabled');
}); });
afterEach(() => { afterEach(() => {
...@@ -40,7 +36,7 @@ describe('Clusters table', () => { ...@@ -40,7 +36,7 @@ describe('Clusters table', () => {
setTimeout(() => { setTimeout(() => {
expect(button.classList).not.toContain('is-loading'); expect(button.classList).not.toContain('is-loading');
expect(button.classList).not.toContain('checked'); expect(button.classList).not.toContain('is-checked');
done(); done();
}, 0); }, 0);
}); });
...@@ -54,7 +50,7 @@ describe('Clusters table', () => { ...@@ -54,7 +50,7 @@ describe('Clusters table', () => {
setTimeout(() => { setTimeout(() => {
expect(button.classList).not.toContain('is-loading'); expect(button.classList).not.toContain('is-loading');
expect(button.classList).toContain('checked'); expect(button.classList).toContain('is-checked');
done(); done();
}, 0); }, 0);
}); });
......
import Vue from 'vue';
import toggleButton from '~/vue_shared/components/toggle_button.vue';
import mountComponent from '../../helpers/vue_mount_component_helper';
describe('Toggle Button', () => {
let vm;
let Component;
beforeEach(() => {
Component = Vue.extend(toggleButton);
});
afterEach(() => {
vm.$destroy();
});
describe('render output', () => {
beforeEach(() => {
vm = mountComponent(Component, {
value: true,
name: 'foo',
});
});
it('renders input with provided name', () => {
expect(vm.$el.querySelector('input').getAttribute('name')).toEqual('foo');
});
it('renders input with provided value', () => {
expect(vm.$el.querySelector('input').getAttribute('value')).toEqual('true');
});
it('renders Enabled and Disabled text data attributes', () => {
expect(vm.$el.querySelector('button').getAttribute('data-enabled-text')).toEqual('Enabled');
expect(vm.$el.querySelector('button').getAttribute('data-disabled-text')).toEqual('Disabled');
});
});
describe('is-checked', () => {
beforeEach(() => {
vm = mountComponent(Component, {
value: true,
});
spyOn(vm, '$emit');
});
it('renders is checked class', () => {
expect(vm.$el.querySelector('button').classList.contains('is-checked')).toEqual(true);
});
it('emits change event when clicked', () => {
vm.$el.querySelector('button').click();
expect(vm.$emit).toHaveBeenCalledWith('change', false);
});
});
describe('is-disabled', () => {
beforeEach(() => {
vm = mountComponent(Component, {
value: true,
disabledInput: true,
});
spyOn(vm, '$emit');
});
it('renders disabled button', () => {
expect(vm.$el.querySelector('button').classList.contains('is-disabled')).toEqual(true);
});
it('does not emit change event when clicked', () => {
vm.$el.querySelector('button').click();
expect(vm.$emit).not.toHaveBeenCalled();
});
});
describe('is-loading', () => {
beforeEach(() => {
vm = mountComponent(Component, {
value: true,
isLoading: true,
});
});
it('renders loading class', () => {
expect(vm.$el.querySelector('button').classList.contains('is-loading')).toEqual(true);
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment