Merge branch 'ui-improvements' into 'master'

Comment UI improvements

* use tabs for preview/write comment form because its better UX
* fix paddings for comment form and unnecessary borders
* remove unused css/js
* cleanup messy comment form html

8 files changed, 81 insertions, 87 deletions

diff --git a/app/assets/javascripts/behaviors/toggler_behavior.coffee b/app/assets/javascripts/behaviors/toggler_behavior.coffee
index 8ac5bfe95d8..1b2ed9efc25 100644
--- a/app/assets/javascripts/behaviors/toggler_behavior.coffee
+++ b/app/assets/javascripts/behaviors/toggler_behavior.coffee
@@ -1,8 +1,4 @@
 $ ->
-  $("body").on "click", ".js-toggler-target", ->
-    container = $(".notes-container")
-    container.toggleClass("on")
-
   # Toggle button. Show/hide content inside parent container.
   # Button does not change visibility. If button has icon - it changes chevron style.
   #
diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee
index 7589fc70cd8..b04a81c85eb 100644
--- a/app/assets/javascripts/merge_request.js.coffee
+++ b/app/assets/javascripts/merge_request.js.coffee
@@ -43,7 +43,7 @@ class MergeRequest
       , 'json'
 
   bindEvents: ->
-    this.$('.nav-tabs').on 'click', 'a', (event) =>
+    this.$('.merge-request-tabs').on 'click', 'a', (event) =>
       a = $(event.currentTarget)
 
       href = a.attr('href')
@@ -51,7 +51,7 @@ class MergeRequest
 
       event.preventDefault()
 
-    this.$('.nav-tabs').on 'click', 'li', (event) =>
+    this.$('.merge-request-tabs').on 'click', 'li', (event) =>
       this.activateTab($(event.currentTarget).data('action'))
 
     this.$('.accept_merge_request').on 'click', ->
@@ -71,15 +71,15 @@ class MergeRequest
       this.$('.remove_source_branch_widget.failed').show()
 
   activateTab: (action) ->
-    this.$('.nav-tabs li').removeClass 'active'
+    this.$('.merge-request-tabs li').removeClass 'active'
     this.$('.tab-content').hide()
     switch action
       when 'diffs'
-        this.$('.nav-tabs .diffs-tab').addClass 'active'
+        this.$('.merge-request-tabs .diffs-tab').addClass 'active'
         this.loadDiff() unless @diffs_loaded
         this.$('.diffs').show()
       else
-        this.$('.nav-tabs .notes-tab').addClass 'active'
+        this.$('.merge-request-tabs .notes-tab').addClass 'active'
         this.$('.notes').show()
 
   showState: (state) ->
@@ -107,7 +107,7 @@ class MergeRequest
   loadDiff: (event) ->
     $.ajax
       type: 'GET'
-      url: this.$('.nav-tabs .diffs-tab a').attr('href')
+      url: this.$('.merge-request-tabs .diffs-tab a').attr('href')
       beforeSend: =>
         this.$('.status').addClass 'loading'
       complete: =>
diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 4510718c2fd..5be001e0a22 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -32,6 +32,9 @@ class Notes
     # Preview button
     $(document).on "click", ".js-note-preview-button", @previewNote
 
+    # Preview button
+    $(document).on "click", ".js-note-write-button", @writeNote
+
     # reset main target form after submit
     $(document).on "ajax:complete", ".js-main-target-form", @resetMainTargetForm
 
@@ -68,6 +71,7 @@ class Notes
     $(document).off "click", ".js-note-delete"
     $(document).off "click", ".js-note-attachment-delete"
     $(document).off "click", ".js-note-preview-button"
+    $(document).off "click", ".js-note-write-button"
     $(document).off "ajax:complete", ".js-main-target-form"
     $(document).off "click", ".js-choose-note-attachment-button"
     $(document).off "click", ".js-discussion-reply-button"
@@ -145,15 +149,35 @@ class Notes
     @removeDiscussionNoteForm(form)
 
   ###
+  Shows write note textarea.
+  ###
+  writeNote: (e) ->
+    e.preventDefault()
+    form = $(this).closest("form")
+    # toggle tabs
+    form.find(".js-note-write-button").parent().addClass "active"
+    form.find(".js-note-preview-button").parent().removeClass "active"
+
+    # toggle content
+    form.find(".note-write-holder").show()
+    form.find(".note-preview-holder").hide()
+
+  ###
   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: (e) ->
     e.preventDefault()
     form = $(this).closest("form")
+    # toggle tabs
+    form.find(".js-note-write-button").parent().removeClass "active"
+    form.find(".js-note-preview-button").parent().addClass "active"
+
+    # toggle content
+    form.find(".note-write-holder").hide()
+    form.find(".note-preview-holder").show()
+
     preview = form.find(".js-note-preview")
     noteText = form.find(".js-note-text").val()
     if noteText.trim().length is 0
@@ -179,7 +203,7 @@ class Notes
     form.find(".js-errors").remove()
 
     # reset text and preview
-    previewContainer = form.find(".js-toggler-container.note_text_and_preview")
+    previewContainer = form.find(".note-edit-and-preview")
     previewContainer.removeClass "on"  if previewContainer.is(".on")
     form.find(".js-note-text").val("").trigger "input"
 
diff --git a/app/assets/stylesheets/behaviors.scss b/app/assets/stylesheets/behaviors.scss
index 64e3c8d9ace..23f206ce3dc 100644
--- a/app/assets/stylesheets/behaviors.scss
+++ b/app/assets/stylesheets/behaviors.scss
@@ -4,20 +4,3 @@
 .js-details-container .content.hide { display: block; }
 .js-details-container.open .content { display: block; }
 .js-details-container.open .content.hide { display: none; }
