Merge branch 'comment-discard-button' into 'master'

Added discard button to comment form

Also changed the labels on the buttons to better match the action they are completing.

Closes #8057

See merge request !3085

12 files changed, 77 insertions, 32 deletions

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index cb6b7b29935..75d7f52bbb6 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -54,6 +54,9 @@ class @Notes
     $(document).on "ajax:complete", ".js-main-target-form", @reenableTargetFormSubmitButton
     $(document).on "ajax:success", ".js-main-target-form", @resetMainTargetForm
 
+    # reset main target form when clicking discard
+    $(document).on "click", ".js-note-discard", @resetMainTargetForm
+
     # update the file name when an attachment is selected
     $(document).on "change", ".js-note-attachment-input", @updateFormAttachment
 
@@ -88,6 +91,7 @@ class @Notes
     $(document).off "keyup", ".js-note-text"
     $(document).off "click", ".js-note-target-reopen"
     $(document).off "click", ".js-note-target-close"
+    $(document).off "click", ".js-note-discard"
 
     $('.note .js-task-list-container').taskList('disable')
     $(document).off 'tasklist:changed', '.note .js-task-list-container'
@@ -222,7 +226,7 @@ class @Notes
   Resets text and preview.
   Resets buttons.
   ###
-  resetMainTargetForm: ->
+  resetMainTargetForm: (e) =>
     form = $(".js-main-target-form")
 
     # remove validation errors
@@ -234,6 +238,8 @@ class @Notes
 
     form.find(".js-note-text").data("autosave").reset()
 
+    @updateTargetButtons(e)
+
   reenableTargetFormSubmitButton: ->
     form = $(".js-main-target-form")
 
@@ -277,8 +283,10 @@ class @Notes
     form.removeClass "js-new-note-form"
     form.find('.div-dropzone').remove()
 
+    # hide discard button
+    form.find('.js-note-discard').hide()
+
     # setup preview buttons
-    form.find(".js-md-write-button, .js-md-preview-button").tooltip placement: "left"
     previewButton = form.find(".js-md-preview-button")
 
     textarea = form.find(".js-note-text")
@@ -476,6 +484,11 @@ class @Notes
     form.find("#note_line_code").val dataHolder.data("lineCode")
     form.find("#note_noteable_type").val dataHolder.data("noteableType")
     form.find("#note_noteable_id").val dataHolder.data("noteableId")
+    form.find('.js-note-discard')
+        .show()
+        .removeClass('js-note-discard')
+        .addClass('js-close-discussion-note-form')
+        .text(form.find('.js-close-discussion-note-form').data('cancel-text'))
     @setupNoteForm form
     form.find(".js-note-text").focus()
     form.addClass "js-discussion-note-form"
@@ -575,21 +588,52 @@ class @Notes
   updateCloseButton: (e) =>
     textarea = $(e.target)
     form = textarea.parents('form')
-    form.find('.js-note-target-close').text('Close')
+    closebtn = form.find('.js-note-target-close')
+    closebtn.text(closebtn.data('original-text'))
 
   updateTargetButtons: (e) =>
     textarea = $(e.target)
     form = textarea.parents('form')
+    reopenbtn = form.find('.js-note-target-reopen')
+    closebtn = form.find('.js-note-target-close')
+    discardbtn = form.find('.js-note-discard')
+
     if textarea.val().trim().length > 0
-      form.find('.js-note-target-reopen').text('Comment & reopen')
-      form.find('.js-note-target-close').text('Comment & close')
-      form.find('.js-note-target-reopen').addClass('btn-comment-and-reopen')
-      form.find('.js-note-target-close').addClass('btn-comment-and-close')
+      reopentext = reopenbtn.data('alternative-text')
+      closetext = closebtn.data('alternative-text')
+
+      if reopenbtn.text() isnt reopentext
+        reopenbtn.text(reopentext)
+
+      if closebtn.text() isnt closetext
+        closebtn.text(closetext)
+
+      if reopenbtn.is(':not(.btn-comment-and-reopen)')
+        reopenbtn.addClass('btn-comment-and-reopen')
+
+      if closebtn.is(':not(.btn-comment-and-close)')
+        closebtn.addClass('btn-comment-and-close')
+
+      if discardbtn.is(':hidden')
+        discardbtn.show()
     else
