diff options
author | Natalia Tepluhina <ntepluhina@gitlab.com> | 2019-07-03 11:26:57 +0300 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2019-07-03 11:26:57 +0300 |
commit | d4151b14c2986db173a7a1a4d293b86bfcdaae3a (patch) | |
tree | f3bb3b6829671ddeb17dd4f373b3e358187df7b8 /app | |
parent | fd547ee4c618ab8d93ba07121e7bc6bf2924a1b3 (diff) |
Rebased and squashed commits
- all commits squashed to make danger review happy
Diffstat (limited to 'app')
18 files changed, 392 insertions, 127 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_discussion_reply.vue b/app/assets/javascripts/diffs/components/diff_discussion_reply.vue new file mode 100644 index 00000000000..2aa5e9b3339 --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_discussion_reply.vue @@ -0,0 +1,55 @@ +<script> +import { mapGetters } from 'vuex'; +import NoteSignedOutWidget from '~/notes/components/note_signed_out_widget.vue'; +import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; +import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; + +export default { + name: 'DiffDiscussionReply', + components: { + NoteSignedOutWidget, + ReplyPlaceholder, + UserAvatarLink, + }, + props: { + hasForm: { + type: Boolean, + required: false, + default: false, + }, + renderReplyPlaceholder: { + type: Boolean, + required: true, + }, + }, + computed: { + ...mapGetters({ + currentUser: 'getUserData', + userCanReply: 'userCanReply', + }), + }, +}; +</script> + +<template> + <div class="discussion-reply-holder d-flex clearfix"> + <template v-if="userCanReply"> + <slot v-if="hasForm" name="form"></slot> + <template v-else-if="renderReplyPlaceholder"> + <user-avatar-link + :link-href="currentUser.path" + :img-src="currentUser.avatar_url" + :img-alt="currentUser.name" + :img-size="40" + class="d-none d-sm-block" + /> + <reply-placeholder + class="qa-discussion-reply" + :button-text="__('Start a new discussion...')" + @onClick="$emit('showNewDiscussionForm')" + /> + </template> + </template> + <note-signed-out-widget v-else /> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index 4c73eea4049..b0460bacff2 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -80,7 +80,6 @@ export default { v-show="isExpanded(discussion)" :discussion="discussion" :render-diff-file="false" - :always-expanded="true" :discussions-by-diff-order="true" :line="line" :help-page-path="helpPagePath" diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index eb9f1465945..4b226e30699 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -151,7 +151,11 @@ export default { stickyMonitor(this.$refs.header, contentTop() - fileHeaderHeight - 1, false); }, methods: { - ...mapActions('diffs', ['toggleFileDiscussions', 'toggleFullDiff']), + ...mapActions('diffs', [ + 'toggleFileDiscussions', + 'toggleFileDiscussionWrappers', + 'toggleFullDiff', + ]), handleToggleFile(e, checkTarget) { if ( !checkTarget || @@ -165,7 +169,7 @@ export default { this.$emit('showForkMessage'); }, handleToggleDiscussions() { - this.toggleFileDiscussions(this.diffFile); + this.toggleFileDiscussionWrappers(this.diffFile); }, handleFileNameClick(e) { const isLinkToOtherPage = diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index e28909b7be3..af5550aec3b 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -1,5 +1,4 @@ <script> -import { mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; import { pluralize, truncate } from '~/lib/utils/text_utility'; import UserAvatarImage from '~/vue_shared/components/user_avatar/user_avatar_image.vue'; @@ -19,11 +18,13 @@ export default { type: Array, required: true, }, + discussionsExpanded: { + type: Boolean, + required: false, + default: false, + }, }, computed: { - discussionsExpanded() { - return this.discussions.every(discussion => discussion.expanded); - }, allDiscussions() { return this.discussions.reduce((acc, note) => acc.concat(note.notes), []); }, @@ -45,26 +46,14 @@ export default { }, }, methods: { - ...mapActions(['toggleDiscussion']), getTooltipText(noteData) { let { note } = noteData; - if (note.length > LENGTH_OF_AVATAR_TOOLTIP) { note = truncate(note, LENGTH_OF_AVATAR_TOOLTIP); } return `${noteData.author.name}: ${note}`; }, - toggleDiscussions() { - const forceExpanded = this.discussions.some(discussion => !discussion.expanded); - - this.discussions.forEach(discussion => { - this.toggleDiscussion({ - discussionId: discussion.id, - forceExpanded, - }); - }); - }, }, }; </script> @@ -76,7 +65,7 @@ export default { type="button" :aria-label="__('Show comments')" class="diff-notes-collapse js-diff-comment-avatar js-diff-comment-button" - @click="toggleDiscussions" + @click="$emit('toggleLineDiscussions')" > <icon :size="12" name="collapse" /> </button> @@ -87,7 +76,7 @@ export default { :img-src="note.author.avatar_url" :tooltip-text="getTooltipText(note)" class="diff-comment-avatar js-diff-comment-avatar" - @click.native="toggleDiscussions" + @click.native="$emit('toggleLineDiscussions')" /> <span v-if="moreText" @@ -97,7 +86,7 @@ export default { data-container="body" data-placement="top" role="button" - @click="toggleDiscussions" + @click="$emit('toggleLineDiscussions')" >+{{ moreCount }}</span > </template> diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index 1281f9b17ef..351110f0a87 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -105,7 +105,13 @@ export default { }, }, methods: { - ...mapActions('diffs', ['loadMoreLines', 'showCommentForm', 'setHighlightedRow']), + ...mapActions('diffs', [ + 'loadMoreLines', + 'showCommentForm', + 'setHighlightedRow', + 'toggleLineDiscussions', + 'toggleLineDiscussionWrappers', + ]), handleCommentButton() { this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash }); }, @@ -184,7 +190,14 @@ export default { @click="setHighlightedRow(lineCode)" > </a> - <diff-gutter-avatars v-if="shouldShowAvatarsOnGutter" :discussions="line.discussions" /> + <diff-gutter-avatars + v-if="shouldShowAvatarsOnGutter" + :discussions="line.discussions" + :discussions-expanded="line.discussionsExpanded" + @toggleLineDiscussions=" + toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded }) + " + /> </template> </div> </template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue index 1faa0493e79..ca3285e9afd 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -1,11 +1,14 @@ <script> -import diffDiscussions from './diff_discussions.vue'; -import diffLineNoteForm from './diff_line_note_form.vue'; +import { mapActions } from 'vuex'; +import DiffDiscussions from './diff_discussions.vue'; +import DiffLineNoteForm from './diff_line_note_form.vue'; +import DiffDiscussionReply from './diff_discussion_reply.vue'; export default { components: { - diffDiscussions, - diffLineNoteForm, + DiffDiscussions, + DiffLineNoteForm, + DiffDiscussionReply, }, props: { line: { @@ -32,10 +35,12 @@ export default { if (!this.line.discussions || !this.line.discussions.length) { return false; } - - return this.line.discussions.every(discussion => discussion.expanded); + return this.line.discussionsExpanded; }, }, + methods: { + ...mapActions('diffs', ['showCommentForm']), + }, }; </script> @@ -49,13 +54,22 @@ export default { :discussions="line.discussions" :help-page-path="helpPagePath" /> - <diff-line-note-form - v-if="line.hasForm" - :diff-file-hash="diffFileHash" - :line="line" - :note-target-line="line" - :help-page-path="helpPagePath" - /> + <diff-discussion-reply + :has-form="line.hasForm" + :render-reply-placeholder="Boolean(line.discussions.length)" + @showNewDiscussionForm=" + showCommentForm({ lineCode: line.line_code, fileHash: diffFileHash }) + " + > + <template #form> + <diff-line-note-form + :diff-file-hash="diffFileHash" + :line="line" + :note-target-line="line" + :help-page-path="helpPagePath" + /> + </template> + </diff-discussion-reply> </div> </td> </tr> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index d2e54edca85..c00b0e010ff 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -1,11 +1,14 @@ <script> -import diffDiscussions from './diff_discussions.vue'; -import diffLineNoteForm from './diff_line_note_form.vue'; +import { mapActions } from 'vuex'; +import DiffDiscussions from './diff_discussions.vue'; +import DiffLineNoteForm from './diff_line_note_form.vue'; +import DiffDiscussionReply from './diff_discussion_reply.vue'; export default { components: { - diffDiscussions, - diffLineNoteForm, + DiffDiscussions, + DiffLineNoteForm, + DiffDiscussionReply, }, props: { line: { @@ -29,24 +32,30 @@ export default { computed: { hasExpandedDiscussionOnLeft() { return this.line.left && this.line.left.discussions.length - ? this.line.left.discussions.every(discussion => discussion.expanded) + ? this.line.left.discussionsExpanded : false; }, hasExpandedDiscussionOnRight() { return this.line.right && this.line.right.discussions.length - ? this.line.right.discussions.every(discussion => discussion.expanded) + ? this.line.right.discussionsExpanded : false; }, hasAnyExpandedDiscussion() { return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; }, shouldRenderDiscussionsOnLeft() { - return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft; + return ( + this.line.left && + this.line.left.discussions && + this.line.left.discussions.length && + this.hasExpandedDiscussionOnLeft + ); }, shouldRenderDiscussionsOnRight() { return ( this.line.right && this.line.right.discussions && + this.line.right.discussions.length && this.hasExpandedDiscussionOnRight && this.line.right.type ); @@ -81,6 +90,22 @@ export default { return hasCommentFormOnLeft || hasCommentFormOnRight; }, + shouldRenderReplyPlaceholderOnLeft() { + return Boolean( + this.line.left && this.line.left.discussions && this.line.left.discussions.length, + ); + }, + shouldRenderReplyPlaceholderOnRight() { + return Boolean( + this.line.right && this.line.right.discussions && this.line.right.discussions.length, + ); + }, + }, + methods: { + ...mapActions('diffs', ['showCommentForm']), + showNewDiscussionForm() { + this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.diffFileHash }); + }, }, }; </script> @@ -90,37 +115,49 @@ export default { <td class="notes-content parallel old" colspan="2"> <div v-if="shouldRenderDiscussionsOnLeft" class="content"> <diff-discussions - v-if="line.left.discussions.length" :discussions="line.left.discussions" :line="line.left" :help-page-path="helpPagePath" /> </div> - <diff-line-note-form - v-if="showLeftSideCommentForm" - :diff-file-hash="diffFileHash" - :line="line.left" - :note-target-line="line.left" - :help-page-path="helpPagePath" - line-position="left" - /> + <diff-discussion-reply + :has-form="showLeftSideCommentForm" + :render-reply-placeholder="shouldRenderReplyPlaceholderOnLeft" + @showNewDiscussionForm="showNewDiscussionForm" + > + <template #form> + <diff-line-note-form + :diff-file-hash="diffFileHash" + :line="line.left" + :note-target-line="line.left" + :help-page-path="helpPagePath" + line-position="left" + /> + </template> + </diff-discussion-reply> </td> <td class="notes-content parallel new" colspan="2"> <div v-if="shouldRenderDiscussionsOnRight" class="content"> <diff-discussions - v-if="line.right.discussions.length" :discussions="line.right.discussions" :line="line.right" :help-page-path="helpPagePath" /> </div> - <diff-line-note-form - v-if="showRightSideCommentForm" - :diff-file-hash="diffFileHash" - :line="line.right" - :note-target-line="line.right" - line-position="right" - /> + <diff-discussion-reply + :has-form="showRightSideCommentForm" + :render-reply-placeholder="shouldRenderReplyPlaceholderOnRight" + @showNewDiscussionForm="showNewDiscussionForm" + > + <template #form> + <diff-line-note-form + :diff-file-hash="diffFileHash" + :line="line.right" + :note-target-line="line.right" + line-position="right" + /> + </template> + </diff-discussion-reply> </td> </tr> </template> diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 79ce49012f0..32e0d8f42ee 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -12,6 +12,7 @@ import { getNoteFormData, convertExpandLines, idleCallback, + allDiscussionWrappersExpanded, } from './utils'; import * as types from './mutation_types'; import { @@ -79,6 +80,7 @@ export const assignDiscussionsToDiff = ( discussions = rootState.notes.discussions, ) => { const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles); + const hash = getLocationHash(); discussions .filter(discussion => discussion.diff_discussion) @@ -86,6 +88,7 @@ export const assignDiscussionsToDiff = ( commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { discussion, diffPositionByLineCode, + hash, }); }); @@ -99,6 +102,10 @@ export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => { commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash: file_hash, lineCode: line_code, id }); }; +export const toggleLineDiscussions = ({ commit }, options) => { + commit(types.TOGGLE_LINE_DISCUSSIONS, options); +}; + export const renderFileForDiscussionId = ({ commit, rootState, state }, discussionId) => { const discussion = rootState.notes.discussions.find(d => d.id === discussionId); @@ -257,6 +264,31 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { }); }; +export const toggleFileDiscussionWrappers = ({ commit }, diff) => { + const discussionWrappersExpanded = allDiscussionWrappersExpanded(diff); + let linesWithDiscussions; + if (diff.highlighted_diff_lines) { + linesWithDiscussions = diff.highlighted_diff_lines.filter(line => line.discussions.length); + } + if (diff.parallel_diff_lines) { + linesWithDiscussions = diff.parallel_diff_lines.filter( + line => + (line.left && line.left.discussions.length) || + (line.right && line.right.discussions.length), + ); + } + + if (linesWithDiscussions.length) { + linesWithDiscussions.forEach(line => { + commit(types.TOGGLE_LINE_DISCUSSIONS, { + fileHash: diff.file_hash, + lineCode: line.line_code, + expanded: !discussionWrappersExpanded, + }); + }); + } +}; + export const saveDiffDiscussion = ({ state, dispatch }, { note, formData }) => { const postData = getNoteFormData({ commit: state.commit, diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 8d6111da500..9db56331faa 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -35,3 +35,5 @@ export const ADD_CURRENT_VIEW_DIFF_FILE_LINES = 'ADD_CURRENT_VIEW_DIFF_FILE_LINE export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE'; export const SET_SHOW_SUGGEST_POPOVER = 'SET_SHOW_SUGGEST_POPOVER'; + +export const TOGGLE_LINE_DISCUSSIONS = 'TOGGLE_LINE_DISCUSSIONS'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 00181a63c43..a66f205bbbd 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -6,6 +6,7 @@ import { addContextLines, prepareDiffData, isDiscussionApplicableToLine, + updateLineInFile, } from './utils'; import * as types from './mutation_types'; @@ -109,7 +110,7 @@ export default { })); }, - [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode }) { + [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) { const { latestDiff } = state; const discussionLineCode = discussion.line_code; @@ -130,13 +131,27 @@ export default { : [], }); + const setDiscussionsExpanded = line => { + const isLineNoteTargeted = line.discussions.some( + disc => disc.notes && disc.notes.find(note => hash === `note_${note.id}`), + ); + + return { + ...line, + discussionsExpanded: + line.discussions && line.discussions.length + ? line.discussions.some(disc => !disc.resolved) || isLineNoteTargeted + : false, + }; + }; + state.diffFiles = state.diffFiles.map(diffFile => { if (diffFile.file_hash === fileHash) { const file = { ...diffFile }; if (file.highlighted_diff_lines) { file.highlighted_diff_lines = file.highlighted_diff_lines.map(line => - lineCheck(line) ? mapDiscussions(line) : line, + setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line), ); } @@ -148,8 +163,10 @@ export default { if (left || right) { return { ...line, - left: line.left ? mapDiscussions(line.left) : null, - right: line.right ? mapDiscussions(line.right, () => !left) : null, + left: line.left ? setDiscussionsExpanded(mapDiscussions(line.left)) : null, + right: line.right + ? setDiscussionsExpanded(mapDiscussions(line.right, () => !left)) + : null, }; } @@ -173,32 +190,11 @@ export default { [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { const selectedFile = state.diffFiles.find(f => f.file_hash === fileHash); if (selectedFile) { - if (selectedFile.parallel_diff_lines) { - const targetLine = selectedFile.parallel_diff_lines.find( - line => - (line.left && line.left.line_code === lineCode) || - (line.right && line.right.line_code === lineCode), - ); - if (targetLine) { - const side = targetLine.left && targetLine.left.line_code === lineCode ? 'left' : 'right'; - - Object.assign(targetLine[side], { - discussions: targetLine[side].discussions.filter(discussion => discussion.notes.length), - }); - } - } - - if (selectedFile.highlighted_diff_lines) { - const targetInlineLine = selectedFile.highlighted_diff_lines.find( - line => line.line_code === lineCode, - ); - - if (targetInlineLine) { - Object.assign(targetInlineLine, { - discussions: targetInlineLine.discussions.filter(discussion => discussion.notes.length), - }); - } - } + updateLineInFile(selectedFile, lineCode, line => + Object.assign(line, { + discussions: line.discussions.filter(discussion => discussion.notes.length), + }), + ); if (selectedFile.discussions && selectedFile.discussions.length) { selectedFile.discussions = selectedFile.discussions.filter( @@ -207,6 +203,15 @@ export default { } } }, + + [types.TOGGLE_LINE_DISCUSSIONS](state, { fileHash, lineCode, expanded }) { + const selectedFile = state.diffFiles.find(f => f.file_hash === fileHash); + + updateLineInFile(selectedFile, lineCode, line => + Object.assign(line, { discussionsExpanded: expanded }), + ); + }, + [types.TOGGLE_FOLDER_OPEN](state, path) { state.treeEntries[path].opened = !state.treeEntries[path].opened; }, diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 71956255eef..1c3ed84001c 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -454,3 +454,48 @@ export const convertExpandLines = ({ }; export const idleCallback = cb => requestIdleCallback(cb); + +export const updateLineInFile = (selectedFile, lineCode, updateFn) => { + if (selectedFile.parallel_diff_lines) { + const targetLine = selectedFile.parallel_diff_lines.find( + line => + (line.left && line.left.line_code === lineCode) || + (line.right && line.right.line_code === lineCode), + ); + if (targetLine) { + const side = targetLine.left && targetLine.left.line_code === lineCode ? 'left' : 'right'; + + updateFn(targetLine[side]); + } + } + if (selectedFile.highlighted_diff_lines) { + const targetInlineLine = selectedFile.highlighted_diff_lines.find( + line => line.line_code === lineCode, + ); + + if (targetInlineLine) { + updateFn(targetInlineLine); + } + } +}; + +export const allDiscussionWrappersExpanded = diff => { + const discussionsExpandedArray = []; + if (diff.parallel_diff_lines) { + diff.parallel_diff_lines.forEach(line => { + if (line.left && line.left.discussions.length) { + discussionsExpandedArray.push(line.left.discussionsExpanded); + } + if (line.right && line.right.discussions.length) { + discussionsExpandedArray.push(line.right.discussionsExpanded); + } + }); + } else if (diff.highlighted_diff_lines) { + diff.parallel_diff_lines.forEach(line => { + if (line.discussions.length) { + discussionsExpandedArray.push(line.discussionsExpanded); + } + }); + } + return discussionsExpandedArray.every(el => el); +}; diff --git a/app/assets/javascripts/notes/components/discussion_actions.vue b/app/assets/javascripts/notes/components/discussion_actions.vue index 1357a5268d6..f4570c1292c 100644 --- a/app/assets/javascripts/notes/components/discussion_actions.vue +++ b/app/assets/javascripts/notes/components/discussion_actions.vue @@ -39,15 +39,23 @@ export default { </script> <template> - <div class="discussion-with-resolve-btn clearfix"> - <reply-placeholder class="qa-discussion-reply" @onClick="$emit('showReplyForm')" /> - - <div class="btn-group discussion-actions" role="group"> - <resolve-discussion-button - v-if="discussion.resolvable" - :is-resolving="isResolving" - :button-title="resolveButtonTitle" - @onClick="$emit('resolve')" + <div class="discussion-with-resolve-btn"> + <reply-placeholder + :button-text="s__('MergeRequests|Reply...')" + class="qa-discussion-reply" + @onClick="$emit('showReplyForm')" + /> + <resolve-discussion-button + v-if="discussion.resolvable" + :is-resolving="isResolving" + :button-title="resolveButtonTitle" + @onClick="$emit('resolve')" + /> + <div v-if="discussion.resolvable" class="btn-group discussion-actions ml-sm-2" role="group"> + <resolve-with-issue-button v-if="resolveWithIssuePath" :url="resolveWithIssuePath" /> + <jump-to-next-discussion-button + v-if="shouldShowJumpToNextDiscussion" + @onClick="$emit('jumpToNextDiscussion')" /> <resolve-with-issue-button v-if="discussion.resolvable && resolveWithIssuePath" diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 30971ad5227..2ff0fee62f3 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -1,11 +1,11 @@ <script> -import { mapGetters } from 'vuex'; +import { mapGetters, mapActions } from 'vuex'; import { SYSTEM_NOTE } from '../constants'; import { __ } from '~/locale'; -import NoteableNote from './noteable_note.vue'; -import PlaceholderNote from '../../vue_shared/components/notes/placeholder_note.vue'; -import PlaceholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue'; +import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue'; +import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue'; 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'; @@ -72,6 +72,7 @@ export default { }, }, methods: { + ...mapActions(['toggleDiscussion']), componentName(note) { if (note.isPlaceholderNote) { if (note.placeholderType === SYSTEM_NOTE) { @@ -101,7 +102,7 @@ export default { <component :is="componentName(firstNote)" :note="componentData(firstNote)" - :line="line" + :line="line || diffLine" :commit="commit" :help-page-path="helpPagePath" :show-reply-button="userCanReply" @@ -118,23 +119,29 @@ export default { /> <slot slot="avatar-badge" name="avatar-badge"></slot> </component> - <toggle-replies-widget - v-if="hasReplies" - :collapsed="!isExpanded" - :replies="replies" - @toggle="$emit('toggleDiscussion')" - /> - <template v-if="isExpanded"> - <component - :is="componentName(note)" - v-for="note in replies" - :key="note.id" - :note="componentData(note)" - :help-page-path="helpPagePath" - :line="line" - @handleDeleteNote="$emit('deleteNote')" + <div + :class="discussion.diff_discussion ? 'discussion-collapsible bordered-box clearfix' : ''" + > + <toggle-replies-widget + v-if="hasReplies" + :collapsed="!isExpanded" + :replies="replies" + :class="{ 'discussion-toggle-replies': discussion.diff_discussion }" + @toggle="toggleDiscussion({ discussionId: discussion.id })" /> - </template> + <template v-if="isExpanded"> + <component + :is="componentName(note)" + v-for="note in replies" + :key="note.id" + :note="componentData(note)" + :help-page-path="helpPagePath" + :line="line" + @handleDeleteNote="$emit('deleteNote')" + /> + </template> + <slot :show-replies="isExpanded || !hasReplies" name="footer"></slot> + </div> </template> <template v-else> <component @@ -148,8 +155,8 @@ export default { > <slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot> </component> + <slot :show-replies="isExpanded || !hasReplies" name="footer"></slot> </template> </ul> - <slot :show-replies="isExpanded || !hasReplies" name="footer"></slot> </div> </template> diff --git a/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue b/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue index ea590905e3c..0204169214b 100644 --- a/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue +++ b/app/assets/javascripts/notes/components/discussion_reply_placeholder.vue @@ -1,6 +1,12 @@ <script> export default { name: 'ReplyPlaceholder', + props: { + buttonText: { + type: String, + required: true, + }, + }, }; </script> @@ -12,6 +18,6 @@ export default { :title="s__('MergeRequests|Add a reply')" @click="$emit('onClick')" > - {{ s__('MergeRequests|Reply...') }} + {{ buttonText }} </button> </template> diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index b8eaff32cce..f6b5fffde29 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -132,7 +132,7 @@ export default { return this.discussion.diff_discussion && this.renderDiffFile; }, shouldGroupReplies() { - return !this.shouldRenderDiffs && !this.discussion.diff_discussion; + return !this.shouldRenderDiffs; }, wrapperComponent() { return this.shouldRenderDiffs ? diffWithNote : 'div'; @@ -250,6 +250,11 @@ export default { clearDraft(this.autosaveKey); }, saveReply(noteText, form, callback) { + if (!noteText) { + this.cancelReplyForm(); + callback(); + return; + } const postData = { in_reply_to_discussion_id: this.discussion.reply_id, target_type: this.getNoteableData.targetType, @@ -363,7 +368,6 @@ Please check your network connection and try again.`; :line="line" :should-group-replies="shouldGroupReplies" @startReplying="showReplyForm" - @toggleDiscussion="toggleDiscussionHandler" @deleteNote="deleteNoteHandler" > <slot slot="avatar-badge" name="avatar-badge"></slot> @@ -376,7 +380,7 @@ Please check your network connection and try again.`; <div v-else-if="showReplies" :class="{ 'is-replying': isReplying }" - class="discussion-reply-holder" + class="discussion-reply-holder clearfix" > <user-avatar-link v-if="!isReplying && userCanReply" diff --git a/app/assets/javascripts/vue_shared/components/notes/placeholder_note.vue b/app/assets/javascripts/vue_shared/components/notes/placeholder_note.vue index baed26a157c..af02b8969ee 100644 --- a/app/assets/javascripts/vue_shared/components/notes/placeholder_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/placeholder_note.vue @@ -39,7 +39,7 @@ export default { </script> <template> - <timeline-entry-item class="note being-posted fade-in-half"> + <timeline-entry-item class="note note-wrapper being-posted fade-in-half"> <div class="timeline-icon"> <user-avatar-link :link-href="getUserData.path" diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index d2d35d91e0b..623c44e062f 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -1093,6 +1093,17 @@ table.code { line-height: 0; } +.discussion-collapsible { + margin: 0 $gl-padding $gl-padding 71px; +} + +.parallel { + .discussion-collapsible { + margin: $gl-padding; + margin-top: 0; + } +} + @media (max-width: map-get($grid-breakpoints, md)-1) { .diffs .files { @include fixed-width-container; @@ -1110,6 +1121,11 @@ table.code { padding-right: 0; } } + + .discussion-collapsible { + margin: $gl-padding; + margin-top: 0; + } } .image-diff-overlay, diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index e880b941d67..7bd1a4138e4 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -134,6 +134,16 @@ $note-form-margin-left: 72px; } } + .discussion-toggle-replies { + border-top: 0; + border-radius: 4px 4px 0 0; + + &.collapsed { + border: 0; + border-radius: 4px; + } + } + .note-created-ago, .note-updated-at { white-space: normal; @@ -462,6 +472,14 @@ $note-form-margin-left: 72px; position: relative; } + .notes-content .discussion-notes.diff-discussions { + border-bottom: 1px solid $border-color; + + &:nth-last-child(1) { + border-bottom: 0; + } + } + .notes_holder { font-family: $regular-font; @@ -517,6 +535,17 @@ $note-form-margin-left: 72px; .discussion-reply-holder { border-radius: 0 0 $border-radius-default $border-radius-default; position: relative; + + .discussion-form { + width: 100%; + background-color: $gray-light; + padding: 0; + } + + .disabled-comment { + padding: $gl-vert-padding 0; + width: 100%; + } } } |