diff --git a/app/assets/javascripts/line_comments/application.js.coffee b/app/assets/javascripts/line_comments/application.js.coffee index d9c0c534726..d6894c54d04 100644 --- a/app/assets/javascripts/line_comments/application.js.coffee +++ b/app/assets/javascripts/line_comments/application.js.coffee @@ -6,11 +6,12 @@ $ => @DiffNotesApp = new Vue - el: '#notes' + el: '#diff-comments-app' components: 'resolve-btn': ResolveBtn + 'resolve-all': ResolveAll new Vue - el: '#resolve-all-app' + el: '#resolve-count-app' components: - 'resolve-all': ResolveAll + 'resolve-count': ResolveCount diff --git a/app/assets/javascripts/line_comments/components/resolve_all.js.coffee b/app/assets/javascripts/line_comments/components/resolve_all.js.coffee index bcc16116be5..3f131376cf5 100644 --- a/app/assets/javascripts/line_comments/components/resolve_all.js.coffee +++ b/app/assets/javascripts/line_comments/components/resolve_all.js.coffee @@ -1,27 +1,22 @@ @ResolveAll = Vue.extend + props: + discussionId: String + namespace: String data: -> comments: CommentsStore.state loading: false - props: - namespace: String computed: - resolved: -> - resolvedCount = 0 - for noteId, resolved of this.comments - resolvedCount++ if resolved - resolvedCount - commentsCount: -> - Object.keys(this.comments).length - buttonText: -> - if this.allResolved then 'Un-resolve all' else 'Resolve all' allResolved: -> - this.resolved is this.commentsCount + isResolved = true + for noteId, resolved of this.comments[this.discussionId] + isResolved = false unless resolved + isResolved + buttonText: -> + if this.allResolved then "Un-resolve all" else "Resolve all" methods: - updateAll: -> - ids = CommentsStore.getAllForState(this.allResolved) + resolve: -> this.loading = true - ResolveService - .resolveAll(this.namespace, ids, !this.allResolved) + .resolveAll(this.namespace, this.discussionId, this.allResolved) .then => this.loading = false diff --git a/app/assets/javascripts/line_comments/components/resolve_btn.js.coffee b/app/assets/javascripts/line_comments/components/resolve_btn.js.coffee index f1f6fd1e353..acf08942f1d 100644 --- a/app/assets/javascripts/line_comments/components/resolve_btn.js.coffee +++ b/app/assets/javascripts/line_comments/components/resolve_btn.js.coffee @@ -1,6 +1,7 @@ @ResolveBtn = Vue.extend props: noteId: Number + discussionId: String resolved: Boolean namespace: String data: -> @@ -9,7 +10,7 @@ computed: buttonText: -> if this.isResolved then "Mark as un-resolved" else "Mark as resolved" - isResolved: -> this.comments[this.noteId] + isResolved: -> CommentsStore.get(this.discussionId, this.noteId) methods: updateTooltip: -> $(this.$els.button) @@ -18,13 +19,13 @@ resolve: -> this.loading = true ResolveService - .resolve(this.namespace, this.noteId, !this.isResolved) + .resolve(this.namespace, this.discussionId, this.noteId, !this.isResolved) .then => this.loading = false this.$nextTick this.updateTooltip compiled: -> $(this.$els.button).tooltip() destroyed: -> - CommentsStore.delete(this.noteId) + CommentsStore.delete(this.discussionId, this.noteId) created: -> - CommentsStore.create(this.noteId, this.resolved) + CommentsStore.create(this.discussionId, this.noteId, this.resolved) diff --git a/app/assets/javascripts/line_comments/components/resolve_count.js.coffee b/app/assets/javascripts/line_comments/components/resolve_count.js.coffee new file mode 100644 index 00000000000..8e063600310 --- /dev/null +++ b/app/assets/javascripts/line_comments/components/resolve_count.js.coffee @@ -0,0 +1,17 @@ +@ResolveCount = Vue.extend + data: -> + comments: CommentsStore.state + loading: false + computed: + resolved: -> + resolvedCount = 0 + for discussionId, comments of this.comments + resolved = true + for noteId, resolved of comments + resolved = false unless resolved + resolvedCount++ if resolved + resolvedCount + commentsCount: -> + Object.keys(this.comments).length + allResolved: -> + this.resolved is this.commentsCount diff --git a/app/assets/javascripts/line_comments/services/resolve.js.coffee b/app/assets/javascripts/line_comments/services/resolve.js.coffee index bb3aa595784..10cf79c39f8 100644 --- a/app/assets/javascripts/line_comments/services/resolve.js.coffee +++ b/app/assets/javascripts/line_comments/services/resolve.js.coffee @@ -12,20 +12,27 @@ class ResolveService Vue.http.headers.common['X-CSRF-Token'] = $.rails.csrfToken() @resource = Vue.resource('notes{/id}', {}, actions) - resolve: (namespace, id, resolve) -> + resolve: (namespace, discussionId, noteId, resolve) -> Vue.http.options.root = "/#{namespace}" @resource - .resolve({ id: id }, { resolved: resolve }) + .resolve({ id: noteId }, { discussion: discussionId, resolved: resolve }) .then (response) -> if response.status is 200 - CommentsStore.update(id, resolve) + CommentsStore.update(discussionId, noteId, resolve) - resolveAll: (namespace, ids, resolve) -> + resolveAll: (namespace, discussionId, allResolve) -> Vue.http.options.root = "/#{namespace}" + + ids = [] + for noteId, resolved of CommentsStore.state[discussionId] + ids.push(noteId) if resolved is allResolve + @resource - .all({}, { ids: ids, resolve: resolve }) + .all({}, { ids: ids, discussion: discussionId, resolved: !allResolve }) .then (response) -> - CommentsStore.updateAll(resolve) + if response.status is 200 + for noteId in ids + CommentsStore.update(discussionId, noteId, !allResolve) $ -> @ResolveService = new ResolveService() diff --git a/app/assets/javascripts/line_comments/stores/comments.js.coffee b/app/assets/javascripts/line_comments/stores/comments.js.coffee index 9bea16d3de0..1635425dbaf 100644 --- a/app/assets/javascripts/line_comments/stores/comments.js.coffee +++ b/app/assets/javascripts/line_comments/stores/comments.js.coffee @@ -1,16 +1,16 @@ @CommentsStore = state: {} - create: (id, resolved) -> - Vue.set(this.state, id, resolved) - update: (id, resolved) -> - this.state[id] = resolved - delete: (id) -> - Vue.delete(this.state, id) - updateAll: (state) -> - for id,resolved of this.state - this.update(id, state) if resolved isnt state - getAllForState: (state) -> - ids = [] - for id,resolved of this.state - ids.push(id) if resolved is state - ids + get: (discussionId, noteId) -> + this.state[discussionId][noteId] + create: (discussionId, noteId, resolved) -> + unless this.state[discussionId]? + Vue.set(this.state, discussionId, {}) + + Vue.set(this.state[discussionId], noteId, resolved) + update: (discussionId, noteId, resolved) -> + this.state[discussionId][noteId] = resolved + delete: (discussionId, noteId) -> + Vue.delete(this.state[discussionId], noteId) + + if Object.keys(this.state[discussionId]).length is 0 + Vue.delete(this.state, discussionId) diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee index 86539e0d725..c976d793cc9 100644 --- a/app/assets/javascripts/merge_request_tabs.js.coffee +++ b/app/assets/javascripts/merge_request_tabs.js.coffee @@ -157,6 +157,11 @@ class @MergeRequestTabs url: "#{source}.json" + @_location.search success: (data) => $('#diffs').html data.html + + if $('resolve-btn, resolve-all').length and DiffNotesApp? + $('resolve-btn, resolve-all').each -> + DiffNotesApp.$compile $(this).get(0) + gl.utils.localTimeAgo($('.js-timeago', 'div#diffs')) $('#diffs .js-syntax-highlight').syntaxHighlight() $('#diffs .diff-file').singleFileDiff() diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 7bd308f6765..00c24c13cb9 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -271,8 +271,8 @@ class @Notes # append new note to all matching discussions discussionContainer.append note_html - if $('resolve-btn').length and DiffNotesApp? - $('resolve-btn').each -> + if $('resolve-btn, resolve-all').length and DiffNotesApp? + $('resolve-btn, resolve-all').each -> DiffNotesApp.$compile $(this).get(0) gl.utils.localTimeAgo($('.js-timeago', note_html), false) @@ -507,10 +507,10 @@ class @Notes replyToDiscussionNote: (e) => form = @formClone.clone() replyLink = $(e.target).closest(".js-discussion-reply-button") - replyLink.hide() - - # insert the form after the button - replyLink.after form + replyLink + .closest('.discussion-reply-holder') + .hide() + .after form # show the form @setupDiscussionNoteForm(replyLink, form) @@ -606,7 +606,9 @@ class @Notes form.find(".js-note-text").data("autosave").reset() # show the reply button (will only work for replies) - form.prev(".js-discussion-reply-button").show() + form + .prev('.discussion-reply-holder') + .show() if row.is(".js-temp-notes-holder") # remove temporary row for diff lines row.remove() diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 3784010348a..a355d802973 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -156,6 +156,27 @@ .discussion-reply-holder { background-color: $white-light; padding: 10px 16px; + + .btn-group-justified { + table-layout: auto; + } + + .btn-group:first-child { + width: 100%; + } + + .discussion-with-resolve-btn { + .btn-group:first-child .btn { + border-top-right-radius: 0; + border-bottom-right-radius: 0; + } + + .btn-group:last-child .btn { + margin-left: -1px; + border-top-left-radius: 0; + border-bottom-left-radius: 0; + } + } } } diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 0f60dd828ab..226f0423132 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -81,10 +81,8 @@ module NotesHelper data = discussion.reply_attributes.merge(line_type: line_type) - content_tag(:div, class: "discussion-reply-holder") do - button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button', - data: data, title: 'Add a reply' - end + button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button', + data: data, title: 'Add a reply' end def note_max_access_for_user(note) diff --git a/app/views/discussions/_diff_discussion.html.haml b/app/views/discussions/_diff_discussion.html.haml index fa1ad9efa73..b9bc8a6304e 100644 --- a/app/views/discussions/_diff_discussion.html.haml +++ b/app/views/discussions/_diff_discussion.html.haml @@ -3,4 +3,10 @@ %td.notes_content %ul.notes{ data: { discussion_id: discussion.id } } = render partial: "projects/notes/note", collection: discussion.notes, as: :note - = link_to_reply_discussion(discussion) + + .discussion-reply-holder + .btn-group-justified.discussion-with-resolve-btn{ role: "group" } + .btn-group{ role: "group" } + = link_to_reply_discussion(note) + .btn-group{ role: "group" } + = render "projects/notes/discussions/resolve_all", note: note diff --git a/app/views/discussions/_parallel_diff_discussion.html.haml b/app/views/discussions/_parallel_diff_discussion.html.haml index a798c438ea0..2ac85699417 100644 --- a/app/views/discussions/_parallel_diff_discussion.html.haml +++ b/app/views/discussions/_parallel_diff_discussion.html.haml @@ -5,7 +5,12 @@ %ul.notes{ data: { discussion_id: discussion_left.id } } = render partial: "projects/notes/note", collection: discussion_left.notes, as: :note - = link_to_reply_discussion(discussion_left, 'old') + .discussion-reply-holder + .btn-group-justified.discussion-with-resolve-btn{ role: "group" } + .btn-group{ role: "group" } + = link_to_reply_discussion(note_left, 'old') + .btn-group{ role: "group" } + = render "projects/notes/discussions/resolve_all", note: note_left - else %td.notes_line.old= "" %td.notes_content.parallel.old= "" @@ -16,7 +21,12 @@ %ul.notes{ data: { discussion_id: discussion_right.id } } = render partial: "projects/notes/note", collection: discussion_right.notes, as: :note - = link_to_reply_discussion(discussion_right, 'new') + .discussion-reply-holder + .btn-group-justified.discussion-with-resolve-btn{ role: "group" } + .btn-group{ role: "group" } + = link_to_reply_discussion(note_right, 'new') + .btn-group{ role: "group" } + = render "projects/notes/discussions/resolve_all", note: note_right - else %td.notes_line.new= "" %td.notes_content.parallel.new= "" diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 43d37c9d545..0ba490c0f7f 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -45,14 +45,11 @@ = link_to "command line", "#modal_merge_info", class: "how_to_merge_link vlink", title: "How To Merge", "data-toggle" => "modal" - if current_user - #resolve-all-app{ "v-cloak" => true } - %resolve-all{ ":namespace" => "'#{@project.namespace.path}/#{@project.path}'", "inline-template" => true } + #resolve-count-app{ "v-cloak" => true } + %resolve-count{ "inline-template" => true } .line-resolve-all{ "v-show" => "commentsCount > 0" } - %button.btn.btn-gray{ type: "button", "aria-label" => "Resolve all", "@click" => "updateAll", ":disabled" => "loading" } - = icon("spinner spin", "v-show" => "loading") - {{ buttonText }} %span.line-resolve-text - {{ resolved }}/{{ commentsCount }} comments resolved + {{ resolved }}/{{ commentsCount }} discussions resolved - if @commits_count.nonzero? %ul.merge-request-tabs.nav-links.no-top.no-bottom @@ -74,7 +71,7 @@ Changes %span.badge= @merge_request.diff_size - .tab-content + .tab-content#diff-comments-app #notes.notes.tab-pane.voting_notes .content-block.content-block-small.oneline-block = render 'award_emoji/awards_block', awardable: @merge_request, inline: true diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 5cfe41de418..5f9eecf27db 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -21,7 +21,7 @@ - if access and not note.system %span.note-role.hidden-xs= access - if !note.system && current_user - %resolve-btn{ ":namespace" => "'#{note.project.namespace.path}/#{note.project.path}'", ":note-id" => note.id, ":resolved" => "false", "inline-template" => true, "v-ref:note_#{note.id}" => true } + %resolve-btn{ ":namespace" => "'#{note.project.namespace.path}/#{note.project.path}'", ":discussion-id" => "'#{note.discussion_id}'", ":note-id" => note.id, ":resolved" => "false", "inline-template" => true, "v-ref:note_#{note.id}" => true } .note-action-button = icon("spin spinner", "v-show" => "loading") %button.line-resolve-btn{ type: "button", ":class" => "{ 'is-active': isResolved }", ":aria-label" => "buttonText", "@click" => "resolve", ":title" => "buttonText", "v-show" => "!loading", "v-el:button" => true } diff --git a/app/views/projects/notes/discussions/_resolve_all.html.haml b/app/views/projects/notes/discussions/_resolve_all.html.haml new file mode 100644 index 00000000000..e94bb743241 --- /dev/null +++ b/app/views/projects/notes/discussions/_resolve_all.html.haml @@ -0,0 +1,4 @@ +%resolve-all{ ":namespace" => "'#{note.project.namespace.path}/#{note.project.path}'", ":discussion-id" => "'#{note.discussion_id}'", "inline-template" => true, "v-cloak" => true } + %button.btn.btn-success{ type: "button", "@click" => "resolve", ":disabled" => "loading" } + = icon("spinner spin", "v-show" => "loading") + {{ buttonText }}