Download as
Browse Code »

Commit 7c6a58a756cc3bff78a6d0a0ca8abfe9b2c147da

Authored by Dmitriy Zaporozhets
2 parents cc28b21a 50f3280e
Exists in spb-stable and in 2 other branches 7-0-stable, 7-0-stable-spb

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
Showing 8 changed files with 81 additions and 87 deletions   Show diff stats
app/assets/javascripts/behaviors/toggler_behavior.coffee
  Diff comments   View file @7c6a58a
1 $ -> 1 $ ->
2 - $("body").on "click", ".js-toggler-target", ->  
3 - container = $(".notes-container")  
4 - container.toggleClass("on")  
5 -  
6 # Toggle button. Show/hide content inside parent container. 2 # Toggle button. Show/hide content inside parent container.
7 # Button does not change visibility. If button has icon - it changes chevron style. 3 # Button does not change visibility. If button has icon - it changes chevron style.
8 # 4 #
app/assets/javascripts/merge_request.js.coffee
  Diff comments   View file @7c6a58a
@@ -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: =>
app/assets/javascripts/notes.js.coffee
  Diff comments   View file @7c6a58a
@@ -32,6 +32,9 @@ class Notes @@ -32,6 +32,9 @@ class Notes
32 # Preview button 32 # Preview button
33 $(document).on "click", ".js-note-preview-button", @previewNote 33 $(document).on "click", ".js-note-preview-button", @previewNote
34 34
  35 + # Preview button
  36 + $(document).on "click", ".js-note-write-button", @writeNote
  37 +
35 # reset main target form after submit 38 # reset main target form after submit
36 $(document).on "ajax:complete", ".js-main-target-form", @resetMainTargetForm 39 $(document).on "ajax:complete", ".js-main-target-form", @resetMainTargetForm
37 40
@@ -68,6 +71,7 @@ class Notes @@ -68,6 +71,7 @@ class Notes
68 $(document).off "click", ".js-note-delete" 71 $(document).off "click", ".js-note-delete"
69 $(document).off "click", ".js-note-attachment-delete" 72 $(document).off "click", ".js-note-attachment-delete"
70 $(document).off "click", ".js-note-preview-button" 73 $(document).off "click", ".js-note-preview-button"
  74 + $(document).off "click", ".js-note-write-button"
71 $(document).off "ajax:complete", ".js-main-target-form" 75 $(document).off "ajax:complete", ".js-main-target-form"
72 $(document).off "click", ".js-choose-note-attachment-button" 76 $(document).off "click", ".js-choose-note-attachment-button"
73 $(document).off "click", ".js-discussion-reply-button" 77 $(document).off "click", ".js-discussion-reply-button"
@@ -145,15 +149,35 @@ class Notes @@ -145,15 +149,35 @@ class Notes
145 @removeDiscussionNoteForm(form) 149 @removeDiscussionNoteForm(form)
146 150
147 ### 151 ###
  152 + Shows write note textarea.
  153 + ###
  154 + writeNote: (e) ->
  155 + e.preventDefault()
  156 + form = $(this).closest("form")
  157 + # toggle tabs
  158 + form.find(".js-note-write-button").parent().addClass "active"
  159 + form.find(".js-note-preview-button").parent().removeClass "active"
  160 +
  161 + # toggle content
  162 + form.find(".note-write-holder").show()
  163 + form.find(".note-preview-holder").hide()
  164 +
  165 + ###
148 Shows the note preview. 166 Shows the note preview.
149 167
150 Lets the server render GFM into Html and displays it. 168 Lets the server render GFM into Html and displays it.
151 -  
152 - Note: uses the Toggler behavior to toggle preview/edit views/buttons  
153 ### 169 ###
154 previewNote: (e) -> 170 previewNote: (e) ->
155 e.preventDefault() 171 e.preventDefault()
156 form = $(this).closest("form") 172 form = $(this).closest("form")
  173 + # toggle tabs
  174 + form.find(".js-note-write-button").parent().removeClass "active"
  175 + form.find(".js-note-preview-button").parent().addClass "active"
  176 +
  177 + # toggle content
  178 + form.find(".note-write-holder").hide()
  179 + form.find(".note-preview-holder").show()
  180 +
