summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKushal Pandya <kushalspandya@gmail.com>2019-07-09 04:22:49 +0000
committerKushal Pandya <kushalspandya@gmail.com>2019-07-09 04:22:49 +0000
commitbf172b115c206cb6ed6c2b271fad600f1548b43c (patch)
treea19d31bb8c5c1266bf0b66e96cc4ee72817bd007
parent56e3fc40b80b8f2231f15d57c73d3f5995f01eca (diff)
parent9613a34a24bda78661ce74348ceb55d1e3b50b53 (diff)
downloadgitlab-ce-bf172b115c206cb6ed6c2b271fad600f1548b43c.tar.gz
Merge branch '64134-edit-comment' into 'master'
Resolve "Clicking edit button in a thread reply doesn’t work" Closes #64134 See merge request gitlab-org/gitlab-ce!30424
-rw-r--r--app/assets/javascripts/notes/components/discussion_notes.vue8
-rw-r--r--app/assets/javascripts/notes/components/discussion_notes_replies_wrapper.vue27
-rw-r--r--app/assets/stylesheets/pages/diff.scss4
-rw-r--r--app/assets/stylesheets/pages/notes.scss1
-rw-r--r--spec/frontend/notes/components/discussion_notes_replies_wrapper_spec.js51
-rw-r--r--spec/javascripts/diffs/components/inline_diff_view_spec.js3
6 files changed, 88 insertions, 6 deletions
diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue
index 2ff0fee62f3..0b136549c14 100644
--- a/app/assets/javascripts/notes/components/discussion_notes.vue
+++ b/app/assets/javascripts/notes/components/discussion_notes.vue
@@ -8,12 +8,14 @@ import SystemNote from '~/vue_shared/components/notes/system_note.vue';
import NoteableNote from './noteable_note.vue';
import ToggleRepliesWidget from './toggle_replies_widget.vue';
import NoteEditedText from './note_edited_text.vue';
+import DiscussionNotesRepliesWrapper from './discussion_notes_replies_wrapper.vue';
export default {
name: 'DiscussionNotes',
components: {
ToggleRepliesWidget,
NoteEditedText,
+ DiscussionNotesRepliesWrapper,
},
props: {
discussion: {
@@ -119,9 +121,7 @@ export default {
/>
<slot slot="avatar-badge" name="avatar-badge"></slot>
</component>
- <div
- :class="discussion.diff_discussion ? 'discussion-collapsible bordered-box clearfix' : ''"
- >
+ <discussion-notes-replies-wrapper :is-diff-discussion="discussion.diff_discussion">
<toggle-replies-widget
v-if="hasReplies"
:collapsed="!isExpanded"
@@ -141,7 +141,7 @@ export default {
/>
</template>
<slot :show-replies="isExpanded || !hasReplies" name="footer"></slot>
- </div>
+ </discussion-notes-replies-wrapper>
</template>
<template v-else>
<component
diff --git a/app/assets/javascripts/notes/components/discussion_notes_replies_wrapper.vue b/app/assets/javascripts/notes/components/discussion_notes_replies_wrapper.vue
new file mode 100644
index 00000000000..2ddca56ddd5
--- /dev/null
+++ b/app/assets/javascripts/notes/components/discussion_notes_replies_wrapper.vue
@@ -0,0 +1,27 @@
+<script>
+/**
+ * Wrapper for discussion notes replies section.
+ *
+ * This is a functional component using the render method because in some cases
+ * the wrapper is not needed and we want to simply render along the children.
+ */
+export default {
+ functional: true,
+ props: {
+ isDiffDiscussion: {
+ type: Boolean,
+ required: false,
+ default: false,
+ },
+ },
+ render(h, { props, children }) {
+ if (props.isDiffDiscussion) {
+ return h('li', { class: 'discussion-collapsible bordered-box clearfix' }, [
+ h('ul', { class: 'notes' }, children),
+ ]);
+ }
+
+ return children;
+ },
+};
+</script>
diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss
index 623c44e062f..3ffe8ae304d 100644
--- a/app/assets/stylesheets/pages/diff.scss
+++ b/app/assets/stylesheets/pages/diff.scss
@@ -1095,6 +1095,10 @@ table.code {
.discussion-collapsible {
margin: 0 $gl-padding $gl-padding 71px;
+
+ .notes {
+ border-radius: $border-radius-default;
+ }
}
.parallel {
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 5db0136e3f1..b9b8eabf909 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -139,7 +139,6 @@ $note-form-margin-left: 72px;
border-radius: 4px 4px 0 0;
&.collapsed {
- border: 0;
border-radius: 4px;
}
}
diff --git a/spec/frontend/notes/components/discussion_notes_replies_wrapper_spec.js b/spec/frontend/notes/components/discussion_notes_replies_wrapper_spec.js
new file mode 100644
index 00000000000..279ca017b44
--- /dev/null
+++ b/spec/frontend/notes/components/discussion_notes_replies_wrapper_spec.js
@@ -0,0 +1,51 @@
+import { mount } from '@vue/test-utils';
+import DiscussionNotesRepliesWrapper from '~/notes/components/discussion_notes_replies_wrapper.vue';
+
+const TEST_CHILDREN = '<li>Hello!</li><li>World!</li>';
+
+// We have to wrap our SUT with a TestComponent because multiple roots are possible
+// because it's a functional component.
+const TestComponent = {
+ components: { DiscussionNotesRepliesWrapper },
+ template: `<ul><discussion-notes-replies-wrapper v-bind="$attrs">${TEST_CHILDREN}</discussion-notes-replies-wrapper></ul>`,
+};
+
+describe('DiscussionNotesRepliesWrapper', () => {
+ let wrapper;
+
+ const createComponent = (props = {}) => {
+ wrapper = mount(TestComponent, {
+ propsData: props,
+ sync: false,
+ });
+ };
+
+ afterEach(() => {
+ wrapper.destroy();
+ });
+
+ describe('when normal discussion', () => {
+ beforeEach(() => {
+ createComponent();
+ });
+
+ it('renders children directly', () => {
+ expect(wrapper.html()).toEqual(`<ul>${TEST_CHILDREN}</ul>`);
+ });
+ });
+
+ describe('when diff discussion', () => {
+ beforeEach(() => {
+ createComponent({
+ isDiffDiscussion: true,
+ });
+ });
+
+ it('wraps children with notes', () => {
+ const notes = wrapper.find('li.discussion-collapsible ul.notes');
+
+ expect(notes.exists()).toBe(true);
+ expect(notes.html()).toEqual(`<ul class="notes">${TEST_CHILDREN}</ul>`);
+ });
+ });
+});
diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js
index 0b3890b68d6..9b61dbe7975 100644
--- a/spec/javascripts/diffs/components/inline_diff_view_spec.js
+++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js
@@ -10,6 +10,7 @@ describe('InlineDiffView', () => {
let component;
const getDiffFileMock = () => Object.assign({}, diffFileMockData);
const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
+ const notesLength = getDiscussionsMockData()[0].notes.length;
beforeEach(done => {
const diffFile = getDiffFileMock();
@@ -40,7 +41,7 @@ describe('InlineDiffView', () => {
Vue.nextTick(() => {
expect(el.querySelectorAll('.notes_holder').length).toEqual(1);
- expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(6);
+ expect(el.querySelectorAll('.notes_holder .note').length).toEqual(notesLength + 1);
expect(el.innerText.indexOf('comment 5')).toBeGreaterThan(-1);
component.$store.dispatch('setInitialNotes', []);