Fix common form and note preview

app/assets/javascripts/notes.js

@@ -10,24 +10,25 @@ var NoteList = {
   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;
-
-    if(this.reversed) {
-      var textarea = $(".note-text");
-      $('.note_advanced_opts').hide();
+    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;
+
+    if(NoteList.reversed) {
+      var form = $("#new_note");
+      form.find(".buttons, .note_advanced_opts").hide();
+      var textarea = form.find(".js-note-text");
       textarea.css("height", "40px");
       textarea.on("focus", function(){
-        $(this).css("height", "80px");
-        $('.note_advanced_opts').show();
+        textarea.css("height", "80px");
+        form.find(".buttons, .note_advanced_opts").show();
       });
     }
     // get initial set of notes
-    this.getContent();
+    NoteList.getContent();
     disableButtonIfEmptyField(".js-note-text", ".js-comment-button");
@@ -37,34 +38,21 @@ var NoteList = {
       $(".file_name").text(filename);
     });
-    // Setup note preview
-    $(document).on('click', '#preview-link', function(e) {
-      $('#preview-note').text('Loading...');
-
-      $(this).text($(this).text() === "Edit" ? "Preview" : "Edit");
-
-      var note_text = $('#note_note').val();
-
-      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);
-        });
-      }
-
-      $('#preview-note, #note_note').toggle();
-    });+
-
+    // add a new diff note
     $(document).on("click",
                     ".js-add-diff-note-button",
                     NoteList.addDiffNote);
-    // reply to diff notes
+    // reply to diff/discussion notes
     $(document).on("click",
                     ".js-discussion-reply-button",
                     NoteList.replyToDiscussionNote);
