diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 12:26:25 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 12:26:25 +0000 |
commit | a09983ae35713f5a2bbb100981116d31ce99826e (patch) | |
tree | 2ee2af7bd104d57086db360a7e6d8c9d5d43667a /app/assets/javascripts/notes/components | |
parent | 18c5ab32b738c0b6ecb4d0df3994000482f34bd8 (diff) | |
download | gitlab-ce-a09983ae35713f5a2bbb100981116d31ce99826e.tar.gz |
Add latest changes from gitlab-org/gitlab@13-2-stable-ee
Diffstat (limited to 'app/assets/javascripts/notes/components')
8 files changed, 214 insertions, 84 deletions
diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index 16dcde46262..ac93d3df654 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -17,7 +17,7 @@ import { import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests'; import * as constants from '../constants'; import eventHub from '../event_hub'; -import issueWarning from '../../vue_shared/components/issue/issue_warning.vue'; +import NoteableWarning from '../../vue_shared/components/notes/noteable_warning.vue'; import markdownField from '../../vue_shared/components/markdown/field.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import loadingButton from '../../vue_shared/components/loading_button.vue'; @@ -28,8 +28,7 @@ import issuableStateMixin from '../mixins/issuable_state'; export default { name: 'CommentForm', components: { - issueWarning, - epicWarning: () => import('ee_component/vue_shared/components/epic/epic_warning.vue'), + NoteableWarning, noteSignedOutWidget, discussionLockedWidget, markdownField, @@ -126,9 +125,13 @@ export default { canToggleIssueState() { return ( this.getNoteableData.current_user.can_update && - this.getNoteableData.state !== constants.MERGED + this.getNoteableData.state !== constants.MERGED && + !this.closedAndLocked ); }, + closedAndLocked() { + return !this.isOpen && this.isLocked(this.getNoteableData); + }, endpoint() { return this.getNoteableData.create_note_path; }, @@ -350,14 +353,15 @@ export default { <form ref="commentForm" class="new-note common-note-form gfm-form js-main-target-form"> <div class="error-alert"></div> - <issue-warning - v-if="hasWarning(getNoteableData) && isIssueType" + <noteable-warning + v-if="hasWarning(getNoteableData)" :is-locked="isLocked(getNoteableData)" :is-confidential="isConfidential(getNoteableData)" - :locked-issue-docs-path="lockedIssueDocsPath" - :confidential-issue-docs-path="confidentialIssueDocsPath" + :noteable-type="noteableType" + :locked-noteable-docs-path="lockedIssueDocsPath" + :confidential-noteable-docs-path="confidentialIssueDocsPath" /> - <epic-warning :is-confidential="isConfidential(getNoteableData)" /> + <markdown-field ref="markdownField" :is-submitting="isSubmitting" @@ -374,20 +378,18 @@ export default { dir="auto" :disabled="isSubmitting" name="note[note]" - class="note-textarea js-vue-comment-form js-note-text -js-gfm-input js-autosize markdown-area js-vue-textarea qa-comment-input" + class="note-textarea js-vue-comment-form js-note-text js-gfm-input js-autosize markdown-area js-vue-textarea qa-comment-input" data-supports-quick-actions="true" :aria-label="__('Description')" :placeholder="__('Write a comment or drag your files hereā¦')" @keydown.up="editCurrentUserLastNote()" @keydown.meta.enter="handleSave()" @keydown.ctrl.enter="handleSave()" - > - </textarea> + ></textarea> </markdown-field> <gl-alert v-if="isToggleBlockedIssueWarning" - class="prepend-top-16" + class="gl-mt-5" :title="__('Are you sure you want to close this blocked issue?')" :primary-button-text="__('Yes, close issue')" :secondary-button-text="__('Cancel')" @@ -417,13 +419,11 @@ js-gfm-input js-autosize markdown-area js-vue-textarea qa-comment-input" </gl-alert> <div class="note-form-actions"> <div - class="btn-group -append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" + class="btn-group gl-mr-3 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" > <button :disabled="isSubmitButtonDisabled" - class="btn btn-success js-comment-button js-comment-submit-button - qa-comment-button" + class="btn btn-success js-comment-button js-comment-submit-button qa-comment-button" type="submit" :data-track-label="trackingLabel" data-track-event="click_button" @@ -440,7 +440,7 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" data-toggle="dropdown" :aria-label="__('Open comment type dropdown')" > - <i aria-hidden="true" class="fa fa-caret-down toggle-icon"> </i> + <i aria-hidden="true" class="fa fa-caret-down toggle-icon"></i> </button> <ul class="note-type-dropdown dropdown-open-top dropdown-menu"> @@ -450,7 +450,7 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" class="btn btn-transparent" @click.prevent="setNoteType('comment')" > - <i aria-hidden="true" class="fa fa-check icon"> </i> + <i aria-hidden="true" class="fa fa-check icon"></i> <div class="description"> <strong>{{ __('Comment') }}</strong> <p> @@ -470,7 +470,7 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" class="btn btn-transparent qa-discussion-option" @click.prevent="setNoteType('discussion')" > - <i aria-hidden="true" class="fa fa-check icon"> </i> + <i aria-hidden="true" class="fa fa-check icon"></i> <div class="description"> <strong>{{ __('Start thread') }}</strong> <p>{{ startDiscussionDescription }}</p> diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 0b136549c14..458da5cf67f 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -74,7 +74,7 @@ export default { }, }, methods: { - ...mapActions(['toggleDiscussion']), + ...mapActions(['toggleDiscussion', 'setSelectedCommentPositionHover']), componentName(note) { if (note.isPlaceholderNote) { if (note.placeholderType === SYSTEM_NOTE) { @@ -99,7 +99,11 @@ export default { <template> <div class="discussion-notes"> - <ul class="notes"> + <ul + class="notes" + @mouseenter="setSelectedCommentPositionHover(discussion.position.line_range)" + @mouseleave="setSelectedCommentPositionHover()" + > <template v-if="shouldGroupReplies"> <component :is="componentName(firstNote)" @@ -108,6 +112,7 @@ export default { :commit="commit" :help-page-path="helpPagePath" :show-reply-button="userCanReply" + :discussion-root="true" @handleDeleteNote="$emit('deleteNote')" @startReplying="$emit('startReplying')" > @@ -151,6 +156,7 @@ export default { :note="componentData(note)" :help-page-path="helpPagePath" :line="diffLine" + :discussion-root="index === 0" @handleDeleteNote="$emit('deleteNote')" > <slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot> diff --git a/app/assets/javascripts/notes/components/multiline_comment_form.vue b/app/assets/javascripts/notes/components/multiline_comment_form.vue index 5fba011a153..bb13eb87af7 100644 --- a/app/assets/javascripts/notes/components/multiline_comment_form.vue +++ b/app/assets/javascripts/notes/components/multiline_comment_form.vue @@ -1,4 +1,5 @@ <script> +import { mapActions } from 'vuex'; import { GlFormSelect, GlSprintf } from '@gitlab/ui'; import { getSymbol, getLineClasses } from './multiline_comment_utils'; @@ -21,19 +22,51 @@ export default { }, data() { return { - commentLineStart: { - lineCode: this.lineRange ? this.lineRange.start_line_code : this.line.line_code, - type: this.lineRange ? this.lineRange.start_line_type : this.line.type, - }, + commentLineStart: {}, + commentLineEndType: this.lineRange?.end?.line_type || this.line.type, }; }, + computed: { + lineNumber() { + return this.commentLineOptions[this.commentLineOptions.length - 1].text; + }, + }, + created() { + const line = this.lineRange?.start || this.line; + + this.commentLineStart = { + line_code: line.line_code, + type: line.type, + old_line: line.old_line, + new_line: line.new_line, + }; + this.highlightSelection(); + }, + destroyed() { + this.setSelectedCommentPosition(); + }, methods: { + ...mapActions(['setSelectedCommentPosition']), getSymbol({ type }) { return getSymbol(type); }, getLineClasses(line) { return getLineClasses(line); }, + updateCommentLineStart(value) { + this.commentLineStart = value; + this.$emit('input', value); + this.highlightSelection(); + }, + highlightSelection() { + const { line_code, new_line, old_line, type } = this.line; + const updatedLineRange = { + start: { ...this.commentLineStart }, + end: { line_code, new_line, old_line, type }, + }; + + this.setSelectedCommentPosition(updatedLineRange); + }, }, }; </script> @@ -55,12 +88,12 @@ export default { :options="commentLineOptions" size="sm" class="gl-w-auto gl-vertical-align-baseline" - @change="$emit('input', $event)" + @change="updateCommentLineStart" /> </template> <template #end> <span :class="getLineClasses(line)"> - {{ getSymbol(line) + (line.new_line || line.old_line) }} + {{ lineNumber }} </span> </template> </gl-sprintf> diff --git a/app/assets/javascripts/notes/components/multiline_comment_utils.js b/app/assets/javascripts/notes/components/multiline_comment_utils.js index dc9c55e9b30..dbae10c8f6c 100644 --- a/app/assets/javascripts/notes/components/multiline_comment_utils.js +++ b/app/assets/javascripts/notes/components/multiline_comment_utils.js @@ -7,11 +7,19 @@ export function getSymbol(type) { } function getLineNumber(lineRange, key) { - if (!lineRange || !key) return ''; - const lineCode = lineRange[`${key}_line_code`] || ''; - const lineType = lineRange[`${key}_line_type`] || ''; - const lines = lineCode.split('_') || []; - const lineNumber = lineType === 'old' ? lines[1] : lines[2]; + if (!lineRange || !key || !lineRange[key]) return ''; + const { new_line: newLine, old_line: oldLine, type } = lineRange[key]; + const otherKey = key === 'start' ? 'end' : 'start'; + + // By default we want to see the "old" or "left side" line number + // The exception is if the "end" line is on the "right" side + // `otherLineType` is only used if `type` is null to make sure the line + // number relfects the "right" side number, if that is the side + // the comment form is located on + const otherLineType = !type ? lineRange[otherKey]?.type : null; + const lineType = type || ''; + let lineNumber = oldLine; + if (lineType === 'new' || otherLineType === 'new') lineNumber = newLine; return (lineNumber && getSymbol(lineType) + lineNumber) || ''; } @@ -37,21 +45,67 @@ export function getLineClasses(line) { ]; } -export function commentLineOptions(diffLines, lineCode) { - const selectedIndex = diffLines.findIndex(line => line.line_code === lineCode); +export function commentLineOptions(diffLines, startingLine, lineCode, side = 'left') { + const preferredSide = side === 'left' ? 'old_line' : 'new_line'; + const fallbackSide = preferredSide === 'new_line' ? 'old_line' : 'new_line'; const notMatchType = l => l.type !== 'match'; + const linesCopy = [...diffLines]; // don't mutate the argument + const startingLineCode = startingLine.line_code; + + const currentIndex = linesCopy.findIndex(line => line.line_code === lineCode); // We're limiting adding comments to only lines above the current line // to make rendering simpler. Future interations will use a more // intuitive dragging interface that will make this unnecessary - const upToSelected = diffLines.slice(0, selectedIndex + 1); + const upToSelected = linesCopy.slice(0, currentIndex + 1); // Only include the lines up to the first "Show unchanged lines" block // i.e. not a "match" type const lines = takeRightWhile(upToSelected, notMatchType); - return lines.map(l => ({ - value: { lineCode: l.line_code, type: l.type }, - text: `${getSymbol(l.type)}${l.new_line || l.old_line}`, - })); + // If the selected line is "hidden" in an unchanged line block + // or "above" the current group of lines add it to the array so + // that the drop down is not defaulted to empty + const selectedIndex = lines.findIndex(line => line.line_code === startingLineCode); + if (selectedIndex < 0) lines.unshift(startingLine); + + return lines.map(l => { + const { line_code, type, old_line, new_line } = l; + return { + value: { line_code, type, old_line, new_line }, + text: `${getSymbol(type)}${l[preferredSide] || l[fallbackSide]}`, + }; + }); +} + +export function formatLineRange(start, end) { + const extractProps = ({ line_code, type, old_line, new_line }) => ({ + line_code, + type, + old_line, + new_line, + }); + return { + start: extractProps(start), + end: extractProps(end), + }; +} + +export function getCommentedLines(selectedCommentPosition, diffLines) { + if (!selectedCommentPosition) { + // This structure simplifies the logic that consumes this result + // by keeping the returned shape the same and adjusting the bounds + // to something unreachable. This way our component logic stays: + // "if index between start and end" + return { + startLine: diffLines.length + 1, + endLine: diffLines.length + 1, + }; + } + + const { start, end } = selectedCommentPosition; + const startLine = diffLines.findIndex(l => l.line_code === start.line_code); + const endLine = diffLines.findIndex(l => l.line_code === end.line_code); + + return { startLine, endLine }; } diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue index f1af8be590a..7615b0518b7 100644 --- a/app/assets/javascripts/notes/components/note_actions.vue +++ b/app/assets/javascripts/notes/components/note_actions.vue @@ -128,6 +128,9 @@ export default { isIssue() { return this.targetType === 'issue'; }, + canAssign() { + return this.getNoteableData.current_user?.can_update && this.isIssue; + }, }, methods: { onEdit() { @@ -257,23 +260,23 @@ export default { {{ __('Copy link') }} </button> </li> - <li v-if="canEdit"> + <li v-if="canAssign"> <button - class="btn btn-transparent js-note-delete js-note-delete" + class="btn-default btn-transparent" + data-testid="assign-user" type="button" - @click.prevent="onDelete" + @click="assignUser" > - <span class="text-danger">{{ __('Delete comment') }}</span> + {{ displayAssignUserText }} </button> </li> - <li v-if="isIssue"> + <li v-if="canEdit"> <button - class="btn-default btn-transparent" - data-testid="assign-user" + class="btn btn-transparent js-note-delete js-note-delete" type="button" - @click="assignUser" + @click.prevent="onDelete" > - {{ displayAssignUserText }} + <span class="text-danger">{{ __('Delete comment') }}</span> </button> </li> </ul> diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue index 795ee10ca0f..24227d55ebf 100644 --- a/app/assets/javascripts/notes/components/note_form.vue +++ b/app/assets/javascripts/notes/components/note_form.vue @@ -2,7 +2,7 @@ import { mapGetters, mapActions, mapState } from 'vuex'; import { mergeUrlParams } from '~/lib/utils/url_utility'; import eventHub from '../event_hub'; -import issueWarning from '../../vue_shared/components/issue/issue_warning.vue'; +import NoteableWarning from '../../vue_shared/components/notes/noteable_warning.vue'; import markdownField from '../../vue_shared/components/markdown/field.vue'; import issuableStateMixin from '../mixins/issuable_state'; import resolvable from '../mixins/resolvable'; @@ -12,7 +12,7 @@ import { getDraft, updateDraft } from '~/lib/utils/autosave'; export default { name: 'NoteForm', components: { - issueWarning, + NoteableWarning, markdownField, }, mixins: [issuableStateMixin, resolvable], @@ -101,6 +101,7 @@ export default { isResolving: this.resolveDiscussion, isUnresolving: !this.resolveDiscussion, resolveAsThread: true, + isSubmittingWithKeydown: false, }; }, computed: { @@ -241,6 +242,10 @@ export default { this.$emit('cancelForm', shouldConfirm, this.noteBody !== this.updatedNoteBody); }, onInput() { + if (this.isSubmittingWithKeydown) { + return; + } + if (this.autosaveKey) { const { autosaveKey, updatedNoteBody: text } = this; updateDraft(autosaveKey, text); @@ -250,6 +255,7 @@ export default { if (this.showBatchCommentsActions) { this.handleAddToReview(); } else { + this.isSubmittingWithKeydown = true; this.handleUpdate(); } }, @@ -303,12 +309,12 @@ export default { ></div> <div class="flash-container timeline-content"></div> <form :data-line-code="lineCode" class="edit-note common-note-form js-quick-submit gfm-form"> - <issue-warning + <noteable-warning v-if="hasWarning(getNoteableData)" :is-locked="isLocked(getNoteableData)" :is-confidential="isConfidential(getNoteableData)" - :locked-issue-docs-path="lockedIssueDocsPath" - :confidential-issue-docs-path="confidentialIssueDocsPath" + :locked-noteable-docs-path="lockedIssueDocsPath" + :confidential-noteable-docs-path="confidentialIssueDocsPath" /> <markdown-field @@ -404,7 +410,7 @@ export default { </button> <button v-if="discussion.resolvable" - class="btn btn-nr btn-default append-right-10 js-comment-resolve-button" + class="btn btn-nr btn-default gl-mr-3 js-comment-resolve-button" @click.prevent="handleUpdate(true)" > {{ resolveButtonTitle }} diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 0e4dd1b9c84..9bf8cffe940 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -21,6 +21,7 @@ import { getEndLineNumber, getLineClasses, commentLineOptions, + formatLineRange, } from './multiline_comment_utils'; import MultilineCommentForm from './multiline_comment_form.vue'; @@ -62,10 +63,15 @@ export default { default: false, }, diffLines: { - type: Object, + type: Array, required: false, default: null, }, + discussionRoot: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -73,10 +79,7 @@ export default { isDeleting: false, isRequesting: false, isResolving: false, - commentLineStart: { - line_code: this.line?.line_code, - type: this.line?.type, - }, + commentLineStart: {}, }; }, computed: { @@ -144,28 +147,46 @@ export default { return getEndLineNumber(this.lineRange); }, showMultiLineComment() { - return ( - this.glFeatures.multilineComments && - this.startLineNumber && - this.endLineNumber && - (this.startLineNumber !== this.endLineNumber || this.isEditing) - ); + if (!this.glFeatures.multilineComments || !this.discussionRoot) return false; + if (this.isEditing) return true; + + return this.line && this.startLineNumber !== this.endLineNumber; }, commentLineOptions() { - if (this.diffLines) { - return commentLineOptions(this.diffLines, this.line.line_code); + if (!this.diffFile || !this.line) return []; + + const sideA = this.line.type === 'new' ? 'right' : 'left'; + const sideB = sideA === 'left' ? 'right' : 'left'; + const lines = this.diffFile.highlighted_diff_lines.length + ? this.diffFile.highlighted_diff_lines + : this.diffFile.parallel_diff_lines.map(l => l[sideA] || l[sideB]); + return commentLineOptions(lines, this.commentLineStart, this.line.line_code, sideA); + }, + diffFile() { + if (this.commentLineStart.line_code) { + const lineCode = this.commentLineStart.line_code.split('_')[0]; + return this.getDiffFileByHash(lineCode); } - const diffFile = this.diffFile || this.getDiffFileByHash(this.targetNoteHash); - if (!diffFile) return null; - return commentLineOptions(diffFile.highlighted_diff_lines, this.line.line_code); + return null; }, }, - created() { + const line = this.note.position?.line_range?.start || this.line; + + this.commentLineStart = line + ? { + line_code: line.line_code, + type: line.type, + old_line: line.old_line, + new_line: line.new_line, + } + : {}; + eventHub.$on('enterEditMode', ({ noteId }) => { if (noteId === this.note.id) { this.isEditing = true; + this.setSelectedCommentPositionHover(); this.scrollToNoteIfNeeded($(this.$el)); } }); @@ -185,9 +206,11 @@ export default { 'toggleResolveNote', 'scrollToNoteIfNeeded', 'updateAssignees', + 'setSelectedCommentPositionHover', ]), editHandler() { this.isEditing = true; + this.setSelectedCommentPositionHover(); this.$emit('handleEdit'); }, deleteHandler() { @@ -224,13 +247,11 @@ export default { formUpdateHandler(noteText, parentElement, callback, resolveDiscussion) { const position = { ...this.note.position, - line_range: { - start_line_code: this.commentLineStart?.lineCode, - start_line_type: this.commentLineStart?.type, - end_line_code: this.line?.line_code, - end_line_type: this.line?.type, - }, }; + + if (this.commentLineStart && this.line) + position.line_range = formatLineRange(this.commentLineStart, this.line); + this.$emit('handleUpdateNote', { note: this.note, noteText, @@ -246,7 +267,7 @@ export default { note: { target_type: this.getNoteableData.targetType, target_id: this.note.noteable_id, - note: { note: noteText }, + note: { note: noteText, position: JSON.stringify(position) }, }, }; this.isRequesting = true; @@ -266,6 +287,7 @@ export default { } else { this.isRequesting = false; this.isEditing = true; + this.setSelectedCommentPositionHover(); this.$nextTick(() => { const msg = __('Something went wrong while editing your comment. Please try again.'); Flash(msg, 'alert', this.$el); @@ -317,14 +339,17 @@ export default { > <div v-if="showMultiLineComment" data-testid="multiline-comment"> <multiline-comment-form - v-if="isEditing && commentLineOptions && line" + v-if="isEditing && note.position" v-model="commentLineStart" :line="line" :comment-line-options="commentLineOptions" :line-range="note.position.line_range" - class="gl-mb-3 gl-text-gray-700 gl-border-gray-200 gl-border-b-solid gl-border-b-1 gl-pb-3" + class="gl-mb-3 gl-text-gray-700 gl-pb-3" /> - <div v-else class="gl-mb-3 gl-text-gray-700"> + <div + v-else + class="gl-mb-3 gl-text-gray-700 gl-border-gray-200 gl-border-b-solid gl-border-b-1 gl-pb-3" + > <gl-sprintf :message="__('Comment on lines %{startLine} to %{endLine}')"> <template #startLine> <span :class="getLineClasses(startLineNumber)">{{ startLineNumber }}</span> diff --git a/app/assets/javascripts/notes/components/sort_discussion.vue b/app/assets/javascripts/notes/components/sort_discussion.vue index 4a7543819eb..60b531d7597 100644 --- a/app/assets/javascripts/notes/components/sort_discussion.vue +++ b/app/assets/javascripts/notes/components/sort_discussion.vue @@ -49,7 +49,10 @@ export default { </script> <template> - <div class="mr-2 d-inline-block align-bottom full-width-mobile"> + <div + data-testid="sort-discussion-filter" + class="gl-mr-2 gl-display-inline-block gl-vertical-align-bottom full-width-mobile" + > <local-storage-sync :value="sortDirection" :storage-key="storageKey" |