diff options
Diffstat (limited to 'app')
55 files changed, 1179 insertions, 686 deletions
diff --git a/app/assets/images/comment_add.png b/app/assets/images/comment_add.png Binary files differdeleted file mode 100644 index 836557ac846..00000000000 --- a/app/assets/images/comment_add.png +++ /dev/null diff --git a/app/assets/images/diff_note_add.png b/app/assets/images/diff_note_add.png Binary files differnew file mode 100644 index 00000000000..8ec15b701fc --- /dev/null +++ b/app/assets/images/diff_note_add.png diff --git a/app/assets/javascripts/behaviors/details_behavior.coffee b/app/assets/javascripts/behaviors/details_behavior.coffee new file mode 100644 index 00000000000..7ad5c818946 --- /dev/null +++ b/app/assets/javascripts/behaviors/details_behavior.coffee @@ -0,0 +1,5 @@ +$ -> + $("body").on "click", ".js-details-target", -> + container = $(@).closest(".js-details-container") + + container.toggleClass("open") diff --git a/app/assets/javascripts/behaviors/toggler_behavior.coffee b/app/assets/javascripts/behaviors/toggler_behavior.coffee new file mode 100644 index 00000000000..3fefbf8e121 --- /dev/null +++ b/app/assets/javascripts/behaviors/toggler_behavior.coffee @@ -0,0 +1,5 @@ +$ -> + $("body").on "click", ".js-toggler-target", -> + container = $(@).closest(".js-toggler-container") + + container.toggleClass("on") diff --git a/app/assets/javascripts/extensions/array.js b/app/assets/javascripts/extensions/array.js new file mode 100644 index 00000000000..7fccc9c9d5f --- /dev/null +++ b/app/assets/javascripts/extensions/array.js @@ -0,0 +1,7 @@ +Array.prototype.first = function() { + return this[0]; +} + +Array.prototype.last = function() { + return this[this.length-1]; +}
\ No newline at end of file diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index b6f65b7aa5e..1eac462172b 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -9,72 +9,311 @@ var NoteList = { loading_more_disabled: false, reversed: false, - init: - function(tid, tt, path) { - this.notes_path = path + ".js"; - this.target_id = tid; - this.target_type = tt; - this.reversed = $("#notes-list").is(".reversed"); - this.target_params = "target_type=" + this.target_type + "&target_id=" + this.target_id; - - // get initial set of notes - this.getContent(); - - $("#notes-list, #new-notes-list").on("ajax:success", ".delete-note", function() { - $(this).closest('li').fadeOut(function() { - $(this).remove(); - NoteList.updateVotes(); - }); + init: function(tid, tt, path) { + NoteList.notes_path = path + ".js"; + NoteList.target_id = tid; + NoteList.target_type = tt; + NoteList.reversed = $("#notes-list").is(".reversed"); + NoteList.target_params = "target_type=" + NoteList.target_type + "&target_id=" + NoteList.target_id; + + NoteList.setupMainTargetNoteForm(); + + if(NoteList.reversed) { + var form = $(".js-main-target-form"); + form.find(".buttons, .note_options").hide(); + var textarea = form.find(".js-note-text"); + textarea.css("height", "40px"); + textarea.on("focus", function(){ + textarea.css("height", "80px"); + form.find(".buttons, .note_options").show(); }); + } - $(".note-form-holder").on("ajax:before", function(){ - $(".submit_note").disable(); - }) + // get initial set of notes + NoteList.getContent(); - $(".note-form-holder").on("ajax:complete", function(){ - $(".submit_note").enable(); - $('#preview-note').hide(); - $('#note_note').show(); - }) + // add a new diff note + $(document).on("click", + ".js-add-diff-note-button", + NoteList.addDiffNote); - disableButtonIfEmptyField(".note-text", ".submit_note"); + // reply to diff/discussion notes + $(document).on("click", + ".js-discussion-reply-button", + NoteList.replyToDiscussionNote); - $("#note_attachment").change(function(e){ - var val = $('.input-file').val(); - var filename = val.replace(/^.*[\\\/]/, ''); - $(".file_name").text(filename); - }); + // setup note preview + $(document).on("click", + ".js-note-preview-button", + NoteList.previewNote); + + // update the file name when an attachment is selected + $(document).on("change", + ".js-note-attachment-input", + NoteList.updateFormAttachment); + + // hide diff note form + $(document).on("click", + ".js-close-discussion-note-form", + NoteList.removeDiscussionNoteForm); + + // remove a note (in general) + $(document).on("click", + ".js-note-delete", + NoteList.removeNote); + + // reset main target form after submit + $(document).on("ajax:complete", + ".js-main-target-form", + NoteList.resetMainTargetForm); + + + $(document).on("click", + ".js-choose-note-attachment-button", + NoteList.chooseNoteAttachment); + }, + + + /** + * When clicking on buttons + */ + + /** + * Called when clicking on the "add a comment" button on the side of a diff line. + * + * Inserts a temporary row for the form below the line. + * Sets up the form and shows it. + */ + addDiffNote: function(e) { + e.preventDefault(); + + // find the form + var form = $(".js-new-note-form"); + var row = $(this).closest("tr"); + var nextRow = row.next(); + + // does it already have notes? + if (nextRow.is(".notes_holder")) { + $.proxy(NoteList.replyToDiscussionNote, + nextRow.find(".js-discussion-reply-button") + ).call(); + } else { + // add a notes row and insert the form + row.after('<tr class="notes_holder js-temp-notes-holder"><td class="notes_line" colspan="2"></td><td class="notes_content"></td></tr>'); + form.clone().appendTo(row.next().find(".notes_content")); + + // show the form + NoteList.setupDiscussionNoteForm($(this), row.next().find("form")); + } + }, + + /** + * Called when clicking the "Choose File" button. + * + * Opesn the file selection dialog. + */ + chooseNoteAttachment: function() { + var form = $(this).closest("form"); + + form.find(".js-note-attachment-input").click(); + }, - if(this.reversed) { - var textarea = $(".note-text"); - $('.note_advanced_opts').hide(); - textarea.css("height", "40px"); - textarea.on("focus", function(){ - $(this).css("height", "80px"); - $('.note_advanced_opts').show(); + /** + * Shows the note preview. + * + * Lets the server render GFM into Html and displays it. + * + * Note: uses the Toggler behavior to toggle preview/edit views/buttons + */ + previewNote: function(e) { + e.preventDefault(); + + var form = $(this).closest("form"); + var preview = form.find('.js-note-preview'); + var noteText = form.find('.js-note-text').val(); + + if(noteText.trim().length === 0) { + preview.text('Nothing to preview.'); + } else { + preview.text('Loading...'); + $.post($(this).data('url'), {note: noteText}) + .success(function(previewData) { + preview.html(previewData); }); - } + } + }, + + /** + * Called in response to "cancel" on a diff note form. + * + * Shows the reply button again. + * Removes the form and if necessary it's temporary row. + */ + removeDiscussionNoteForm: function() { + var form = $(this).closest("form"); + var row = form.closest("tr"); + + // show the reply button (will only work for replys) + form.prev(".js-discussion-reply-button").show(); + + if (row.is(".js-temp-notes-holder")) { + // remove temporary row for diff lines + row.remove(); + } else { + // only remove the form + form.remove(); + } + }, - // Setup note preview - $(document).on('click', '#preview-link', function(e) { - $('#preview-note').text('Loading...'); + /** + * Called in response to deleting a note of any kind. + * + * Removes the actual note from view. + * Removes the whole discussion if the last note is being removed. + */ + removeNote: function() { + var note = $(this).closest(".note"); + var notes = note.closest(".notes"); - $(this).text($(this).text() === "Edit" ? "Preview" : "Edit"); + // check if this is the last note for this line + if (notes.find(".note").length === 1) { + // for discussions + notes.closest(".discussion").remove(); - var note_text = $('#note_note').val(); + // for diff lines + notes.closest("tr").remove(); + } - if(note_text.trim().length === 0) { - $('#preview-note').text('Nothing to preview.'); - } else { - $.post($(this).attr('href'), {note: note_text}).success(function(data) { - $('#preview-note').html(data); - }); - } + note.remove(); + NoteList.updateVotes(); + }, - $('#preview-note, #note_note').toggle(); - e.preventDefault(); - }); - }, + /** + * Called when clicking on the "reply" button for a diff line. + * + * Shows the note form below the notes. + */ + replyToDiscussionNote: function() { + // find the form + var form = $(".js-new-note-form"); + + // hide reply button + $(this).hide(); + // insert the form after the button + form.clone().insertAfter($(this)); + + // show the form + NoteList.setupDiscussionNoteForm($(this), $(this).next("form")); + }, + + + /** + * Helper for inserting and setting up note forms. + */ + + + /** + * Called in response to creating a note failing validation. + * + * Adds the rendered errors to the respective form. + * If "discussionId" is null or undefined, the main target form is assumed. + */ + errorsOnForm: function(errorsHtml, discussionId) { + // find the form + if (discussionId) { + var form = $("form[rel='"+discussionId+"']"); + } else { + var form = $(".js-main-target-form"); + } + + form.find(".js-errors").remove(); + form.prepend(errorsHtml); + + form.find(".js-note-text").focus(); + }, + + + /** + * Shows the diff/discussion form and does some setup on it. + * + * Sets some hidden fields in the form. + * + * Note: dataHolder must have the "discussionId", "lineCode", "noteableType" + * and "noteableId" data attributes set. + */ + setupDiscussionNoteForm: function(dataHolder, form) { + // setup note target + form.attr("rel", dataHolder.data("discussionId")); + form.find("#note_commit_id").val(dataHolder.data("commitId")); + form.find("#note_line_code").val(dataHolder.data("lineCode")); + form.find("#note_noteable_type").val(dataHolder.data("noteableType")); + form.find("#note_noteable_id").val(dataHolder.data("noteableId")); + + NoteList.setupNoteForm(form); + + form.find(".js-note-text").focus(); + }, + + /** + * Shows the main form and does some setup on it. + * + * Sets some hidden fields in the form. + */ + setupMainTargetNoteForm: function() { + // find the form + var form = $(".js-new-note-form"); + // insert the form after the button + form.clone().replaceAll($(".js-main-target-form")); + + form = form.prev("form"); + + // show the form + NoteList.setupNoteForm(form); + + // fix classes + form.removeClass("js-new-note-form"); + form.addClass("js-main-target-form"); + + // remove unnecessary fields and buttons + form.find("#note_line_code").remove(); + form.find(".js-close-discussion-note-form").remove(); + }, + + /** + * General note form setup. + * + * * deactivates the submit button when text is empty + * * hides the preview button when text is empty + * * setup GFM auto complete + * * show the form + */ + setupNoteForm: function(form) { + disableButtonIfEmptyField(form.find(".js-note-text"), form.find(".js-comment-button")); + + form.removeClass("js-new-note-form"); + + // setup preview buttons + form.find(".js-note-edit-button, .js-note-preview-button") + .tooltip({ placement: 'left' }); + + previewButton = form.find(".js-note-preview-button"); + form.find(".js-note-text").on("input", function() { + if ($(this).val().trim() !== "") { + previewButton.removeClass("turn-off").addClass("turn-on"); + } else { + previewButton.removeClass("turn-on").addClass("turn-off"); + } + }); + + // remove notify commit author checkbox for non-commit notes + if (form.find("#note_noteable_type").val() !== "Commit") { + form.find(".js-notify-commit-author").remove(); + } + + GitLab.GfmAutoComplete.setup(); + + form.show(); + }, /** @@ -86,40 +325,39 @@ var NoteList = { /** * Gets an inital set of notes. */ - getContent: - function() { - $.ajax({ - type: "GET", - url: this.notes_path, - data: this.target_params, - complete: function(){ $('.notes-status').removeClass("loading")}, - beforeSend: function() { $('.notes-status').addClass("loading") }, - dataType: "script"}); - }, + getContent: function() { + $.ajax({ + url: NoteList.notes_path, + data: NoteList.target_params, + complete: function(){ $('.js-notes-busy').removeClass("loading")}, + beforeSend: function() { $('.js-notes-busy').addClass("loading") }, + dataType: "script" + }); + }, /** * Called in response to getContent(). * Replaces the content of #notes-list with the given html. */ - setContent: - function(first_id, last_id, html) { - this.top_id = first_id; - this.bottom_id = last_id; - $("#notes-list").html(html); + setContent: function(newNoteIds, html) { + NoteList.top_id = newNoteIds.first(); + NoteList.bottom_id = newNoteIds.last(); + $("#notes-list").html(html); + // for the wall + if (NoteList.reversed) { // init infinite scrolling - this.initLoadMore(); + NoteList.initLoadMore(); // init getting new notes - if (this.reversed) { - this.initRefreshNew(); - } - }, + NoteList.initRefreshNew(); + } + }, /** * Handle loading more notes when scrolling to the bottom of the page. - * The id of the last note in the list is in this.bottom_id. + * The id of the last note in the list is in NoteList.bottom_id. * * Set up refreshing only new notes after all notes have been loaded. */ @@ -128,65 +366,57 @@ var NoteList = { /** * Initializes loading more notes when scrolling to the bottom of the page. */ - initLoadMore: - function() { - $(document).endlessScroll({ - bottomPixels: 400, - fireDelay: 1000, - fireOnce:true, - ceaseFire: function() { - return NoteList.loading_more_disabled; - }, - callback: function(i) { - NoteList.getMore(); - } - }); - }, + initLoadMore: function() { + $(document).endlessScroll({ + bottomPixels: 400, + fireDelay: 1000, + fireOnce:true, + ceaseFire: function() { + return NoteList.loading_more_disabled; + }, + callback: function(i) { + NoteList.getMore(); + } + }); + }, /** * Gets an additional set of notes. */ - getMore: - function() { - // only load more notes if there are no "new" notes - $('.loading').show(); - $.ajax({ - type: "GET", - url: this.notes_path, - data: this.target_params + "&loading_more=1&" + (this.reversed ? "before_id" : "after_id") + "=" + this.bottom_id, - complete: function(){ $('.notes-status').removeClass("loading")}, - beforeSend: function() { $('.notes-status').addClass("loading") }, - dataType: "script"}); - }, + getMore: function() { + // only load more notes if there are no "new" notes + $('.loading').show(); + $.ajax({ + url: NoteList.notes_path, + data: NoteList.target_params + "&loading_more=1&" + (NoteList.reversed ? "before_id" : "after_id") + "=" + NoteList.bottom_id, + complete: function(){ $('.js-notes-busy').removeClass("loading")}, + beforeSend: function() { $('.js-notes-busy').addClass("loading") }, + dataType: "script" + }); + }, /** * Called in response to getMore(). * Append notes to #notes-list. */ - appendMoreNotes: - function(id, html) { - if(id != this.bottom_id) { - this.bottom_id = id; - $("#notes-list").append(html); - } - }, + appendMoreNotes: function(newNoteIds, html) { + var lastNewNoteId = newNoteIds.last(); + if(lastNewNoteId != NoteList.bottom_id) { + NoteList.bottom_id = lastNewNoteId; + $("#notes-list").append(html); + } + }, /** * Called in response to getMore(). * Disables loading more notes when scrolling to the bottom of the page. - * Initalizes refreshing new notes. */ - finishedLoadingMore: - function() { - this.loading_more_disabled = true; + finishedLoadingMore: function() { + NoteList.loading_more_disabled = true; - // from now on only get new notes - if (!this.reversed) { - this.initRefreshNew(); - } - // make sure we are up to date - this.updateVotes(); - }, + // make sure we are up to date + NoteList.updateVotes(); + }, /** @@ -194,52 +424,118 @@ var NoteList = { * * New notes are all notes that are created after the site has been loaded. * The "old" notes are in #notes-list the "new" ones will be in #new-notes-list. - * The id of the last "old" note is in this.bottom_id. + * The id of the last "old" note is in NoteList.bottom_id. */ /** * Initializes getting new notes every n seconds. + * + * Note: only used on wall. */ - initRefreshNew: - function() { - setInterval("NoteList.getNew()", 10000); - }, + initRefreshNew: function() { + setInterval("NoteList.getNew()", 10000); + }, /** * Gets the new set of notes. + * + * Note: only used on wall. */ - getNew: - function() { - $.ajax({ - type: "GET", - url: this.notes_path, - data: this.target_params + "&loading_new=1&after_id=" + (this.reversed ? this.top_id : this.bottom_id), - dataType: "script"}); - }, + getNew: function() { + $.ajax({ + url: NoteList.notes_path, + data: NoteList.target_params + "&loading_new=1&after_id=" + (NoteList.reversed ? NoteList.top_id : NoteList.bottom_id), + dataType: "script" + }); + }, /** * Called in response to getNew(). * Replaces the content of #new-notes-list with the given html. + * + * Note: only used on wall. */ - replaceNewNotes: - function(html) { - $("#new-notes-list").html(html); - this.updateVotes(); - }, + replaceNewNotes: function(newNoteIds, html) { + $("#new-notes-list").html(html); + NoteList.updateVotes(); + }, /** - * Adds a single note to #new-notes-list. + * Adds a single common note to #notes-list. */ - appendNewNote: - function(id, html) { - if (this.reversed) { - $("#new-notes-list").prepend(html); - } else { - $("#new-notes-list").append(html); - } - this.updateVotes(); - }, + appendNewNote: function(id, html) { + $("#notes-list").append(html); + NoteList.updateVotes(); + }, + + /** + * Adds a single discussion note to #notes-list. + * + * Also removes the corresponding form. + */ + appendNewDiscussionNote: function(discussionId, diffRowHtml, noteHtml) { + var form = $("form[rel='"+discussionId+"']"); + var row = form.closest("tr"); + + // is this the first note of discussion? + if (row.is(".js-temp-notes-holder")) { + // insert the note and the reply button after the temp row + row.after(diffRowHtml); + // remove the note (will be added again below) + row.next().find(".note").remove(); + } + + // append new note to all matching discussions + $(".notes[rel='"+discussionId+"']").append(noteHtml); + + // cleanup after successfully creating a diff/discussion note + $.proxy(NoteList.removeDiscussionNoteForm, form).call(); + }, + + /** + * Adds a single wall note to #new-notes-list. + * + * Note: only used on wall. + */ + appendNewWallNote: function(id, html) { + $("#new-notes-list").prepend(html); + }, + + /** + * Called in response the main target form has been successfully submitted. + * + * Removes any errors. + * Resets text and preview. + * Resets buttons. + */ + resetMainTargetForm: function(){ + var form = $(this); + + // remove validation errors + form.find(".js-errors").remove(); + + // reset text and preview + var previewContainer = form.find(".js-toggler-container.note_text_and_preview"); + if (previewContainer.is(".on")) { + previewContainer.removeClass("on"); + } + form.find(".js-note-text").val("").trigger("input"); + }, + + /** + * Called after an attachment file has been selected. + * + * Updates the file name for the selected attachment. + */ + updateFormAttachment: function() { + var form = $(this).closest("form"); + + // get only the basename + var filename = $(this).val().replace(/^.*[\\\/]/, ''); + + form.find(".js-attachment-filename").text(filename); + }, /** * Recalculates the votes and updates them (if they are displayed at all). @@ -249,67 +545,24 @@ var NoteList = { * Might produce inaccurate results when not all notes have been loaded and a * recalculation is triggered (e.g. when deleting a note). */ - updateVotes: - function() { - var votes = $("#votes .votes"); - var notes = $("#notes-list, #new-notes-list").find(".note .vote"); - - // only update if there is a vote display - if (votes.size()) { - var upvotes = notes.filter(".upvote").size(); - var downvotes = notes.filter(".downvote").size(); - var votesCount = upvotes + downvotes; - var upvotesPercent = votesCount ? (100.0 / votesCount * upvotes) : 0; - var downvotesPercent = votesCount ? (100.0 - upvotesPercent) : 0; - - // change vote bar lengths - votes.find(".bar-success").css("width", upvotesPercent+"%"); - votes.find(".bar-danger").css("width", downvotesPercent+"%"); - // replace vote numbers - votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes)); - votes.find(".downvotes").text(votes.find(".downvotes").text().replace(/\d+/, downvotes)); - } + updateVotes: function() { + var votes = $("#votes .votes"); + var notes = $("#notes-list .note .vote"); + + // only update if there is a vote display + if (votes.size()) { + var upvotes = notes.filter(".upvote").size(); + var downvotes = notes.filter(".downvote").size(); + var votesCount = upvotes + downvotes; + var upvotesPercent = votesCount ? (100.0 / votesCount * upvotes) : 0; + var downvotesPercent = votesCount ? (100.0 - upvotesPercent) : 0; + + // change vote bar lengths + votes.find(".bar-success").css("width", upvotesPercent+"%"); + votes.find(".bar-danger").css("width", downvotesPercent+"%"); + // replace vote numbers + votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes)); + votes.find(".downvotes").text(votes.find(".downvotes").text().replace(/\d+/, downvotes)); } + } }; - -var PerLineNotes = { - init: - function() { - /** - * Called when clicking on the "add note" or "reply" button for a diff line. - * - * Shows the note form below the line. - * Sets some hidden fields in the form. - */ - $(".diff_file_content").on("click", ".line_note_link, .line_note_reply_link", function(e) { - var form = $(".per_line_form"); - $(this).closest("tr").after(form); - form.find("#note_line_code").val($(this).data("lineCode")); - form.show(); - e.preventDefault(); - }); - - disableButtonIfEmptyField(".line-note-text", ".submit_inline_note"); - - /** - * Called in response to successfully deleting a note on a diff line. - * - * Removes the actual note from view. - * Removes the reply button if the last note for that line has been removed. - */ - $(".diff_file_content").on("ajax:success", ".delete-note", function() { - var trNote = $(this).closest("tr"); - trNote.fadeOut(function() { - $(this).remove(); - }); - - // check if this is the last note for this line - // elements must really be removed for this to work reliably - var trLine = trNote.prev(); - var trRpl = trNote.next(); - if (trLine.is(".line_holder") && trRpl.is(".reply")) { - trRpl.fadeOut(function() { $(this).remove(); }); - } - }); - } -} diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 54690e73f81..f93246c13c7 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -45,3 +45,8 @@ @import "themes/ui_gray.scss"; @import "themes/ui_color.scss"; +/** + * Styles for JS behaviors. + */ +@import "behaviors.scss"; + diff --git a/app/assets/stylesheets/behaviors.scss b/app/assets/stylesheets/behaviors.scss new file mode 100644 index 00000000000..3fdc20485f2 --- /dev/null +++ b/app/assets/stylesheets/behaviors.scss @@ -0,0 +1,14 @@ +// Details +//-------- +.js-details-container .content { display: none; } +.js-details-container .content.hide { display: block; } +.js-details-container.open .content { display: block; } +.js-details-container.open .content.hide { display: none; } + + +// Toggler +//-------- +.js-toggler-container .turn-on { display: inherit; } +.js-toggler-container .turn-off { display: none; } +.js-toggler-container.on .turn-on { display: none; } +.js-toggler-container.on .turn-off { display: inherit; } diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index bcaa6a2f76e..e192bb426a1 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -532,3 +532,9 @@ h1.http_status_code { } } } + +img.emoji { + height: 20px; + vertical-align: middle; + width: 20px; +} diff --git a/app/assets/stylesheets/sections/commits.scss b/app/assets/stylesheets/sections/commits.scss index ebec51c9715..c60aae49eaa 100644 --- a/app/assets/stylesheets/sections/commits.scss +++ b/app/assets/stylesheets/sections/commits.scss @@ -119,12 +119,14 @@ } } } - .old_line, .new_line { - margin: 0px; - padding: 0px; - border: none; - background: #EEE; - color: #666; + .new_line, + .old_line, + .notes_line { + margin:0px; + padding:0px; + border:none; + background:#EEE; + color:#666; padding: 0px 5px; border-right: 1px solid #ccc; text-align: right; @@ -134,6 +136,7 @@ moz-user-select: none; -khtml-user-select: none; user-select: none; + a { float: left; width: 35px; diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss index 34c7391da18..93ad0d45037 100644 --- a/app/assets/stylesheets/sections/notes.scss +++ b/app/assets/stylesheets/sections/notes.scss @@ -1,233 +1,309 @@ /** * Notes - * */ -#notes-list, -#new-notes-list { +ul.notes { display: block; list-style: none; margin: 0px; padding: 0px; -} -.issue_notes, -.wiki_notes { - .note_content { - float: left; - width: 400px; - } -} + .discussion-header, + .note-header { + @extend .cgray; + padding-top: 5px; + padding-bottom: 15px; -/* Note textare */ -#note_note { - height: 80px; - width: 98%; - font-size: 14px; -} + .avatar { + float: left; + margin-right: 10px; + } -#new_note { - .attach_holder { - display: none; + .discussion-last-update, + .note-last-update { + font-style: italic; + } + .note-author { + color: $style_color; + font-weight: bold; + &:hover { + color: $primary_color; + } + } } -} - -.preview_note { - margin: 2px; - border: 1px solid #ddd; - padding: 10px; - min-height: 60px; - background: #f5f5f5; -} -.note { - padding: 8px 0; - overflow: hidden; - display: block; - position: relative; - img {float: left; margin-right: 10px;} - img.emoji {float: none;margin: 0;} - .note-author cite{font-style: italic;} - p { color: $style_color; } - .note-author { color: $style_color;} + .discussion { + padding: 8px 0; + overflow: hidden; + display: block; + position:relative; - .note-title { margin-left: 45px; padding-top: 5px;} - .avatar { - margin-top: 3px; - } + .discussion-body { + margin-left: 50px; - .delete-note { - display: none; - position: absolute; - right: 0; - top: 0; - } + .diff_file, + .discussion-hidden, + .notes { + @extend .borders; + background-color: #F9F9F9; + } + .diff_file .notes { + /* reset */ + background: inherit; + border: none; + @include box-shadow(none); - &:hover { - .delete-note { display: block; } + } + .discussion-hidden .note { + @extend .cgray; + padding: 8px; + text-align: center; + } + .notes .note { + border-color: #ddd; + padding: 8px; + } + .reply-btn { + margin-top: 8px; + } + } } -} -#notes-list:not(.reversed) .note, -#new-notes-list:not(.reversed) .note { - border-bottom: 1px solid #eee; -} -#notes-list.reversed .note, -#new-notes-list.reversed .note { - border-top: 1px solid #eee; -} -/* mark vote notes */ -.voting_notes .note { - padding: 8px 0; -} + .note { + padding: 8px 0; + overflow: hidden; + display: block; + position:relative; + p { color: $style_color; } -.notes-status { - margin: 18px; -} + .avatar { + margin-top: 3px; + } + .attachment { + font-size: 16px; + margin-top: -20px; + .icon-attachment { + @extend .icon-paper-clip; + font-size: 24px; + position: relative; + text-align: right; + top: 6px; + } + } + .note-body { + margin-left: 45px; + padding-top: 5px; + } + .note-header { + padding-bottom: 5px; + } + } -p.notify_controls input{ - margin: 5px; + // paint top or bottom borders depending on notes direction + &:not(.reversed) .note, + &:not(.reversed) .discussion { + border-bottom: 1px solid #eee; + } + &.reversed .note, + &.reversed .discussion { + border-top: 1px solid #eee; + } } -p.notify_controls span{ - font-weight: 700; -} +.diff_file .notes_holder { + font-family: $sansFontFamily; + font-size: 13px; + line-height: 18px; -tr.line_notes_row { - border-bottom: 1px solid #DDD; - border-left: 7px solid #2A79A3; + td { + border: 1px solid #ddd; + border-left: none; - &.reply { - background: #eee; - border-left: 7px solid #2A79A3; - border-top: 1px solid #ddd; - td { - padding: 7px 10px; + &.notes_line { + text-align: center; + padding: 10px 0; } - a.line_note_reply_link { - border: 1px solid #eaeaea; - @include border-radius(4px); - padding: 3px 10px; - margin-left: 5px; - color: white; - background: #2A79A3; - border-color: #2A79A3; + &.notes_content { + background-color: $white; + border-width: 1px 0; + padding-top: 0; } } - ul { - margin: 0; - li { - padding: 0; - border: none; - } + + .reply-btn { + margin-top: 8px; } } -.line_notes_row, .per_line_form { font-family: "Helvetica Neue", Helvetica, Arial, sans-serif; } -.per_line_form { - background: #f5f5f5; - border-top: 1px solid #eee; - form { margin: 0; } - td { - border-bottom: 1px solid #ddd; + +/** + * Actions for Discussions/Notes + */ + +.discussion, +.note { + &.note:hover { + .note-actions { display: block; } + } + .discussion-header:hover { + .discussion-actions { display: block; } } - .note_actions { - margin: 0; - padding-top: 10px; - .buttons { - float: left; - width: 300px; - } - .options { - .labels { - float: left; - padding-left: 10px; - label { - padding: 6px 0; - margin: 0; - width: 120px; - } + .discussion-actions, + .note-actions { + display: none; + float: right; + + [class^="icon-"], + [class*="icon-"] { + font-size: 16px; + line-height: 16px; + vertical-align: middle; + } + + a { + @extend .cgray; + + &:hover { + color: $primary_color; + &.danger { @extend .cred; } } } } } +.diff_file .note .note-actions { + right: 0; + top: 0; +} + + + +/** + * Line note button on the side of diffs + */ -td .line_note_link { - position: absolute; - margin-left:-70px; - margin-top:-10px; - z-index: 10; - background: url("comment_add.png") no-repeat left 0; - width: 32px; - height: 32px; +.diff_file tr.line_holder { + .add-diff-note { + background: url("diff_note_add.png") no-repeat left 0; + height: 22px; + margin-left: -65px; + position: absolute; + width: 22px; + z-index: 10; - opacity: 0.0; - filter: alpha(opacity=0); + // "hide" it by default + opacity: 0.0; + filter: alpha(opacity=0); - &:hover { - opacity: 1.0; - filter: alpha(opacity=100); + &:hover { + opacity: 1.0; + filter: alpha(opacity=100); + } + } + + // "show" the icon also if we just hover somwhere over the line + &:hover > td { + background: $hover !important; + + .add-diff-note { + opacity: 1.0; + filter: alpha(opacity=100); + } } } -.diff_file_content tr.line_holder:hover > td { background: $hover !important; } -.diff_file_content tr.line_holder:hover > td .line_note_link { - opacity: 1.0; - filter: alpha(opacity=100); + + +/** + * Note Form + */ + +.comment-btn, +.reply-btn { + @extend .save-btn; } +.diff_file, +.discussion { + .new_note { + margin: 8px 5px 8px 0; -.new_note { - .input-file { - font: 500px monospace; - opacity: 0; - filter: alpha(opacity=0); - position: absolute; - z-index: 1; - top: 0; - right: 0; - padding: 0; - margin: 0; + .note_options { + // because of the smaller width and the extra "cancel" button + margin-top: 8px; + } } +} +.new_note { + display: none; - .note_advanced_opts { + .buttons { + float: left; + margin-top: 8px; + } + .clearfix { + margin-bottom: 0; + } + .note_options { h6 { - line-height: 32px; - padding-right: 15px; + @extend .left; + line-height: 20px; + padding-right: 16px; + padding-bottom: 16px; + } + label { + padding: 0; } - } - .attachments { - position: relative; - width: 350px; - height: 50px; - overflow: hidden; - margin:0 0 5px !important; + .attachment { + @extend .right; + position: relative; + width: 350px; + height: 50px; + margin:0 0 5px !important; - .input_file { - .file_upload { - position: absolute; - right: 14px; - top: 7px; + // hide the actual file field + input { + display: none; } - .file_name { - line-height: 30px; - width: 240px; - height: 28px; - overflow: hidden; - } - .input-file { - width: 260px; - height: 41px; + .choose-btn { float: right; } } + .notify_options { + @extend .right; + } + } + .note_text_and_preview { + // makes the "absolute" position for links relative to this + position: relative; + + // preview/edit buttons + > a { + font-size: 24px; + padding: 4px; + position: absolute; + right: 0; + } + .note_preview { + background: #f5f5f5; + border: 1px solid #ddd; + @include border-radius(4px); + min-height: 80px; + padding: 4px 6px; + } + .note_text { + border: 1px solid #DDD; + box-shadow: none; + font-size: 14px; + height: 80px; + width: 98.6%; + } } } -.note-text { - border: 1px solid #DDD; - box-shadow: none; +/* loading indicator */ +.notes-busy { + margin: 18px; } diff --git a/app/contexts/notes/create_context.rb b/app/contexts/notes/create_context.rb index d93adb835ef..1367dff4699 100644 --- a/app/contexts/notes/create_context.rb +++ b/app/contexts/notes/create_context.rb @@ -3,8 +3,8 @@ module Notes def execute note = project.notes.new(params[:note]) note.author = current_user - note.notify = true if params[:notify] == '1' - note.notify_author = true if params[:notify_author] == '1' + note.notify = params[:notify].present? + note.notify_author = params[:notify_author].present? note.save note end diff --git a/app/contexts/notes/load_context.rb b/app/contexts/notes/load_context.rb index a8f617a782b..e3875e1d4e2 100644 --- a/app/contexts/notes/load_context.rb +++ b/app/contexts/notes/load_context.rb @@ -9,11 +9,11 @@ module Notes @notes = case target_type when "commit" - project.notes.for_commit_id(target_id).not_inline.fresh.limit(20) + project.notes.for_commit_id(target_id).not_inline.fresh when "issue" - project.issues.find(target_id).notes.inc_author.fresh.limit(20) + project.issues.find(target_id).notes.inc_author.fresh when "merge_request" - project.merge_requests.find(target_id).mr_and_commit_notes.inc_author.fresh.limit(20) + project.merge_requests.find(target_id).mr_and_commit_notes.inc_author.fresh when "snippet" project.snippets.find(target_id).notes.fresh when "wall" diff --git a/app/controllers/commit_controller.rb b/app/controllers/commit_controller.rb index d671e9f9e9e..1329410891d 100644 --- a/app/controllers/commit_controller.rb +++ b/app/controllers/commit_controller.rb @@ -13,11 +13,17 @@ class CommitController < ProjectResourceController @commit = result[:commit] git_not_found! unless @commit - @suppress_diff = result[:suppress_diff] - @note = result[:note] - @line_notes = result[:line_notes] - @notes_count = result[:notes_count] - @comments_allowed = true + @suppress_diff = result[:suppress_diff] + + @note = result[:note] + @line_notes = result[:line_notes] + @notes_count = result[:notes_count] + @target_type = :commit + @target_id = @commit.id + + @comments_allowed = @reply_allowed = true + @comments_target = { noteable_type: 'Commit', + commit_id: @commit.id } respond_to do |format| format.html do diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 5a1ce2cfcc4..9917d198cbf 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -35,6 +35,8 @@ class IssuesController < ProjectResourceController def show @note = @project.notes.new(noteable: @issue) + @target_type = :issue + @target_id = @issue.id respond_to do |format| format.html diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 6ead406aac5..ab6bf595982 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -18,6 +18,9 @@ class MergeRequestsController < ProjectResourceController end def show + @target_type = :merge_request + @target_id = @merge_request.id + respond_to do |format| format.html format.js @@ -31,7 +34,9 @@ class MergeRequestsController < ProjectResourceController @diffs = @merge_request.diffs @commit = @merge_request.last_commit - @comments_allowed = true + @comments_allowed = @reply_allowed = true + @comments_target = { noteable_type: 'MergeRequest', + noteable_id: @merge_request.id } @line_notes = @merge_request.notes.where("line_code is not null") end diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index d794f368f57..000c7bbb641 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -6,10 +6,12 @@ class NotesController < ProjectResourceController respond_to :js def index - notes + @notes = Notes::LoadContext.new(project, current_user, params).execute + @target_type = params[:target_type].camelize + @target_id = params[:target_id] + if params[:target_type] == "merge_request" - @mixed_targets = true - @main_target_type = params[:target_type].camelize + @discussions = discussions_from_notes end respond_with(@notes) @@ -17,6 +19,8 @@ class NotesController < ProjectResourceController def create @note = Notes::CreateContext.new(project, current_user, params).execute + @target_type = params[:target_type].camelize + @target_id = params[:target_id] respond_to do |format| format.html {redirect_to :back} @@ -40,7 +44,34 @@ class NotesController < ProjectResourceController protected - def notes - @notes = Notes::LoadContext.new(project, current_user, params).execute + def discussion_notes_for(note) + @notes.select do |other_note| + note.discussion_id == other_note.discussion_id + end + end + + def discussions_from_notes + discussion_ids = [] + discussions = [] + + @notes.each do |note| + next if discussion_ids.include?(note.discussion_id) + + # don't group notes for the main target + if note_for_main_target?(note) + discussions << [note] + else + discussions << discussion_notes_for(note) + discussion_ids << note.discussion_id + end + end + + discussions + end + + # Helps to distinguish e.g. commit notes in mr notes list + def note_for_main_target?(note) + note.for_wall? || + (@target_type.camelize == note.noteable_type && !note.for_diff_line?) end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 1a9c890ca80..6f1180eaa6b 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -80,7 +80,10 @@ class ProjectsController < ProjectResourceController def wall return render_404 unless @project.wall_enabled - @note = Note.new + + @target_type = :wall + @target_id = nil + @note = @project.notes.new respond_to do |format| format.html diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 119ef9b2be3..26898abfa82 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -50,6 +50,8 @@ class SnippetsController < ProjectResourceController def show @note = @project.notes.new(noteable: @snippet) + @target_type = :snippet + @target_id = @snippet.id end def destroy diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index ffcc7acc8da..fd920e23c19 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -1,24 +1,32 @@ module NotesHelper - def loading_more_notes? - params[:loading_more].present? + # Helps to distinguish e.g. commit notes in mr notes list + def note_for_main_target?(note) + note.for_wall? || + (@target_type.camelize == note.noteable_type && !note.for_diff_line?) end - def loading_new_notes? - params[:loading_new].present? + def note_target_fields + hidden_field_tag(:target_type, @target_type) + + hidden_field_tag(:target_id, @target_id) end - # Helps to distinguish e.g. commit notes in mr notes list - def note_for_main_target?(note) - !@mixed_targets || @main_target_type == note.noteable_type + def link_to_commit_diff_line_note(note) + if note.for_commit_diff_line? + link_to "#{note.diff_file_name}:L#{note.diff_new_line}", project_commit_path(@project, note.noteable, anchor: note.line_code) + end end - def link_to_commit_diff_line_note(note) - commit = note.noteable - diff_index, diff_old_line, diff_new_line = note.line_code.split('_') + def link_to_merge_request_diff_line_note(note) + if note.for_merge_request_diff_line? + link_to "#{note.diff_file_name}:L#{note.diff_new_line}", diffs_project_merge_request_path(note.project, note.noteable_id, anchor: note.line_code) + end + end - link_file = commit.diffs[diff_index.to_i].new_path - link_line = diff_new_line + def loading_more_notes? + params[:loading_more].present? + end - link_to "#{link_file}:L#{link_line}", project_commit_path(@project, commit, anchor: note.line_code) + def loading_new_notes? + params[:loading_new].present? end end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index c672940a05e..706d7a8e4ed 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -63,12 +63,12 @@ class Notify < ActionMailer::Base # Note # - def note_commit_email(recipient_id, note_id) + def note_commit_email(commit_autor_email, note_id) @note = Note.find(note_id) @commit = @note.noteable @commit = CommitDecorator.decorate(@commit) @project = @note.project - mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title)) + mail(to: recipient(commit_autor_email), subject: subject("note for commit #{@commit.short_id}", @commit.title)) end def note_issue_email(recipient_id, note_id) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f9dd74f9cab..d1717d3bbee 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -69,29 +69,33 @@ module Issuable closed_changed? && !closed end - # Return the number of +1 comments (upvotes) - def upvotes - notes.select(&:upvote?).size + # + # Votes + # + + # Return the number of -1 comments (downvotes) + def downvotes + notes.select(&:downvote?).size end - def upvotes_in_percent + def downvotes_in_percent if votes_count.zero? 0 else - 100.0 / votes_count * upvotes + 100.0 - upvotes_in_percent end end - # Return the number of -1 comments (downvotes) - def downvotes - notes.select(&:downvote?).size + # Return the number of +1 comments (upvotes) + def upvotes + notes.select(&:upvote?).size end - def downvotes_in_percent + def upvotes_in_percent if votes_count.zero? 0 else - 100.0 - upvotes_in_percent + 100.0 / votes_count * upvotes end end diff --git a/app/models/note.rb b/app/models/note.rb index b055ae623b2..3ad03cc601b 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -19,7 +19,6 @@ require 'carrierwave/orm/activerecord' require 'file_size_validator' class Note < ActiveRecord::Base - attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, :attachment, :line_code, :commit_id @@ -34,6 +33,7 @@ class Note < ActiveRecord::Base delegate :name, :email, to: :author, prefix: true validates :note, :project, presence: true + validates :line_code, format: { with: /\A\d+_\d+_\d+\Z/ }, allow_blank: true validates :attachment, file_size: { maximum: 10.megabytes.to_i } validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' } @@ -60,12 +60,68 @@ class Note < ActiveRecord::Base }, without_protection: true) end - def notify - @notify ||= false + def commit_author + @commit_author ||= + project.users.find_by_email(noteable.author_email) || + project.users.find_by_name(noteable.author_name) + rescue + nil end - def notify_author - @notify_author ||= false + def diff + noteable.diffs[diff_file_index] + end + + def diff_file_index + line_code.split('_')[0].to_i + end + + def diff_file_name + diff.b_path + end + + def diff_new_line + line_code.split('_')[2].to_i + end + + def discussion_id + @discussion_id ||= [:discussion, noteable_type.underscore, noteable_id, line_code].join("-").to_sym + end + + # Returns true if this is a downvote note, + # otherwise false is returned + def downvote? + votable? && (note.start_with?('-1') || + note.start_with?(':-1:') + ) + end + + def for_commit? + noteable_type == "Commit" + end + + def for_commit_diff_line? + for_commit? && for_diff_line? + end + + def for_diff_line? + line_code.present? + end + + def for_issue? + noteable_type == "Issue" + end + + def for_merge_request? + noteable_type == "MergeRequest" + end + + def for_merge_request_diff_line? + for_merge_request? && for_diff_line? + end + + def for_wall? + noteable_type.blank? end # override to return commits, which are not active record @@ -81,50 +137,24 @@ class Note < ActiveRecord::Base nil end - # Check if we can notify commit author - # with email about our comment - # - # If commit author email exist in project - # and commit author is not passed user we can - # send email to him - # - # params: - # user - current user - # - # return: - # Boolean - # - def notify_only_author?(user) - for_commit? && commit_author && - commit_author.email != user.email - end - - def for_commit? - noteable_type == "Commit" - end - - def for_diff_line? - line_code.present? + def notify + @notify ||= false end - def commit_author - @commit_author ||= - project.users.find_by_email(noteable.author_email) || - project.users.find_by_name(noteable.author_name) - rescue - nil + def notify_author + @notify_author ||= false end # Returns true if this is an upvote note, # otherwise false is returned def upvote? - note.start_with?('+1') || note.start_with?(':+1:') + votable? && (note.start_with?('+1') || + note.start_with?(':+1:') + ) end - # Returns true if this is a downvote note, - # otherwise false is returned - def downvote? - note.start_with?('-1') || note.start_with?(':-1:') + def votable? + for_issue? || (for_merge_request? && !for_diff_line?) end def noteable_type_name diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb index 3f6d1dfcb70..2ec644ef7c1 100644 --- a/app/observers/note_observer.rb +++ b/app/observers/note_observer.rb @@ -11,7 +11,7 @@ class NoteObserver < ActiveRecord::Observer notify_team(note) elsif note.notify_author # Notify only author of resource - Notify.delay.note_commit_email(note.commit_author.id, note.id) + Notify.delay.note_commit_email(note.noteable.author_email, note.id) else # Otherwise ignore it nil diff --git a/app/views/commit/show.html.haml b/app/views/commit/show.html.haml index 0144e4754c5..f920534e03a 100644 --- a/app/views/commit/show.html.haml +++ b/app/views/commit/show.html.haml @@ -7,12 +7,10 @@ %span.cred #{@commit.stats.deletions} deletions = render "commits/diffs", diffs: @commit.diffs -= render "notes/notes_with_form", tid: @commit.id, tt: "commit" -= render "notes/per_line_form" += render "notes/notes_with_form" :javascript $(function(){ - PerLineNotes.init(); var w, h; $('.diff_file').each(function(){ $('.image.diff_removed img', this).on('load', $.proxy(function(event){ diff --git a/app/views/commits/_diffs.html.haml b/app/views/commits/_diffs.html.haml index e7733f0506b..7fe45aa25bc 100644 --- a/app/views/commits/_diffs.html.haml +++ b/app/views/commits/_diffs.html.haml @@ -38,10 +38,10 @@ %br/ .diff_file_content - -# Skipp all non non-supported blobs + -# Skip all non-supported blobs - next unless file.respond_to?('text?') - if file.text? - = render "commits/text_file", diff: diff, index: i + = render "commits/text_diff", diff: diff, index: i - elsif file.image? - old_file = (@commit.prev_commit.tree / diff.old_path) if !@commit.prev_commit.nil? - if diff.renamed_file || diff.new_file || diff.deleted_file diff --git a/app/views/commits/_text_file.html.haml b/app/views/commits/_text_diff.html.haml index ecdae2f3715..ce07b87d548 100644 --- a/app/views/commits/_text_file.html.haml +++ b/app/views/commits/_text_diff.html.haml @@ -13,11 +13,11 @@ %td.old_line = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code - if @comments_allowed - = render "notes/per_line_note_link", line_code: line_code + = render "notes/diff_note_link", line_code: line_code %td.new_line= link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line) - - if @comments_allowed - - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at) - - unless comments.empty? - = render "notes/per_line_notes_with_reply", notes: comments + - if @reply_allowed + - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at) + - unless comments.empty? + = render "notes/diff_notes_with_reply", notes: comments diff --git a/app/views/issues/show.html.haml b/app/views/issues/show.html.haml index b96af36fcc0..55fc0aee0df 100644 --- a/app/views/issues/show.html.haml +++ b/app/views/issues/show.html.haml @@ -55,4 +55,4 @@ = markdown @issue.description -.issue_notes.voting_notes#notes= render "notes/notes_with_form", tid: @issue.id, tt: "issue" +.voting_notes#notes= render "notes/notes_with_form" diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml index b65d1596f53..cefd33c0cdf 100644 --- a/app/views/merge_requests/_show.html.haml +++ b/app/views/merge_requests/_show.html.haml @@ -12,20 +12,18 @@ %li.notes-tab{data: {action: 'notes'}} = link_to project_merge_request_path(@project, @merge_request) do %i.icon-comment - Comments + Discussion %li.diffs-tab{data: {action: 'diffs'}} = link_to diffs_project_merge_request_path(@project, @merge_request) do %i.icon-list-alt Diff .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" } - = render("notes/notes_with_form", tid: @merge_request.id, tt: "merge_request") + = render "notes/notes_with_form" .diffs.tab-content = render "merge_requests/show/diffs" if @diffs .status - = render "notes/per_line_form" - :javascript var merge_request; $(function(){ diff --git a/app/views/merge_requests/diffs.html.haml b/app/views/merge_requests/diffs.html.haml index a755491c42e..2a5b8b1441e 100644 --- a/app/views/merge_requests/diffs.html.haml +++ b/app/views/merge_requests/diffs.html.haml @@ -1,6 +1 @@ = render "show" - -:javascript - $(function(){ - PerLineNotes.init(); - }); diff --git a/app/views/merge_requests/diffs.js.haml b/app/views/merge_requests/diffs.js.haml index 1d92f1a6fb8..266892c01ef 100644 --- a/app/views/merge_requests/diffs.js.haml +++ b/app/views/merge_requests/diffs.js.haml @@ -1,5 +1,2 @@ :plain merge_request.$(".diffs").html("#{escape_javascript(render(partial: "merge_requests/show/diffs"))}"); - - PerLineNotes.init(); - diff --git a/app/views/merge_requests/show.js.haml b/app/views/merge_requests/show.js.haml index a2a79307453..2ce6eb63290 100644 --- a/app/views/merge_requests/show.js.haml +++ b/app/views/merge_requests/show.js.haml @@ -1,2 +1,2 @@ :plain - merge_request.$(".notes").html("#{escape_javascript(render "notes/notes_with_form", tid: @merge_request.id, tt: "merge_request")}"); + merge_request.$(".notes").html("#{escape_javascript(render "notes/notes_with_form")}"); diff --git a/app/views/notes/_common_form.html.haml b/app/views/notes/_common_form.html.haml deleted file mode 100644 index d76be75bc27..00000000000 --- a/app/views/notes/_common_form.html.haml +++ /dev/null @@ -1,39 +0,0 @@ -.note-form-holder - = form_for [@project, @note], remote: "true", multipart: true do |f| - %h3.page_title Leave a comment - -if @note.errors.any? - .alert-message.block-message.error - - @note.errors.full_messages.each do |msg| - %div= msg - - = f.hidden_field :noteable_id - = f.hidden_field :commit_id - = f.hidden_field :noteable_type - = f.text_area :note, size: 255, class: 'note-text js-gfm-input' - #preview-note.preview_note.hide - .hint - .right Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}. - .clearfix - - .row.note_advanced_opts - .span3 - = f.submit 'Add Comment', class: "btn success submit_note grouped", id: "submit_note" - = link_to 'Preview', preview_project_notes_path(@project), class: 'btn grouped', id: 'preview-link' - .span4.notify_opts - %h6.left Notify via email: - = label_tag :notify do - = check_box_tag :notify, 1, @note.noteable_type != "Commit" - %span Project team - - - if @note.notify_only_author?(current_user) - = label_tag :notify_author do - = check_box_tag :notify_author, 1 , @note.noteable_type == "Commit" - %span Commit author - .span5.attachments - %h6.left Attachment: - %span.file_name File name... - - .input.input_file - %a.file_upload.btn.small Upload File - = f.file_field :attachment, class: "input-file" - %span.hint Any file less than 10 MB diff --git a/app/views/notes/_create_common_note.js.haml b/app/views/notes/_create_common_note.js.haml deleted file mode 100644 index 217c908064a..00000000000 --- a/app/views/notes/_create_common_note.js.haml +++ /dev/null @@ -1,13 +0,0 @@ -- if note.valid? - :plain - $(".note-form-holder .error").remove(); - $('.note-form-holder textarea').val(""); - $('.note-form-holder #preview-link').text('Preview'); - $('.note-form-holder #preview-note').hide(); - $('.note-form-holder').show(); - NoteList.appendNewNote(#{note.id}, "#{escape_javascript(render "notes/note", note: note)}"); - -- else - :plain - $(".note-form-holder").replaceWith("#{escape_javascript(render 'notes/common_form')}"); - GitLab.GfmAutoComplete.setup(); diff --git a/app/views/notes/_create_per_line_note.js.haml b/app/views/notes/_create_per_line_note.js.haml deleted file mode 100644 index 180960e38e4..00000000000 --- a/app/views/notes/_create_per_line_note.js.haml +++ /dev/null @@ -1,19 +0,0 @@ -- if note.valid? - :plain - // hide and reset the form - $(".per_line_form").hide(); - $('.line-note-form-holder textarea').val(""); - - // find the reply button for this line - // (might not be there if this is the first note) - var trRpl = $("a.line_note_reply_link[data-line-code='#{note.line_code}']").closest("tr"); - if (trRpl.size() == 0) { - // find the commented line ... - var trEl = $(".#{note.line_code}").parent(); - // ... and insert the note and the reply button after it - trEl.after("#{escape_javascript(render "notes/per_line_reply_button", line_code: note.line_code)}"); - trEl.after("#{escape_javascript(render "notes/per_line_note", note: note)}"); - } else { - // instert new note before reply button - trRpl.before("#{escape_javascript(render "notes/per_line_note", note: note)}"); - } diff --git a/app/views/notes/_diff_note_link.html.haml b/app/views/notes/_diff_note_link.html.haml new file mode 100644 index 00000000000..377c926a204 --- /dev/null +++ b/app/views/notes/_diff_note_link.html.haml @@ -0,0 +1,10 @@ +- note = @project.notes.new(@comments_target.merge({ line_code: line_code })) += link_to "", + "javascript:;", + class: "add-diff-note js-add-diff-note-button", + data: { noteable_type: note.noteable_type, + noteable_id: note.noteable_id, + commit_id: note.commit_id, + line_code: note.line_code, + discussion_id: note.discussion_id }, + title: "Add a comment to this line" diff --git a/app/views/notes/_diff_notes_with_reply.html.haml b/app/views/notes/_diff_notes_with_reply.html.haml new file mode 100644 index 00000000000..0808f86b090 --- /dev/null +++ b/app/views/notes/_diff_notes_with_reply.html.haml @@ -0,0 +1,11 @@ +- note = notes.first # example note +%tr.notes_holder + %td.notes_line{ colspan: 2 } + %span.btn.disabled + %i.icon-comment + = notes.count + %td.notes_content + %ul.notes{ rel: note.discussion_id } + = render notes + + = render "notes/discussion_reply_button", note: note diff --git a/app/views/notes/_discussion.html.haml b/app/views/notes/_discussion.html.haml new file mode 100644 index 00000000000..031f4477ce3 --- /dev/null +++ b/app/views/notes/_discussion.html.haml @@ -0,0 +1,47 @@ +- note = discussion_notes.first +.discussion.js-details-container.js-toggler-container.open{ class: note.discussion_id } + .discussion-header + .discussion-actions + = link_to "javascript:;", class: "js-details-target turn-on js-toggler-target" do + %i.icon-eye-close + Hide discussion + = link_to "javascript:;", class: "js-details-target turn-off js-toggler-target" do + %i.icon-eye-open + Show discussion + = image_tag gravatar_icon(note.author.email), class: "avatar s32" + %div + = link_to note.author_name, project_team_member_path(@project, @project.team_member_by_id(note.author)), class: "note-author" + - if note.for_merge_request? + started a discussion on this merge request diff + = link_to_merge_request_diff_line_note(note) + - elsif note.for_commit? + started a discussion on commit + #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)} + = link_to_commit_diff_line_note(note) if note.for_diff_line? + - else + %cite.cgray started a discussion + %div + - last_note = discussion_notes.last + last updated by + = link_to last_note.author_name, project_team_member_path(@project, @project.team_member_by_id(last_note.author)), class: "note-author" + %span.discussion-last-update + = time_ago_in_words(last_note.updated_at) + ago + .discussion-body + - if note.for_diff_line? + .content + .diff_file= render "notes/discussion_diff", discussion_notes: discussion_notes, note: note + - else + .content + .notes{ rel: discussion_notes.first.discussion_id } + = render discussion_notes + = render "notes/discussion_reply_button", note: discussion_notes.first + + -# will be shown when the other one is hidden + .discussion-hidden.content.hide + .note + %em Hidden discussion. + = link_to "javascript:;", class: "js-details-target js-toggler-target" do + %i.icon-eye-open + Show + diff --git a/app/views/notes/_discussion_diff.html.haml b/app/views/notes/_discussion_diff.html.haml new file mode 100644 index 00000000000..78f06f78f69 --- /dev/null +++ b/app/views/notes/_discussion_diff.html.haml @@ -0,0 +1,25 @@ +- diff = note.diff +.diff_file_header + - if diff.deleted_file + %span= diff.old_path + - else + %span= diff.new_path + - if diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode + %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}" + %br/ +.diff_file_content + %table + - each_diff_line(diff.diff.lines.to_a, note.diff_file_index) do |line, type, line_code, line_new, line_old| + %tr.line_holder{ id: line_code } + - if type == "match" + %td.old_line= "..." + %td.new_line= "..." + %td.line_content.matched= line + - else + %td.old_line= raw(type == "new" ? " " : line_old) + %td.new_line= raw(type == "old" ? " " : line_new) + %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw "#{line} " + + - if line_code == note.line_code + = render "notes/diff_notes_with_reply", notes: discussion_notes + - break # cut off diff after notes diff --git a/app/views/notes/_discussion_reply_button.html.haml b/app/views/notes/_discussion_reply_button.html.haml new file mode 100644 index 00000000000..d1c5ccc29db --- /dev/null +++ b/app/views/notes/_discussion_reply_button.html.haml @@ -0,0 +1,10 @@ += link_to "javascript:;", + class: "btn reply-btn js-discussion-reply-button", + data: { noteable_type: note.noteable_type, + noteable_id: note.noteable_id, + commit_id: note.commit_id, + line_code: note.line_code, + discussion_id: note.discussion_id }, + title: "Add a reply" do + %i.icon-comment + Reply diff --git a/app/views/notes/_form.html.haml b/app/views/notes/_form.html.haml new file mode 100644 index 00000000000..d094119a9da --- /dev/null +++ b/app/views/notes/_form.html.haml @@ -0,0 +1,44 @@ += form_for [@project, @note], remote: true, html: { multipart: true, id: nil, class: "new_note js-new-note-form" } do |f| + + = note_target_fields + = f.hidden_field :commit_id + = f.hidden_field :line_code + = f.hidden_field :noteable_id + = f.hidden_field :noteable_type + + .note_text_and_preview.js-toggler-container + %a.js-note-preview-button.js-toggler-target.turn-off{ href: "javascript:;", title: "Preview", data: {url: preview_project_notes_path(@project)} } + %i.icon-eye-open + %a.js-note-edit-button.js-toggler-target.turn-off{ href: "javascript:;", title: "Edit" } + %i.icon-edit + + = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input turn-on' + .note_preview.js-note-preview.turn-off + + .buttons + = f.submit 'Add Comment', class: "btn comment-btn grouped js-comment-button" + %a.btn.grouped.js-close-discussion-note-form Cancel + .hint + .right Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}. + .clearfix + + .note_options + .attachment + %h6 Attachment: + .file_name.js-attachment-filename File name... + %a.choose-btn.btn.small.js-choose-note-attachment-button Choose File ... + .hint Any file up to 10 MB + + = f.file_field :attachment, class: "js-note-attachment-input" + + .notify_options + %h6 Notify via email: + = label_tag :notify do + = check_box_tag :notify, 1, !@note.for_commit? + Project team + + .js-notify-commit-author + = label_tag :notify_author do + = check_box_tag :notify_author, 1 , @note.for_commit? + Commit author + .clearfix diff --git a/app/views/notes/_form_errors.html.haml b/app/views/notes/_form_errors.html.haml new file mode 100644 index 00000000000..0851536f0da --- /dev/null +++ b/app/views/notes/_form_errors.html.haml @@ -0,0 +1,3 @@ +.error_message.js-errors + - note.errors.full_messages.each do |msg| + %div= msg diff --git a/app/views/notes/_note.html.haml b/app/views/notes/_note.html.haml index 8591e7bd86b..6f7eea9827f 100644 --- a/app/views/notes/_note.html.haml +++ b/app/views/notes/_note.html.haml @@ -1,42 +1,37 @@ -%li{id: dom_id(note), class: "note"} - = image_tag gravatar_icon(note.author.email), class: "avatar s32" - %div.note-author - %strong= note.author_name - = link_to "##{dom_id(note)}", name: dom_id(note) do - %cite.cgray - = time_ago_in_words(note.updated_at) - ago +%li{ id: dom_id(note), class: dom_class(note), data: { discussion: note.discussion_id } } + .note-header + .note-actions + = link_to "##{dom_id(note)}", name: dom_id(note) do + %i.icon-link + Link here + + - if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project) + = link_to project_note_path(@project, note), title: "Remove comment", method: :delete, confirm: 'Are you sure?', remote: true, class: "danger js-note-delete" do + %i.icon-remove-circle + = image_tag gravatar_icon(note.author.email), class: "avatar s32" + = link_to note.author_name, project_team_member_path(@project, @project.team_member_by_id(note.author)), class: "note-author" + %span.note-last-update + = time_ago_in_words(note.updated_at) + ago - - unless note_for_main_target?(note) - - if note.for_commit? - %span.cgray - on #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)} - = link_to_commit_diff_line_note(note) if note.for_diff_line? + - if note.upvote? + %span.vote.upvote.label.label-success + %i.icon-thumbs-up + \+1 + - if note.downvote? + %span.vote.downvote.label.label-error + %i.icon-thumbs-down + \-1 - -# only show vote if it's a note for the main target - - if note_for_main_target?(note) - - if note.upvote? - %span.vote.upvote.label.label-success - %i.icon-thumbs-up - \+1 - - if note.downvote? - %span.vote.downvote.label.label-error - %i.icon-thumbs-down - \-1 - -# remove button - - if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project) - = link_to [@project, note], confirm: 'Are you sure?', method: :delete, remote: true, class: "cred delete-note btn very_small" do - %i.icon-trash - Remove - - %div.note-title + .note-body = preserve do = markdown(note.note) - - if note.attachment.url - - if note.attachment.image? - = image_tag note.attachment.url, class: 'thumbnail span4' - .right - %div.file - = link_to note.attachment_identifier, note.attachment.url, target: "_blank" + - if note.attachment.url + - if note.attachment.image? + = image_tag note.attachment.url, class: 'thumbnail span4' + .attachment.right + = link_to note.attachment.url, target: "_blank" do + %i.icon-attachment + = note.attachment_identifier .clear diff --git a/app/views/notes/_notes.html.haml b/app/views/notes/_notes.html.haml index adb5dfcbf18..4904249aeff 100644 --- a/app/views/notes/_notes.html.haml +++ b/app/views/notes/_notes.html.haml @@ -1,4 +1,11 @@ -- @notes.each do |note| - - next unless note.author - = render "note", note: note - +- if @discussions.present? + - @discussions.each do |discussion_notes| + - note = discussion_notes.first + - if note_for_main_target?(note) + = render discussion_notes + - else + = render 'discussion', discussion_notes: discussion_notes +- else + - @notes.each do |note| + - next unless note.author + = render 'note', note: note diff --git a/app/views/notes/_notes_with_form.html.haml b/app/views/notes/_notes_with_form.html.haml index 53716c1d3f4..2566edd81ad 100644 --- a/app/views/notes/_notes_with_form.html.haml +++ b/app/views/notes/_notes_with_form.html.haml @@ -1,11 +1,11 @@ -%ul#notes-list -%ul#new-notes-list -.notes-status +%ul#notes-list.notes +.js-notes-busy +.js-main-target-form - if can? current_user, :write_note, @project - = render "notes/common_form" + = render "notes/form" :javascript $(function(){ - NoteList.init("#{tid}", "#{tt}", "#{project_notes_path(@project)}"); + NoteList.init("#{@target_id}", "#{@target_type}", "#{project_notes_path(@project)}"); }); diff --git a/app/views/notes/_per_line_form.html.haml b/app/views/notes/_per_line_form.html.haml deleted file mode 100644 index ff80ad4e0d5..00000000000 --- a/app/views/notes/_per_line_form.html.haml +++ /dev/null @@ -1,40 +0,0 @@ -%table{style: "display:none;"} - %tr.per_line_form - %td{colspan: 3 } - .line-note-form-holder - = form_for [@project, @note], remote: "true", multipart: true do |f| - %h3.page_title Leave a note - %div.span10 - -if @note.errors.any? - .alert-message.block-message.error - - @note.errors.full_messages.each do |msg| - %div= msg - - = f.hidden_field :noteable_id - = f.hidden_field :commit_id - = f.hidden_field :noteable_type - = f.hidden_field :line_code - = f.text_area :note, size: 255, class: 'line-note-text js-gfm-input' - .note_actions - .buttons - = f.submit 'Add note', class: "btn save-btn submit_note submit_inline_note", id: "submit_note" - = link_to "Cancel", "#", class: "btn hide-button" - .options - %h6.left Notify via email: - .labels - = label_tag :notify do - = check_box_tag :notify, 1, @note.noteable_type != "Commit" - %span Project team - - - if @note.notify_only_author?(current_user) - = label_tag :notify_author do - = check_box_tag :notify_author, 1 , @note.noteable_type == "Commit" - %span Commit author - -:javascript - $(function(){ - $(".per_line_form .hide-button").bind("click", function(){ - $('.per_line_form').hide(); - return false; - }); - }); diff --git a/app/views/notes/_per_line_note.html.haml b/app/views/notes/_per_line_note.html.haml deleted file mode 100644 index 28bcd6e0c94..00000000000 --- a/app/views/notes/_per_line_note.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -%tr.line_notes_row - %td{colspan: 3} - %ul - = render "notes/note", note: note - diff --git a/app/views/notes/_per_line_note_link.html.haml b/app/views/notes/_per_line_note_link.html.haml deleted file mode 100644 index 72b59a596a3..00000000000 --- a/app/views/notes/_per_line_note_link.html.haml +++ /dev/null @@ -1 +0,0 @@ -= link_to "", "#", class: "line_note_link", data: { line_code: line_code }, title: "Add note for this line" diff --git a/app/views/notes/_per_line_notes_with_reply.html.haml b/app/views/notes/_per_line_notes_with_reply.html.haml deleted file mode 100644 index 1bcfc41fe20..00000000000 --- a/app/views/notes/_per_line_notes_with_reply.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -- notes.each do |note| - = render "notes/per_line_note", note: note -= render "notes/per_line_reply_button", line_code: notes.first.line_code diff --git a/app/views/notes/_per_line_reply_button.html.haml b/app/views/notes/_per_line_reply_button.html.haml deleted file mode 100644 index 42c737c75ae..00000000000 --- a/app/views/notes/_per_line_reply_button.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%tr.line_notes_row.reply - %td{colspan: 3} - %i.icon-comment - = link_to "Reply", "#", class: "line_note_reply_link", data: { line_code: line_code }, title: "Add note for this line" diff --git a/app/views/notes/_reversed_notes_with_form.html.haml b/app/views/notes/_reversed_notes_with_form.html.haml index 24d599244b4..bb583b8c3b8 100644 --- a/app/views/notes/_reversed_notes_with_form.html.haml +++ b/app/views/notes/_reversed_notes_with_form.html.haml @@ -1,11 +1,12 @@ +.js-main-target-form - if can? current_user, :write_note, @project - = render "notes/common_form" + = render "notes/form" -%ul.reversed#new-notes-list -%ul.reversed#notes-list -.notes-status +%ul#new-notes-list.reversed.notes +%ul#notes-list.reversed.notes +.notes-busy.js-notes-busy :javascript $(function(){ - NoteList.init("#{tid}", "#{tt}", "#{project_notes_path(@project)}"); + NoteList.init("#{@target_id}", "#{@target_type}", "#{project_notes_path(@project)}"); }); diff --git a/app/views/notes/create.js.haml b/app/views/notes/create.js.haml index 03866591c4f..c573d406b73 100644 --- a/app/views/notes/create.js.haml +++ b/app/views/notes/create.js.haml @@ -1,8 +1,21 @@ -- if @note.line_code - = render "create_per_line_note", note: @note -- else - = render "create_common_note", note: @note +- if @note.valid? + var noteHtml = "#{escape_javascript(render "notes/note", note: @note)}"; + + - if note_for_main_target?(@note) + - if @note.for_wall? + NoteList.appendNewWallNote(#{@note.id}, noteHtml); + - else + NoteList.appendNewNote(#{@note.id}, noteHtml); + - else + :plain + var firstDiscussionNoteHtml = "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [@note])}"; + NoteList.appendNewDiscussionNote("#{@note.discussion_id}", + firstDiscussionNoteHtml, + noteHtml); --# Enable submit button -:plain - $("#submit_note").removeAttr("disabled"); +- else + var errorsHtml = "#{escape_javascript(render 'notes/form_errors', note: @note)}"; + - if note_for_main_target?(@note) + NoteList.errorsOnForm(errorsHtml); + - else + NoteList.errorsOnForm(errorsHtml, "#{@note.discussion_id}");
\ No newline at end of file diff --git a/app/views/notes/index.js.haml b/app/views/notes/index.js.haml index 3814dbd46a2..f0826100fbf 100644 --- a/app/views/notes/index.js.haml +++ b/app/views/notes/index.js.haml @@ -1,17 +1,15 @@ - unless @notes.blank? + var notesHtml = "#{escape_javascript(render 'notes/notes')}"; + - new_note_ids = @notes.map(&:id) - if loading_more_notes? - :plain - NoteList.appendMoreNotes(#{@notes.last.id}, "#{escape_javascript(render 'notes/notes')}"); + NoteList.appendMoreNotes(#{new_note_ids}, notesHtml); - elsif loading_new_notes? - :plain - NoteList.replaceNewNotes("#{escape_javascript(render 'notes/notes')}"); + NoteList.replaceNewNotes(#{new_note_ids}, notesHtml); - else - :plain - NoteList.setContent(#{@notes.first.id}, #{@notes.last.id}, "#{escape_javascript(render 'notes/notes')}"); + NoteList.setContent(#{new_note_ids}, notesHtml); - else - if loading_more_notes? - :plain - NoteList.finishedLoadingMore(); + NoteList.finishedLoadingMore(); diff --git a/app/views/projects/wall.html.haml b/app/views/projects/wall.html.haml index 591a8cd06d4..82b565def43 100644 --- a/app/views/projects/wall.html.haml +++ b/app/views/projects/wall.html.haml @@ -1,2 +1,2 @@ %div.wall_page - = render "notes/reversed_notes_with_form", tid: nil, tt: "wall" + = render "notes/reversed_notes_with_form" diff --git a/app/views/snippets/show.html.haml b/app/views/snippets/show.html.haml index ad399533a3d..02022185f9a 100644 --- a/app/views/snippets/show.html.haml +++ b/app/views/snippets/show.html.haml @@ -8,4 +8,4 @@ %br %div= render 'blob' -%div#notes= render "notes/notes_with_form", tid: @snippet.id, tt: "snippet" +%div#notes= render "notes/notes_with_form" |