+    // setup note preview
+    $(document).on("click",
+                    ".js-note-preview-button",
+                    NoteList.previewNote);
+
     // hide diff note form
     $(document).on("click",
                     ".js-close-discussion-note-form",
@@ -83,8 +71,12 @@ var NoteList = {
     // clean up previews for forms
     $(document).on("ajax:complete", ".note-form-holder", function(){
-      $(this).find('#preview-note').hide();
-      $(this).find('#note_note').show();
+      //$(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();
     });
   },
@@ -123,6 +115,31 @@ var NoteList = {
     e.preventDefault();
   },
+  /**
+   * 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) {
+    var form = $(this).closest("form");
+    var preview = form.find('.js-note-preview');
+    var note_text = form.find('.js-note-text').val();
+
+    if(note_text.trim().length === 0) {
+      preview.text('Nothing to preview.');
+    } else if($(this).data('url')) {
+      preview.text('Loading...');
+      $.post($(this).data('url'), {note: note_text})
+        .success(function(previewData) {
+          preview.html(previewData);
+        });
+    }
+
+    e.preventDefault();
+  },
+
   /**
    * Called in response to deleting a note on a diff line.
    *
@@ -233,8 +250,8 @@ var NoteList = {
    */
   getContent: function() {
     $.ajax({
-      url: this.notes_path,
-      data: this.target_params,
+      url: NoteList.notes_path,
+      data: NoteList.target_params,
       complete: function(){ $('.notes-status').removeClass("loading")},
       beforeSend: function() { $('.notes-status').addClass("loading") },
       dataType: "script"
@@ -246,23 +263,23 @@ var NoteList = {
    * Replaces the content of #notes-list with the given html.
    */
   setContent: function(newNoteIds, html) {
-    this.top_id = newNoteIds.first();
-    this.bottom_id = newNoteIds.last();
+    NoteList.top_id = newNoteIds.first();
+    NoteList.bottom_id = newNoteIds.last();
     $("#notes-list").html(html);
-    if (this.reversed) {
+    if (NoteList.reversed) {
       // init infinite scrolling
-      this.initLoadMore();
+      NoteList.initLoadMore();
       // init getting new notes
-      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.
    */
@@ -292,8 +309,8 @@ var NoteList = {
     // only load more notes if there are no "new" notes
     $('.loading').show();
     $.ajax({
-      url: this.notes_path,
-      data: this.target_params + "&loading_more=1&" + (this.reversed ? "before_id" : "after_id") + "=" + this.bottom_id,
+      url: NoteList.notes_path,
+      data: NoteList.target_params + "&loading_more=1&" + (NoteList.reversed ? "before_id" : "after_id") + "=" + NoteList.bottom_id,
       complete: function(){ $('.notes-status').removeClass("loading")},
       beforeSend: function() { $('.notes-status').addClass("loading") },
       dataType: "script"
@@ -306,8 +323,8 @@ var NoteList = {
    */
   appendMoreNotes: function(newNoteIds, html) {
     var lastNewNoteId = newNoteIds.last();
-    if(lastNewNoteId != this.bottom_id) {
-      this.bottom_id = lastNewNoteId;
+    if(lastNewNoteId != NoteList.bottom_id) {
+      NoteList.bottom_id = lastNewNoteId;
       $("#notes-list").append(html);
     }
   },
@@ -315,17 +332,12 @@ var NoteList = {
   /**
    * 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;
-    // from now on only get new notes
-    if (!this.reversed) {
-      this.initRefreshNew();
-    }
+    NoteList.loading_more_disabled = true;
     // make sure we are up to date
-    this.updateVotes();
+    NoteList.updateVotes();
   },
@@ -334,7 +346,7 @@ 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.
    */
@@ -350,8 +362,8 @@ var NoteList = {
    */
   getNew: function() {
     $.ajax({
-      url: this.notes_path,
-      data: this.target_params + "&loading_new=1&after_id=" + (this.reversed ? this.top_id : this.bottom_id),
+      url: NoteList.notes_path,
+      data: NoteList.target_params + "&loading_new=1&after_id=" + (NoteList.reversed ? NoteList.top_id : NoteList.bottom_id),
       dataType: "script"
     });
   },
@@ -362,21 +374,20 @@ var NoteList = {
    */
   replaceNewNotes: function(newNoteIds, html) {
     $("#new-notes-list").html(html);
-    this.updateVotes();
+    NoteList.updateVotes();
   },
   /**
-   * Adds a single note to #new-notes-list.
+   * Adds a single common note to #(new-)notes-list.
    */
   appendNewNote: function(id, html) {
-    if (this.reversed) {
-      $("#notes-list").prepend(html);
-    } else {
-      $("#notes-list").append(html);
-    }
-    this.updateVotes();
+    $("#notes-list").append(html);
+    NoteList.updateVotes();
   },
+  /**
+   * Adds a single discussion note to #(new-)notes-list.
+   */
   appendNewDiscussionNote: function(discussionId, diffRowHtml, noteHtml) {
     // is this the first note of discussion?
     var row = $("form[rel='"+discussionId+"']").closest("tr");
@@ -401,7 +412,7 @@ var NoteList = {
    */
   updateVotes: function() {
     var votes = $("#votes .votes");
-    var notes = $("#notes-list").find(".note .vote");
+    var notes = $("#notes-list .note .vote");
     // only update if there is a vote display
     if (votes.size()) {

app/assets/stylesheets/behaviors.scss

@@ -8,7 +8,7 @@
 // Toggler
 //--------
-.js-toggler-container .turn-on  { display: inline-block; }
+.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: inline-block; }
+.js-toggler-container.on .turn-off { display: inherit; }

app/assets/stylesheets/sections/notes.scss

@@ -310,24 +310,26 @@ p.notify_controls span{
       padding-right: 15px;
     }
   }
-  .note-text {
+  .note_preview {
+    margin: 2px;
+    border: 1px solid #ddd;
+    padding: 10px;
+    min-height: 60px;
+    background:#f5f5f5;
+  }
+  .note_text {
     border: 1px solid #aaa;
-    box-shadow:none;
+    box-shadow: none;
   }
   // TODO: end cleanup
 }
 // hide the new discussion note form template
-.note-forms {
+#note-forms {
+  .note-form-holder {
+    margin-top: 5px;
+  }
   .new_discussion_note {
     display: none;
   }
 }
-
-.preview_note {
-  margin: 2px;
-  border: 1px solid #ddd;
-  padding: 10px;
-  min-height: 60px;
-  background:#f5f5f5;
-}

app/views/notes/_common_form.html.haml

@@ -11,17 +11,32 @@
         - @note.errors.full_messages.each do |msg|
           %div= msg
-    = f.text_area :note, size: 255, class: 'js-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'}.
+    .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
-    .row.note_advanced_opts
-      .span3
+      .buttons
         = f.submit 'Add Comment', class: "btn comment-btn grouped js-comment-button"
-        = link_to 'Preview', preview_project_notes_path(@project), class: 'btn grouped', id: 'preview-link'
-      .span4.notify_opts
+        %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"
@@ -31,11 +46,4 @@
           = 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
+  .clearfix

app/views/notes/_create_common_note.js.haml

 - 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();
+  $(".note-form-holder").replaceWith("#{escape_javascript(render 'notes/common_form')}");
+  GitLab.GfmAutoComplete.setup();

app/views/notes/_create_discussion_note.js.haml

 - if note.valid?
   :plain
     NoteList.appendNewDiscussionNote("#{note.discussion_id}",
-                                     "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [note])}",
-                                     "#{escape_javascript(render "notes/note", note: note)}");
+                                      "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [note])}",
+                                      "#{escape_javascript(render "notes/note", note: note)}");

app/views/notes/_discussion_note_form.html.haml

@@ -10,7 +10,7 @@
       - @note.errors.full_messages.each do |msg|
         %div= msg
-  = f.text_area :note,  size: 255, class: 'js-note-text js-gfm-input'
+  = 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"

app/views/notes/_notes_with_form.html.haml

 %ul#notes-list.notes
+.notes-status
 - if can? current_user, :write_note, @project
-  .note-forms.js-note-forms
+  #note-forms.js-note-forms
     = render "notes/common_form"
     = render "notes/discussion_note_form"