Merge branch 'note-form-design' into 'master'

Notes form design update

Part of #3401

See merge request !3442

app/assets/javascripts/issue.js.coffee

@@ -6,25 +6,10 @@ class @Issue
   constructor: ->
     # Prevent duplicate event bindings
     @disableTaskList()
-    @fixAffixScroll()
     if $('a.btn-close').length
       @initTaskList()
       @initIssueBtnEventListeners()
 
-  fixAffixScroll: ->
-    fixAffix = ->
-      $discussion = $('.issuable-discussion')
-      $sidebar = $('.issuable-sidebar')
-      if $sidebar.hasClass('no-affix')
-        $sidebar.removeClass(['affix-top','affix'])
-      discussionHeight = $discussion.height()
-      sidebarHeight = $sidebar.height()
-      if sidebarHeight > discussionHeight
-        $discussion.height(sidebarHeight + 50)
-        $sidebar.addClass('no-affix')
-    $(window).on('resize', fixAffix)
-    fixAffix()
-
   initTaskList: ->
     $('.detail-page-description .js-task-list-container').taskList('enable')
     $(document).on 'tasklist:changed', '.detail-page-description .js-task-list-container', @updateTaskList

app/assets/javascripts/merge_request.js.coffee

@@ -15,8 +15,6 @@ class @MergeRequest
     this.$('.show-all-commits').on 'click', =>
       this.showAllCommits()
 
-    @fixAffixScroll();
-
     @initTabs()
 
     # Prevent duplicate event bindings
@@ -30,20 +28,6 @@ class @MergeRequest
   $: (selector) ->
     this.$el.find(selector)
 
-  fixAffixScroll: ->
-    fixAffix = ->
-      $discussion = $('.issuable-discussion')
-      $sidebar = $('.issuable-sidebar')
-      if $sidebar.hasClass('no-affix')
-        $sidebar.removeClass(['affix-top','affix'])
-      discussionHeight = $discussion.height()
-      sidebarHeight = $sidebar.height()
-      if sidebarHeight > discussionHeight
-        $discussion.height(sidebarHeight + 50)
-        $sidebar.addClass('no-affix')
-    $(window).on('resize', fixAffix)
-    fixAffix()
-
   initTabs: ->
     if @opts.action != 'new'
       # `MergeRequests#new` has no tab-persisting or lazy-loading behavior

app/assets/javascripts/notes.js.coffee

@@ -251,13 +251,11 @@ class @Notes
   Sets some hidden fields in the form.
   ###
   setupMainTargetNoteForm: ->
-
     # find the form
     form = $(".js-new-note-form")
 
-    # insert the form after the button
-    form.clone().replaceAll $(".js-main-target-form")
-    form = form.prev("form")
+    # Set a global clone of the form for later cloning
+    @formClone = form.clone()
 
     # show the form
     @setupNoteForm(form)
@@ -266,9 +264,7 @@ class @Notes
     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.
@@ -297,7 +293,14 @@ class @Notes
       else
         previewButton.removeClass("turn-on").addClass "turn-off"
 
+    textarea.on 'focus', ->
+      $(this).closest('.md-area').addClass 'is-focused'
+
+    textarea.on 'blur', ->
+      $(this).closest('.md-area').removeClass 'is-focused'
+
     autosize(textarea)
+
     new Autosave textarea, [
       "Note"
       form.find("#note_commit_id").val()
@@ -307,7 +310,6 @@ class @Notes
     ]
 
     # remove notify commit author checkbox for non-commit notes
-    form.find(".js-notify-commit-author").remove()  if form.find("#note_noteable_type").val() isnt "Commit"
     GitLab.GfmAutoComplete.setup()
     new DropzoneInput(form)
     form.show()
@@ -455,15 +457,15 @@ class @Notes
   Shows the note form below the notes.
   ###
   replyToDiscussionNote: (e) =>
