From 6fc10fa2564538bb892090759fdb05604b0e5567 Mon Sep 17 00:00:00 2001 From: Riyad Preukschas <riyad@informatik.uni-bremen.de> Date: Sun, 2 Dec 2012 20:47:09 +0100 Subject: [PATCH] Unify forms for discussions and main target. Allows previews and uploads in all forms. Also fixes #1730 --- app/assets/javascripts/notes.js | 121 +++++++++++++----- app/assets/stylesheets/sections/notes.scss | 96 ++++++-------- app/views/notes/_common_form.html.haml | 96 +++++++------- app/views/notes/_create_common_note.js.haml | 3 +- .../notes/_discussion_note_form.html.haml | 28 ---- app/views/notes/_notes_with_form.html.haml | 5 +- .../notes/_reversed_notes_with_form.html.haml | 1 + 7 files changed, 178 insertions(+), 172 deletions(-) delete mode 100644 app/views/notes/_discussion_note_form.html.haml diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index da356b2ef..c9137c887 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -16,22 +16,22 @@ var NoteList = { 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 = $("#new_note"); - form.find(".buttons, .note_advanced_opts").hide(); + 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_advanced_opts").show(); + form.find(".buttons, .note_options").show(); }); } // get initial set of notes NoteList.getContent(); - disableButtonIfEmptyField(".js-note-text", ".js-comment-button"); - $("#note_attachment").change(function(e){ var val = $('.input-file').val(); var filename = val.replace(/^.*[\\\/]/, ''); @@ -70,10 +70,7 @@ var NoteList = { NoteList.removeNote); // clean up previews for forms - $(document).on("ajax:complete", ".note-form-holder", function(){ - //$(this).find('.js-note-preview-button').text('Preview'); - //$(this).find('.js-note-preview').hide(); - + $(document).on("ajax:complete", ".js-main-target-form", function(){ $(this).find('.error').remove(); $(this).find('.js-note-text').val(""); $(this).show(); @@ -93,8 +90,10 @@ var NoteList = { * Sets up the form and shows it. */ addDiffNote: function(e) { + e.preventDefault(); + // find the form - var form = $(".js-note-forms .js-discussion-note-form"); + var form = $(".js-new-note-form"); var row = $(this).closest("tr"); var nextRow = row.next(); @@ -111,8 +110,6 @@ var NoteList = { // show the form NoteList.setupDiscussionNoteForm($(this), row.next().find("form")); } - - e.preventDefault(); }, /** @@ -123,21 +120,23 @@ var NoteList = { * 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 note_text = form.find('.js-note-text').val(); + var noteText = form.find('.js-note-text').val(); + + console.log("preview", noteText); - if(note_text.trim().length === 0) { + if(noteText.trim().length === 0) { preview.text('Nothing to preview.'); - } else if($(this).data('url')) { + } else { preview.text('Loading...'); - $.post($(this).data('url'), {note: note_text}) + $.post($(this).data('url'), {note: noteText}) .success(function(previewData) { preview.html(previewData); }); } - - e.preventDefault(); }, /** @@ -168,6 +167,8 @@ var NoteList = { * Removes the form and if necessary it's temporary row. */ removeDiscussionNoteForm: function(e) { + e.preventDefault(); + var form = $(this).closest("form"); var row = form.closest("tr"); @@ -181,8 +182,6 @@ var NoteList = { // only remove the form form.remove(); } - - e.preventDefault(); }, /** @@ -202,7 +201,7 @@ var NoteList = { */ replyToDiscussionNote: function() { // find the form - var form = $(".js-note-forms .js-discussion-note-form"); + var form = $(".js-new-note-form"); // hide reply button $(this).hide(); @@ -213,27 +212,83 @@ var NoteList = { NoteList.setupDiscussionNoteForm($(this), $(this).next("form")); }, + + /** + * Helper for inserting and setting up note forms. + */ + + /** - * Shows the diff line form and does some setup. + * Shows the diff/discussion form and does some setup on it. * * Sets some hidden fields in the form. * - * Note: "this" must have the "discussionId", "lineCode", "noteableType" and - * "noteableId" data attributes set. + * Note: dataHolder must have the "discussionId", "lineCode", "noteableType" + * and "noteableId" data attributes set. */ - setupDiscussionNoteForm: function(data_holder, form) { + setupDiscussionNoteForm: function(dataHolder, form) { // setup note target - form.attr("rel", data_holder.data("discussionId")); - form.find("#note_line_code").val(data_holder.data("lineCode")); - form.find("#note_noteable_type").val(data_holder.data("noteableType")); - form.find("#note_noteable_id").val(data_holder.data("noteableId")); + form.attr("rel", dataHolder.data("discussionId")); + 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")); - // setup interaction - disableButtonIfEmptyField(form.find(".js-note-text"), form.find(".js-comment-button")); - GitLab.GfmAutoComplete.setup(); + NoteList.setupNoteForm(form); - // cleanup after successfully creating a diff note + // cleanup after successfully creating a diff/discussion note form.on("ajax:success", NoteList.removeDiscussionNoteForm); + }, + + /** + * 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")); + + // setup preview buttons + $(".js-note-edit-button, .js-note-preview-button").tooltip({ placement: 'left' }); + + previewButton = $(".js-note-preview-button"); + previewButton.hide(); + form.find(".js-note-text").on("input", function() { + if ($(this).val().trim() !== "") { + previewButton.fadeIn(); + } else { + previewButton.fadeOut(); + } + }); + + GitLab.GfmAutoComplete.setup(); form.show(); }, diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss index e8bf3d631..6a2f784e7 100644 --- a/app/assets/stylesheets/sections/notes.scss +++ b/app/assets/stylesheets/sections/notes.scss @@ -239,46 +239,51 @@ p.notify_controls span{ .reply-btn { @extend .save-btn; } -.new_discussion_note { - // hide it by default +.diff_file, +.discussion { + .new_note { + margin: 8px 5px 8px 0; + } +} +.new_note { display: none; - margin: 8px 5px 8px 0; - // TODO: start cleanup - .note_actions { - margin:0; - padding-top: 10px; + .buttons { + float: left; + margin-top: 8px; + } + .note_options { + h6 { + line-height: 32px; + padding-right: 15px; + } + } + .note_text_and_preview { + // makes the "absolute" position for links relative to this + position: relative; - .buttons { - float:left; - width:300px; + // preview/edit buttons + > a { + font-size: 24px; + padding: 4px; + position: absolute; + right: 0; } - .options { - .labels { - float:left; - padding-left:10px; - label { - padding: 6px 0; - margin: 0; - width:120px; - } - } + .note_preview { + background: #f5f5f5; + border: 1px solid #ddd; + @include border-radius(4px); + min-height: 80px; + padding: 4px 6px; + } + .note_text { + font-size: 14px; + height: 80px; + width: 98.6%; } - } - // TODO: end cleanup -} -.new_note { - textarea { - height:80px; - width:99%; - font-size:14px; } // TODO: start cleanup - .attach_holder { - display:none; - } - .attachments { position: relative; width: 350px; @@ -316,32 +321,5 @@ p.notify_controls span{ padding:0; margin: 0; } - .note_advanced_opts { - h6 { - line-height: 32px; - padding-right: 15px; - } - } - .note_preview { - margin: 2px; - border: 1px solid #ddd; - padding: 10px; - min-height: 60px; - background:#f5f5f5; - } - .note_text { - border: 1px solid #aaa; - box-shadow: none; - } // TODO: end cleanup } - -// hide the new discussion note form template -#note-forms { - .note-form-holder { - margin-top: 5px; - } - .new_discussion_note { - display: none; - } -} diff --git a/app/views/notes/_common_form.html.haml b/app/views/notes/_common_form.html.haml index 3dfbe4b8a..9d6004472 100644 --- a/app/views/notes/_common_form.html.haml +++ b/app/views/notes/_common_form.html.haml @@ -1,49 +1,49 @@ -.note-form-holder - = form_for [@project, @note], remote: "true", multipart: true do |f| - - = note_target_fields - = f.hidden_field :noteable_id - = f.hidden_field :noteable_type - - %h3.page_title Leave a comment - -if @note.errors.any? - .alert-message.block-message.error - - @note.errors.full_messages.each do |msg| - %div= msg - - .js-toggler-container - = f.text_area :note, size: 255, class: 'note_text turn-on js-note-text js-gfm-input' - .note_preview.js-note-preview.hide.turn-off - - .hint - .right Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}. - .clearfix - - .buttons - = f.submit 'Add Comment', class: "btn comment-btn grouped js-comment-button" - %button.btn.grouped.js-note-preview-button.js-toggler-target.turn-on{ data: {url: preview_project_notes_path(@project)} } - Preview - %button.btn.grouped.js-note-preview-button.js-toggler-target.turn-off - Edit - - .note_advanced_opts - .attachments.right - %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 - - .notify_opts.right - %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 += 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 :line_code + = f.hidden_field :noteable_id + = f.hidden_field :noteable_type + + - if @note.errors.any? + .alert-message.block-message.error + - @note.errors.full_messages.each do |msg| + %div= msg + + .note_text_and_preview.js-toggler-container + %a.js-note-preview-button.js-toggler-target.turn-on{ href: "javascript:;", data: {title: "Preview", url: preview_project_notes_path(@project)} } + %i.icon-eye-open + %a.js-note-edit-button.js-toggler-target.turn-off{ href: "javascript:;", data: {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 + .attachments.right + %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 + + .notify_opts.right + %h6.left Notify via email: + = label_tag :notify do + = check_box_tag :notify, 1, !@note.for_commit? + %span Project team + + - if @note.notify_only_author?(current_user) + = label_tag :notify_author do + = check_box_tag :notify_author, 1 , !@note.for_commit? + %span Commit author .clearfix diff --git a/app/views/notes/_create_common_note.js.haml b/app/views/notes/_create_common_note.js.haml index 20bc07568..7ea8c4cd8 100644 --- a/app/views/notes/_create_common_note.js.haml +++ b/app/views/notes/_create_common_note.js.haml @@ -5,5 +5,6 @@ NoteList.appendNewNote(#{note.id}, "#{escape_javascript(render "notes/note", note: note)}"); - else - $(".note-form-holder").replaceWith("#{escape_javascript(render 'notes/common_form')}"); + -# TODO: insert form correctly + $(".js-main-target-note").replaceWith("#{escape_javascript(render 'notes/common_form')}"); GitLab.GfmAutoComplete.setup(); diff --git a/app/views/notes/_discussion_note_form.html.haml b/app/views/notes/_discussion_note_form.html.haml deleted file mode 100644 index 85b9a35e1..000000000 --- a/app/views/notes/_discussion_note_form.html.haml +++ /dev/null @@ -1,28 +0,0 @@ -= form_for [@project, @note], remote: true, html: { multipart: true, class: "new_note new_discussion_note js-discussion-note-form" } do |f| - - = note_target_fields - = f.hidden_field :line_code - = f.hidden_field :noteable_id - = f.hidden_field :noteable_type - - -if @note.errors.any? - .alert-message.block-message.error - - @note.errors.full_messages.each do |msg| - %div= msg - - = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input' - .note_actions - .buttons - = f.submit 'Add Comment', class: "btn comment-btn js-comment-button" - %button.btn.js-close-discussion-note-form Cancel - .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 diff --git a/app/views/notes/_notes_with_form.html.haml b/app/views/notes/_notes_with_form.html.haml index 117556d62..2511cbe52 100644 --- a/app/views/notes/_notes_with_form.html.haml +++ b/app/views/notes/_notes_with_form.html.haml @@ -1,10 +1,9 @@ %ul#notes-list.notes .notes-status +.js-main-target-form - if can? current_user, :write_note, @project - #note-forms.js-note-forms - = render "notes/common_form" - = render "notes/discussion_note_form" + = render "notes/common_form" :javascript $(function(){ diff --git a/app/views/notes/_reversed_notes_with_form.html.haml b/app/views/notes/_reversed_notes_with_form.html.haml index 50fdad2ce..6a7f08f9b 100644 --- a/app/views/notes/_reversed_notes_with_form.html.haml +++ b/app/views/notes/_reversed_notes_with_form.html.haml @@ -1,3 +1,4 @@ +.js-main-target-form - if can? current_user, :write_note, @project = render "notes/common_form" -- 2.30.9