Commit b47173da6a0fea0982d009f91e2c4d042f9b5c37
1 parent
4ed82788
Exists in
master
and in
4 other branches
Revamped note form options.
Showing
4 changed files
with
100 additions
and
89 deletions
Show diff stats
app/assets/javascripts/notes.js
| @@ -32,12 +32,6 @@ var NoteList = { | @@ -32,12 +32,6 @@ var NoteList = { | ||
| 32 | // get initial set of notes | 32 | // get initial set of notes |
| 33 | NoteList.getContent(); | 33 | NoteList.getContent(); |
| 34 | 34 | ||
| 35 | - $("#note_attachment").change(function(e){ | ||
| 36 | - var val = $('.input-file').val(); | ||
| 37 | - var filename = val.replace(/^.*[\\\/]/, ''); | ||
| 38 | - $(".file_name").text(filename); | ||
| 39 | - }); | ||
| 40 | - | ||
| 41 | // add a new diff note | 35 | // add a new diff note |
| 42 | $(document).on("click", | 36 | $(document).on("click", |
| 43 | ".js-add-diff-note-button", | 37 | ".js-add-diff-note-button", |
| @@ -53,6 +47,11 @@ var NoteList = { | @@ -53,6 +47,11 @@ var NoteList = { | ||
| 53 | ".js-note-preview-button", | 47 | ".js-note-preview-button", |
| 54 | NoteList.previewNote); | 48 | NoteList.previewNote); |
| 55 | 49 | ||
| 50 | + // update the file name when an attachment is selected | ||
| 51 | + $(document).on("change", | ||
| 52 | + ".js-note-attachment-input", | ||
| 53 | + NoteList.updateFormAttachment); | ||
| 54 | + | ||
| 56 | // hide diff note form | 55 | // hide diff note form |
| 57 | $(document).on("click", | 56 | $(document).on("click", |
| 58 | ".js-close-discussion-note-form", | 57 | ".js-close-discussion-note-form", |
| @@ -63,34 +62,21 @@ var NoteList = { | @@ -63,34 +62,21 @@ var NoteList = { | ||
| 63 | ".js-note-delete", | 62 | ".js-note-delete", |
| 64 | NoteList.removeNote); | 63 | NoteList.removeNote); |
| 65 | 64 | ||
| 66 | - // clean up previews for main target form | 65 | + // reset main target form after submit |
| 67 | $(document).on("ajax:complete", | 66 | $(document).on("ajax:complete", |
| 68 | ".js-main-target-form", | 67 | ".js-main-target-form", |
| 69 | - NoteList.cleanupMainTargetForm); | ||
| 70 | - }, | 68 | + NoteList.resetMainTargetForm); |
| 71 | 69 | ||
| 72 | 70 | ||
| 73 | - /** | ||
| 74 | - * Event handlers | ||
| 75 | - */ | 71 | + $(document).on("click", |
| 72 | + ".js-choose-note-attachment-button", | ||
| 73 | + NoteList.chooseNoteAttachment); | ||
| 74 | + }, | ||
| 76 | 75 | ||
| 77 | 76 | ||
| 78 | /** | 77 | /** |
| 79 | - * | 78 | + * When clicking on buttons |
| 80 | */ | 79 | */ |
| 81 | - cleanupMainTargetForm: function(){ | ||
| 82 | - var form = $(this); | ||
| 83 | - | ||
| 84 | - // remove validation errors | ||
| 85 | - form.find(".js-errors").remove(); | ||
| 86 | - | ||
| 87 | - // reset text and preview | ||
| 88 | - var previewContainer = form.find(".js-toggler-container.note_text_and_preview"); | ||
| 89 | - if (previewContainer.is(".on")) { | ||
| 90 | - previewContainer.removeClass("on"); | ||
| 91 | - } | ||
| 92 | - form.find(".js-note-text").val("").trigger("input"); | ||
| 93 | - }, | ||
| 94 | 80 | ||
| 95 | /** | 81 | /** |
| 96 | * Called when clicking on the "add a comment" button on the side of a diff line. | 82 | * Called when clicking on the "add a comment" button on the side of a diff line. |
| @@ -122,6 +108,17 @@ var NoteList = { | @@ -122,6 +108,17 @@ var NoteList = { | ||
| 122 | }, | 108 | }, |
| 123 | 109 | ||
| 124 | /** | 110 | /** |
| 111 | + * Called when clicking the "Choose File" button. | ||
| 112 | + * | ||
| 113 | + * Opesn the file selection dialog. | ||
| 114 | + */ | ||
| 115 | + chooseNoteAttachment: function() { | ||
| 116 | + var form = $(this).closest("form"); | ||
| 117 | + | ||
| 118 | + form.find(".js-note-attachment-input").click(); | ||
| 119 | + }, | ||
| 120 | + | ||
| 121 | + /** | ||
| 125 | * Shows the note preview. | 122 | * Shows the note preview. |
| 126 | * | 123 | * |
| 127 | * Lets the server render GFM into Html and displays it. | 124 | * Lets the server render GFM into Html and displays it. |
| @@ -307,6 +304,11 @@ var NoteList = { | @@ -307,6 +304,11 @@ var NoteList = { | ||
| 307 | } | 304 | } |
| 308 | }); | 305 | }); |
| 309 | 306 | ||
| 307 | + // remove notify commit author checkbox for non-commit notes | ||
| 308 | + if (form.find("#note_noteable_type").val() !== "Commit") { | ||
| 309 | + form.find(".js-notify-commit-author").remove(); | ||
| 310 | + } | ||
| 311 | + | ||
| 310 | GitLab.GfmAutoComplete.setup(); | 312 | GitLab.GfmAutoComplete.setup(); |
| 311 | 313 | ||
| 312 | form.show(); | 314 | form.show(); |
| @@ -500,6 +502,41 @@ var NoteList = { | @@ -500,6 +502,41 @@ var NoteList = { | ||
| 500 | }, | 502 | }, |
| 501 | 503 | ||
| 502 | /** | 504 | /** |
| 505 | + * Called in response the main target form has been successfully submitted. | ||
| 506 | + * | ||
| 507 | + * Removes any errors. | ||
| 508 | + * Resets text and preview. | ||
| 509 | + * Resets buttons. | ||
| 510 | + */ | ||
| 511 | + resetMainTargetForm: function(){ | ||
| 512 | + var form = $(this); | ||
| 513 | + | ||
| 514 | + // remove validation errors | ||
| 515 | + form.find(".js-errors").remove(); | ||
| 516 | + | ||
| 517 | + // reset text and preview | ||
| 518 | + var previewContainer = form.find(".js-toggler-container.note_text_and_preview"); | ||
| 519 | + if (previewContainer.is(".on")) { | ||
| 520 | + previewContainer.removeClass("on"); | ||
| 521 | + } | ||
| 522 | + form.find(".js-note-text").val("").trigger("input"); | ||
| 523 | + }, | ||
| 524 | + | ||
| 525 | + /** | ||
| 526 | + * Called after an attachment file has been selected. | ||
| 527 | + * | ||
| 528 | + * Updates the file name for the selected attachment. | ||
| 529 | + */ | ||
| 530 | + updateFormAttachment: function() { | ||
| 531 | + var form = $(this).closest("form"); | ||
| 532 | + | ||
| 533 | + // get only the basename | ||
| 534 | + var filename = $(this).val().replace(/^.*[\\\/]/, ''); | ||
| 535 | + | ||
| 536 | + form.find(".js-attachment-filename").text(filename); | ||
| 537 | + }, | ||
| 538 | + | ||
| 539 | + /** | ||
| 503 | * Recalculates the votes and updates them (if they are displayed at all). | 540 | * Recalculates the votes and updates them (if they are displayed at all). |
| 504 | * | 541 | * |
| 505 | * Assumes all relevant notes are displayed (i.e. there are no more notes to | 542 | * Assumes all relevant notes are displayed (i.e. there are no more notes to |
app/assets/stylesheets/sections/notes.scss
| @@ -227,6 +227,11 @@ ul.notes { | @@ -227,6 +227,11 @@ ul.notes { | ||
| 227 | .discussion { | 227 | .discussion { |
| 228 | .new_note { | 228 | .new_note { |
| 229 | margin: 8px 5px 8px 0; | 229 | margin: 8px 5px 8px 0; |
| 230 | + | ||
| 231 | + .note_options { | ||
| 232 | + // because of the smaller width and the extra "cancel" button | ||
| 233 | + margin-top: 8px; | ||
| 234 | + } | ||
| 230 | } | 235 | } |
| 231 | } | 236 | } |
| 232 | .new_note { | 237 | .new_note { |
| @@ -236,51 +241,39 @@ ul.notes { | @@ -236,51 +241,39 @@ ul.notes { | ||
| 236 | float: left; | 241 | float: left; |
| 237 | margin-top: 8px; | 242 | margin-top: 8px; |
| 238 | } | 243 | } |
| 244 | + .clearfix { | ||
| 245 | + margin-bottom: 0; | ||
| 246 | + } | ||
| 239 | .note_options { | 247 | .note_options { |
| 240 | h6 { | 248 | h6 { |
| 241 | - line-height: 32px; | ||
| 242 | - padding-right: 15px; | 249 | + @extend .left; |
| 250 | + line-height: 20px; | ||
| 251 | + padding-right: 16px; | ||
| 252 | + padding-bottom: 16px; | ||
| 253 | + } | ||
| 254 | + label { | ||
| 255 | + padding: 0; | ||
| 243 | } | 256 | } |
| 244 | 257 | ||
| 245 | - // TODO: start cleanup | ||
| 246 | - .attachments { | 258 | + .attachment { |
| 259 | + @extend .right; | ||
| 247 | position: relative; | 260 | position: relative; |
| 248 | width: 350px; | 261 | width: 350px; |
| 249 | height: 50px; | 262 | height: 50px; |
| 250 | - overflow: hidden; | ||
| 251 | margin:0 0 5px !important; | 263 | margin:0 0 5px !important; |
| 252 | 264 | ||
| 253 | - .input_file { | ||
| 254 | - .file_name { | ||
| 255 | - line-height: 30px; | ||
| 256 | - width: 240px; | ||
| 257 | - height: 28px; | ||
| 258 | - overflow: hidden; | ||
| 259 | - } | ||
| 260 | - .file_upload { | ||
| 261 | - position: absolute; | ||
| 262 | - right:14px; | ||
| 263 | - top:7px; | ||
| 264 | - } | ||
| 265 | - .input-file { | ||
| 266 | - width: 260px; | ||
| 267 | - height: 41px; | ||
| 268 | - float: right; | ||
| 269 | - } | 265 | + // hide the actual file field |
| 266 | + input { | ||
| 267 | + display: none; | ||
| 268 | + } | ||
| 269 | + | ||
| 270 | + .choose-btn { | ||
| 271 | + float: right; | ||
| 270 | } | 272 | } |
| 271 | } | 273 | } |
| 272 | - .input-file { | ||
| 273 | - font: 500px monospace; | ||
| 274 | - opacity:0; | ||
| 275 | - filter: alpha(opacity=0); | ||
| 276 | - position: absolute; | ||
| 277 | - z-index: 1; | ||
| 278 | - top:0; | ||
| 279 | - right:0; | ||
| 280 | - padding:0; | ||
| 281 | - margin: 0; | 274 | + .notify_options { |
| 275 | + @extend .right; | ||
| 282 | } | 276 | } |
| 283 | - // TODO: end cleanup | ||
| 284 | } | 277 | } |
| 285 | .note_text_and_preview { | 278 | .note_text_and_preview { |
| 286 | // makes the "absolute" position for links relative to this | 279 | // makes the "absolute" position for links relative to this |
app/models/note.rb
| @@ -140,24 +140,6 @@ class Note < ActiveRecord::Base | @@ -140,24 +140,6 @@ class Note < ActiveRecord::Base | ||
| 140 | @notify_author ||= false | 140 | @notify_author ||= false |
| 141 | end | 141 | end |
| 142 | 142 | ||
| 143 | - # Check if we can notify commit author | ||
| 144 | - # with email about our comment | ||
| 145 | - # | ||
| 146 | - # If commit author email exist in project | ||
| 147 | - # and commit author is not passed user we can | ||
| 148 | - # send email to him | ||
| 149 | - # | ||
| 150 | - # params: | ||
| 151 | - # user - current user | ||
| 152 | - # | ||
| 153 | - # return: | ||
| 154 | - # Boolean | ||
| 155 | - # | ||
| 156 | - def notify_only_author?(user) | ||
| 157 | - for_commit? && commit_author && | ||
| 158 | - commit_author.email != user.email | ||
| 159 | - end | ||
| 160 | - | ||
| 161 | # Returns true if this is an upvote note, | 143 | # Returns true if this is an upvote note, |
| 162 | # otherwise false is returned | 144 | # otherwise false is returned |
| 163 | def upvote? | 145 | def upvote? |
app/views/notes/_form.html.haml
| @@ -22,23 +22,22 @@ | @@ -22,23 +22,22 @@ | ||
| 22 | .clearfix | 22 | .clearfix |
| 23 | 23 | ||
| 24 | .note_options | 24 | .note_options |
| 25 | - .attachments.right | ||
| 26 | - %h6.left Attachment: | ||
| 27 | - %span.file_name File name... | 25 | + .attachment |
| 26 | + %h6 Attachment: | ||
| 27 | + .file_name.js-attachment-filename File name... | ||
| 28 | + %a.choose-btn.btn.small.js-choose-note-attachment-button Choose File ... | ||
| 29 | + .hint Any file up to 10 MB | ||
| 28 | 30 | ||
| 29 | - .input.input_file | ||
| 30 | - %a.file_upload.btn.small Upload File | ||
| 31 | - = f.file_field :attachment, class: "input-file" | ||
| 32 | - %span.hint Any file less than 10 MB | 31 | + = f.file_field :attachment, class: "js-note-attachment-input" |
| 33 | 32 | ||
| 34 | - .notify_opts.right | ||
| 35 | - %h6.left Notify via email: | 33 | + .notify_options |
| 34 | + %h6 Notify via email: | ||
| 36 | = label_tag :notify do | 35 | = label_tag :notify do |
| 37 | = check_box_tag :notify, 1, !@note.for_commit? | 36 | = check_box_tag :notify, 1, !@note.for_commit? |
| 38 | - %span Project team | 37 | + Project team |
| 39 | 38 | ||
| 40 | - - if @note.notify_only_author?(current_user) # FIXME: put in JS | 39 | + .js-notify-commit-author |
| 41 | = label_tag :notify_author do | 40 | = label_tag :notify_author do |
| 42 | = check_box_tag :notify_author, 1 , !@note.for_commit? | 41 | = check_box_tag :notify_author, 1 , !@note.for_commit? |
| 43 | - %span Commit author | 42 | + Commit author |
| 44 | .clearfix | 43 | .clearfix |