From b47173da6a0fea0982d009f91e2c4d042f9b5c37 Mon Sep 17 00:00:00 2001 From: Riyad Preukschas Date: Mon, 3 Dec 2012 22:34:50 +0100 Subject: [PATCH] Revamped note form options. --- app/assets/javascripts/notes.js | 91 +++++++++++++++------- app/assets/stylesheets/sections/notes.scss | 59 +++++++------- app/models/note.rb | 18 ----- app/views/notes/_form.html.haml | 23 +++--- 4 files changed, 101 insertions(+), 90 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index f9dda3c7..fa0edd2d 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -32,12 +32,6 @@ var NoteList = { // get initial set of notes NoteList.getContent(); - $("#note_attachment").change(function(e){ - var val = $('.input-file').val(); - var filename = val.replace(/^.*[\\\/]/, ''); - $(".file_name").text(filename); - }); - // add a new diff note $(document).on("click", ".js-add-diff-note-button", @@ -53,6 +47,11 @@ var NoteList = { ".js-note-preview-button", NoteList.previewNote); + // update the file name when an attachment is selected + $(document).on("change", + ".js-note-attachment-input", + NoteList.updateFormAttachment); + // hide diff note form $(document).on("click", ".js-close-discussion-note-form", @@ -63,35 +62,22 @@ var NoteList = { ".js-note-delete", NoteList.removeNote); - // clean up previews for main target form + // reset main target form after submit $(document).on("ajax:complete", ".js-main-target-form", - NoteList.cleanupMainTargetForm); + NoteList.resetMainTargetForm); + + + $(document).on("click", + ".js-choose-note-attachment-button", + NoteList.chooseNoteAttachment); }, /** - * Event handlers + * When clicking on buttons */ - - /** - * - */ - 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"); - }, - /** * Called when clicking on the "add a comment" button on the side of a diff line. * @@ -121,6 +107,17 @@ var NoteList = { } }, + /** + * Called when clicking the "Choose File" button. + * + * Opesn the file selection dialog. + */ + chooseNoteAttachment: function() { + var form = $(this).closest("form"); + + form.find(".js-note-attachment-input").click(); + }, + /** * Shows the note preview. * @@ -307,6 +304,11 @@ var NoteList = { } }); + // remove notify commit author checkbox for non-commit notes + if (form.find("#note_noteable_type").val() !== "Commit") { + form.find(".js-notify-commit-author").remove(); + } + GitLab.GfmAutoComplete.setup(); form.show(); @@ -499,6 +501,41 @@ var NoteList = { $("#new-notes-list").prepend(html); }, + /** + * Called in response the main target form has been successfully submitted. + * + * Removes any errors. + * Resets text and preview. + * Resets buttons. + */ + resetMainTargetForm: 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"); + }, + + /** + * Called after an attachment file has been selected. + * + * Updates the file name for the selected attachment. + */ + updateFormAttachment: function() { + var form = $(this).closest("form"); + + // get only the basename + var filename = $(this).val().replace(/^.*[\\\/]/, ''); + + form.find(".js-attachment-filename").text(filename); + }, + /** * Recalculates the votes and updates them (if they are displayed at all). * diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss index 981186a5..465d578f 100644 --- a/app/assets/stylesheets/sections/notes.scss +++ b/app/assets/stylesheets/sections/notes.scss @@ -227,6 +227,11 @@ ul.notes { .discussion { .new_note { margin: 8px 5px 8px 0; + + .note_options { + // because of the smaller width and the extra "cancel" button + margin-top: 8px; + } } } .new_note { @@ -236,51 +241,39 @@ ul.notes { float: left; margin-top: 8px; } + .clearfix { + margin-bottom: 0; + } .note_options { h6 { - line-height: 32px; - padding-right: 15px; + @extend .left; + line-height: 20px; + padding-right: 16px; + padding-bottom: 16px; + } + label { + padding: 0; } - // TODO: start cleanup - .attachments { + .attachment { + @extend .right; position: relative; width: 350px; height: 50px; - overflow: hidden; margin:0 0 5px !important; - .input_file { - .file_name { - line-height: 30px; - width: 240px; - height: 28px; - overflow: hidden; - } - .file_upload { - position: absolute; - right:14px; - top:7px; - } - .input-file { - width: 260px; - height: 41px; - float: right; - } + // hide the actual file field + input { + display: none; + } + + .choose-btn { + float: right; } } - .input-file { - font: 500px monospace; - opacity:0; - filter: alpha(opacity=0); - position: absolute; - z-index: 1; - top:0; - right:0; - padding:0; - margin: 0; + .notify_options { + @extend .right; } - // TODO: end cleanup } .note_text_and_preview { // makes the "absolute" position for links relative to this diff --git a/app/models/note.rb b/app/models/note.rb index 0027c57b..a8ae9080 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -140,24 +140,6 @@ class Note < ActiveRecord::Base @notify_author ||= false end - # Check if we can notify commit author - # with email about our comment - # - # If commit author email exist in project - # and commit author is not passed user we can - # send email to him - # - # params: - # user - current user - # - # return: - # Boolean - # - def notify_only_author?(user) - for_commit? && commit_author && - commit_author.email != user.email - end - # Returns true if this is an upvote note, # otherwise false is returned def upvote? diff --git a/app/views/notes/_form.html.haml b/app/views/notes/_form.html.haml index cb96877f..57811daf 100644 --- a/app/views/notes/_form.html.haml +++ b/app/views/notes/_form.html.haml @@ -22,23 +22,22 @@ .clearfix .note_options - .attachments.right - %h6.left Attachment: - %span.file_name File name... + .attachment + %h6 Attachment: + .file_name.js-attachment-filename File name... + %a.choose-btn.btn.small.js-choose-note-attachment-button Choose File ... + .hint Any file up to 10 MB - .input.input_file - %a.file_upload.btn.small Upload File - = f.file_field :attachment, class: "input-file" - %span.hint Any file less than 10 MB + = f.file_field :attachment, class: "js-note-attachment-input" - .notify_opts.right - %h6.left Notify via email: + .notify_options + %h6 Notify via email: = label_tag :notify do = check_box_tag :notify, 1, !@note.for_commit? - %span Project team + Project team - - if @note.notify_only_author?(current_user) # FIXME: put in JS + .js-notify-commit-author = label_tag :notify_author do = check_box_tag :notify_author, 1 , !@note.for_commit? - %span Commit author + Commit author .clearfix