-      form.find('.js-note-target-reopen').text('Reopen')
-      form.find('.js-note-target-close').text('Close')
-      form.find('.js-note-target-reopen').removeClass('btn-comment-and-reopen')
-      form.find('.js-note-target-close').removeClass('btn-comment-and-close')
+      reopentext = reopenbtn.data('original-text')
+      closetext = closebtn.data('original-text')
+
+      if reopenbtn.text() isnt reopentext
+        reopenbtn.text(reopentext)
+
+      if closebtn.text() isnt closetext
+        closebtn.text(closetext)
+
+      if reopenbtn.is(':not(.btn-comment-and-reopen)')
+        reopenbtn.removeClass('btn-comment-and-reopen')
+
+      if closebtn.is(':not(.btn-comment-and-close)')
+        closebtn.removeClass('btn-comment-and-close')
+
+      if discardbtn.is(':visible')
+        discardbtn.hide()
 
   initTaskList: ->
     @enableTaskList()
diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml
index eb9c225df2f..b151393abab 100644
--- a/app/views/projects/issues/_discussion.html.haml
+++ b/app/views/projects/issues/_discussion.html.haml
@@ -1,7 +1,7 @@
 - content_for :note_actions do
   - if can?(current_user, :update_issue, @issue)
-    = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn btn-nr btn-grouped btn-reopen btn-comment js-note-target-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
-    = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn btn-nr btn-grouped btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'
+    = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, status_only: true, format: 'json'), data: {no_turbolink: true, original_text: "Reopen issue", alternative_text: "Comment & reopen issue"}, class: "btn btn-nr btn-grouped btn-reopen btn-comment js-note-target-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
+    = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, status_only: true, format: 'json'), data: {no_turbolink: true, original_text: "Close issue", alternative_text: "Comment & close issue"}, class: "btn btn-nr btn-grouped btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'
 
 #notes
   = render 'projects/notes/notes_with_form'
diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml
index 1c7de94acfd..393998f15b9 100644
--- a/app/views/projects/merge_requests/_discussion.html.haml
+++ b/app/views/projects/merge_requests/_discussion.html.haml
@@ -1,8 +1,8 @@
 - content_for :note_actions do
   - if can?(current_user, :update_merge_request, @merge_request)
     - if @merge_request.open?
-      = link_to 'Close', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request"
+      = link_to 'Close merge request', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request", data: {original_text: "Close merge request", alternative_text: "Comment & close merge request"}
     - if @merge_request.closed?
-      = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request"
+      = link_to 'Reopen merge request', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request", data: {original_text: "Reopen merge request", alternative_text: "Comment & reopen merge request"}
 
 #notes= render "projects/notes/notes_with_form"
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index 09740d8ea12..f675f092da1 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -13,6 +13,7 @@
     .error-alert
 
   .note-form-actions.clearfix
-    = f.submit 'Add Comment', class: "btn btn-nr btn-create comment-btn btn-grouped js-comment-button"
+    = f.submit 'Comment', class: "btn btn-nr btn-create comment-btn btn-grouped js-comment-button"
     = yield(:note_actions)
-    %a.btn.btn-nr.btn-cancel.js-close-discussion-note-form Cancel
+    %a.btn.btn-cancel.js-note-discard{role: "button", data: {cancel_text: "Cancel"}}
+      Discard draft
diff --git a/features/steps/project/issues/award_emoji.rb b/features/steps/project/issues/award_emoji.rb
index 277c63914d1..937fbbd34eb 100644
--- a/features/steps/project/issues/award_emoji.rb
+++ b/features/steps/project/issues/award_emoji.rb
@@ -79,7 +79,7 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps
   step 'I leave comment with a single emoji' do
     page.within('.js-main-target-form') do
       fill_in 'note[note]', with: ':smile:'
-      click_button 'Add Comment'
+      click_button 'Comment'
     end
   end
 
diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb
index 383ab4e14da..8c31fa890b2 100644
--- a/features/steps/project/issues/issues.rb
+++ b/features/steps/project/issues/issues.rb
@@ -268,7 +268,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
   step 'I leave a comment with code block' do
     page.within(".js-main-target-form") do
       fill_in "note[note]", with: "```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```"
-      click_button "Add Comment"
+      click_button "Comment"
       sleep 0.05
     end
   end
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index c19b15bc9ed..df4259b9ddf 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -419,7 +419,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
     page.within(".js-discussion-note-form") do
       fill_in "note_note", with: "Line is correct"
