Commit 7978f8dd2b62baacb0d045b65282b758a76da118
1 parent
c1ffee4e
Exists in
master
and in
4 other branches
Fix handling form errors.
Showing
4 changed files
with
68 additions
and
21 deletions
Show diff stats
app/assets/javascripts/notes.js
| ... | ... | @@ -69,12 +69,10 @@ var NoteList = { |
| 69 | 69 | ".js-note-delete", |
| 70 | 70 | NoteList.removeNote); |
| 71 | 71 | |
| 72 | - // clean up previews for forms | |
| 73 | - $(document).on("ajax:complete", ".js-main-target-form", function(){ | |
| 74 | - $(this).find('.error').remove(); | |
| 75 | - $(this).find('.js-note-text').val(""); | |
| 76 | - $(this).show(); | |
| 77 | - }); | |
| 72 | + // clean up previews for main target form | |
| 73 | + $(document).on("ajax:complete", | |
| 74 | + ".js-main-target-form", | |
| 75 | + NoteList.cleanupMainTargetForm); | |
| 78 | 76 | }, |
| 79 | 77 | |
| 80 | 78 | |
| ... | ... | @@ -84,6 +82,26 @@ var NoteList = { |
| 84 | 82 | |
| 85 | 83 | |
| 86 | 84 | /** |
| 85 | + * | |
| 86 | + */ | |
| 87 | + cleanupMainTargetForm: function(){ | |
| 88 | + var form = $(this); | |
| 89 | + | |
| 90 | + // remove validation errors | |
| 91 | + form.find(".js-errors").remove(); | |
| 92 | + | |
| 93 | + // reset text and preview | |
| 94 | + var previewContainer = form.find(".js-toggler-container.note_text_and_preview"); | |
| 95 | + if (previewContainer.is(".on")) { | |
| 96 | + previewContainer.removeClass("on"); | |
| 97 | + } | |
| 98 | + form.find(".js-note-text").val("").trigger("input"); | |
| 99 | + | |
| 100 | + // re-enable submit button | |
| 101 | + form.find(".js-comment-button").enable(); | |
| 102 | + }, | |
| 103 | + | |
| 104 | + /** | |
| 87 | 105 | * Called when clicking on the "add a comment" button on the side of a diff line. |
| 88 | 106 | * |
| 89 | 107 | * Inserts a temporary row for the form below the line. |
| ... | ... | @@ -219,6 +237,27 @@ var NoteList = { |
| 219 | 237 | |
| 220 | 238 | |
| 221 | 239 | /** |
| 240 | + * Called in response to creating a note failing validation. | |
| 241 | + * | |
| 242 | + * Adds the rendered errors to the respective form. | |
| 243 | + * If "discussionId" is null or undefined, the main target form is assumed. | |
| 244 | + */ | |
| 245 | + errorsOnForm: function(errorsHtml, discussionId) { | |
| 246 | + // find the form | |
| 247 | + if (discussionId) { | |
| 248 | + var form = $("form[rel='"+discussionId+"']"); | |
| 249 | + } else { | |
| 250 | + var form = $(".js-main-target-form"); | |
| 251 | + } | |
| 252 | + | |
| 253 | + form.find(".js-errors").remove(); | |
| 254 | + form.prepend(errorsHtml); | |
| 255 | + | |
| 256 | + form.find(".js-note-text").focus(); | |
| 257 | + }, | |
| 258 | + | |
| 259 | + | |
| 260 | + /** | |
| 222 | 261 | * Shows the diff/discussion form and does some setup on it. |
| 223 | 262 | * |
| 224 | 263 | * Sets some hidden fields in the form. |
| ... | ... | @@ -235,8 +274,6 @@ var NoteList = { |
| 235 | 274 | |
| 236 | 275 | NoteList.setupNoteForm(form); |
| 237 | 276 | |
| 238 | - // cleanup after successfully creating a diff/discussion note | |
| 239 | - form.on("ajax:success", NoteList.removeDiscussionNoteForm); | |
| 240 | 277 | }, |
| 241 | 278 | |
| 242 | 279 | /** |
| ... | ... | @@ -449,19 +486,26 @@ var NoteList = { |
| 449 | 486 | |
| 450 | 487 | /** |
| 451 | 488 | * Adds a single discussion note to #notes-list. |
| 489 | + * | |
| 490 | + * Also removes the corresponding form. | |
| 452 | 491 | */ |
| 453 | 492 | appendNewDiscussionNote: function(discussionId, diffRowHtml, noteHtml) { |
| 493 | + var form = $("form[rel='"+discussionId+"']"); | |
| 494 | + var row = form.closest("tr"); | |
| 495 | + | |
| 454 | 496 | // is this the first note of discussion? |
| 455 | - var row = $("form[rel='"+discussionId+"']").closest("tr"); | |
| 456 | 497 | if (row.is(".js-temp-notes-holder")) { |
| 457 | - // insert the note and the reply button after it | |
| 498 | + // insert the note and the reply button after the temp row | |
| 458 | 499 | row.after(diffRowHtml); |
| 459 | - // will be added again below | |
| 500 | + // remove the note (will be added again below) | |
| 460 | 501 | row.next().find(".note").remove(); |
| 461 | 502 | } |
| 462 | 503 | |
| 463 | 504 | // append new note to all matching discussions |
| 464 | 505 | $(".notes[rel='"+discussionId+"']").append(noteHtml); |
| 506 | + | |
| 507 | + // cleanup after successfully creating a diff/discussion note | |
| 508 | + $.proxy(NoteList.removeDiscussionNoteForm, form).call(); | |
| 465 | 509 | }, |
| 466 | 510 | |
| 467 | 511 | /** | ... | ... |
app/views/notes/_form.html.haml
| ... | ... | @@ -5,11 +5,6 @@ |
| 5 | 5 | = f.hidden_field :noteable_id |
| 6 | 6 | = f.hidden_field :noteable_type |
| 7 | 7 | |
| 8 | - - if @note.errors.any? | |
| 9 | - .alert-message.block-message.error | |
| 10 | - - @note.errors.full_messages.each do |msg| | |
| 11 | - %div= msg | |
| 12 | - | |
| 13 | 8 | .note_text_and_preview.js-toggler-container |
| 14 | 9 | %a.js-note-preview-button.js-toggler-target.turn-on{ href: "javascript:;", data: {title: "Preview", url: preview_project_notes_path(@project)} } |
| 15 | 10 | %i.icon-eye-open | ... | ... |
app/views/notes/create.js.haml
| ... | ... | @@ -7,10 +7,15 @@ |
| 7 | 7 | - else |
| 8 | 8 | NoteList.appendNewNote(#{@note.id}, noteHtml); |
| 9 | 9 | - else |
| 10 | - var firstDiscussionNoteHtml = "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [@note])}"; | |
| 11 | - NoteList.appendNewDiscussionNote("#{@note.discussion_id}", firstDiscussionNoteHtml, noteHtml); | |
| 10 | + :plain | |
| 11 | + var firstDiscussionNoteHtml = "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [@note])}"; | |
| 12 | + NoteList.appendNewDiscussionNote("#{@note.discussion_id}", | |
| 13 | + firstDiscussionNoteHtml, | |
| 14 | + noteHtml); | |
| 12 | 15 | |
| 13 | 16 | - else |
| 14 | - -# TODO: insert form correctly | |
| 15 | - $(".js-main-target-note").replaceWith("#{escape_javascript(render 'notes/common_form')}"); | |
| 16 | - GitLab.GfmAutoComplete.setup(); | |
| 17 | 17 | \ No newline at end of file |
| 18 | + var errorsHtml = "#{escape_javascript(render 'notes/form_errors', note: @note)}"; | |
| 19 | + - if note_for_main_target?(@note) | |
| 20 | + NoteList.errorsOnForm(errorsHtml); | |
| 21 | + - else | |
| 22 | + NoteList.errorsOnForm(errorsHtml, "#{@note.discussion_id}"); | |
| 18 | 23 | \ No newline at end of file | ... | ... |