Commit 89f0db81 authored by Phil Hughes's avatar Phil Hughes

Improve draft note header design

Moves the pending badge into the header.
Enables the toggle resolve button to toggle the resolve status
of a draft note.

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/7905
parent 1f5d50b1
......@@ -2,6 +2,7 @@
import { mapGetters } from 'vuex';
import Icon from '~/vue_shared/components/icon.vue';
import { GlLoadingIcon, GlTooltipDirective } from '@gitlab/ui';
import resolvedStatusMixin from 'ee/batch_comments/mixins/resolved_status';
export default {
name: 'NoteActions',
......@@ -12,6 +13,7 @@ export default {
directives: {
GlTooltip: GlTooltipDirective,
},
mixins: [resolvedStatusMixin],
props: {
authorId: {
type: Number,
......@@ -93,6 +95,8 @@ export default {
return this.getUserDataByProp('id');
},
resolveButtonTitle() {
if (this.discussionId) return this.resolvedStatusMessage;
let title = 'Mark as resolved';
if (this.resolvedBy) {
......@@ -113,6 +117,7 @@ export default {
this.$emit('handleResolve');
},
},
showStaysResolved: true,
};
</script>
......
<script>
import { mapGetters } from 'vuex';
import $ from 'jquery';
import noteEditedText from './note_edited_text.vue';
import noteAwardsList from './note_awards_list.vue';
......@@ -30,9 +31,15 @@ export default {
},
},
computed: {
...mapGetters(['getDiscussion']),
noteBody() {
return this.note.note;
},
discussion() {
if (!this.note.isDraft) return {};
return this.getDiscussion(this.note.discussion_id);
},
},
mounted() {
this.renderGFM();
......@@ -56,8 +63,8 @@ export default {
renderGFM() {
$(this.$refs['note-body']).renderGFM();
},
handleFormUpdate(note, parentElement, callback) {
this.$emit('handleFormUpdate', note, parentElement, callback);
handleFormUpdate(note, parentElement, callback, resolveDiscussion) {
this.$emit('handleFormUpdate', note, parentElement, callback, resolveDiscussion);
},
formCancelHandler(shouldConfirm, isDirty) {
this.$emit('cancelForm', shouldConfirm, isDirty);
......@@ -76,6 +83,8 @@ export default {
:note-body="noteBody"
:note-id="note.id"
:markdown-version="note.cached_markdown_version"
:discussion="discussion"
:resolve-discussion="note.resolve_discussion"
@handleFormUpdate="handleFormUpdate"
@cancelForm="formCancelHandler"
/>
......
......@@ -51,14 +51,19 @@ export default {
required: false,
default: '',
},
resolveDiscussion: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
updatedNoteBody: this.noteBody,
conflictWhileEditing: false,
isSubmitting: false,
isResolving: false,
isUnresolving: false,
isResolving: this.resolveDiscussion,
isUnresolving: !this.resolveDiscussion,
resolveAsThread: true,
};
},
......@@ -122,13 +127,19 @@ export default {
const beforeSubmitDiscussionState = this.discussionResolved;
this.isSubmitting = true;
this.$emit('handleFormUpdate', this.updatedNoteBody, this.$refs.editNoteForm, () => {
this.isSubmitting = false;
this.$emit(
'handleFormUpdate',
this.updatedNoteBody,
this.$refs.editNoteForm,
() => {
this.isSubmitting = false;
if (this.shouldToggleResolved(shouldResolve, beforeSubmitDiscussionState)) {
this.resolveHandler(beforeSubmitDiscussionState);
}
});
if (this.shouldToggleResolved(shouldResolve, beforeSubmitDiscussionState)) {
this.resolveHandler(beforeSubmitDiscussionState);
}
},
this.discussionResolved ? !this.isUnresolving : this.isResolving,
);
},
editMyLastNote() {
if (this.updatedNoteBody === '') {
......@@ -153,7 +164,7 @@ export default {
<div ref="editNoteForm" class="note-edit-form current-note-edit-form js-discussion-note-form">
<div v-if="conflictWhileEditing" class="js-conflict-edit-warning alert alert-danger">
This comment has changed since you started editing, please review the
<a :href="noteHash" target="_blank" rel="noopener noreferrer"> updated comment </a> to ensure
<a :href="noteHash" target="_blank" rel="noopener noreferrer">updated comment</a> to ensure
information is not lost.
</div>
<div class="flash-container timeline-content"></div>
......@@ -178,71 +189,63 @@ export default {
v-model="updatedNoteBody"
:data-supports-quick-actions="!isEditing"
name="note[note]"
class="note-textarea js-gfm-input js-note-text
js-autosize markdown-area js-vue-issue-note-form js-vue-textarea qa-reply-input"
class="note-textarea js-gfm-input js-note-text js-autosize markdown-area js-vue-issue-note-form js-vue-textarea qa-reply-input"
aria-label="Description"
placeholder="Write a comment or drag your files here…"
@keydown.meta.enter="handleUpdate();"
@keydown.ctrl.enter="handleUpdate();"
@keydown.up="editMyLastNote();"
@keydown.esc="cancelHandler(true);"
>
</textarea>
></textarea>
</markdown-field>
<div class="note-form-actions clearfix">
<template v-if="showBatchCommentsActions">
<p v-if="discussion && discussion.id">
<label>
<template v-if="discussionResolved">
<input
v-model="isUnresolving"
type="checkbox"
class="qa-unresolve-review-discussion"
/>
{{ __('Unresolve discussion') }}
</template>
<template v-else>
<input v-model="isResolving" type="checkbox" class="qa-resolve-review-discussion" />
{{ __('Resolve discussion') }}
</template>
</label>
</p>
<div>
<button
:disabled="isDisabled"
type="button"
class="btn btn-success qa-start-review"
@click="handleAddToReview();"
>
<template v-if="hasDrafts">
{{ __('Add to review') }}
</template>
<template v-else>
{{ __('Start a review') }}
</template>
</button>
<button
:disabled="isDisabled"
type="button"
class="btn qa-comment-now"
@click="handleUpdate();"
>
{{ __('Add comment now') }}
</button>
<button
class="btn btn-cancel note-edit-cancel js-close-discussion-note-form"
type="button"
@click="cancelHandler();"
>
{{ __('Cancel') }}
</button>
</div>
</template>
<p v-if="(discussion && discussion.id) || isDraft">
<label>
<template v-if="discussionResolved">
<input
v-model="isUnresolving"
type="checkbox"
class="qa-unresolve-review-discussion"
/>
{{ __('Unresolve discussion') }}
</template>
<template v-else>
<input v-model="isResolving" type="checkbox" class="qa-resolve-review-discussion" />
{{ __('Resolve discussion') }}
</template>
</label>
</p>
<div v-if="showBatchCommentsActions">
<button
:disabled="isDisabled"
type="button"
class="btn btn-success qa-start-review"
@click="handleAddToReview"
>
<template v-if="hasDrafts">{{ __('Add to review') }}</template>
<template v-else>{{ __('Start a review') }}</template>
</button>
<button
:disabled="isDisabled"
type="button"
class="btn qa-comment-now"
@click="handleUpdate();"
>
{{ __('Add comment now') }}
</button>
<button
class="btn btn-cancel note-edit-cancel js-close-discussion-note-form"
type="button"
@click="cancelHandler();"
>
{{ __('Cancel') }}
</button>
</div>
<template v-else>
<button
:disabled="isDisabled"
type="button"
class="js-vue-issue-save btn btn-success js-comment-button "
class="js-vue-issue-save btn btn-success js-comment-button"
@click="handleUpdate();"
>
{{ saveButtonTitle }}
......
......@@ -69,24 +69,23 @@ export default {
type="button"
@click="handleToggle"
>
<i :class="toggleChevronClass" class="fa" aria-hidden="true"> </i>
<i :class="toggleChevronClass" class="fa" aria-hidden="true"></i>
{{ __('Toggle discussion') }}
</button>
</div>
<a v-if="hasAuthor" v-once :href="author.path">
<slot name="note-header-info"></slot>
<span class="note-header-author-name">{{ author.name }}</span>
<span v-if="author.status_tooltip_html" v-html="author.status_tooltip_html"></span>
<span class="note-headline-light"> @{{ author.username }} </span>
<span class="note-headline-light">@{{ author.username }}</span>
</a>
<span v-else> {{ __('A deleted user') }} </span>
<span v-else>{{ __('A deleted user') }}</span>
<span class="note-headline-light">
<span class="note-headline-meta">
<span class="system-note-message"> <slot></slot> </span>
<template v-if="createdAt">
<span class="system-note-separator">
<template v-if="actionText">
{{ actionText }}
</template>
<template v-if="actionText">{{ actionText }}</template>
</span>
<a
:href="noteTimestampLink"
......@@ -100,8 +99,7 @@ export default {
class="fa fa-spinner fa-spin editing-spinner"
aria-label="Comment is being updated"
aria-hidden="true"
>
</i>
></i>
</span>
</span>
</div>
......
......@@ -111,10 +111,11 @@ export default {
this.$refs.noteBody.resetAutoSave();
this.$emit('updateSuccess');
},
formUpdateHandler(noteText, parentElement, callback) {
formUpdateHandler(noteText, parentElement, callback, resolveDiscussion) {
this.$emit('handleUpdateNote', {
note: this.note,
noteText,
resolveDiscussion,
callback: () => this.updateSuccess(),
});
......@@ -198,7 +199,9 @@ export default {
:created-at="note.created_at"
:note-id="note.id"
action-text="commented"
/>
>
<slot slot="note-header-info" name="note-header-info"> </slot>
</note-header>
<note-actions
:author-id="author.id"
:note-id="note.id"
......@@ -208,12 +211,16 @@ export default {
:can-award-emoji="note.current_user.can_award_emoji"
:can-delete="note.current_user.can_edit"
:can-report-as-abuse="canReportAsAbuse"
:can-resolve="note.current_user.can_resolve"
:can-resolve="
note.current_user.can_resolve || (note.isDraft && note.discussion_id !== null)
"
:report-abuse-path="note.report_abuse_path"
:resolvable="note.resolvable"
:is-resolved="note.resolved"
:resolvable="note.resolvable || note.isDraft"
:is-resolved="note.resolved || note.resolve_discussion"
:is-resolving="isResolving"
:resolved-by="note.resolved_by"
:discussion-id="note.isDraft && note.discussion_id"
:resolve-discussion="note.isDraft && note.resolve_discussion"
@handleEdit="editHandler"
@handleDelete="deleteHandler"
@handleResolve="resolveHandler"
......
......@@ -31,12 +31,16 @@ export default {
},
methods: {
resolveHandler(resolvedState = false) {
if (this.note && this.note.isDraft) {
return this.$emit('toggleResolveStatus');
}
this.isResolving = true;
const isResolved = this.discussionResolved || resolvedState;
const discussion = this.resolveAsThread;
const endpoint = discussion ? this.discussion.resolve_path : `${this.note.path}/resolve`;
this.toggleResolveNote({ endpoint, isResolved, discussion })
return this.toggleResolveNote({ endpoint, isResolved, discussion })
.then(() => {
this.isResolving = false;
})
......
......@@ -3,7 +3,6 @@ import { mapActions, mapGetters, mapState } from 'vuex';
import Icon from '~/vue_shared/components/icon.vue';
import NoteableNote from '~/notes/components/noteable_note.vue';
import LoadingButton from '~/vue_shared/components/loading_button.vue';
import resolvedStatusMixin from '../mixins/resolved_status';
import PublishButton from './publish_button.vue';
export default {
......@@ -13,7 +12,6 @@ export default {
Icon,
LoadingButton,
},
mixins: [resolvedStatusMixin],
props: {
draft: {
type: Object,
......@@ -43,6 +41,7 @@ export default {
'updateDraft',
'publishSingleDraft',
'scrollToDraft',
'toggleResolveDiscussion',
]),
update(data) {
this.updateDraft(data);
......@@ -57,18 +56,10 @@ export default {
this.isEditingDraft = false;
},
},
showStaysResolved: true,
};
</script>
<template>
<article :class="componentClasses" class="draft-note-component note-wrapper">
<header class="draft-note-header">
<strong class="badge draft-pending-label"> {{ __('Pending') }} </strong>
<p v-if="draft.discussion_id" class="draft-note-resolution">
<Icon :size="16" name="status_success" />
{{ __(resolvedStatusMessage) }}
</p>
</header>
<article class="draft-note-component note-wrapper">
<ul class="notes draft-notes">
<noteable-note
:note="draft"
......@@ -78,7 +69,12 @@ export default {
@updateSuccess="handleNotEditing"
@handleDeleteNote="deleteDraft"
@handleUpdateNote="update"
/>
@toggleResolveStatus="toggleResolveDiscussion(draft.id);"
>
<strong slot="note-header-info" class="badge draft-pending-label append-right-4">
{{ __('Pending') }}
</strong>
</noteable-note>
</ul>
<template v-if="!isEditingDraft">
......@@ -88,11 +84,11 @@ export default {
v-html="draftCommands"
></div>
<p class="draft-note-actions">
<p class="draft-note-actions d-flex">
<publish-button
:show-count="true"
:should-publish="false"
class="btn btn-success btn-inverted"
class="btn btn-success btn-inverted append-right-8"
/>
<loading-button
:loading="isPublishingDraft(draft.id) || isPublishing"
......
import { mapGetters, mapState } from 'vuex';
export default {
props: {
isDraft: {
type: Boolean,
required: false,
default: false,
},
},
computed: {
...mapState({
withBatchComments: state => state.batchComments && state.batchComments.withBatchComments,
......@@ -21,20 +28,6 @@ export default {
return resolveStatus;
},
handleUpdate(resolveStatus) {
const beforeSubmitDiscussionState = this.discussionResolved;
this.isSubmitting = true;
const shouldBeResolved = this.shouldBeResolved(resolveStatus) !== beforeSubmitDiscussionState;
this.$emit('handleFormUpdate', this.updatedNoteBody, this.$refs.editNoteForm, () => {
this.isSubmitting = false;
if (resolveStatus || (shouldBeResolved && this.withBatchComments)) {
this.resolveHandler(beforeSubmitDiscussionState); // this will toggle the state
}
});
},
handleAddToReview() {
// check if draft should resolve discussion
const shouldResolve =
......
......@@ -2,12 +2,28 @@ import { mapGetters } from 'vuex';
import { s__ } from '~/locale';
export default {
props: {
discussionId: {
type: String,
required: false,
default: null,
},
resolveDiscussion: {
type: Boolean,
required: false,
default: false,
},
},
computed: {
...mapGetters(['isDiscussionResolved']),
resolvedStatusMessage() {
let message;
const discussionResolved = this.isDiscussionResolved(this.draft.discussion_id);
const discussionToBeResolved = this.draft.resolve_discussion;
const discussionResolved = this.isDiscussionResolved(
this.draft ? this.draft.discussion_id : this.discussionId,
);
const discussionToBeResolved = this.draft
? this.draft.resolve_discussion
: this.resolveDiscussion;
if (discussionToBeResolved && discussionResolved && !this.$options.showStaysResolved) {
return undefined;
......@@ -15,22 +31,20 @@ export default {
if (discussionToBeResolved) {
if (discussionResolved) {
message = s__('MergeRequests|Discussion stays resolved.');
message = s__('MergeRequests|Discussion stays resolved');
} else {
message = s__('MergeRequests|Discussion will be resolved.');
message = s__('MergeRequests|Discussion will be resolved');
}
} else if (discussionResolved) {
message = s__('MergeRequests|Discussion will be unresolved.');
message = s__('MergeRequests|Discussion will be unresolved');
} else if (this.$options.showStaysResolved) {
message = s__('MergeRequests|Discussion stays unresolved.');
message = s__('MergeRequests|Discussion stays unresolved');
}
return message;
},
componentClasses() {
return this.draft.resolve_discussion
? 'is-resolving-discussion'
: 'is-unresolving-discussion';
return this.resolveDiscussion ? 'is-resolving-discussion' : 'is-unresolving-discussion';
},
},
};
......@@ -28,7 +28,11 @@ export default {
discard(endpoint) {
return Vue.http.delete(endpoint);
},
update(endpoint, { draftId, note }) {
return Vue.http.put(`${endpoint}/${draftId}`, { draft_note: { note } }, { emulateJSON: true });
update(endpoint, { draftId, note, resolveDiscussion }) {
return Vue.http.put(
`${endpoint}/${draftId}`,
{ draft_note: { note, resolve_discussion: resolveDiscussion } },
{ emulateJSON: true },
);
},
};
......@@ -88,9 +88,13 @@ export const discardReview = ({ commit, getters }) => {
.catch(() => commit(types.RECEIVE_DISCARD_REVIEW_ERROR));
};
export const updateDraft = ({ commit, getters }, { note, noteText, callback }) =>
export const updateDraft = ({ commit, getters }, { note, noteText, resolveDiscussion, callback }) =>
service
.update(getters.getNotesData.draftsPath, { draftId: note.id, note: noteText })
.update(getters.getNotesData.draftsPath, {
draftId: note.id,
note: noteText,
resolveDiscussion,
})
.then(res => res.json())
.then(data => commit(types.RECEIVE_DRAFT_UPDATE_SUCCESS, data))
.then(callback)
......@@ -139,5 +143,9 @@ export const expandAllDiscussions = ({ dispatch, state }) =>
dispatch('expandDiscussion', { discussionId: draft.discussion_id }, { root: true });
});
export const toggleResolveDiscussion = ({ commit }, draftId) => {
commit(types.TOGGLE_RESOLVE_DISCUSSION, draftId);
};
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};
......@@ -19,3 +19,5 @@ export const RECEIVE_DRAFT_UPDATE_SUCCESS = 'RECEIVE_DRAFT_UPDATE_SUCCESS';
export const OPEN_REVIEW_DROPDOWN = 'OPEN_REVIEW_DROPDOWN';
export const CLOSE_REVIEW_DROPDOWN = 'CLOSE_REVIEW_DROPDOWN';
export const TOGGLE_RESOLVE_DISCUSSION = 'TOGGLE_RESOLVE_DISCUSSION';
......@@ -70,4 +70,16 @@ export default {
[types.CLOSE_REVIEW_DROPDOWN](state) {
state.showPreviewDropdown = false;
},
[types.TOGGLE_RESOLVE_DISCUSSION](state, draftId) {
state.drafts = state.drafts.map(draft => {
if (draft.id === draftId) {
return {
...draft,
resolve_discussion: !draft.resolve_discussion,
};
}
return draft;
});
},
};
......@@ -47,37 +47,6 @@ button[disabled] {
}
}
.draft-note-header {
display: flex;
justify-content: space-between;
align-items: center;
}
.draft-note-resolution {
padding: 0 $gl-padding;
line-height: 1;
font-size: $label-font-size;
color: $theme-gray-700;
flex-grow: 1;
svg {
vertical-align: text-bottom;
display: inline-block;
}
.is-resolving-discussion & {
svg {
color: $green-600;
}
}
.is-unresolving-discussion & {
svg {
color: $gray-darkest;
}
}
}
.draft-pending-label {
background: $orange-600;
color: $white-light;
......@@ -88,14 +57,10 @@ button[disabled] {
.diff-file {
.notes .note {
&.draft-note {
margin: $gl-padding-8 0 $gl-padding;
margin: 0 0 $gl-padding;
padding: 0;
border: 0;
.note-actions {
margin-top: -26px;
}
&.is-editing {
margin-bottom: 0;
}
......
......@@ -35,58 +35,13 @@ describe('Batch comments draft note component', () => {
describe('in discussion', () => {
beforeEach(done => {
vm.draft.discussion_id = 123;
vm.draft.discussion_id = '123';
vm.$nextTick(done);
});
it('renders resolution status', () => {
expect(vm.$el.querySelector('.draft-note-resolution')).not.toBe(null);
});
describe('resolvedStatusMessage', () => {
describe('resolve discussion', () => {
it('return will be resolved text', () => {
vm.draft.resolve_discussion = true;
expect(vm.resolvedStatusMessage).toBe('Discussion will be resolved.');
});
it('return will be stays resolved text', () => {
spyOnProperty(vm, 'isDiscussionResolved').and.returnValue(() => true);
vm.draft.resolve_discussion = true;
expect(vm.resolvedStatusMessage).toBe('Discussion stays resolved.');
});
});
describe('unresolve discussion', () => {
it('return will be stays unresolved text', () => {
expect(vm.resolvedStatusMessage).toBe('Discussion stays unresolved.');
});
it('return will be unresolved text', () => {
spyOnProperty(vm, 'isDiscussionResolved').and.returnValue(() => true);
vm.$forceUpdate();
expect(vm.resolvedStatusMessage).toBe('Discussion stays unresolved.');
});
});
it('adds resolving class to element', done => {
vm.draft.resolve_discussion = true;
vm.$nextTick(() => {
expect(vm.$el.classList).toContain('is-resolving-discussion');
done();
});
});
it('adds unresolving class to element', () => {
expect(vm.$el.classList).toContain('is-unresolving-discussion');
});
expect(vm.$el.querySelector('.line-resolve-btn')).not.toBe(null);
});
});
......@@ -121,6 +76,7 @@ describe('Batch comments draft note component', () => {
expect(vm.$store.dispatch).toHaveBeenCalledWith('batchComments/updateDraft', {
note: draft,
noteText: 'a',
resolveDiscussion: false,
callback: jasmine.any(Function),
});
})
......
......@@ -5202,16 +5202,16 @@ msgstr ""
msgid "MergeRequests|An error occurred while saving the draft comment."
msgstr ""
msgid "MergeRequests|Discussion stays resolved."
msgid "MergeRequests|Discussion stays resolved"
msgstr ""
msgid "MergeRequests|Discussion stays unresolved."
msgid "MergeRequests|Discussion stays unresolved"
msgstr ""
msgid "MergeRequests|Discussion will be resolved."
msgid "MergeRequests|Discussion will be resolved"
msgstr ""
msgid "MergeRequests|Discussion will be unresolved."
msgid "MergeRequests|Discussion will be unresolved"
msgstr ""
msgid "MergeRequests|Resolve this discussion in a new issue"
......
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