diff options
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', []); |