Revamped note form options.

4 files changed, 100 insertions, 89 deletions

diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index f9dda3c70a1..fa0edd2d1f4 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -32,12 +32,6 @@ var NoteList = {
     // get initial set of notes
     NoteList.getContent();
 
-    $("#note_attachment").change(function(e){
-      var val = $('.input-file').val();
-      var filename = val.replace(/^.*[\\\/]/, '');
-      $(".file_name").text(filename);
-    });
-
     // add a new diff note
     $(document).on("click",
                     ".js-add-diff-note-button",
@@ -53,6 +47,11 @@ var NoteList = {
                     ".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",
@@ -63,34 +62,21 @@ var NoteList = {
                     ".js-note-delete",
                     NoteList.removeNote);
 
-    // clean up previews for main target form
+    // reset main target form after submit
     $(document).on("ajax:complete",
                    ".js-main-target-form",
-                   NoteList.cleanupMainTargetForm);
-  },
+                   NoteList.resetMainTargetForm);
 
 
-  /**
-   * Event handlers
-   */
+    $(document).on("click",
+      ".js-choose-note-attachment-button",
+      NoteList.chooseNoteAttachment);
+  },
 
 
   /**
-   *
+   * When clicking on buttons
    */
-  cleanupMainTargetForm: 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 when clicking on the "add a comment" button on the side of a diff line.
@@ -122,6 +108,17 @@ var NoteList = {
   },
 
   /**
+   * 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();
+  },
+
+  /**
    * Shows the note preview.
    *
    * Lets the server render GFM into Html and displays it.
@@ -307,6 +304,11 @@ var NoteList = {
       }
     });
 
+    // 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();
@@ -500,6 +502,41 @@ var NoteList = {
   },
 
   /**
+   * 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).
    *
    * Assumes all relevant notes are displayed (i.e. there are no more notes to
diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss
index 981186a5850..465d578f3b7 100644
--- a/app/assets/stylesheets/sections/notes.scss
+++ b/app/assets/stylesheets/sections/notes.scss
@@ -227,6 +227,11 @@ ul.notes {
 .discussion {
   .new_note {
     margin: 8px 5px 8px 0;
+
+    .note_options {
+      // because of the smaller width and the extra "cancel" button
+      margin-top: 8px;
+    }
   }
 }
 .new_note {
@@ -236,51 +241,39 @@ ul.notes {
     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;
     }
 
-    // TODO: start cleanup
-    .attachments {
+    .attachment {
+      @extend .right;
       position: relative;
       width: 350px;
       height: 50px;
-      overflow: hidden;
       margin:0 0 5px !important;
 
-      .input_file {
-        .file_name {
-          line-height: 30px;
-          width: 240px;
-          height: 28px;
-          overflow: hidden;
-        }
-        .file_upload {
-          position: absolute;
-          right:14px;
-          top:7px;
-        }
-        .input-file {
-          width: 260px;
-          height: 41px;
-          float: right;
-        }
+      // hide the actual file field
+      input {
+        display: none;
+      }
+
+      .choose-btn {
+        float: right;
       }
     }
-    .input-file {
-      font: 500px monospace;
-      opacity:0;
-      filter: alpha(opacity=0);
-      position: absolute;
-      z-index: 1;
-      top:0;
-      right:0;
-      padding:0;
-      margin: 0;
+    .notify_options {
+      @extend .right;
     }
-    // TODO: end cleanup
   }
   .note_text_and_preview {
     // makes the "absolute" position for links relative to this
diff --git a/app/models/note.rb b/app/models/note.rb
index 0027c57b63b..a8ae9080627 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -140,24 +140,6 @@ class Note < ActiveRecord::Base
     @notify_author ||= false
   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
-
   # Returns true if this is an upvote note,
   # otherwise false is returned
   def upvote?
diff --git a/app/views/notes/_form.html.haml b/app/views/notes/_form.html.haml
index cb96877f766..57811daf919 100644
--- a/app/views/notes/_form.html.haml
+++ b/app/views/notes/_form.html.haml
@@ -22,23 +22,22 @@
   .clearfix
 
   .note_options
-    .attachments.right
-      %h6.left Attachment:
-      %span.file_name File name...
+    .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
 
-      .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
+      = f.file_field :attachment, class: "js-note-attachment-input"
 
-    .notify_opts.right
-      %h6.left Notify via email:
+    .notify_options
+      %h6 Notify via email:
       = label_tag :notify do
         = check_box_tag :notify, 1, !@note.for_commit?
-        %span Project team
+        Project team
 
-      - if @note.notify_only_author?(current_user) # FIXME: put in JS
+      .js-notify-commit-author
         = label_tag :notify_author do
           = check_box_tag :notify_author, 1 , !@note.for_commit?
-          %span Commit author
+          Commit author
   .clearfix