Improve notes form UI/UX

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Improve notes form UI/UX
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Dmitriy Zaporozhets
1 parent 0853eebd
Showing 7 changed files with 75 additions and 58 deletions Show diff stats
app/assets/javascripts/behaviors/toggler_behavior.coffee
app/assets/javascripts/merge_request.js.coffee
app/assets/javascripts/notes.js.coffee
app/assets/stylesheets/generic/markdown_area.scss
app/assets/stylesheets/sections/notes.scss
app/views/projects/merge_requests/_show.html.haml
app/views/projects/notes/_form.html.haml
 $ ->
-  $("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.
   #
@@ -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: =>
@@ -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"
@@ -43,12 +43,6 @@
       display: none;
     }
   }
-
-  .hint {
-    float: left;
-    padding: 0;
-    margin: 0;
-  }
 }
 .div-dropzone-alert {
@@ -135,10 +135,6 @@ ul.notes {
       border-width: 1px 0;
       padding-top: 0;
       vertical-align: top;
-
-      li {
-        padding: 5px;
-      }
     }
   }
@@ -266,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%;
     }
   }
 }
@@ -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
@@ -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 &amp; 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}";
	@@ -43,7 +43,7 @@ class MergeRequest		@@ -43,7 +43,7 @@ class MergeRequest
43	, 'json'	43	, 'json'
44		44
45	bindEvents: ->	45	bindEvents: ->
46	- this.$('.nav-tabs').on 'click', 'a', (event) =>	46	+ this.$('.merge-request-tabs').on 'click', 'a', (event) =>
47	a = $(event.currentTarget)	47	a = $(event.currentTarget)
48		48
49	href = a.attr('href')	49	href = a.attr('href')
	@@ -51,7 +51,7 @@ class MergeRequest		@@ -51,7 +51,7 @@ class MergeRequest
51		51
52	event.preventDefault()	52	event.preventDefault()
53		53
54	- this.$('.nav-tabs').on 'click', 'li', (event) =>	54	+ this.$('.merge-request-tabs').on 'click', 'li', (event) =>
55	this.activateTab($(event.currentTarget).data('action'))	55	this.activateTab($(event.currentTarget).data('action'))
56		56
57	this.$('.accept_merge_request').on 'click', ->	57	this.$('.accept_merge_request').on 'click', ->
	@@ -71,15 +71,15 @@ class MergeRequest		@@ -71,15 +71,15 @@ class MergeRequest
71	this.$('.remove_source_branch_widget.failed').show()	71	this.$('.remove_source_branch_widget.failed').show()
72		72
73	activateTab: (action) ->	73	activateTab: (action) ->
74	- this.$('.nav-tabs li').removeClass 'active'	74	+ this.$('.merge-request-tabs li').removeClass 'active'
75	this.$('.tab-content').hide()	75	this.$('.tab-content').hide()
76	switch action	76	switch action
77	when 'diffs'	77	when 'diffs'
78	- this.$('.nav-tabs .diffs-tab').addClass 'active'	78	+ this.$('.merge-request-tabs .diffs-tab').addClass 'active'
79	this.loadDiff() unless @diffs_loaded	79	this.loadDiff() unless @diffs_loaded
80	this.$('.diffs').show()	80	this.$('.diffs').show()
81	else	81	else
82	- this.$('.nav-tabs .notes-tab').addClass 'active'	82	+ this.$('.merge-request-tabs .notes-tab').addClass 'active'
83	this.$('.notes').show()	83	this.$('.notes').show()
84		84
85	showState: (state) ->	85	showState: (state) ->
	@@ -107,7 +107,7 @@ class MergeRequest		@@ -107,7 +107,7 @@ class MergeRequest
107	loadDiff: (event) ->	107	loadDiff: (event) ->
108	$.ajax	108	$.ajax
109	type: 'GET'	109	type: 'GET'
110	- url: this.$('.nav-tabs .diffs-tab a').attr('href')	110	+ url: this.$('.merge-request-tabs .diffs-tab a').attr('href')
111	beforeSend: =>	111	beforeSend: =>
112	this.$('.status').addClass 'loading'	112	this.$('.status').addClass 'loading'
113	complete: =>	113	complete: =>
	@@ -43,12 +43,6 @@		@@ -43,12 +43,6 @@
43	display: none;	43	display: none;
44	}	44	}
45	}	45	}
46	-
47	- .hint {
48	- float: left;
49	- padding: 0;
50	- margin: 0;
51	- }
52	}	46	}
53		47
54	.div-dropzone-alert {	48	.div-dropzone-alert {
	@@ -7,7 +7,7 @@		@@ -7,7 +7,7 @@
7	= render "projects/merge_requests/show/participants"	7	= render "projects/merge_requests/show/participants"
8		8
9	- if @commits.present?	9	- if @commits.present?
10	- %ul.nav.nav-tabs	10	+ %ul.nav.nav-tabs.merge-request-tabs
11	%li.notes-tab{data: {action: 'notes'}}	11	%li.notes-tab{data: {action: 'notes'}}
12	= link_to project_merge_request_path(@project, @merge_request) do	12	= link_to project_merge_request_path(@project, @merge_request) do
13	%i.icon-comment	13	%i.icon-comment