diff options
author | Mike Greiling <mike@pixelcog.com> | 2018-11-08 07:58:45 +0000 |
---|---|---|
committer | Mike Greiling <mike@pixelcog.com> | 2018-11-08 07:58:45 +0000 |
commit | 15e519451e843b6d55a2c35bbefe039003b15c25 (patch) | |
tree | d7deeb2234938180fda92f17ebcfad22611fd07f | |
parent | b5a79f15369678fd959e6db290e2f29eab3ca84c (diff) | |
parent | cd5ddc4f2e3088a906417741a019292479ab84a6 (diff) | |
download | gitlab-ce-15e519451e843b6d55a2c35bbefe039003b15c25.tar.gz |
Merge branch '_acet-discussion-redesign' into 'master'
Discussions redesign
Closes #29294
See merge request gitlab-org/gitlab-ce!22333
24 files changed, 629 insertions, 276 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index e19207bdc95..b9de487a737 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -76,7 +76,6 @@ export default { <noteable-discussion v-show="isExpanded(discussion)" :discussion="discussion" - :render-header="false" :render-diff-file="false" :always-expanded="true" :discussions-by-diff-order="true" 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 3339c56cbb6..3b71c0a1fd4 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -76,8 +76,9 @@ export default { :class="className" class="notes_holder" > - <td class="notes_line old"></td> - <td class="notes_content parallel old"> + <td + class="notes_content parallel old" + colspan="2"> <div v-if="shouldRenderDiscussionsOnLeft" class="content" @@ -95,8 +96,9 @@ export default { line-position="left" /> </td> - <td class="notes_line new"></td> - <td class="notes_content parallel new"> + <td + class="notes_content parallel new" + colspan="2"> <div v-if="shouldRenderDiscussionsOnRight" class="content" diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index 554db102027..754c6e79ee4 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -321,10 +321,10 @@ Please check your network connection and try again.`; v-else-if="!canCreateNote" :issuable-type="issuableTypeTitle" /> - <ul + <div v-else-if="canCreateNote" class="notes notes-form timeline"> - <li class="timeline-entry"> + <div class="timeline-entry note-form"> <div class="timeline-entry-inner"> <div class="flash-container error-alert timeline-content"></div> <div class="timeline-icon d-none d-sm-none d-md-block"> @@ -462,7 +462,7 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" </form> </div> </div> - </li> - </ul> + </div> + </div> </div> </template> diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue index 1f80f24e045..a4d76a70696 100644 --- a/app/assets/javascripts/notes/components/discussion_counter.vue +++ b/app/assets/javascripts/notes/components/discussion_counter.vue @@ -1,9 +1,6 @@ <script> import { mapActions, mapGetters } from 'vuex'; -import resolveSvg from 'icons/_icon_resolve_discussion.svg'; -import resolvedSvg from 'icons/_icon_status_success_solid.svg'; -import mrIssueSvg from 'icons/_icon_mr_issue.svg'; -import nextDiscussionSvg from 'icons/_next_discussion.svg'; +import Icon from '~/vue_shared/components/icon.vue'; import { pluralize } from '../../lib/utils/text_utility'; import discussionNavigation from '../mixins/discussion_navigation'; import tooltip from '../../vue_shared/directives/tooltip'; @@ -12,6 +9,9 @@ export default { directives: { tooltip, }, + components: { + Icon, + }, mixins: [discussionNavigation], computed: { ...mapGetters([ @@ -37,12 +37,6 @@ export default { return this.getNoteableData.create_issue_to_resolve_discussions_path; }, }, - created() { - this.resolveSvg = resolveSvg; - this.resolvedSvg = resolvedSvg; - this.mrIssueSvg = mrIssueSvg; - this.nextDiscussionSvg = nextDiscussionSvg; - }, methods: { ...mapActions(['expandDiscussion']), jumpToFirstUnresolvedDiscussion() { @@ -66,15 +60,9 @@ export default { <span :class="{ 'is-active': allResolved }" class="line-resolve-btn is-disabled" - type="button"> - <span - v-if="allResolved" - v-html="resolvedSvg" - ></span> - <span - v-else - v-html="resolveSvg" - ></span> + type="button" + > + <icon name="check-circle" /> </span> <span class="line-resolve-text"> {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ countText }} resolved @@ -90,7 +78,7 @@ export default { :title="s__('Resolve all discussions in new issue')" data-container="body" class="new-issue-for-discussion btn btn-default discussion-create-issue-btn"> - <span v-html="mrIssueSvg"></span> + <icon name="issue-new" /> </a> </div> <div @@ -103,7 +91,7 @@ export default { data-container="body" class="btn btn-default discussion-next-btn" @click="jumpToFirstUnresolvedDiscussion"> - <span v-html="nextDiscussionSvg"></span> + <icon name="comment-next" /> </button> </div> </div> diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue index 01cbe40f444..f7a61fbfcd4 100644 --- a/app/assets/javascripts/notes/components/note_actions.vue +++ b/app/assets/javascripts/notes/components/note_actions.vue @@ -1,12 +1,5 @@ <script> import { mapGetters } from 'vuex'; -import emojiSmiling from 'icons/_emoji_slightly_smiling_face.svg'; -import emojiSmile from 'icons/_emoji_smile.svg'; -import emojiSmiley from 'icons/_emoji_smiley.svg'; -import editSvg from 'icons/_icon_pencil.svg'; -import resolveDiscussionSvg from 'icons/_icon_resolve_discussion.svg'; -import resolvedDiscussionSvg from 'icons/_icon_status_success_solid.svg'; -import ellipsisSvg from 'icons/_ellipsis_v.svg'; import Icon from '~/vue_shared/components/icon.vue'; import tooltip from '~/vue_shared/directives/tooltip'; import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; @@ -110,15 +103,6 @@ export default { return title; }, }, - created() { - this.emojiSmiling = emojiSmiling; - this.emojiSmile = emojiSmile; - this.emojiSmiley = emojiSmiley; - this.editSvg = editSvg; - this.ellipsisSvg = ellipsisSvg; - this.resolveDiscussionSvg = resolveDiscussionSvg; - this.resolvedDiscussionSvg = resolvedDiscussionSvg; - }, methods: { onEdit() { this.$emit('handleEdit'); @@ -152,12 +136,7 @@ export default { class="line-resolve-btn note-action-button" @click="onResolve"> <template v-if="!isResolving"> - <div - v-if="isResolved" - v-html="resolvedDiscussionSvg"></div> - <div - v-else - v-html="resolveDiscussionSvg"></div> + <icon name="check-circle" /> </template> <gl-loading-icon v-else @@ -179,18 +158,18 @@ export default { title="Add reaction" > <gl-loading-icon inline/> - <span - class="link-highlight award-control-icon-neutral" - v-html="emojiSmiling"> - </span> - <span - class="link-highlight award-control-icon-positive" - v-html="emojiSmiley"> - </span> - <span - class="link-highlight award-control-icon-super-positive" - v-html="emojiSmile"> - </span> + <icon + css-classes="link-highlight award-control-icon-neutral" + name="emoji_slightly_smiling_face" + /> + <icon + css-classes="link-highlight award-control-icon-positive" + name="emoji_smiley" + /> + <icon + css-classes="link-highlight award-control-icon-super-positive" + name="emoji_smiley" + /> </a> </div> <div @@ -204,10 +183,10 @@ export default { data-container="body" data-placement="bottom" @click="onEdit"> - <span - class="link-highlight" - v-html="editSvg"> - </span> + <icon + name="pencil" + css-classes="link-highlight" + /> </button> </div> <div @@ -240,10 +219,10 @@ export default { data-toggle="dropdown" data-container="body" data-placement="bottom"> - <span - class="icon" - v-html="ellipsisSvg"> - </span> + <icon + css-classes="icon" + name="ellipsis_v" + /> </button> <ul class="dropdown-menu more-actions-dropdown dropdown-open-left"> <li v-if="canReportAsAbuse"> diff --git a/app/assets/javascripts/notes/components/note_awards_list.vue b/app/assets/javascripts/notes/components/note_awards_list.vue index df7ab4502a6..401bcfabbe4 100644 --- a/app/assets/javascripts/notes/components/note_awards_list.vue +++ b/app/assets/javascripts/notes/components/note_awards_list.vue @@ -1,13 +1,14 @@ <script> import { mapActions, mapGetters } from 'vuex'; -import emojiSmiling from 'icons/_emoji_slightly_smiling_face.svg'; -import emojiSmile from 'icons/_emoji_smile.svg'; -import emojiSmiley from 'icons/_emoji_smiley.svg'; +import Icon from '~/vue_shared/components/icon.vue'; import Flash from '../../flash'; import { glEmojiTag } from '../../emoji'; import tooltip from '../../vue_shared/directives/tooltip'; export default { + components: { + Icon, + }, directives: { tooltip, }, @@ -72,11 +73,6 @@ export default { return this.noteAuthorId === this.getUserData.id; }, }, - created() { - this.emojiSmiling = emojiSmiling; - this.emojiSmile = emojiSmile; - this.emojiSmiley = emojiSmiley; - }, methods: { ...mapActions(['toggleAwardRequest']), getAwardHTML(name) { @@ -196,17 +192,14 @@ export default { data-boundary="viewport" data-placement="bottom" type="button"> - <span - class="award-control-icon award-control-icon-neutral" - v-html="emojiSmiling"> + <span class="award-control-icon award-control-icon-neutral"> + <icon name="emoji_slightly_smiling_face" /> </span> - <span - class="award-control-icon award-control-icon-positive" - v-html="emojiSmiley"> + <span class="award-control-icon award-control-icon-positive"> + <icon name="emoji_smiley" /> </span> - <span - class="award-control-icon award-control-icon-super-positive" - v-html="emojiSmile"> + <span class="award-control-icon award-control-icon-super-positive"> + <icon name="emoji_smiley" /> </span> <i aria-hidden="true" diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index 7b6e7b72caf..dd7313d7b10 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -45,6 +45,9 @@ export default { noteTimestampLink() { return `#note_${this.noteId}`; }, + hasAuthor() { + return this.author && Object.keys(this.author).length; + }, }, methods: { ...mapActions(['setTargetNoteHash']), @@ -76,7 +79,7 @@ export default { </button> </div> <a - v-if="Object.keys(author).length" + v-if="hasAuthor" :href="author.path" > <span class="note-header-author-name">{{ author.name }}</span> @@ -92,9 +95,6 @@ export default { </span> <span class="note-headline-light"> <span class="note-headline-meta"> - <template v-if="actionText"> - {{ actionText }} - </template> <span class="system-note-message"> <slot></slot> </span> @@ -102,7 +102,9 @@ export default { v-if="createdAt" > <span class="system-note-separator"> - · + <template v-if="actionText"> + {{ actionText }} + </template> </span> <a :href="noteTimestampLink" diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 07115ca07c4..a153edd0476 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -1,16 +1,16 @@ <script> import { mapActions, mapGetters } from 'vuex'; -import resolveDiscussionsSvg from 'icons/_icon_mr_issue.svg'; -import nextDiscussionsSvg from 'icons/_next_discussion.svg'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { truncateSha } from '~/lib/utils/text_utility'; -import systemNote from '~/vue_shared/components/notes/system_note.vue'; import { s__ } from '~/locale'; +import systemNote from '~/vue_shared/components/notes/system_note.vue'; +import icon from '~/vue_shared/components/icon.vue'; import Flash from '../../flash'; import { SYSTEM_NOTE } from '../constants'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import noteableNote from './noteable_note.vue'; import noteHeader from './note_header.vue'; +import toggleRepliesWidget from './toggle_replies_widget.vue'; import noteSignedOutWidget from './note_signed_out_widget.vue'; import noteEditedText from './note_edited_text.vue'; import noteForm from './note_form.vue'; @@ -26,6 +26,7 @@ import tooltip from '../../vue_shared/directives/tooltip'; export default { name: 'NoteableDiscussion', components: { + icon, noteableNote, diffWithNote, userAvatarLink, @@ -33,6 +34,7 @@ export default { noteSignedOutWidget, noteEditedText, noteForm, + toggleRepliesWidget, placeholderNote, placeholderSystemNote, systemNote, @@ -46,11 +48,6 @@ export default { type: Object, required: true, }, - renderHeader: { - type: Boolean, - required: false, - default: true, - }, renderDiffFile: { type: Boolean, required: false, @@ -72,6 +69,7 @@ export default { isReplying: false, isResolving: false, resolveAsThread: true, + isRepliesCollapsed: (!this.discussion.diff_discussion && this.discussion.resolved) || false, }; }, computed: { @@ -112,6 +110,15 @@ export default { newNotePath() { return this.getNoteableData.create_note_path; }, + hasReplies() { + return this.discussion.notes.length > 1; + }, + initialDiscussion() { + return this.discussion.notes.slice(0, 1)[0]; + }, + replies() { + return this.discussion.notes.slice(1); + }, lastUpdatedBy() { const { notes } = this.discussion; @@ -147,6 +154,12 @@ export default { return diffDiscussion && diffFile && this.renderDiffFile; }, + shouldGroupReplies() { + return !this.shouldRenderDiffs && !this.transformedDiscussion.diffDiscussion; + }, + shouldRenderHeader() { + return this.shouldRenderDiffs; + }, wrapperComponent() { return this.shouldRenderDiffs ? diffWithNote : 'div'; }, @@ -160,6 +173,22 @@ export default { wrapperClass() { return this.isDiffDiscussion ? '' : 'card discussion-wrapper'; }, + componentClassName() { + if (this.shouldRenderDiffs) { + if (!this.lastUpdatedAt && !this.discussion.resolved) { + return 'unresolved'; + } + } + + return ''; + }, + shouldShowDiscussions() { + const isExpanded = this.discussion.expanded; + const { diffDiscussion, resolved } = this.transformedDiscussion; + const isResolvedNonDiffDiscussion = !diffDiscussion && resolved; + + return isExpanded || this.alwaysExpanded || isResolvedNonDiffDiscussion; + }, }, watch: { isReplying() { @@ -173,10 +202,6 @@ export default { } }, }, - created() { - this.resolveDiscussionsSvg = resolveDiscussionsSvg; - this.nextDiscussionsSvg = nextDiscussionsSvg; - }, methods: { ...mapActions([ 'saveNote', @@ -207,6 +232,9 @@ export default { toggleDiscussionHandler() { this.toggleDiscussion({ discussionId: this.discussion.id }); }, + toggleReplies() { + this.isRepliesCollapsed = !this.isRepliesCollapsed; + }, showReplyForm() { this.isReplying = true; }, @@ -274,26 +302,29 @@ Please check your network connection and try again.`; </script> <template> - <li class="note note-discussion timeline-entry"> + <li + class="note note-discussion timeline-entry" + :class="componentClassName" + > <div class="timeline-entry-inner"> - <div class="timeline-icon"> - <user-avatar-link - v-if="author" - :link-href="author.path" - :img-src="author.avatar_url" - :img-alt="author.name" - :img-size="40" - /> - </div> <div class="timeline-content"> <div :data-discussion-id="transformedDiscussion.discussion_id" class="discussion js-discussion-container" > <div - v-if="renderHeader" - class="discussion-header" + v-if="shouldRenderHeader" + class="discussion-header note-wrapper" > + <div class="timeline-icon"> + <user-avatar-link + v-if="author" + :link-href="author.path" + :img-src="author.avatar_url" + :img-alt="author.name" + :img-size="40" + /> + </div> <note-header :author="author" :created-at="transformedDiscussion.created_at" @@ -339,7 +370,7 @@ Please check your network connection and try again.`; /> </div> <div - v-if="discussion.expanded || alwaysExpanded" + v-if="shouldShowDiscussions" class="discussion-body"> <component :is="wrapperComponent" @@ -348,45 +379,70 @@ Please check your network connection and try again.`; > <div class="discussion-notes"> <ul class="notes"> - <component - :is="componentName(note)" - v-for="(note, index) in discussion.notes" - :key="note.id" - :note="componentData(note)" - @handleDeleteNote="deleteNoteHandler" - > - <slot - v-if="index === 0" - slot="avatar-badge" - name="avatar-badge" + <template v-if="shouldGroupReplies"> + <component + :is="componentName(initialDiscussion)" + :note="componentData(initialDiscussion)" + @handleDeleteNote="deleteNoteHandler" > - </slot> - </component> + <slot + slot="avatar-badge" + name="avatar-badge" + > + </slot> + </component> + <toggle-replies-widget + v-if="hasReplies" + :collapsed="isRepliesCollapsed" + :replies="replies" + @toggle="toggleReplies" + /> + <template v-if="!isRepliesCollapsed"> + <component + :is="componentName(note)" + v-for="note in replies" + :key="note.id" + :note="componentData(note)" + @handleDeleteNote="deleteNoteHandler" + /> + </template> + </template> + <template v-else> + <component + :is="componentName(note)" + v-for="(note, index) in discussion.notes" + :key="note.id" + :note="componentData(note)" + @handleDeleteNote="deleteNoteHandler" + > + <slot + v-if="index === 0" + slot="avatar-badge" + name="avatar-badge" + > + </slot> + </component> + </template> </ul> <div + v-if="!isRepliesCollapsed" :class="{ 'is-replying': isReplying }" class="discussion-reply-holder" > <template v-if="!isReplying && canReply"> - <div - class="btn-group d-flex discussion-with-resolve-btn" - role="group"> - <div - class="btn-group w-100" - role="group"> - <button - type="button" - class="js-vue-discussion-reply btn btn-text-field mr-2 qa-discussion-reply" - title="Add a reply" - @click="showReplyForm">Reply...</button> - </div> - <div - v-if="discussion.resolvable" - class="btn-group" - role="group"> + <div class="discussion-with-resolve-btn"> + <button + type="button" + class="js-vue-discussion-reply btn btn-text-field mr-2 qa-discussion-reply" + title="Add a reply" + @click="showReplyForm" + > + Reply... + </button> + <div v-if="discussion.resolvable"> <button type="button" - class="btn btn-default" + class="btn btn-default mx-sm-2" @click="resolveHandler()" > <i @@ -414,7 +470,7 @@ Please check your network connection and try again.`; btn-default discussion-create-issue-btn" data-container="body" > - <span v-html="resolveDiscussionsSvg"></span> + <icon name="issue-new" /> </a> </div> <div @@ -428,7 +484,7 @@ Please check your network connection and try again.`; data-container="body" @click="jumpToNextDiscussion" > - <span v-html="nextDiscussionsSvg"></span> + <icon name="comment-next" /> </button> </div> </div> diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 40222ac4a80..e302a89ee95 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -173,7 +173,7 @@ export default { :class="classNameBindings" :data-award-url="note.toggle_award_path" :data-note-id="note.id" - class="note timeline-entry" + class="note timeline-entry note-wrapper" > <div class="timeline-entry-inner"> <div class="timeline-icon"> @@ -196,6 +196,7 @@ export default { :author="author" :created-at="note.created_at" :note-id="note.id" + action-text="commented" /> <note-actions :author-id="author.id" diff --git a/app/assets/javascripts/notes/components/toggle_replies_widget.vue b/app/assets/javascripts/notes/components/toggle_replies_widget.vue new file mode 100644 index 00000000000..78ecbbb9247 --- /dev/null +++ b/app/assets/javascripts/notes/components/toggle_replies_widget.vue @@ -0,0 +1,94 @@ +<script> +import _ from 'underscore'; +import Icon from '~/vue_shared/components/icon.vue'; +import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; +import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; + +export default { + components: { + Icon, + UserAvatarLink, + TimeAgoTooltip, + }, + props: { + collapsed: { + type: Boolean, + required: true, + }, + replies: { + type: Array, + required: true, + }, + }, + computed: { + lastReply() { + return this.replies[this.replies.length - 1]; + }, + uniqueAuthors() { + const authors = this.replies.map(reply => reply.author || {}); + + return _.uniq(authors, author => author.username); + }, + className() { + return this.collapsed ? 'collapsed' : 'expanded'; + }, + }, + methods: { + toggle() { + this.$emit('toggle'); + }, + }, +}; +</script> + +<template> + <li + :class="className" + class="replies-toggle" + > + <template v-if="collapsed"> + <icon + name="chevron-right" + @click.native="toggle" + /> + <div> + <user-avatar-link + v-for="author in uniqueAuthors" + :key="author.username" + :link-href="author.path" + :img-alt="author.name" + :img-src="author.avatar_url" + :img-size="26" + :tooltip-text="author.name" + tooltip-placement="bottom" + /> + </div> + <button + class="btn btn-link js-replies-text" + type="button" + @click="toggle" + > + {{ replies.length }} {{ n__('reply', 'replies', replies.length) }} + </button> + {{ __('Last reply by') }} + <a + :href="lastReply.author.path" + class="btn btn-link author-link" + > + {{ lastReply.author.name }} + </a> + <time-ago-tooltip + :time="lastReply.created_at" + tooltip-placement="bottom" + /> + </template> + <span + v-else + class="collapse-replies-btn js-collapse-replies" + @click="toggle" + > + <icon name="chevron-down" /> + {{ s__('Notes|Collapse replies') }} + </span> + </li> +</template> diff --git a/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue b/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue index f56414c3c63..8cb72afcdc0 100644 --- a/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue @@ -10,7 +10,7 @@ export default { </script> <template> - <li class="timeline-entry note"> + <li class="timeline-entry note note-wrapper"> <div class="timeline-entry-inner"> <div class="timeline-icon"> </div> diff --git a/app/assets/javascripts/vue_shared/components/notes/system_note.vue b/app/assets/javascripts/vue_shared/components/notes/system_note.vue index de3c7a80365..6a44e6a29ed 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -76,7 +76,7 @@ export default { <li :id="noteAnchorId" :class="{ target: isTargetNote }" - class="note system-note timeline-entry"> + class="note system-note timeline-entry note-wrapper"> <div class="timeline-entry-inner"> <div class="timeline-icon" diff --git a/app/assets/stylesheets/framework/awards.scss b/app/assets/stylesheets/framework/awards.scss index 702276780e9..7a95db5976d 100644 --- a/app/assets/stylesheets/framework/awards.scss +++ b/app/assets/stylesheets/framework/awards.scss @@ -148,10 +148,7 @@ .award-control-icon svg { background: $award-emoji-positive-add-bg; - - path { - fill: $award-emoji-positive-add-lines; - } + fill: $award-emoji-positive-add-lines; } .award-control-icon-neutral { diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index 1dbb5ca3e4c..219fd99b097 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -222,6 +222,25 @@ } } + &.btn-text-field { + width: 100%; + text-align: left; + padding: 6px 16px; + border-color: $border-color; + color: $gray-darkest; + background-color: $gray-light; + + &:hover, + &:active, + &:focus { + cursor: text; + box-shadow: none; + border-color: lighten($blue-300, 20%); + color: $gray-darkest; + background-color: $gray-light; + } + } + &.dot-highlight::after { content: ''; background-color: $blue-500; @@ -339,25 +358,6 @@ } } -.btn-text-field { - width: 100%; - text-align: left; - padding: 6px 16px; - border-color: $border-color; - color: $gray-darkest; - background-color: $gray-light; - - &:hover, - &:active, - &:focus { - cursor: text; - box-shadow: none; - border-color: lighten($blue-300, 20%); - color: $gray-darkest; - background-color: $gray-light; - } -} - .btn-build { margin-left: 10px; diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 53f198b47c6..6bdcb20210b 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -36,7 +36,6 @@ text-align: left; padding: 10px $gl-padding; word-wrap: break-word; - border-radius: $border-radius-default $border-radius-default 0 0; &.file-title-clear { padding-left: 0; diff --git a/app/assets/stylesheets/framework/timeline.scss b/app/assets/stylesheets/framework/timeline.scss index dfb145debe7..4a311da1675 100644 --- a/app/assets/stylesheets/framework/timeline.scss +++ b/app/assets/stylesheets/framework/timeline.scss @@ -1,7 +1,7 @@ .timeline { - @include basic-list; margin: 0; padding: 0; + list-style: none; &::before { @include notes-media('max', map-get($grid-breakpoints, sm)) { @@ -26,10 +26,8 @@ } .timeline-entry { - border-color: $white-normal; color: $gl-text-color; - border-bottom: 1px solid $border-white-light; - background: $white-light; + background-color: $white-light; .timeline-entry-inner { position: relative; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 19bc4262e21..6d998fa1e07 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -59,6 +59,7 @@ margin: 0; padding: 0; table-layout: fixed; + border-radius: 0 0 $border-radius-default $border-radius-default; .diff-line-num { width: 50px; @@ -859,7 +860,7 @@ } .diff-file .note-container > .new-note, -.note-container .discussion-notes { +.note-container .discussion-notes.diff-discussions { margin-left: 100px; border-left: 1px solid $white-normal; } diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index c60bb360a03..855d73a9939 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -239,6 +239,7 @@ .discussion-reply-holder { background-color: $white-light; padding: 10px 16px; + border-radius: 0 0 $border-radius-default $border-radius-default; &.is-replying { padding-bottom: $gl-padding; @@ -247,10 +248,15 @@ } .discussion-with-resolve-btn { + @include media-breakpoint-up(sm) { + display: flex; + } + + .discussion-actions { display: table; - .btn-default path { + svg { fill: $gray-darkest; } @@ -270,6 +276,12 @@ .btn { width: 100%; } + + .btn-text-field { + @include media-breakpoint-down(xs) { + margin-bottom: $gl-padding-8; + } + } } .discussion-notes-count { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index be535ade0a6..dcb1275d182 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -1,26 +1,135 @@ -/** - * Notes - */ +$system-note-icon-size: 32px; +$system-note-svg-size: 16px; +$note-form-margin-left: 70px; -@-webkit-keyframes targe3-note { - from { - background: $note-targe3-outside; +@mixin vertical-line($left) { + &::before { + content: ''; + border-left: 2px solid $theme-gray-100; + position: absolute; + top: 0; + bottom: 0; + left: $left; } +} - 50% { - background: $note-targe3-inside; - } +.note-wrapper { + padding: $gl-padding; +} + +.issuable-discussion { + .notes.timeline > .timeline-entry { + border: 1px solid $border-color; + border-radius: $border-radius-default; + margin: $gl-padding 0; + + &.system-note, + &.note-form { + border: 0; + } + + &.note-form { + margin-left: 0; - to { - background: $note-targe3-outside; + @include notes-media('min', map-get($grid-breakpoints, md)) { + margin-left: $note-form-margin-left; + } + + .timeline-icon { + @include notes-media('min', map-get($grid-breakpoints, sm)) { + margin-left: -$note-icon-gutter-width; + } + } + + .timeline-content { + margin-left: 0; + } + } + + .notes_content { + border: 0; + border-top: 1px solid $border-color; + } } } -ul.notes { +.main-notes-list { + @include vertical-line(39px); +} + +.notes { display: block; list-style: none; margin: 0; padding: 0; + position: relative; + + > .note-discussion { + .card { + border: 0; + } + + li.note { + border-bottom: 1px solid $border-color; + + &:first-child { + border-radius: $border-radius-default $border-radius-default 0 0; + } + } + } + + .replies-toggle { + background-color: $gray-light; + padding: $gl-padding-8 $gl-padding; + + .collapse-replies-btn:hover { + color: $blue-600; + } + + &.expanded { + border-bottom: 1px solid $border-color; + + span { + cursor: pointer; + } + + svg { + position: relative; + top: 3px; + } + } + + &.collapsed { + color: $gl-text-color-secondary; + + svg { + float: left; + position: relative; + top: $gl-padding-4; + margin-right: $gl-padding-8; + cursor: pointer; + } + + img { + margin: -2px 4px 0 0; + } + + .author-link { + color: $gl-text-color; + } + } + + .user-avatar-link { + &:last-child img { + margin-right: $gl-padding-8; + } + } + + .btn-link { + border: 0; + vertical-align: baseline; + } + } .note-created-ago, .note-updated-at { @@ -28,8 +137,6 @@ ul.notes { } .discussion-body { - padding-top: 8px; - .card { margin-bottom: 0; } @@ -46,21 +153,10 @@ ul.notes { } > li { - // .timeline-entry - padding: 0; display: block; position: relative; border-bottom: 0; - @include notes-media('min', map-get($grid-breakpoints, sm)) { - padding-left: $note-icon-gutter-width; - } - - .timeline-entry-inner { - padding: $gl-padding $gl-btn-padding; - border-bottom: 1px solid $white-normal; - } - &:target, &.target { border-bottom: 1px solid $white-normal; @@ -75,23 +171,10 @@ ul.notes { } } - .timeline-icon { - @include notes-media('min', map-get($grid-breakpoints, sm)) { - margin-left: -$note-icon-gutter-width; - } - } - - .timeline-content { - margin-left: $note-icon-gutter-width; - - @include notes-media('min', map-get($grid-breakpoints, sm)) { - margin-left: 0; - } - } - &.being-posted { pointer-events: none; opacity: 0.5; + padding: $gl-padding; .dummy-avatar { background-color: $gl-gray-200; @@ -104,12 +187,6 @@ ul.notes { } } - &.note-discussion { - .timeline-entry-inner { - padding: $gl-padding 10px; - } - } - .editing-spinner { display: none; } @@ -191,8 +268,9 @@ ul.notes { } .system-note { - font-size: 14px; - clear: both; + padding: 6px $gl-padding-24; + margin: $gl-padding-24 0; + background-color: transparent; .note-header-info { padding-bottom: 0; @@ -225,17 +303,21 @@ ul.notes { .timeline-icon { float: left; - - @include notes-media('min', map-get($grid-breakpoints, sm)) { - margin-left: 0; - width: auto; - } + display: flex; + align-items: center; + background-color: $white-light; + width: $system-note-icon-size; + height: $system-note-icon-size; + border: 1px solid $border-color; + border-radius: $system-note-icon-size; + margin: -6px $gl-padding 0 0; svg { - width: 16px; - height: 16px; + width: $system-note-svg-size; + height: $system-note-svg-size; fill: $gray-darkest; - margin-top: 2px; + display: block; + margin: 0 auto; } } @@ -302,10 +384,17 @@ ul.notes { .discussion-body .diff-file { .file-title { cursor: default; + line-height: 42px; + padding: 0 $gl-padding; + border-top: 1px solid $border-color; &:hover { background-color: $gray-light; } + + .btn-clipboard { + top: 10px; + } } .line_content { @@ -320,6 +409,23 @@ ul.notes { } } + .discussion-notes { + &:not(:first-child) { + border-top: 1px solid $white-normal; + margin-top: 20px; + } + + &:not(:last-child) { + border-bottom: 1px solid $white-normal; + margin-bottom: 20px; + } + + .system-note { + margin: 0; + padding: $gl-padding; + } + } + // Merge request notes in diffs // Diff is inline .notes_content .note-header .note-headline-light { @@ -335,7 +441,6 @@ ul.notes { border-left: 0; &.notes_content { - background-color: $gray-light; border-width: 1px 0; padding: 0; vertical-align: top; @@ -349,18 +454,6 @@ ul.notes { } } - .discussion-notes { - &:not(:first-child) { - border-top: 1px solid $white-normal; - margin-top: 20px; - } - - &:not(:last-child) { - border-bottom: 1px solid $white-normal; - margin-bottom: 20px; - } - } - .notes { background-color: $white-light; } @@ -374,6 +467,30 @@ ul.notes { } } +.diffs { + .discussion-notes { + margin-left: 0; + border-left: 0; + + .notes { + position: relative; + @include vertical-line(52px); + } + } + + .note-wrapper { + margin: $gl-padding; + border: 1px solid $border-color; + border-radius: $border-radius-default; + } + + .discussion-reply-holder { + border-radius: 0 0 $border-radius-default $border-radius-default; + border-top: 1px solid $border-color; + position: relative; + } +} + .discussion-header, .note-header-info { a { @@ -399,7 +516,17 @@ ul.notes { } .discussion-header { - font-size: 14px; + min-height: 72px; + + .note-header-info { + padding-bottom: 0; + } +} + +.unresolved { + .note-header-info { + margin-top: $gl-padding-8; + } } .note-header { @@ -409,7 +536,7 @@ ul.notes { .note-header-info { min-width: 0; - padding-bottom: 8px; + padding-bottom: $gl-padding-8; &.discussion { padding-bottom: 0; @@ -471,9 +598,18 @@ ul.notes { margin-left: 10px; color: $gray-darkest; + @include media-breakpoint-down(xs) { + width: 100%; + margin: $gl-padding-8 0; + } + .btn-group > .discussion-next-btn { margin-left: -1px; } + + svg { + height: 15px; + } } .note-actions { @@ -585,19 +721,6 @@ ul.notes { z-index: 10; } -.discussion-body, -.diff-file { - .notes .note { - border-bottom: 1px solid $white-normal; - - .timeline-entry-inner { - padding-left: $gl-padding; - padding-right: $gl-padding; - border-bottom: 0; - } - } -} - .disabled-comment { background-color: $gray-light; border-radius: $border-radius-base; @@ -634,7 +757,7 @@ ul.notes { } .btn { - svg path { + svg { fill: $gray-darkest; } @@ -659,7 +782,7 @@ ul.notes { .line-resolve-all { vertical-align: middle; display: inline-block; - padding: 5px 10px 6px; + padding: 6px 10px; background-color: $gray-light; border: 1px solid $border-color; border-radius: $border-radius-default; diff --git a/app/views/shared/notes/_notes_with_form.html.haml b/app/views/shared/notes/_notes_with_form.html.haml index 9dd1c24fdfa..ec1e10bb0c1 100644 --- a/app/views/shared/notes/_notes_with_form.html.haml +++ b/app/views/shared/notes/_notes_with_form.html.haml @@ -7,8 +7,8 @@ = render 'shared/notes/edit_form', project: @project - if can_create_note? - %ul.notes.notes-form.timeline - %li.timeline-entry + .notes.notes-form.timeline + .timeline-entry .timeline-entry-inner .flash-container.timeline-content diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3182ffb27b9..3d66b092143 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3591,6 +3591,9 @@ msgstr "" msgid "Last edited by %{name}" msgstr "" +msgid "Last reply by" +msgstr "" + msgid "Last update" msgstr "" @@ -4192,6 +4195,9 @@ msgstr "" msgid "Notes|Are you sure you want to cancel creating this comment?" msgstr "" +msgid "Notes|Collapse replies" +msgstr "" + msgid "Notes|Show all activity" msgstr "" @@ -7568,6 +7574,11 @@ msgstr "" msgid "remove due date" msgstr "" +msgid "reply" +msgid_plural "replies" +msgstr[0] "" +msgstr[1] "" + msgid "source" msgstr "" diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index b447e79b0df..81cb3e1f74d 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -3,6 +3,7 @@ import createStore from '~/notes/stores'; import noteableDiscussion from '~/notes/components/noteable_discussion.vue'; import '~/behaviors/markdown/render_gfm'; import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data'; +import mockDiffFile from '../../diffs/mock_data/diff_file'; const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; @@ -33,9 +34,20 @@ describe('noteable_discussion component', () => { expect(vm.$el.querySelector('.user-avatar-link')).not.toBeNull(); }); + it('should not render discussion header for non diff discussions', () => { + expect(vm.$el.querySelector('.discussion-header')).toBeNull(); + }); + it('should render discussion header', () => { - expect(vm.$el.querySelector('.discussion-header')).not.toBeNull(); - expect(vm.$el.querySelector('.notes').children.length).toEqual(discussionMock.notes.length); + const discussion = { ...discussionMock }; + discussion.diff_file = mockDiffFile; + discussion.diff_discussion = true; + const diffDiscussionVm = new Component({ + store, + propsData: { discussion }, + }).$mount(); + + expect(diffDiscussionVm.$el.querySelector('.discussion-header')).not.toBeNull(); }); describe('actions', () => { diff --git a/spec/javascripts/notes/components/toggle_replies_widget_spec.js b/spec/javascripts/notes/components/toggle_replies_widget_spec.js new file mode 100644 index 00000000000..2ead8cc6e6a --- /dev/null +++ b/spec/javascripts/notes/components/toggle_replies_widget_spec.js @@ -0,0 +1,78 @@ +import Vue from 'vue'; +import toggleRepliesWidget from '~/notes/components/toggle_replies_widget.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import { note } from '../mock_data'; + +const deepCloneObject = obj => JSON.parse(JSON.stringify(obj)); + +describe('toggle replies widget for notes', () => { + let vm; + let ToggleRepliesWidget; + const noteFromOtherUser = deepCloneObject(note); + noteFromOtherUser.author.username = 'fatihacet'; + + const noteFromAnotherUser = deepCloneObject(note); + noteFromAnotherUser.author.username = 'mgreiling'; + noteFromAnotherUser.author.name = 'Mike Greiling'; + + const replies = [note, note, note, noteFromOtherUser, noteFromAnotherUser]; + + beforeEach(() => { + ToggleRepliesWidget = Vue.extend(toggleRepliesWidget); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('collapsed state', () => { + beforeEach(() => { + vm = mountComponent(ToggleRepliesWidget, { + replies, + collapsed: true, + }); + }); + + it('should render the collapsed', () => { + const vmTextContent = vm.$el.textContent.replace(/\s\s+/g, ' '); + + expect(vm.$el.classList.contains('collapsed')).toEqual(true); + expect(vm.$el.querySelectorAll('.user-avatar-link').length).toEqual(3); + expect(vm.$el.querySelector('time')).not.toBeNull(); + expect(vmTextContent).toContain('5 replies'); + expect(vmTextContent).toContain(`Last reply by ${noteFromAnotherUser.author.name}`); + }); + + it('should emit toggle event when the replies text clicked', () => { + const spy = spyOn(vm, '$emit'); + + vm.$el.querySelector('.js-replies-text').click(); + + expect(spy).toHaveBeenCalledWith('toggle'); + }); + }); + + describe('expanded state', () => { + beforeEach(() => { + vm = mountComponent(ToggleRepliesWidget, { + replies, + collapsed: false, + }); + }); + + it('should render expanded state', () => { + const vmTextContent = vm.$el.textContent.replace(/\s\s+/g, ' '); + + expect(vm.$el.querySelector('.collapse-replies-btn')).not.toBeNull(); + expect(vmTextContent).toContain('Collapse replies'); + }); + + it('should emit toggle event when the collapse replies text called', () => { + const spy = spyOn(vm, '$emit'); + + vm.$el.querySelector('.js-collapse-replies').click(); + + expect(spy).toHaveBeenCalledWith('toggle'); + }); + }); +}); diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index 80604395adf..18cf08f0b9e 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -150,17 +150,25 @@ shared_examples 'discussion comments' do |resource_name| end if resource_name == 'merge request' - let(:note_id) { find("#{comments_selector} .note", match: :first)['data-note-id'] } + let(:note_id) { find("#{comments_selector} .note:first-child", match: :first)['data-note-id'] } + let(:reply_id) { find("#{comments_selector} .note:last-child", match: :first)['data-note-id'] } it 'shows resolved discussion when toggled' do + find("#{comments_selector} .js-vue-discussion-reply").click + find("#{comments_selector} .note-textarea").send_keys('a') + + click_button "Comment" + wait_for_requests + click_button "Resolve discussion" + wait_for_requests expect(page).to have_selector(".note-row-#{note_id}", visible: true) refresh - click_button "Toggle discussion" + click_button "1 reply" - expect(page).to have_selector(".note-row-#{note_id}", visible: true) + expect(page).to have_selector(".note-row-#{reply_id}", visible: true) end end end |