157 preview = form.find(".js-note-preview") 181 preview = form.find(".js-note-preview")
158 noteText = form.find(".js-note-text").val() 182 noteText = form.find(".js-note-text").val()
159 if noteText.trim().length is 0 183 if noteText.trim().length is 0
@@ -179,7 +203,7 @@ class Notes @@ -179,7 +203,7 @@ class Notes
179 form.find(".js-errors").remove() 203 form.find(".js-errors").remove()
180 204
181 # reset text and preview 205 # reset text and preview
182 - previewContainer = form.find(".js-toggler-container.note_text_and_preview") 206 + previewContainer = form.find(".note-edit-and-preview")
183 previewContainer.removeClass "on" if previewContainer.is(".on") 207 previewContainer.removeClass "on" if previewContainer.is(".on")
184 form.find(".js-note-text").val("").trigger "input" 208 form.find(".js-note-text").val("").trigger "input"
185 209
app/assets/stylesheets/behaviors.scss
  Diff comments   View file @7c6a58a
@@ -4,20 +4,3 @@ @@ -4,20 +4,3 @@
4 .js-details-container .content.hide { display: block; } 4 .js-details-container .content.hide { display: block; }
5 .js-details-container.open .content { display: block; } 5 .js-details-container.open .content { display: block; }
6 .js-details-container.open .content.hide { display: none; } 6 .js-details-container.open .content.hide { display: none; }
7 -  
8 -// Toggler  
9 -//--------  
10 -.write-preview-btn .turn-on { display: inherit; }  
11 -.write-preview-btn .turn-off { display: none; }  
12 -  
13 -.js-toggler-container .turn-off { display: none; }  
14 -.js-toggler-container.on .turn-on { display: none; }  
15 -.js-toggler-container.on .turn-off { display: inherit; }  
16 -  
17 -.js-toggler-container.on ~ .note-form-actions {  
18 - .write-preview-btn .turn-on { display: none; }  
19 -}  
20 -  
21 -.js-toggler-container.on ~ .note-form-actions {  
22 - .write-preview-btn .turn-off { display: inherit; }  
23 -}  
app/assets/stylesheets/generic/markdown_area.scss
  Diff comments   View file @7c6a58a
