Unify forms for discussions and main target.

Allows previews and uploads in all forms.
Also fixes #1730

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");
+      var form = $(".js-main-target-form");
-      form.find(".buttons, .note_advanced_opts").hide();
+      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(){
+    $(document).on("ajax:complete", ".js-main-target-form", function(){
-      //$(this).find('.js-note-preview-button').text('Preview');
-      //$(this).find('.js-note-preview').hide();
-
       $(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();
 
-    if(note_text.trim().length === 0) {
+    console.log("preview", noteText);
+
+    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"));
   },
 
+
   /**
-   * Shows the diff line form and does some setup.
+   * Helper for inserting and setting up note forms.
+   */
+
+
+  /**
+   * 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
+   * Note: dataHolder must have the "discussionId", "lineCode", "noteableType"
-   *       "noteableId" data attributes set.
+   *       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.attr("rel", dataHolder.data("discussionId"));
-    form.find("#note_line_code").val(data_holder.data("lineCode"));
+    form.find("#note_line_code").val(dataHolder.data("lineCode"));
-    form.find("#note_noteable_type").val(data_holder.data("noteableType"));
+    form.find("#note_noteable_type").val(dataHolder.data("noteableType"));
-    form.find("#note_noteable_id").val(data_holder.data("noteableId"));
+    form.find("#note_noteable_id").val(dataHolder.data("noteableId"));
 
-    // setup interaction
+    NoteList.setupNoteForm(form);
-    disableButtonIfEmptyField(form.find(".js-note-text"), form.find(".js-comment-button"));
-    GitLab.GfmAutoComplete.setup();
 
-    // 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();
   },

app/assets/stylesheets/sections/notes.scss

@@ -239,46 +239,51 @@ p.notify_controls span{
 .reply-btn {
   @extend .save-btn;
 }
-.new_discussion_note {
+.diff_file,
-  // hide it by default
+.discussion {
-  display: none;
+  .new_note {
-  margin: 8px 5px 8px 0;
+    margin: 8px 5px 8px 0;
-
-  // TODO: start cleanup
-  .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;
-        }
-      }
-    }
   }
-  // TODO: end cleanup
 }
 .new_note {
-  textarea {
+  display: none;
-    height:80px;
+
-    width:99%;
+  .buttons {
-    font-size:14px;
+    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;
+
+    // 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 {
+      font-size: 14px;
+      height: 80px;
+      width: 98.6%;
+    }
   }
 
   // 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;
-  }
-}

app/views/notes/_common_form.html.haml

@@ -1,49 +1,49 @@
-.note-form-holder
+= form_for [@project, @note], remote: true, html: { multipart: true, id: nil, class: "new_note js-new-note-form" } do |f|
-  = form_for [@project, @note], remote: "true", multipart: true do |f|
 
-    = note_target_fields
+  = note_target_fields
-    = f.hidden_field :noteable_id
+  = f.hidden_field :line_code
-    = f.hidden_field :noteable_type
+  = f.hidden_field :noteable_id
+  = f.hidden_field :noteable_type
 
-    %h3.page_title Leave a comment
+  - if @note.errors.any?
-    -if @note.errors.any?
+    .alert-message.block-message.error
-      .alert-message.block-message.error
+      - @note.errors.full_messages.each do |msg|
-        - @note.errors.full_messages.each do |msg|
+        %div= msg
-          %div= msg
 
-    .js-toggler-container
+  .note_text_and_preview.js-toggler-container
-      = f.text_area :note, size: 255, class: 'note_text turn-on js-note-text js-gfm-input'
+    %a.js-note-preview-button.js-toggler-target.turn-on{ href: "javascript:;", data: {title: "Preview", url: preview_project_notes_path(@project)} }
-      .note_preview.js-note-preview.hide.turn-off
+      %i.icon-eye-open
+    %a.js-note-edit-button.js-toggler-target.turn-off{ href: "javascript:;", data: {title: "Edit"} }
+      %i.icon-edit
 
-      .hint
+    = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input turn-on'
-        .right Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
+    .note_preview.js-note-preview.turn-off
-      .clearfix
 
-      .buttons
+  .buttons
-        = f.submit 'Add Comment', class: "btn comment-btn grouped js-comment-button"
+    = 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)} }
+    %a.btn.grouped.js-close-discussion-note-form Cancel
-          Preview
+  .hint
-        %button.btn.grouped.js-note-preview-button.js-toggler-target.turn-off
+    .right Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
-          Edit
+  .clearfix
-
+
-    .note_advanced_opts
+  .note_options
-      .attachments.right
+    .attachments.right
-        %h6.left Attachment:
+      %h6.left Attachment:
-        %span.file_name File name...
+      %span.file_name File name...
-
+
-        .input.input_file
+      .input.input_file
-          %a.file_upload.btn.small Upload File
+        %a.file_upload.btn.small Upload File
-          = f.file_field :attachment, class: "input-file"
+        = f.file_field :attachment, class: "input-file"
-        %span.hint Any file less than 10 MB
+      %span.hint Any file less than 10 MB
-
+
-      .notify_opts.right
+    .notify_opts.right
-        %h6.left Notify via email:
+      %h6.left Notify via email:
-        = label_tag :notify do
+      = label_tag :notify do
-          = check_box_tag :notify, 1, @note.noteable_type != "Commit"
+        = check_box_tag :notify, 1, !@note.for_commit?
-          %span Project team
+        %span Project team
-
+
-        - if @note.notify_only_author?(current_user)
+      - if @note.notify_only_author?(current_user)
-          = label_tag :notify_author do
+        = label_tag :notify_author do
-            = check_box_tag :notify_author, 1 , @note.noteable_type == "Commit"
+          = check_box_tag :notify_author, 1 , !@note.for_commit?
-            %span Commit author
+          %span Commit author
   .clearfix

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();

app/views/notes/_discussion_note_form.html.haml

@@ -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

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/common_form"
-    = render "notes/discussion_note_form"
 
 :javascript
   $(function(){

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"