diff --git a/app/assets/javascripts/droplab/drop_down.js b/app/assets/javascripts/droplab/drop_down.js index f522859c457..9588921ebcd 100644 --- a/app/assets/javascripts/droplab/drop_down.js +++ b/app/assets/javascripts/droplab/drop_down.js @@ -37,7 +37,7 @@ Object.assign(DropDown.prototype, { clickEvent: function(e) { if (e.target.tagName === 'UL') return; - var selected = utils.closest(e.target, 'LI', ''); + var selected = utils.closest(e.target, 'LI'); if (!selected) return; this.addSelectedClass(selected); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 66e89b7607a..9dd516d93fd 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -143,7 +143,7 @@ require('./task_list'); form.querySelector('.js-comment-type-dropdown .dropdown-menu'), form.querySelector('#note_type'), form.querySelector('.js-comment-type-dropdown .js-comment-submit-button'), - form.querySelector('.js-note-target-close'), + form.querySelector('.js-note-target-close:not(.hidden)') || form.querySelector('.js-note-target-reopen'), ); this.commentTypeToggle.initDroplab(); diff --git a/app/views/projects/notes/_comment_button.html.haml b/app/views/projects/notes/_comment_button.html.haml index 008363263b3..2e65a3bf294 100644 --- a/app/views/projects/notes/_comment_button.html.haml +++ b/app/views/projects/notes/_comment_button.html.haml @@ -1,4 +1,5 @@ - noteable_name = @note.noteable.human_class_name +- noteable_state_action = noteable_name =~ /(merge request|issue)/ && @note.noteable['state'] == 'closed' ? 'reopen' : 'close' .pull-left.btn-group.append-right-10.comment-type-dropdown.js-comment-type-dropdown %input.btn.btn-nr.btn-create.comment-btn.js-comment-button.js-comment-submit-button{ type: 'submit', value: 'Comment' } @@ -8,7 +9,7 @@ = icon('caret-down') %ul#resolvable-comment-menu.dropdown-menu{ data: { dropdown: true } } - %li#comment.droplab-item-selected{ data: { value: '', 'button-text' => 'Comment', 'secondary-button-text' => "Comment & close #{noteable_name}" } } + %li#comment.droplab-item-selected{ data: { value: '', 'button-text' => 'Comment', 'secondary-button-text' => "Comment & #{noteable_state_action} #{noteable_name}" } } = icon('check') .description %strong Comment @@ -17,7 +18,7 @@ %li.divider - %li#discussion{ data: { value: 'DiscussionNote', 'button-text' => 'Start discussion', 'secondary-button-text' => "Start discussion & close #{noteable_name}" } } + %li#discussion{ data: { value: 'DiscussionNote', 'button-text' => 'Start discussion', 'secondary-button-text' => "Start discussion & #{noteable_state_action} #{noteable_name}" } } = icon('check') .description %strong Start discussion diff --git a/spec/features/discussion_comments_spec.rb b/spec/features/discussion_comments_spec.rb index 9e99fe064e8..ae778118c5c 100644 --- a/spec/features/discussion_comments_spec.rb +++ b/spec/features/discussion_comments_spec.rb @@ -93,9 +93,9 @@ shared_examples 'discussion comments' do |resource_name| end it 'clicking the ul padding should not change the text' do - find(menu_selector).click + find(menu_selector).trigger 'click' - expect(find(submit_selector)).to have_content 'Comment' + expect(find(dropdown_selector)).to have_content 'Comment' end describe 'when selecting "Start discussion"' do @@ -109,7 +109,7 @@ shared_examples 'discussion comments' do |resource_name| end it 'updates the submit button text' do - expect(find(dropdown_selector)).to have_content "Start discussion" + expect(find(dropdown_selector)).to have_content 'Start discussion' end if resource_name =~ /(issue|merge request)/ @@ -181,7 +181,7 @@ shared_examples 'discussion comments' do |resource_name| end it 'updates the submit button text' do - expect(find(dropdown_selector)).to have_content "Comment" + expect(find(dropdown_selector)).to have_content 'Comment' end if resource_name =~ /(issue|merge request)/ @@ -222,20 +222,22 @@ shared_examples 'discussion comments' do |resource_name| if resource_name =~ /(issue|merge request)/ describe "on a closed #{resource_name}" do before do - find("#{form_selector} .close-mr-link").click + find("#{form_selector} .js-note-target-close").click + + find("#{form_selector} .note-textarea").send_keys('a') end - it 'should show a "Comment & reopen #{resource_name}" button' do - expect(find(close_selector)).to have_content "Comment & reopen #{resource_name}" + it "should show a 'Comment & reopen #{resource_name}' button" do + expect(find("#{form_selector} .js-note-target-reopen")).to have_content "Comment & reopen #{resource_name}" end - it 'should show a "Start discussion & reopen #{resource_name}" button when "Start discussion" is selected' do + it "should show a 'Start discussion & reopen #{resource_name}' button when 'Start discussion' is selected" do find(toggle_selector).click find("#{menu_selector} li", match: :first) all("#{menu_selector} li").last.click - expect(find(close_selector)).to have_content "Start discussion & reopen #{resource_name}" + expect(find("#{form_selector} .js-note-target-reopen")).to have_content "Start discussion & reopen #{resource_name}" end end end diff --git a/spec/javascripts/droplab/drop_down_spec.js b/spec/javascripts/droplab/drop_down_spec.js index bbf953658c8..802e2435672 100644 --- a/spec/javascripts/droplab/drop_down_spec.js +++ b/spec/javascripts/droplab/drop_down_spec.js @@ -130,7 +130,7 @@ describe('DropDown', function () { beforeEach(function () { this.list = { dispatchEvent: () => {} }; this.dropdown = { hide: () => {}, list: this.list, addSelectedClass: () => {} }; - this.event = { preventDefault: () => {}, target: 'target' }; + this.event = { preventDefault: () => {}, target: {} }; this.customEvent = {}; this.closestElement = {}; @@ -168,6 +168,21 @@ describe('DropDown', function () { expect(this.list.dispatchEvent).toHaveBeenCalledWith(this.customEvent); }); + describe('if the target is a UL element', function () { + beforeEach(function () { + this.event = { preventDefault: () => {}, target: { tagName: 'UL' } }; + + spyOn(this.event, 'preventDefault'); + utils.closest.calls.reset(); + + DropDown.prototype.clickEvent.call(this.dropdown, this.event); + }); + + it('should return immediately', function () { + expect(utils.closest).not.toHaveBeenCalled(); + }); + }); + describe('if no selected element exists', function () { beforeEach(function () { this.event.preventDefault.calls.reset(); diff --git a/spec/javascripts/droplab/plugins/input_setter_spec.js b/spec/javascripts/droplab/plugins/input_setter_spec.js index 412d1054385..bd625f4ae80 100644 --- a/spec/javascripts/droplab/plugins/input_setter_spec.js +++ b/spec/javascripts/droplab/plugins/input_setter_spec.js @@ -2,7 +2,7 @@ import InputSetter from '~/droplab/plugins/input_setter'; -fdescribe('InputSetter', function () { +describe('InputSetter', function () { describe('init', function () { beforeEach(function () { this.config = { InputSetter: {} };