@@ -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 {
app/assets/stylesheets/sections/notes.scss
  Diff comments   View file @7c6a58a
@@ -83,6 +83,7 @@ ul.notes { @@ -83,6 +83,7 @@ ul.notes {
83 overflow: hidden; 83 overflow: hidden;
84 display: block; 84 display: block;
85 position:relative; 85 position:relative;
  86 + border-bottom: 1px solid #eee;
86 p { color: $style_color; } 87 p { color: $style_color; }
87 88
88 .avatar { 89 .avatar {
@@ -98,22 +99,16 @@ ul.notes { @@ -98,22 +99,16 @@ ul.notes {
98 .note-header { 99 .note-header {
99 padding-bottom: 3px; 100 padding-bottom: 3px;
100 } 101 }
  102 +
  103 + &:last-child {
  104 + border-bottom: none;
  105 + }
101 } 106 }
102 107
103 .note:target { 108 .note:target {
104 -webkit-animation:target-note 2s linear; 109 -webkit-animation:target-note 2s linear;
105 background: #fffff0; 110 background: #fffff0;
106 } 111 }
107 -  
108 - // paint top or bottom borders depending on notes direction  
109 - &:not(.reversed) .note,  
110 - &:not(.reversed) .discussion {  
111 - border-bottom: 1px solid #eee;  
112 - }  
113 - &.reversed .note,  
114 - &.reversed .discussion {  
115 - border-top: 1px solid #eee;  
116 - }  
117 } 112 }
118 113
119 .diff-file .notes_holder { 114 .diff-file .notes_holder {
@@ -140,10 +135,6 @@ ul.notes { @@ -140,10 +135,6 @@ ul.notes {
140 border-width: 1px 0; 135 border-width: 1px 0;
141 padding-top: 0; 136 padding-top: 0;
142 vertical-align: top; 137 vertical-align: top;
143 -  
144 - li {  
145 - padding: 5px;  
146 - }  
147 } 138 }
148 } 139 }
149 140
@@ -271,24 +262,33 @@ ul.notes { @@ -271,24 +262,33 @@ ul.notes {
271 .clearfix { 262 .clearfix {
272 margin-bottom: 0; 263 margin-bottom: 0;
273 } 264 }
274 - .note_text_and_preview {  
275 - .note_preview {  
276 - background: #f5f5f5;  
277 - border: 1px solid #ddd;  
278 - @include border-radius(4px);  
279 - min-height: 80px;  
280 - padding: 4px 6px;  
281 -  
282 - > p {  
283 - overflow-x: auto;  
284 - } 265 +
  266 + .note-preview-holder,
  267 + .note_text {
  268 + background: #FFF;
  269 + border: 1px solid #ddd;
  270 + min-height: 100px;
  271 + padding: 5px;
  272 + font-size: 14px;
  273 + box-shadow: none;
  274 + }
  275 +
  276 + .note-preview-holder {
  277 + > p {
  278 + overflow-x: auto;
285 } 279 }
286 - .note_text { 280 + }
  281 +
  282 + .note_text {
  283 + width: 100%;
  284 + }
  285 + .nav-tabs {
  286 + margin-bottom: 0;
  287 + border: none;
  288 +
  289 + li a,
  290 + li.active a {
287 border: 1px solid #DDD; 291 border: 1px solid #DDD;
288 - box-shadow: none;  
289 - font-size: 14px;  
290 - height: 80px;  
291 - width: 100%;  
292 } 292 }
293 } 293 }
294 } 294 }
@@ -308,14 +308,13 @@ ul.notes { @@ -308,14 +308,13 @@ ul.notes {
308 .common-note-form { 308 .common-note-form {
309 margin: 0; 309 margin: 0;
310 background: #F9F9F9; 310 background: #F9F9F9;
311 - padding: 3px; 311 + padding: 5px;
312 border: 1px solid #DDD; 312 border: 1px solid #DDD;
313 } 313 }
314 314
315 .note-form-actions { 315 .note-form-actions {
316 background: #F9F9F9; 316 background: #F9F9F9;
317 height: 45px; 317 height: 45px;
318 - padding: 0 5px;  
319 318
320 .note-form-option { 319 .note-form-option {
321 margin-top: 8px; 320 margin-top: 8px;
app/views/projects/merge_requests/_show.html.haml
  Diff comments   View file @7c6a58a
@@ -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
app/views/projects/notes/_form.html.haml
  Diff comments   View file @7c6a58a
@@ -5,21 +5,28 @@ @@ -5,21 +5,28 @@
5 = f.hidden_field :noteable_id 5 = f.hidden_field :noteable_id
6 = f.hidden_field :noteable_type 6 = f.hidden_field :noteable_type
7 7
8 - .note_text_and_preview.js-toggler-container.notes-container  
9 - = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input turn-on markdown-area'  
10 - .note_preview.js-note-preview.turn-off 8 + %ul.nav.nav-tabs
  9 + %li.active
  10 + = link_to '#note-write-holder', class: 'js-note-write-button' do
  11 + Write
  12 + %li
  13 + = link_to '#note-preview-holder', class: 'js-note-preview-button', data: { url: preview_project_notes_path(@project) } do
  14 + Preview
  15 + %div
  16 + .note-write-holder
  17 + = f.text_area :note, size: 255, class: 'note_text js-note-text js-gfm-input markdown-area'
11 18
12 - .hint  
13 - .pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.  
14 - .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.  
15 - .clearfix  
16 - .error-alert 19 + .light.clearfix
  20 + .pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
  21 + .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }.
  22 +
  23 + .note-preview-holder.hide
  24 + .js-note-preview
17 25
18 .note-form-actions 26 .note-form-actions
19 .buttons 27 .buttons
20 = f.submit 'Add Comment', class: "btn comment-btn btn-grouped js-comment-button" 28 = f.submit 'Add Comment', class: "btn comment-btn btn-grouped js-comment-button"
21 = yield(:note_actions) 29 = yield(:note_actions)
22 -  
23 %a.btn.grouped.js-close-discussion-note-form Cancel 30 %a.btn.grouped.js-close-discussion-note-form Cancel
24 31
25 .note-form-option 32 .note-form-option
@@ -30,14 +37,5 @@ @@ -30,14 +37,5 @@
30 %span.file_name.js-attachment-filename File name... 37 %span.file_name.js-attachment-filename File name...
31 = f.file_field :attachment, class: "js-note-attachment-input hidden" 38 = f.file_field :attachment, class: "js-note-attachment-input hidden"
32 39
33 - .write-preview-btn  
34 - %a.btn.js-note-preview-button.js-toggler-target.turn-off{ href: "javascript:;", data: {url: preview_project_notes_path(@project)} }  
35 - %i.icon-eye-open  
36 - Preview  
37 - %a.btn.btn-primary.js-note-edit-button.js-toggler-target.turn-off{ href: "javascript:;" }  
38 - %i.icon-edit  
39 - Write  
40 - .clearfix  
41 -  
42 :javascript 40 :javascript
43 window.project_image_path_upload = "#{upload_image_project_path @project}"; 41 window.project_image_path_upload = "#{upload_image_project_path @project}";