-
-// Toggler
-//--------
-.write-preview-btn .turn-on  { display: inherit; }
-.write-preview-btn .turn-off { display: none; }
-
-.js-toggler-container .turn-off { display: none; }
-.js-toggler-container.on .turn-on  { display: none; }
-.js-toggler-container.on .turn-off { display: inherit; }
-
-.js-toggler-container.on ~ .note-form-actions { 
-	.write-preview-btn .turn-on { display: none; }
-}
-
-.js-toggler-container.on ~ .note-form-actions { 
-	.write-preview-btn .turn-off { display: inherit; }
-}
diff --git a/app/assets/stylesheets/generic/markdown_area.scss b/app/assets/stylesheets/generic/markdown_area.scss
index a1fe18b02fa..fbfa72c5e5e 100644
--- a/app/assets/stylesheets/generic/markdown_area.scss
+++ b/app/assets/stylesheets/generic/markdown_area.scss
@@ -43,12 +43,6 @@
       display: none;
     }
   }
-
-  .hint {
-    float: left;
-    padding: 0;
-    margin: 0;
-  }
 }
 
 .div-dropzone-alert {
diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss
index 755295be1f4..2758f57bd54 100644
--- a/app/assets/stylesheets/sections/notes.scss
+++ b/app/assets/stylesheets/sections/notes.scss
@@ -83,6 +83,7 @@ ul.notes {
     overflow: hidden;
     display: block;
     position:relative;
+    border-bottom: 1px solid #eee;
     p { color: $style_color; }
 
     .avatar {
@@ -98,22 +99,16 @@ ul.notes {
     .note-header {
       padding-bottom: 3px;
     }
+
+    &:last-child {
+      border-bottom: none;
+    }
   }
 
   .note:target {
     -webkit-animation:target-note 2s linear;
     background: #fffff0;
   }
-
-  // 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;
-  }
 }
 
 .diff-file .notes_holder {
@@ -140,10 +135,6 @@ ul.notes {
       border-width: 1px 0;
       padding-top: 0;
       vertical-align: top;
-
-      li {
-        padding: 5px;
-      }
     }
   }
 
@@ -271,24 +262,33 @@ ul.notes {
   .clearfix {
     margin-bottom: 0;
   }
-  .note_text_and_preview {
-    .note_preview {
-      background: #f5f5f5;
-      border: 1px solid #ddd;
-      @include border-radius(4px);
-      min-height: 80px;
-      padding: 4px 6px;
-
-      > p {
-        overflow-x: auto;
-      }
+
+  .note-preview-holder,
+  .note_text {
+    background: #FFF;
+    border: 1px solid #ddd;
+    min-height: 100px;
+    padding: 5px;
+    font-size: 14px;
+    box-shadow: none;
+  }
+
+  .note-preview-holder {
+    > p {
+      overflow-x: auto;
     }
-    .note_text {
+  }
+
+  .note_text {
+    width: 100%;
+  }
+  .nav-tabs {
+    margin-bottom: 0;
+    border: none;
+
+    li a,
+    li.active a {
       border: 1px solid #DDD;
-      box-shadow: none;
-      font-size: 14px;
-      height: 80px;
-      width: 100%;
     }
   }
 }
@@ -308,14 +308,13 @@ ul.notes {
 .common-note-form {
   margin: 0;
   background: #F9F9F9;
-  padding: 3px;
+  padding: 5px;
   border: 1px solid #DDD;
 }
 
 .note-form-actions {
   background: #F9F9F9;
   height: 45px;
-  padding: 0 5px;
 
   .note-form-option {
     margin-top: 8px;
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index 193c600f774..0166b988e58 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -7,7 +7,7 @@
   = render "projects/merge_requests/show/participants"
 
   - if @commits.present?
-    %ul.nav.nav-tabs
+    %ul.nav.nav-tabs.merge-request-tabs
       %li.notes-tab{data: {action: 'notes'}}
         = link_to project_merge_request_path(@project, @merge_request) do
           %i.icon-comment
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index cadae9da805..2e84dcea899 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -5,21 +5,28 @@
   = f.hidden_field :noteable_id
   = f.hidden_field :noteable_type
 
-  .note_text_and_preview.js-toggler-container.notes-container
-    = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input turn-on markdown-area'
-    .note_preview.js-note-preview.turn-off
+  %ul.nav.nav-tabs
+    %li.active
+      = link_to '#note-write-holder', class: 'js-note-write-button' do
+        Write
+    %li
+      = link_to '#note-preview-holder', class: 'js-note-preview-button', data: { url: preview_project_notes_path(@project) } do
+        Preview
+  %div
+    .note-write-holder
+      = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input markdown-area'
 
-  .hint
-    .pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
-    .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.
-  .clearfix
-  .error-alert
+      .light.clearfix
+        .pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
+        .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.
+
+    .note-preview-holder.hide
+      .js-note-preview
 
   .note-form-actions
     .buttons
       = f.submit 'Add Comment', class: "btn comment-btn btn-grouped js-comment-button"
       = yield(:note_actions)
-
       %a.btn.grouped.js-close-discussion-note-form Cancel
 
     .note-form-option
@@ -30,14 +37,5 @@
       %span.file_name.js-attachment-filename File name...
       = f.file_field :attachment, class: "js-note-attachment-input hidden"
 
-    .write-preview-btn
-      %a.btn.js-note-preview-button.js-toggler-target.turn-off{ href: "javascript:;", data: {url: preview_project_notes_path(@project)} }
-        %i.icon-eye-open
-        Preview
-      %a.btn.btn-primary.js-note-edit-button.js-toggler-target.turn-off{ href: "javascript:;" }
-        %i.icon-edit
-        Write
-    .clearfix
-
 :javascript
   window.project_image_path_upload = "#{upload_image_project_path @project}";