-    form = $(".js-new-note-form")
+    form = @formClone.clone()
     replyLink = $(e.target).closest(".js-discussion-reply-button")
     replyLink.hide()
 
     # insert the form after the button
-    form.clone().insertAfter replyLink
+    replyLink.after form
 
     # show the form
-    @setupDiscussionNoteForm(replyLink, replyLink.next("form"))
+    @setupDiscussionNoteForm(replyLink, form)
 
   ###
   Shows the diff or discussion form and does some setup on it.
@@ -488,7 +490,9 @@ class @Notes
         .text(form.find('.js-close-discussion-note-form').data('cancel-text'))
     @setupNoteForm form
     form.find(".js-note-text").focus()
-    form.addClass "js-discussion-note-form"
+    form
+      .removeClass('js-main-target-form')
+      .addClass("discussion-form js-discussion-note-form")
 
   ###
   Called when clicking on the "add a comment" button on the side of a diff line.
@@ -498,9 +502,8 @@ class @Notes
   ###
   addDiffNote: (e) =>
     e.preventDefault()
-    link = e.currentTarget
-    form = $(".js-new-note-form")
-    row = $(link).closest("tr")
+    $link = $(e.currentTarget)
+    row = $link.closest("tr")
     nextRow = row.next()
     hasNotes = nextRow.is(".notes_holder")
     addForm = false
@@ -509,7 +512,7 @@ class @Notes
 
     # In parallel view, look inside the correct left/right pane
     if @isParallelView()
-      lineType = $(link).data("lineType")
+      lineType = $link.data("lineType")
       targetContent += "." + lineType
       rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\"></td><td class=\"notes_content parallel old\"></td><td class=\"notes_line\"></td><td class=\"notes_content parallel new\"></td></tr>"
 
@@ -531,11 +534,11 @@ class @Notes
       addForm = true
 
     if addForm
-      newForm = form.clone()
+      newForm = @formClone.clone()
       newForm.appendTo row.next().find(targetContent)
 
       # show the form
-      @setupDiscussionNoteForm $(link), newForm
+      @setupDiscussionNoteForm $link, newForm
 
   ###
   Called in response to "cancel" on a diff note form.
@@ -560,7 +563,6 @@ class @Notes
 
   cancelDiscussionForm: (e) =>
     e.preventDefault()
-    form = $(".js-new-note-form")
     form = $(e.target).closest(".js-discussion-note-form")
     @removeDiscussionNoteForm(form)
 

app/assets/javascripts/zen_mode.js.coffee

@@ -42,7 +42,7 @@ class @ZenMode
       $(e.currentTarget).trigger('zen_mode:leave')
 
     $(document).on 'zen_mode:enter', (e) =>
-      @enter(e.target.parentNode)
+      @enter($(e.target).closest('.md-area').find('.zen-backdrop'))
     $(document).on 'zen_mode:leave', (e) =>
       @exit()
 

app/assets/stylesheets/framework/common.scss

@@ -125,13 +125,6 @@ p.time {
   height: 150px;
 }
 