-      click_button "Add Comment"
+      click_button "Comment"
     end
 
     page.within ".files [id^=diff]:nth-child(2) .note-body > .note-text" do
@@ -432,7 +432,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
     page.within(".js-discussion-note-form") do
       fill_in "note_note", with: "Line is wrong on here"
-      click_button "Add Comment"
+      click_button "Comment"
     end
   end
 
@@ -528,7 +528,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   def leave_comment(message)
     page.within(".js-discussion-note-form", visible: true) do
       fill_in "note_note", with: message
-      click_button "Add Comment"
+      click_button "Comment"
     end
     page.within(".notes_holder", visible: true) do
       expect(page).to have_content message
diff --git a/features/steps/project/snippets.rb b/features/steps/project/snippets.rb
index 504654f90dd..786a0cad975 100644
--- a/features/steps/project/snippets.rb
+++ b/features/steps/project/snippets.rb
@@ -77,7 +77,7 @@ class Spinach::Features::ProjectSnippets < Spinach::FeatureSteps
   step 'I leave a comment like "Good snippet!"' do
     page.within('.js-main-target-form') do
       fill_in "note_note", with: "Good snippet!"
-      click_button "Add Comment"
+      click_button "Comment"
     end
   end
 
diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb
index 06e69441894..906b66a4a63 100644
--- a/features/steps/shared/diff_note.rb
+++ b/features/steps/shared/diff_note.rb
@@ -93,14 +93,14 @@ module SharedDiffNote
 
       page.within("form[id$='#{sample_commit.line_code}']") do
         fill_in 'note[note]', with: ':smile:'
-        click_button('Add Comment')
+        click_button('Comment')
       end
     end
   end
 
   step 'I submit the diff comment' do
     page.within(diff_file_selector) do
-      click_button("Add Comment")
+      click_button("Comment")
     end
   end
 
diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb
index ae10c6069a9..e59bfbea998 100644
--- a/features/steps/shared/issuable.rb
+++ b/features/steps/shared/issuable.rb
@@ -182,7 +182,7 @@ module SharedIssuable
 
     page.within('.js-main-target-form') do
       fill_in 'note[note]', with: "##{issuable.to_reference(project)}"
-      click_button 'Add Comment'
+      click_button 'Comment'
     end
   end
 
diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb
index 444d6726f99..fb0462d6e04 100644
--- a/features/steps/shared/note.rb
+++ b/features/steps/shared/note.rb
@@ -17,7 +17,7 @@ module SharedNote
   step 'I leave a comment like "XML attached"' do
     page.within(".js-main-target-form") do
       fill_in "note[note]", with: "XML attached"
-      click_button "Add Comment"
+      click_button "Comment"
     end
   end
 
@@ -30,7 +30,7 @@ module SharedNote
 
   step 'I submit the comment' do
     page.within(".js-main-target-form") do
-      click_button "Add Comment"
+      click_button "Comment"
     end
   end
 
@@ -115,7 +115,7 @@ module SharedNote
   step 'I leave a comment with a header containing "Comment with a header"' do
     page.within(".js-main-target-form") do
       fill_in "note[note]", with: "# Comment with a header"
-      click_button "Add Comment"
+      click_button "Comment"
       sleep 0.05
     end
   end
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index 1a360cd1ebc..d9a8058efd9 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -22,7 +22,7 @@ describe 'Comments', feature: true do
       it 'should be valid' do
         is_expected.to have_css('.js-main-target-form', visible: true, count: 1)
         expect(find('.js-main-target-form input[type=submit]').value).
-          to eq('Add Comment')
+          to eq('Comment')
         page.within('.js-main-target-form') do
           expect(page).not_to have_link('Cancel')
         end
@@ -49,7 +49,7 @@ describe 'Comments', feature: true do
         page.within('.js-main-target-form') do
           fill_in 'note[note]', with: 'This is awsome!'
           find('.js-md-preview-button').click
-          click_button 'Add Comment'
+          click_button 'Comment'
         end
       end
 
@@ -202,7 +202,7 @@ describe 'Comments', feature: true do
         before do
           page.within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do
             fill_in 'note[note]', with: 'Another comment on line 10'
-            click_button('Add Comment')
+            click_button('Comment')
           end
         end