From 7978f8dd2b62baacb0d045b65282b758a76da118 Mon Sep 17 00:00:00 2001 From: Riyad Preukschas Date: Mon, 3 Dec 2012 20:35:09 +0100 Subject: [PATCH] Fix handling form errors. --- app/assets/javascripts/notes.js | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----------- app/views/notes/_form.html.haml | 5 ----- app/views/notes/_form_errors.html.haml | 3 +++ app/views/notes/create.js.haml | 15 ++++++++++----- 4 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 app/views/notes/_form_errors.html.haml diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index c9137c8..8c4577b 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -69,12 +69,10 @@ var NoteList = { ".js-note-delete", NoteList.removeNote); - // clean up previews for forms - $(document).on("ajax:complete", ".js-main-target-form", function(){ - $(this).find('.error').remove(); - $(this).find('.js-note-text').val(""); - $(this).show(); - }); + // clean up previews for main target form + $(document).on("ajax:complete", + ".js-main-target-form", + NoteList.cleanupMainTargetForm); }, @@ -84,6 +82,26 @@ var NoteList = { /** + * + */ + cleanupMainTargetForm: function(){ + var form = $(this); + + // remove validation errors + form.find(".js-errors").remove(); + + // reset text and preview + var previewContainer = form.find(".js-toggler-container.note_text_and_preview"); + if (previewContainer.is(".on")) { + previewContainer.removeClass("on"); + } + form.find(".js-note-text").val("").trigger("input"); + + // re-enable submit button + form.find(".js-comment-button").enable(); + }, + + /** * Called when clicking on the "add a comment" button on the side of a diff line. * * Inserts a temporary row for the form below the line. @@ -219,6 +237,27 @@ var NoteList = { /** + * Called in response to creating a note failing validation. + * + * Adds the rendered errors to the respective form. + * If "discussionId" is null or undefined, the main target form is assumed. + */ + errorsOnForm: function(errorsHtml, discussionId) { + // find the form + if (discussionId) { + var form = $("form[rel='"+discussionId+"']"); + } else { + var form = $(".js-main-target-form"); + } + + form.find(".js-errors").remove(); + form.prepend(errorsHtml); + + form.find(".js-note-text").focus(); + }, + + + /** * Shows the diff/discussion form and does some setup on it. * * Sets some hidden fields in the form. @@ -235,8 +274,6 @@ var NoteList = { NoteList.setupNoteForm(form); - // cleanup after successfully creating a diff/discussion note - form.on("ajax:success", NoteList.removeDiscussionNoteForm); }, /** @@ -449,19 +486,26 @@ var NoteList = { /** * Adds a single discussion note to #notes-list. + * + * Also removes the corresponding form. */ appendNewDiscussionNote: function(discussionId, diffRowHtml, noteHtml) { + var form = $("form[rel='"+discussionId+"']"); + var row = form.closest("tr"); + // is this the first note of discussion? - var row = $("form[rel='"+discussionId+"']").closest("tr"); if (row.is(".js-temp-notes-holder")) { - // insert the note and the reply button after it + // insert the note and the reply button after the temp row row.after(diffRowHtml); - // will be added again below + // remove the note (will be added again below) row.next().find(".note").remove(); } // append new note to all matching discussions $(".notes[rel='"+discussionId+"']").append(noteHtml); + + // cleanup after successfully creating a diff/discussion note + $.proxy(NoteList.removeDiscussionNoteForm, form).call(); }, /** diff --git a/app/views/notes/_form.html.haml b/app/views/notes/_form.html.haml index 9d60044..f4a79b0 100644 --- a/app/views/notes/_form.html.haml +++ b/app/views/notes/_form.html.haml @@ -5,11 +5,6 @@ = f.hidden_field :noteable_id = f.hidden_field :noteable_type - - if @note.errors.any? - .alert-message.block-message.error - - @note.errors.full_messages.each do |msg| - %div= msg - .note_text_and_preview.js-toggler-container %a.js-note-preview-button.js-toggler-target.turn-on{ href: "javascript:;", data: {title: "Preview", url: preview_project_notes_path(@project)} } %i.icon-eye-open diff --git a/app/views/notes/_form_errors.html.haml b/app/views/notes/_form_errors.html.haml new file mode 100644 index 0000000..0851536 --- /dev/null +++ b/app/views/notes/_form_errors.html.haml @@ -0,0 +1,3 @@ +.error_message.js-errors + - note.errors.full_messages.each do |msg| + %div= msg diff --git a/app/views/notes/create.js.haml b/app/views/notes/create.js.haml index 6ffb0cd..c573d40 100644 --- a/app/views/notes/create.js.haml +++ b/app/views/notes/create.js.haml @@ -7,10 +7,15 @@ - else NoteList.appendNewNote(#{@note.id}, noteHtml); - else - var firstDiscussionNoteHtml = "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [@note])}"; - NoteList.appendNewDiscussionNote("#{@note.discussion_id}", firstDiscussionNoteHtml, noteHtml); + :plain + var firstDiscussionNoteHtml = "#{escape_javascript(render "notes/diff_notes_with_reply", notes: [@note])}"; + NoteList.appendNewDiscussionNote("#{@note.discussion_id}", + firstDiscussionNoteHtml, + noteHtml); - else - -# TODO: insert form correctly - $(".js-main-target-note").replaceWith("#{escape_javascript(render 'notes/common_form')}"); - GitLab.GfmAutoComplete.setup(); \ No newline at end of file + var errorsHtml = "#{escape_javascript(render 'notes/form_errors', note: @note)}"; + - if note_for_main_target?(@note) + NoteList.errorsOnForm(errorsHtml); + - else + NoteList.errorsOnForm(errorsHtml, "#{@note.discussion_id}"); \ No newline at end of file -- libgit2 0.21.2