-// Fixes alignment on notes.
-.new_note {
-  label {
-    text-align: left;
-  }
-}
-
 // Fix issue with notes & lists creating a bunch of bottom borders.
 li.note {
   img { max-width: 100% }

app/assets/stylesheets/framework/markdown_area.scss

 .div-dropzone-wrapper {
   .div-dropzone {
     position: relative;
-    padding: 0;
-    border: 0;
-    margin-bottom: 5px;
+    margin-bottom: -5px;
 
     .div-dropzone-focus {
       border-color: #66afe9 !important;
@@ -25,12 +23,10 @@
 
     .div-dropzone-spinner {
       position: absolute;
-      top: 100%;
-      left: 100%;
-      margin-top: -1.1em;
-      margin-left: -1.1em;
+      bottom: 10px;
+      right: 5px;
       opacity: 0;
-      font-size: 30px;
+      font-size: 20px;
       transition: opacity 200ms ease-in-out;
     }
 
@@ -65,17 +61,29 @@
   position: relative;
 }
 
+.md-header {
+  .nav-links {
+    .active {
+      a {
+        border-bottom-color: #000;
+      }
+    }
+
+    a {
+      padding-top: 0;
+      line-height: 1;
+    }
+  }
+}
+
 .referenced-users {
   color: #4c4e54;
   padding-top: 10px;
 }
 
 .md-preview-holder {
-  background: #fff;
-  border: 1px solid #ddd;
-  min-height: 169px;
-  padding: 5px;
-  box-shadow: none;
+  min-height: 167px;
+  padding: 10px 0;
 }
 
 .markdown-area {

app/assets/stylesheets/framework/typography.scss

@@ -250,7 +250,7 @@ a > code {
  * Textareas intended for GFM
  *
  */
-textarea.js-gfm-input {
+.js-gfm-input {
   font-family: $monospace_font;
   color: $gl-text-color;
 }

app/assets/stylesheets/framework/variables.scss

@@ -217,3 +217,9 @@ $notes-light-color: #8e8e8e;
 $notes-action-color: #c3c3c3;
 $notes-role-color: #8e8e8e;
 $notes-role-border-color: #e4e4e4;
+
+$note-disabled-comment-color: #b2b2b2;
+$note-form-border-color: #e5e5e5;
+$note-toolbar-color: #959494;
+
+$zen-control-hover-color: #111;

app/assets/stylesheets/framework/zen.scss

-.zennable {
-  a.js-zen-enter {
-    color: $gl-gray;
-    position: absolute;
-    top: 0;
-    right: 4px;
-    line-height: 56px;
-  }
-
-  a.js-zen-leave {
-    display: none;
-    color: $gl-text-color;
-    position: absolute;
-    top: 10px;
-    right: 10px;
-    padding: 5px;
-    font-size: 36px;
-
-    &:hover {
-      color: #111;
-    }
-  }
-
-  .zen-backdrop {
+.zen-backdrop {
   &.fullscreen {
     background-color: white;
     position: fixed;
@@ -47,15 +24,39 @@
       margin: 0 auto;
     }
 
-      a.js-zen-enter {
-        display: none;
-      }
-
-      a.js-zen-leave {
+    .zen-control-leave {
       display: block;
       position: absolute;
       top: 0;
     }
   }
+}
+
+.zen-cotrol {
+  padding: 0;
+  color: #555;
+  background: none;
+  border: 0;
+}
+
+.zen-control-full {
+  color: $note-toolbar-color;
+
+  &:hover {
+    color: $gl-link-color;
+    text-decoration: none;
+  }
+}
+
+.zen-control-leave {
+  display: none;
+  color: $gl-text-color;
+  position: absolute;
+  right: 10px;
+  padding: 5px;
+  font-size: 36px;
+
+  &:hover {
+    color: $zen-control-hover-color;
   }
 }

app/assets/stylesheets/pages/merge_requests.scss

@@ -195,42 +195,6 @@
   line-height: 31px;
 }
 
-.disabled-comment-area {
-  padding: 16px 0;
-
-  .disabled-profile {
-    width: 40px;
-    height: 40px;
-    background: $border-gray-dark;
-    border-radius: 20px;
-    display: inline-block;
-    margin-right: 10px;
-  }
-
-  .disabled-comment {
-    background: $gray-light;
-    display: inline-block;
-    vertical-align: top;
-    height: 200px;
-    border-radius: 4px;
-    border: 1px solid $border-gray-normal;
-    padding-top: 90px;
-    text-align: center;
-    right: 20px;
-    position: absolute;
-    left: 70px;
-    margin-bottom: 20px;
-
-    span {
-      color: #b2b2b2;
-
-      a {
-        color: $md-link-color;
-      }
-    }
-  }
-}
-
 .builds {
   .table-holder {
     overflow-x: scroll;

app/assets/stylesheets/pages/note_form.scss

 /**
  * Note Form
  */
-
-.comment-btn {
-  @extend .btn-create;
-}
 .reply-btn {
   @extend .btn-primary;
   margin: 10px $gl-padding;
@@ -17,16 +13,17 @@
 }
 .diff-file,
 .discussion {
-  .new_note {
+  .new-note {
     margin: 0;
     border: none;
   }
 }
-.new_note {
+
+.new-note {
   display: none;
 }
 
-.new_note, .note-edit-form {
+.new-note, .note-edit-form {
   .note-form-actions {
     margin-top: $gl-padding;
   }
@@ -40,21 +37,18 @@
   img {
     max-width: 100%;
   }
+}
 
-  .note_text {
-    width: 100%;
-  }
+.note-textarea {
+  padding: 10px 0;
+  font-family: $regular_font;
+  border: 0;
 
-  .comment-hints {
-    margin-top: -12px;
+  &:focus {
+    outline: 0;
   }
 }
 
-/* loading indicator */
-.notes-busy {
-  margin: 18px;
-}
-
 .note-image-attach {
   @extend .col-md-4;
   margin-left: 45px;
@@ -62,38 +56,29 @@
 }
 
 .common-note-form {
-  margin: 0;
-  background: #fff;
-  padding: $gl-padding;
-  margin-left: -$gl-padding;
-  margin-right: -$gl-padding;
-  margin-bottom: -$gl-padding;
-}
+  .md-area {
+    padding: $gl-padding-top $gl-padding;
+    border: 1px solid $note-form-border-color;
+    border-radius: $border-radius-base;
 
-.note-form-actions {
-  .note-form-option {
-    margin-top: 8px;
-    margin-left: 30px;
-    @extend .pull-left;
-  }
+    &.is-focused {
+      border-color: $focus-border-color;
+      box-shadow: 0 0 2px rgba(#000, .2),
+                  0 0 4px rgba($focus-border-color, .4);
 
-  .js-notify-commit-author {
-    float: left;
+      .comment-toolbar,
+      .nav-links {
+        border-color: $focus-border-color;
       }
-
-  .write-preview-btn {
-    // makes the "absolute" position for links relative to this
-    position: relative;
-
-    // preview/edit buttons
-    > a {
-      position: absolute;
-      right: 5px;
-      top: 8px;
     }
   }
 }
 
+.discussion-form {
+  padding: $gl-padding-top $gl-padding;
+  background-color: #fff;
+}
+
 .note-edit-form {
   display: none;
   font-size: 15px;
@@ -152,11 +137,49 @@
   }
 }
 
-.comment-hints {
-  color: #999;
-  background: #fff;
-  padding: 7px;
-  margin-top: -7px;
-  border: 1px solid $border-color;
-  font-size: 13px;
+.comment-toolbar {
+  padding-top: $gl-padding-top;
+  color: $note-toolbar-color;
+  border-top: 1px solid $border-color;
+}
+
+.toolbar-button {
+  padding: 0;
+  background: none;
+  border: 0;
+  font-size: 14px;
+  line-height: 16px;
+
+  &:hover,
+  &:focus {
+    color: $gl-link-color;
+    outline: 0;
+  }
+
+  @media (min-width: $screen-md-min) {
+    float: left;
+    margin-right: $gl-padding;
+
+    &:last-child {
+      float: right;
+      margin-right: 0;
+    }
+  }
+}
+
+.toolbar-button-icon {
+  position: relative;
+  top: 1px;
+  margin-right: 3px;
+  color: inherit;
+  font-size: 16px;
+}
+
+.toolbar-text {
+  font-size: 14px;
+  line-height: 16px;
+
+  @media (min-width: $screen-md-min) {
+    float: left;
+  }
 }

app/assets/stylesheets/pages/notes.scss

@@ -20,6 +20,12 @@ ul.notes {
 
   .timeline-content {
     margin-left: 55px;
+
+    &.timeline-content-form {
+      @media (max-width: $screen-sm-max) {
+        margin-left: 0;
+      }
+    }
   }
 
   .note-created-ago, .note-updated-at {
@@ -149,7 +155,7 @@ ul.notes {
     &.notes_content {
       background-color: #fff;
       border-width: 1px 0;
-      padding-top: 0;
+      padding: 0;
       vertical-align: top;
       &.parallel {
         border-width: 1px;
@@ -281,3 +287,21 @@ ul.notes {
     }
   }
 }
+
+.disabled-comment {
+  margin-left: -$gl-padding-top;
+  margin-right: -$gl-padding-top;
+  background-color: $gray-light;
+  border-radius: $border-radius-base;
+  border: 1px solid $border-gray-normal;
+  color: $note-disabled-comment-color;
+  line-height: 200px;
+
+  .disabled-comment-text {
+    line-height: normal;
+  }
+
+  a {
+    color: $gl-link-color;
+  }
+}

app/views/projects/_md_preview.html.haml

 .md-area
-  .md-header.clearfix
+  .md-header
     %ul.nav-links
       %li.active
-        %a.js-md-write-button(href="#md-write-holder" tabindex="-1")
+        %a.js-md-write-button{ href: "#md-write-holder" }
           Write
       %li
-        %a.js-md-preview-button(href="#md-preview-holder" tabindex="-1")
+        %a.js-md-preview-button{ href: "#md-preview-holder" }
           Preview
+      %li.pull-right
+        %button.zen-cotrol.zen-control-full.js-zen-enter{ type: 'button' }
+          Go full screen
 
-  %div
   .md-write-holder
     = yield
-    .md.md-preview-holder.hide
-      .js-md-preview{class: (preview_class if defined?(preview_class))}
+  .md.md-preview-holder.js-md-preview.hide{class: (preview_class if defined?(preview_class))}
 
   - if defined?(referenced_users) && referenced_users
     %div.referenced-users.hide

app/views/projects/_zen.html.haml

-.zennable
-  .zen-backdrop
+.zen-backdrop
   - classes << ' js-gfm-input js-autosize markdown-area'
   - if defined?(f) && f
-      = f.text_area attr, class: classes
+    = f.text_area attr, class: classes, placeholder: "Write a comment or drag your files here..."
   - else
-      = text_area_tag attr, nil, class: classes
-    %a.js-zen-enter(tabindex="-1" href="#")
-      = icon('expand')
-      Edit in fullscreen
-    %a.js-zen-leave(tabindex="-1" href="#")
+    = text_area_tag attr, nil, class: classes, placeholder: "Write a comment or drag your files here..."
+  %a.zen-cotrol.zen-control-leave.js-zen-leave{ href: "#" }
     = icon('compress')

app/views/projects/notes/_edit_form.html.haml

 .note-edit-form
-  = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true, html: { class: 'edit-note js-quick-submit' } do |f|
+  = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true, html: { class: 'edit-note common-note-form js-quick-submit' } do |f|
     = note_target_fields(note)
     = render layout: 'projects/md_preview', locals: { preview_class: 'md-preview' } do
-      = render 'projects/zen', f: f, attr: :note, classes: 'note_text js-note-text js-task-list-field'
+      = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text js-task-list-field'
       = render 'projects/notes/hints'
 
     .note-form-actions.clearfix
       = f.submit 'Save Comment', class: 'btn btn-nr btn-save btn-grouped js-comment-button'
-      = link_to  'Cancel', '#', class: 'btn btn-nr btn-cancel note-edit-cancel'
+      %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' }
+        Cancel

app/views/projects/notes/_form.html.haml

-= form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form js-quick-submit common-note-form gfm-form" }, authenticity_token: true do |f|
+= form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form gfm-form" }, authenticity_token: true do |f|
   = hidden_field_tag :view, diff_view
   = hidden_field_tag :line_type
   = note_target_fields(@note)
@@ -8,7 +8,7 @@
   = f.hidden_field :noteable_type
 
   = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do
-    = render 'projects/zen', f: f, attr: :note, classes: 'note_text js-note-text'
+    = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text'
     = render 'projects/notes/hints'
     .error-alert
 

app/views/projects/notes/_hints.html.haml

-.comment-hints.clearfix
-  .pull-left
+.comment-toolbar.clearfix
+  .toolbar-text
+    Styling with
     = link_to 'Markdown', help_page_path('markdown', 'markdown'), target: '_blank', tabindex: -1
-    tip:
-    = random_markdown_tip
-  .pull-right
-    = link_to '#', class: 'markdown-selector', tabindex: -1 do
-      = icon('paperclip')
+    is supported
+  %button.toolbar-button.markdown-selector{ type: 'button', tabindex: '-1' }
+    = icon('file-image-o', class: 'toolbar-button-icon')
     Attach a file

app/views/projects/notes/_notes_with_form.html.haml

 %ul#notes-list.notes.main-notes-list.timeline
   = render "projects/notes/notes"
-.js-notes-busy
-
-.js-main-target-form
-- if can? current_user, :create_note, @project
+%ul.notes.timeline
+  %li.timeline-entry
+    - if can? current_user, :create_note, @project
+      .timeline-icon.hidden-xs.hidden-sm
+        %a.author_link{ href: user_path(current_user) }
+          = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40'
+      .timeline-content.timeline-content-form
         = render "projects/notes/form", view: diff_view
-- else
-  .disabled-comment-area
-    .disabled-profile
-    .disabled-comment
-      %span
+    - else
+      .disabled-comment.text-center
+        .disabled-comment-text.inline
           Please
           = link_to "register",new_user_session_path
           or

features/steps/shared/diff_note.rb

@@ -125,7 +125,7 @@ module SharedDiffNote
 
   step 'I should only see one diff form' do
     page.within(diff_file_selector) do
-      expect(page).to have_css("form.new_note", count: 1)
+      expect(page).to have_css("form.new-note", count: 1)
     end
   end
 
@@ -161,7 +161,7 @@ module SharedDiffNote
 
   step 'I should see a temporary diff comment form' do
     page.within(diff_file_selector) do
-      expect(page).to have_css(".js-temp-notes-holder form.new_note")
+      expect(page).to have_css(".js-temp-notes-holder form.new-note")
     end
   end
 

features/steps/shared/note.rb

@@ -2,7 +2,7 @@ module SharedNote
   include Spinach::DSL
 
   step 'I delete a comment' do
-    page.within('.notes') do
+    page.within('.main-notes-list') do
       find('.note').hover
       find(".js-note-delete").click
     end
@@ -128,7 +128,7 @@ module SharedNote
   end
 
   step 'I edit the last comment with a +1' do
-    page.within(".notes") do
+    page.within(".main-notes-list") do
       find(".note").hover
       find('.js-note-edit').click
     end

spec/features/issues_spec.rb

@@ -22,7 +22,7 @@ describe 'Issues', feature: true do
 
     before do
       visit edit_namespace_project_issue_path(project.namespace, project, issue)
-      click_link "Edit"
+      click_button "Go full screen"
     end
 
     it 'should open new issue popup' do

spec/features/notes_on_merge_requests_spec.rb

@@ -152,7 +152,7 @@ describe 'Comments', feature: true do
 
         it 'has .new_note css class' do
           page.within('.js-temp-notes-holder') do
-            expect(subject).to have_css('.new_note')
+            expect(subject).to have_css('.new-note')
           end
         end
       end
@@ -225,6 +225,6 @@ describe 'Comments', feature: true do
   end
 
   def click_diff_line(data = line_code)
-    page.find(%Q{button[data-line-code="#{data}"]}, visible: false).click
+    execute_script("$('button[data-line-code=\"#{data}\"]').click()")
   end
 end

spec/javascripts/fixtures/zen_mode.html.haml

-.zennable
+.md-area
   .zen-backdrop
     %textarea#note_note.js-gfm-input.markdown-area
     %a.js-zen-enter(tabindex="-1" href="#")