Commit e84c943f authored by Filipa Lacerda's avatar Filipa Lacerda

Fix loading icon being visible in the wrong button

parent bd26e012
...@@ -52,6 +52,7 @@ ...@@ -52,6 +52,7 @@
'getNoteableData', 'getNoteableData',
'getNotesData', 'getNotesData',
'openState', 'openState',
'isToggleStateButtonLoading',
]), ]),
noteableDisplayName() { noteableDisplayName() {
return this.noteableType.replace(/_/g, ' '); return this.noteableType.replace(/_/g, ' ');
...@@ -143,6 +144,7 @@ ...@@ -143,6 +144,7 @@
'closeIssue', 'closeIssue',
'reopenIssue', 'reopenIssue',
'toggleIssueLocalState', 'toggleIssueLocalState',
'toggleStateButtonLoading',
]), ]),
setIsSubmitButtonDisabled(note, isSubmitting) { setIsSubmitButtonDisabled(note, isSubmitting) {
if (!_.isEmpty(note) && !isSubmitting) { if (!_.isEmpty(note) && !isSubmitting) {
...@@ -170,13 +172,14 @@ ...@@ -170,13 +172,14 @@
if (this.noteType === constants.DISCUSSION) { if (this.noteType === constants.DISCUSSION) {
noteData.data.note.type = constants.DISCUSSION_NOTE; noteData.data.note.type = constants.DISCUSSION_NOTE;
} }
this.note = ''; // Empty textarea while being requested. Repopulate in catch this.note = ''; // Empty textarea while being requested. Repopulate in catch
this.resizeTextarea(); this.resizeTextarea();
this.stopPolling(); this.stopPolling();
this.saveNote(noteData) this.saveNote(noteData)
.then((res) => { .then((res) => {
this.isSubmitting = false; this.enableButton();
this.restartPolling(); this.restartPolling();
if (res.errors) { if (res.errors) {
...@@ -198,7 +201,7 @@ ...@@ -198,7 +201,7 @@
} }
}) })
.catch(() => { .catch(() => {
this.isSubmitting = false; this.enableButton();
this.discard(false); this.discard(false);
const msg = const msg =
`Your comment could not be submitted! `Your comment could not be submitted!
...@@ -220,6 +223,7 @@ Please check your network connection and try again.`; ...@@ -220,6 +223,7 @@ Please check your network connection and try again.`;
.then(() => this.enableButton()) .then(() => this.enableButton())
.catch(() => { .catch(() => {
this.enableButton(); this.enableButton();
this.toggleStateButtonLoading(false);
Flash( Flash(
sprintf( sprintf(
__('Something went wrong while closing the %{issuable}. Please try again later'), __('Something went wrong while closing the %{issuable}. Please try again later'),
...@@ -232,6 +236,7 @@ Please check your network connection and try again.`; ...@@ -232,6 +236,7 @@ Please check your network connection and try again.`;
.then(() => this.enableButton()) .then(() => this.enableButton())
.catch(() => { .catch(() => {
this.enableButton(); this.enableButton();
this.toggleStateButtonLoading(false);
Flash( Flash(
sprintf( sprintf(
__('Something went wrong while reopening the %{issuable}. Please try again later'), __('Something went wrong while reopening the %{issuable}. Please try again later'),
...@@ -419,13 +424,13 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" ...@@ -419,13 +424,13 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown"
<loading-button <loading-button
v-if="canUpdateIssue" v-if="canUpdateIssue"
:loading="isSubmitting" :loading="isToggleStateButtonLoading"
@click="handleSave(true)" @click="handleSave(true)"
:container-class="[ :container-class="[
actionButtonClassNames, actionButtonClassNames,
'btn btn-comment btn-comment-and-close js-action-button' 'btn btn-comment btn-comment-and-close js-action-button'
]" ]"
:disabled="isSubmitting" :disabled="isToggleStateButtonLoading || isSubmitting"
:label="issueActionButtonTitle" :label="issueActionButtonTitle"
/> />
......
...@@ -71,21 +71,32 @@ export const toggleResolveNote = ({ commit }, { endpoint, isResolved, discussion ...@@ -71,21 +71,32 @@ export const toggleResolveNote = ({ commit }, { endpoint, isResolved, discussion
commit(mutationType, res); commit(mutationType, res);
}); });
export const closeIssue = ({ commit, dispatch, state }) => service export const closeIssue = ({ commit, dispatch, state }) => {
dispatch('toggleStateButtonLoading', true);
return service
.toggleIssueState(state.notesData.closePath) .toggleIssueState(state.notesData.closePath)
.then(res => res.json()) .then(res => res.json())
.then((data) => { .then((data) => {
commit(types.CLOSE_ISSUE); commit(types.CLOSE_ISSUE);
dispatch('emitStateChangedEvent', data); dispatch('emitStateChangedEvent', data);
dispatch('toggleStateButtonLoading', false);
}); });
};
export const reopenIssue = ({ commit, dispatch, state }) => service export const reopenIssue = ({ commit, dispatch, state }) => {
dispatch('toggleStateButtonLoading', true);
return service
.toggleIssueState(state.notesData.reopenPath) .toggleIssueState(state.notesData.reopenPath)
.then(res => res.json()) .then(res => res.json())
.then((data) => { .then((data) => {
commit(types.REOPEN_ISSUE); commit(types.REOPEN_ISSUE);
dispatch('emitStateChangedEvent', data); dispatch('emitStateChangedEvent', data);
dispatch('toggleStateButtonLoading', false);
}); });
};
export const toggleStateButtonLoading = ({ commit }, value) =>
commit(types.TOGGLE_STATE_BUTTON_LOADING, value);
export const emitStateChangedEvent = ({ commit, getters }, data) => { export const emitStateChangedEvent = ({ commit, getters }, data) => {
const event = new CustomEvent('issuable_vue_app:change', { detail: { const event = new CustomEvent('issuable_vue_app:change', { detail: {
......
...@@ -9,6 +9,7 @@ export const getNotesDataByProp = state => prop => state.notesData[prop]; ...@@ -9,6 +9,7 @@ export const getNotesDataByProp = state => prop => state.notesData[prop];
export const getNoteableData = state => state.noteableData; export const getNoteableData = state => state.noteableData;
export const getNoteableDataByProp = state => prop => state.noteableData[prop]; export const getNoteableDataByProp = state => prop => state.noteableData[prop];
export const openState = state => state.noteableData.state; export const openState = state => state.noteableData.state;
export const isToggleStateButtonLoading = state => state.isToggleStateButtonLoading;
export const getUserData = state => state.userData || {}; export const getUserData = state => state.userData || {};
export const getUserDataByProp = state => prop => state.userData && state.userData[prop]; export const getUserDataByProp = state => prop => state.userData && state.userData[prop];
......
...@@ -12,6 +12,9 @@ export default new Vuex.Store({ ...@@ -12,6 +12,9 @@ export default new Vuex.Store({
targetNoteHash: null, targetNoteHash: null,
lastFetchedAt: null, lastFetchedAt: null,
// View layer
isToggleStateButtonLoading: false,
// holds endpoints and permissions provided through haml // holds endpoints and permissions provided through haml
notesData: {}, notesData: {},
userData: {}, userData: {},
......
...@@ -17,3 +17,4 @@ export const UPDATE_DISCUSSION = 'UPDATE_DISCUSSION'; ...@@ -17,3 +17,4 @@ export const UPDATE_DISCUSSION = 'UPDATE_DISCUSSION';
// Issue // Issue
export const CLOSE_ISSUE = 'CLOSE_ISSUE'; export const CLOSE_ISSUE = 'CLOSE_ISSUE';
export const REOPEN_ISSUE = 'REOPEN_ISSUE'; export const REOPEN_ISSUE = 'REOPEN_ISSUE';
export const TOGGLE_STATE_BUTTON_LOADING = 'TOGGLE_STATE_BUTTON_LOADING';
...@@ -197,4 +197,8 @@ export default { ...@@ -197,4 +197,8 @@ export default {
[types.REOPEN_ISSUE](state) { [types.REOPEN_ISSUE](state) {
Object.assign(state.noteableData, { state: constants.REOPENED }); Object.assign(state.noteableData, { state: constants.REOPENED });
}, },
[types.TOGGLE_STATE_BUTTON_LOADING](state, value) {
Object.assign(state, { isToggleStateButtonLoading: value });
},
}; };
---
title: Fix broken loading state for close issue button
merge_request:
author:
type: fixed
...@@ -200,6 +200,20 @@ describe('issue_comment_form component', () => { ...@@ -200,6 +200,20 @@ describe('issue_comment_form component', () => {
done(); done();
}); });
}); });
describe('when clicking close/reopen button', () => {
it('should disable button and show a loading spinner', (done) => {
const toggleStateButton = vm.$el.querySelector('.js-action-button');
toggleStateButton.click();
Vue.nextTick(() => {
expect(toggleStateButton.disabled).toEqual(true);
expect(toggleStateButton.querySelector('.js-loading-button-icon')).not.toBeNull();
done();
});
});
});
}); });
describe('issue is confidential', () => { describe('issue is confidential', () => {
......
...@@ -87,6 +87,7 @@ describe('Actions Notes Store', () => { ...@@ -87,6 +87,7 @@ describe('Actions Notes Store', () => {
store.dispatch('closeIssue', { notesData: { closeIssuePath: '' } }) store.dispatch('closeIssue', { notesData: { closeIssuePath: '' } })
.then(() => { .then(() => {
expect(store.state.noteableData.state).toEqual('closed'); expect(store.state.noteableData.state).toEqual('closed');
expect(store.state.isToggleStateButtonLoading).toEqual(false);
done(); done();
}) })
.catch(done.fail); .catch(done.fail);
...@@ -98,6 +99,7 @@ describe('Actions Notes Store', () => { ...@@ -98,6 +99,7 @@ describe('Actions Notes Store', () => {
store.dispatch('reopenIssue', { notesData: { reopenIssuePath: '' } }) store.dispatch('reopenIssue', { notesData: { reopenIssuePath: '' } })
.then(() => { .then(() => {
expect(store.state.noteableData.state).toEqual('reopened'); expect(store.state.noteableData.state).toEqual('reopened');
expect(store.state.isToggleStateButtonLoading).toEqual(false);
done(); done();
}) })
.catch(done.fail); .catch(done.fail);
...@@ -116,6 +118,20 @@ describe('Actions Notes Store', () => { ...@@ -116,6 +118,20 @@ describe('Actions Notes Store', () => {
}); });
}); });
describe('toggleStateButtonLoading', () => {
it('should set loading as true', (done) => {
testAction(actions.toggleStateButtonLoading, true, {}, [
{ type: 'TOGGLE_STATE_BUTTON_LOADING', payload: true },
], done);
});
it('should set loading as false', (done) => {
testAction(actions.toggleStateButtonLoading, false, {}, [
{ type: 'TOGGLE_STATE_BUTTON_LOADING', payload: false },
], done);
});
});
describe('toggleIssueLocalState', () => { describe('toggleIssueLocalState', () => {
it('sets issue state as closed', (done) => { it('sets issue state as closed', (done) => {
testAction(actions.toggleIssueLocalState, 'closed', {}, [ testAction(actions.toggleIssueLocalState, 'closed', {}, [
......
...@@ -217,4 +217,70 @@ describe('Notes Store mutations', () => { ...@@ -217,4 +217,70 @@ describe('Notes Store mutations', () => {
expect(state.notes[0].notes[0].note).toEqual('Foo'); expect(state.notes[0].notes[0].note).toEqual('Foo');
}); });
}); });
describe('CLOSE_ISSUE', () => {
it('should set issue as closed', () => {
const state = {
notes: [],
targetNoteHash: null,
lastFetchedAt: null,
isToggleStateButtonLoading: false,
notesData: {},
userData: {},
noteableData: {},
};
mutations.CLOSE_ISSUE(state);
expect(state.noteableData.state).toEqual('closed');
});
});
describe('REOPEN_ISSUE', () => {
it('should set issue as closed', () => {
const state = {
notes: [],
targetNoteHash: null,
lastFetchedAt: null,
isToggleStateButtonLoading: false,
notesData: {},
userData: {},
noteableData: {},
};
mutations.REOPEN_ISSUE(state);
expect(state.noteableData.state).toEqual('reopened');
});
});
describe('TOGGLE_STATE_BUTTON_LOADING', () => {
it('should set isToggleStateButtonLoading as true', () => {
const state = {
notes: [],
targetNoteHash: null,
lastFetchedAt: null,
isToggleStateButtonLoading: false,
notesData: {},
userData: {},
noteableData: {},
};
mutations.TOGGLE_STATE_BUTTON_LOADING(state, true);
expect(state.isToggleStateButtonLoading).toEqual(true);
});
it('should set isToggleStateButtonLoading as false', () => {
const state = {
notes: [],
targetNoteHash: null,
lastFetchedAt: null,
isToggleStateButtonLoading: true,
notesData: {},
userData: {},
noteableData: {},
};
mutations.TOGGLE_STATE_BUTTON_LOADING(state, false);
expect(state.isToggleStateButtonLoading).toEqual(false);
});
});
}); });
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