Commit 11070779 authored by Nicolò Maria Mezzopera's avatar Nicolò Maria Mezzopera

Merge branch 'ntepluhina-fix-checkbox-regression-2' into 'master'

Fix design comment checkbox regression

See merge request gitlab-org/gitlab!33844
parents bfc4c29b 34c3d203
...@@ -53,6 +53,10 @@ export default { ...@@ -53,6 +53,10 @@ export default {
type: Boolean, type: Boolean,
required: true, required: true,
}, },
discussionWithOpenForm: {
type: String,
required: true,
},
}, },
apollo: { apollo: {
activeDiscussion: { activeDiscussion: {
...@@ -127,6 +131,9 @@ export default { ...@@ -127,6 +131,9 @@ export default {
isReplyPlaceholderVisible() { isReplyPlaceholderVisible() {
return this.areRepliesShown || !this.discussionReplies.length; return this.areRepliesShown || !this.discussionReplies.length;
}, },
isFormVisible() {
return this.isFormRendered && this.discussionWithOpenForm === this.discussion.id;
},
}, },
methods: { methods: {
addDiscussionComment( addDiscussionComment(
...@@ -158,14 +165,9 @@ export default { ...@@ -158,14 +165,9 @@ export default {
this.discussionComment = ''; this.discussionComment = '';
}, },
showForm() { showForm() {
this.$emit('openForm', this.discussion.id);
this.isFormRendered = true; this.isFormRendered = true;
}, },
handleReplyFormBlur() {
if (this.discussionComment) {
return;
}
this.isFormRendered = false;
},
toggleResolvedStatus() { toggleResolvedStatus() {
this.isResolving = true; this.isResolving = true;
this.$apollo this.$apollo
...@@ -256,10 +258,10 @@ export default { ...@@ -256,10 +258,10 @@ export default {
/> />
<li v-show="isReplyPlaceholderVisible" class="reply-wrapper"> <li v-show="isReplyPlaceholderVisible" class="reply-wrapper">
<reply-placeholder <reply-placeholder
v-if="!isFormRendered" v-if="!isFormVisible"
class="qa-discussion-reply" class="qa-discussion-reply"
:button-text="__('Reply...')" :button-text="__('Reply...')"
@onMouseDown="showForm" @onClick="showForm"
/> />
<apollo-mutation <apollo-mutation
v-else v-else
...@@ -278,7 +280,6 @@ export default { ...@@ -278,7 +280,6 @@ export default {
:markdown-preview-path="markdownPreviewPath" :markdown-preview-path="markdownPreviewPath"
@submitForm="mutate" @submitForm="mutate"
@cancelForm="hideForm" @cancelForm="hideForm"
@onBlur="handleReplyFormBlur"
> >
<template v-if="discussion.resolvable" #resolveCheckbox> <template v-if="discussion.resolvable" #resolveCheckbox>
<label data-testid="resolve-checkbox"> <label data-testid="resolve-checkbox">
......
...@@ -103,7 +103,6 @@ export default { ...@@ -103,7 +103,6 @@ export default {
@keydown.meta.enter="submitForm" @keydown.meta.enter="submitForm"
@keydown.ctrl.enter="submitForm" @keydown.ctrl.enter="submitForm"
@keyup.esc.stop="cancelComment" @keyup.esc.stop="cancelComment"
@blur="$emit('onBlur')"
> >
</textarea> </textarea>
</template> </template>
......
...@@ -34,6 +34,7 @@ export default { ...@@ -34,6 +34,7 @@ export default {
data() { data() {
return { return {
isResolvedCommentsPopoverHidden: parseBoolean(Cookies.get(this.$options.cookieKey)), isResolvedCommentsPopoverHidden: parseBoolean(Cookies.get(this.$options.cookieKey)),
discussionWithOpenForm: '',
}; };
}, },
computed: { computed: {
...@@ -78,6 +79,9 @@ export default { ...@@ -78,6 +79,9 @@ export default {
this.comment = ''; this.comment = '';
this.$emit('closeCommentForm'); this.$emit('closeCommentForm');
}, },
updateDiscussionWithOpenForm(id) {
this.discussionWithOpenForm = id;
},
}, },
resolveCommentsToggleText: s__('DesignManagement|Resolved Comments'), resolveCommentsToggleText: s__('DesignManagement|Resolved Comments'),
cookieKey: 'hide_design_resolved_comments_popover', cookieKey: 'hide_design_resolved_comments_popover',
...@@ -114,11 +118,13 @@ export default { ...@@ -114,11 +118,13 @@ export default {
:noteable-id="design.id" :noteable-id="design.id"
:markdown-preview-path="markdownPreviewPath" :markdown-preview-path="markdownPreviewPath"
:resolved-discussions-expanded="resolvedDiscussionsExpanded" :resolved-discussions-expanded="resolvedDiscussionsExpanded"
:discussion-with-open-form="discussionWithOpenForm"
data-testid="unresolved-discussion" data-testid="unresolved-discussion"
@createNoteError="$emit('onDesignDiscussionError', $event)" @createNoteError="$emit('onDesignDiscussionError', $event)"
@updateNoteError="$emit('updateNoteError', $event)" @updateNoteError="$emit('updateNoteError', $event)"
@resolveDiscussionError="$emit('resolveDiscussionError', $event)" @resolveDiscussionError="$emit('resolveDiscussionError', $event)"
@click.native.stop="updateActiveDiscussion(discussion.notes[0].id)" @click.native.stop="updateActiveDiscussion(discussion.notes[0].id)"
@openForm="updateDiscussionWithOpenForm"
/> />
<template v-if="resolvedDiscussions.length > 0"> <template v-if="resolvedDiscussions.length > 0">
<gl-button <gl-button
...@@ -158,9 +164,11 @@ export default { ...@@ -158,9 +164,11 @@ export default {
:noteable-id="design.id" :noteable-id="design.id"
:markdown-preview-path="markdownPreviewPath" :markdown-preview-path="markdownPreviewPath"
:resolved-discussions-expanded="resolvedDiscussionsExpanded" :resolved-discussions-expanded="resolvedDiscussionsExpanded"
:discussion-with-open-form="discussionWithOpenForm"
data-testid="resolved-discussion" data-testid="resolved-discussion"
@error="$emit('onDesignDiscussionError', $event)" @error="$emit('onDesignDiscussionError', $event)"
@updateNoteError="$emit('updateNoteError', $event)" @updateNoteError="$emit('updateNoteError', $event)"
@openForm="updateDiscussionWithOpenForm"
@click.native.stop="updateActiveDiscussion(discussion.notes[0].id)" @click.native.stop="updateActiveDiscussion(discussion.notes[0].id)"
/> />
</gl-collapse> </gl-collapse>
......
...@@ -17,7 +17,6 @@ export default { ...@@ -17,7 +17,6 @@ export default {
class="js-vue-discussion-reply btn btn-text-field" class="js-vue-discussion-reply btn btn-text-field"
:title="s__('MergeRequests|Add a reply')" :title="s__('MergeRequests|Add a reply')"
@click="$emit('onClick')" @click="$emit('onClick')"
@mousedown.prevent="$emit('onMouseDown')"
> >
{{ buttonText }} {{ buttonText }}
</button> </button>
......
...@@ -53,6 +53,7 @@ describe('Design discussions component', () => { ...@@ -53,6 +53,7 @@ describe('Design discussions component', () => {
noteableId: 'noteable-id', noteableId: 'noteable-id',
designId: 'design-id', designId: 'design-id',
discussionIndex: 1, discussionIndex: 1,
discussionWithOpenForm: '',
...props, ...props,
}, },
data() { data() {
...@@ -119,7 +120,8 @@ describe('Design discussions component', () => { ...@@ -119,7 +120,8 @@ describe('Design discussions component', () => {
}); });
it('renders a checkbox with Resolve thread text in reply form', () => { it('renders a checkbox with Resolve thread text in reply form', () => {
findReplyPlaceholder().vm.$emit('onMouseDown'); findReplyPlaceholder().vm.$emit('onClick');
wrapper.setProps({ discussionWithOpenForm: discussion.id });
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(findResolveCheckbox().text()).toBe('Resolve thread'); expect(findResolveCheckbox().text()).toBe('Resolve thread');
...@@ -199,7 +201,8 @@ describe('Design discussions component', () => { ...@@ -199,7 +201,8 @@ describe('Design discussions component', () => {
}); });
it('renders a checkbox with Unresolve thread text in reply form', () => { it('renders a checkbox with Unresolve thread text in reply form', () => {
findReplyPlaceholder().vm.$emit('onMouseDown'); findReplyPlaceholder().vm.$emit('onClick');
wrapper.setProps({ discussionWithOpenForm: discussion.id });
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(findResolveCheckbox().text()).toBe('Unresolve thread'); expect(findResolveCheckbox().text()).toBe('Unresolve thread');
...@@ -210,7 +213,8 @@ describe('Design discussions component', () => { ...@@ -210,7 +213,8 @@ describe('Design discussions component', () => {
it('hides reply placeholder and opens form on placeholder click', () => { it('hides reply placeholder and opens form on placeholder click', () => {
createComponent(); createComponent();
findReplyPlaceholder().vm.$emit('onMouseDown'); findReplyPlaceholder().vm.$emit('onClick');
wrapper.setProps({ discussionWithOpenForm: discussion.id });
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(findReplyPlaceholder().exists()).toBe(false); expect(findReplyPlaceholder().exists()).toBe(false);
...@@ -219,7 +223,10 @@ describe('Design discussions component', () => { ...@@ -219,7 +223,10 @@ describe('Design discussions component', () => {
}); });
it('calls mutation on submitting form and closes the form', () => { it('calls mutation on submitting form and closes the form', () => {
createComponent({}, { discussionComment: 'test', isFormRendered: true }); createComponent(
{ discussionWithOpenForm: discussion.id },
{ discussionComment: 'test', isFormRendered: true },
);
findReplyForm().vm.$emit('submitForm'); findReplyForm().vm.$emit('submitForm');
expect(mutate).toHaveBeenCalledWith(mutationVariables); expect(mutate).toHaveBeenCalledWith(mutationVariables);
...@@ -234,7 +241,10 @@ describe('Design discussions component', () => { ...@@ -234,7 +241,10 @@ describe('Design discussions component', () => {
}); });
it('clears the discussion comment on closing comment form', () => { it('clears the discussion comment on closing comment form', () => {
createComponent({}, { discussionComment: 'test', isFormRendered: true }); createComponent(
{ discussionWithOpenForm: discussion.id },
{ discussionComment: 'test', isFormRendered: true },
);
return wrapper.vm return wrapper.vm
.$nextTick() .$nextTick()
...@@ -265,24 +275,6 @@ describe('Design discussions component', () => { ...@@ -265,24 +275,6 @@ describe('Design discussions component', () => {
); );
}); });
it('closes the form on blur if the form was empty', () => {
createComponent({}, { discussionComment: '', isFormRendered: true });
findReplyForm().vm.$emit('onBlur');
return wrapper.vm.$nextTick().then(() => {
expect(findReplyForm().exists()).toBe(false);
});
});
it('keeps the form open on blur if the form had text', () => {
createComponent({}, { discussionComment: 'test', isFormRendered: true });
findReplyForm().vm.$emit('onBlur');
return wrapper.vm.$nextTick().then(() => {
expect(findReplyForm().exists()).toBe(true);
});
});
it('calls toggleResolveDiscussion mutation on resolve thread button click', () => { it('calls toggleResolveDiscussion mutation on resolve thread button click', () => {
createComponent(); createComponent();
findResolveButton().trigger('click'); findResolveButton().trigger('click');
...@@ -299,7 +291,10 @@ describe('Design discussions component', () => { ...@@ -299,7 +291,10 @@ describe('Design discussions component', () => {
}); });
it('calls toggleResolveDiscussion mutation after adding a note if checkbox was checked', () => { it('calls toggleResolveDiscussion mutation after adding a note if checkbox was checked', () => {
createComponent({}, { discussionComment: 'test', isFormRendered: true }); createComponent(
{ discussionWithOpenForm: discussion.id },
{ discussionComment: 'test', isFormRendered: true },
);
findResolveButton().trigger('click'); findResolveButton().trigger('click');
findReplyForm().vm.$emit('submitForm'); findReplyForm().vm.$emit('submitForm');
...@@ -313,4 +308,11 @@ describe('Design discussions component', () => { ...@@ -313,4 +308,11 @@ describe('Design discussions component', () => {
}); });
}); });
}); });
it('emits openForm event on opening the form', () => {
createComponent();
findReplyPlaceholder().vm.$emit('onClick');
expect(wrapper.emitted('openForm')).toBeTruthy();
});
}); });
...@@ -39,13 +39,6 @@ describe('Design reply form component', () => { ...@@ -39,13 +39,6 @@ describe('Design reply form component', () => {
expect(findTextarea().element).toEqual(document.activeElement); expect(findTextarea().element).toEqual(document.activeElement);
}); });
it('textarea emits onBlur event on blur', () => {
createComponent();
findTextarea().trigger('blur');
expect(wrapper.emitted('onBlur')).toBeTruthy();
});
it('renders button text as "Comment" when creating a comment', () => { it('renders button text as "Comment" when creating a comment', () => {
createComponent(); createComponent();
......
...@@ -159,6 +159,14 @@ describe('Design management design sidebar component', () => { ...@@ -159,6 +159,14 @@ describe('Design management design sidebar component', () => {
findFirstDiscussion().vm.$emit('resolveDiscussionError', 'payload'); findFirstDiscussion().vm.$emit('resolveDiscussionError', 'payload');
expect(wrapper.emitted('resolveDiscussionError')).toEqual([['payload']]); expect(wrapper.emitted('resolveDiscussionError')).toEqual([['payload']]);
}); });
it('changes prop correctly on opening discussion form', () => {
findFirstDiscussion().vm.$emit('openForm', 'some-id');
return wrapper.vm.$nextTick().then(() => {
expect(findFirstDiscussion().props('discussionWithOpenForm')).toBe('some-id');
});
});
}); });
describe('when all discussions are resolved', () => { describe('when all discussions are resolved', () => {
......
...@@ -59,6 +59,7 @@ exports[`Design management design index page renders design index 1`] = ` ...@@ -59,6 +59,7 @@ exports[`Design management design index page renders design index 1`] = `
data-testid="unresolved-discussion" data-testid="unresolved-discussion"
designid="test" designid="test"
discussion="[object Object]" discussion="[object Object]"
discussionwithopenform=""
markdownpreviewpath="//preview_markdown?target_type=Issue" markdownpreviewpath="//preview_markdown?target_type=Issue"
noteableid="design-id" noteableid="design-id"
/> />
...@@ -106,6 +107,7 @@ exports[`Design management design index page renders design index 1`] = ` ...@@ -106,6 +107,7 @@ exports[`Design management design index page renders design index 1`] = `
data-testid="resolved-discussion" data-testid="resolved-discussion"
designid="test" designid="test"
discussion="[object Object]" discussion="[object Object]"
discussionwithopenform=""
markdownpreviewpath="//preview_markdown?target_type=Issue" markdownpreviewpath="//preview_markdown?target_type=Issue"
noteableid="design-id" noteableid="design-id"
/> />
......
...@@ -30,16 +30,6 @@ describe('ReplyPlaceholder', () => { ...@@ -30,16 +30,6 @@ describe('ReplyPlaceholder', () => {
}); });
}); });
it('emits onMouseDown event on button mousedown', () => {
findButton().trigger('mousedown');
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.emitted()).toEqual({
onMouseDown: [[]],
});
});
});
it('should render reply button', () => { it('should render reply button', () => {
expect(findButton().text()).toEqual(buttonText); expect(findButton().text()).toEqual(buttonText);
}); });
......
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