diff options
| -rw-r--r-- | app/assets/javascripts/droplab/drop_down.js | 4 | ||||
| -rw-r--r-- | app/assets/javascripts/droplab/plugins/input_setter.js | 2 | ||||
| -rw-r--r-- | spec/features/discussion_comments_spec.rb | 86 | ||||
| -rw-r--r-- | spec/javascripts/droplab/plugins/input_setter.js | 36 |
4 files changed, 63 insertions, 65 deletions
diff --git a/app/assets/javascripts/droplab/drop_down.js b/app/assets/javascripts/droplab/drop_down.js index f686ad33f6f..f522859c457 100644 --- a/app/assets/javascripts/droplab/drop_down.js +++ b/app/assets/javascripts/droplab/drop_down.js @@ -35,7 +35,9 @@ Object.assign(DropDown.prototype, { }, clickEvent: function(e) { - var selected = utils.closest(e.target, 'LI'); + if (e.target.tagName === 'UL') return; + + var selected = utils.closest(e.target, 'LI', ''); if (!selected) return; this.addSelectedClass(selected); diff --git a/app/assets/javascripts/droplab/plugins/input_setter.js b/app/assets/javascripts/droplab/plugins/input_setter.js index c292cfa7b8f..d01fbc5830d 100644 --- a/app/assets/javascripts/droplab/plugins/input_setter.js +++ b/app/assets/javascripts/droplab/plugins/input_setter.js @@ -35,8 +35,6 @@ const InputSetter = { const newValue = selectedItem.getAttribute(config.valueAttribute); const inputAttribute = config.inputAttribute; - if (!newValue) return; - if (input.hasAttribute(inputAttribute)) return input.setAttribute(inputAttribute, newValue); if (input.tagName === 'INPUT') return input.value = newValue; return input.textContent = newValue; diff --git a/spec/features/discussion_comments_spec.rb b/spec/features/discussion_comments_spec.rb index 3ffe88e4c59..9e99fe064e8 100644 --- a/spec/features/discussion_comments_spec.rb +++ b/spec/features/discussion_comments_spec.rb @@ -7,31 +7,33 @@ shared_examples 'discussion comments' do |resource_name| let(:menu_selector) { "#{dropdown_selector} .dropdown-menu" } let(:submit_selector) { "#{form_selector} .js-comment-submit-button" } let(:close_selector) { "#{form_selector} .btn-comment-and-close" } + let(:comments_selector) { '.timeline > .note.timeline-entry' } it 'should show a comment type toggle' do expect(page).to have_selector toggle_selector end - it '"Comment" will post a comment' do + it 'clicking "Comment" will post a comment' do find("#{form_selector} .note-textarea").send_keys('a') find(submit_selector).click - find('.timeline .timeline-entry', match: :first) - new_comment = all('.timeline .timeline-entry').last + find(comments_selector, match: :first) + new_comment = all(comments_selector).last expect(new_comment).to have_content 'a' expect(new_comment).not_to have_selector '.discussion' end if resource_name == 'issue' - it "'Comment & close #{resource_name}' will post a comment and close the #{resource_name}" do + it "clicking 'Comment & close #{resource_name}' will post a comment and close the #{resource_name}" do find("#{form_selector} .note-textarea").send_keys('a') find(close_selector).click - find('.timeline .timeline-entry', match: :first) - entries = all('.timeline .timeline-entry') + find(comments_selector, match: :first) + find("#{comments_selector}.system-note") + entries = all(comments_selector) close_note = entries.last new_comment = entries[-2] @@ -62,7 +64,7 @@ shared_examples 'discussion comments' do |resource_name| menu = find(menu_selector) expect(menu).to have_content 'Start discussion' - expect(menu).to have_content 'Discuss a specific suggestion or question that needs to be resolved.' + expect(menu).to have_content "Discuss a specific suggestion or question#{' that needs to be resolved' if resource_name == 'merge request'}." end it 'has the "Comment" item selected by default' do @@ -90,17 +92,20 @@ shared_examples 'discussion comments' do |resource_name| expect(page).not_to have_selector menu_selector end + it 'clicking the ul padding should not change the text' do + find(menu_selector).click + + expect(find(submit_selector)).to have_content 'Comment' + end + describe 'when selecting "Start discussion"' do before do - screenshot_and_open_image find("#{menu_selector} li", match: :first) - p first("#{menu_selector} li")['class'] - p first("#{menu_selector} li").text - first("#{menu_selector} li").click + all("#{menu_selector} li").last.click end it 'updates the note_type input to "DiscussionNote"' do - expect(find("#{form_selector} #note_type", visible: false).value).to be 'DiscussionNote' + expect(find("#{form_selector} #note_type", visible: false).value).to eq('DiscussionNote') end it 'updates the submit button text' do @@ -111,28 +116,35 @@ shared_examples 'discussion comments' do |resource_name| it 'updates the close button text' do expect(find(close_selector)).to have_content "Start discussion & close #{resource_name}" end + + it 'typing does not change the close button text' do + find("#{form_selector} .note-textarea").send_keys('b') + + expect(find(close_selector)).to have_content "Start discussion & close #{resource_name}" + end end it 'closes the dropdown' do expect(page).not_to have_selector menu_selector end - it '"Start discussion" will post a discussion' do + it 'clicking "Start discussion" will post a discussion' do find(submit_selector).click - find('.timeline .timeline-entry', match: :first) - new_comment = all('.timeline .timeline-entry').last + find(comments_selector, match: :first) + new_comment = all(comments_selector).last expect(new_comment).to have_content 'a' expect(new_comment).to have_selector '.discussion' end if resource_name == 'issue' - it "'Start discussion & close #{resource_name}' will post a discussion and close the #{resource_name}" do + it "clicking 'Start discussion & close #{resource_name}' will post a discussion and close the #{resource_name}" do find(close_selector).click - find('.timeline .timeline-entry', match: :first) - entries = all('.timeline .timeline-entry') + find(comments_selector, match: :first) + find("#{comments_selector}.system-note") + entries = all(comments_selector) close_note = entries.last new_discussion = entries[-2] @@ -152,21 +164,20 @@ shared_examples 'discussion comments' do |resource_name| expect(items.first).to have_content 'Comment' expect(items.first).not_to have_selector '.fa-check' - expect(items.first).not_to have_selector '.droplab-item-selected' + expect(items.first['class']).not_to match 'droplab-item-selected' expect(items.last).to have_content 'Start discussion' expect(items.last).to have_selector '.fa-check' - expect(items.last).to have_selector '.droplab-item-selected' + expect(items.last['class']).to match 'droplab-item-selected' end describe 'when selecting "Comment"' do before do - find("#{menu_selector} li", match: :first) - all("#{menu_selector} li").last.click + find("#{menu_selector} li", match: :first).click end it 'clears the note_type input"' do - expect(find("#{form_selector} #note_type").value).to be '' + expect(find("#{form_selector} #note_type", visible: false).value).to eq('') end it 'updates the submit button text' do @@ -177,6 +188,12 @@ shared_examples 'discussion comments' do |resource_name| it 'updates the close button text' do expect(find(close_selector)).to have_content "Comment & close #{resource_name}" end + + it 'typing does not change the close button text' do + find("#{form_selector} .note-textarea").send_keys('b') + + expect(find(close_selector)).to have_content "Comment & close #{resource_name}" + end end it 'closes the dropdown' do @@ -191,11 +208,11 @@ shared_examples 'discussion comments' do |resource_name| expect(items.first).to have_content 'Comment' expect(items.first).to have_selector '.fa-check' - expect(items.first).to have_selector '.droplab-item-selected' + expect(items.first['class']).to match 'droplab-item-selected' expect(items.last).to have_content 'Start discussion' expect(items.last).not_to have_selector '.fa-check' - expect(items.last).not_to have_selector '.droplab-item-selected' + expect(items.last['class']).not_to match 'droplab-item-selected' end end end @@ -204,7 +221,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 + end + + it 'should show a "Comment & reopen #{resource_name}" button' do + expect(find(close_selector)).to have_content "Comment & reopen #{resource_name}" + end + + 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}" + end end end end @@ -242,10 +274,10 @@ describe 'Discussion Comments', :feature, :js do end describe 'on an snippet' do - let(:snippet) { create(:personal_snippet, :public, author: user) } + let(:snippet) { create(:project_snippet, :private, project: project, author: user) } before do - visit snippet_path(snippet) + visit namespace_project_snippet_path(project.namespace, project, snippet) end it_behaves_like 'discussion comments', 'snippet' diff --git a/spec/javascripts/droplab/plugins/input_setter.js b/spec/javascripts/droplab/plugins/input_setter.js index c9b7b2b23dc..412d1054385 100644 --- a/spec/javascripts/droplab/plugins/input_setter.js +++ b/spec/javascripts/droplab/plugins/input_setter.js @@ -2,7 +2,7 @@ import InputSetter from '~/droplab/plugins/input_setter'; -describe('InputSetter', function () { +fdescribe('InputSetter', function () { describe('init', function () { beforeEach(function () { this.config = { InputSetter: {} }; @@ -140,22 +140,6 @@ describe('InputSetter', function () { expect(this.input.value).toBe(this.newValue); }); - describe('if there is no newValue', function () { - beforeEach(function () { - this.newValue = ''; - this.inputSetter = { hook: { trigger: {} } }; - this.config = { valueAttribute: {}, input: this.input }; - this.input = { value: 'oldValue', tagName: 'INPUT' }; - this.selectedItem = { getAttribute: () => {} }; - - InputSetter.setInput.call(this.inputSetter, this.config, this.selectedItem); - }); - - it('should not set the value of the input', function () { - expect(this.input.value).toBe('oldValue'); - }) - }); - describe('if no config.input is provided', function () { beforeEach(function () { this.config = { valueAttribute: {} }; @@ -181,24 +165,6 @@ describe('InputSetter', function () { it('should set the textContent of the input', function () { expect(this.input.textContent).toBe(this.newValue); }); - - describe('if there is no new value', function () { - beforeEach(function () { - this.selectedItem = { getAttribute: () => {} }; - this.input = { textContent: 'oldValue', tagName: 'INPUT', hasAttribute: () => {} }; - this.config = { valueAttribute: {}, input: this.input }; - this.inputSetter = { hook: { trigger: {} } }; - this.newValue = 'newValue'; - - spyOn(this.selectedItem, 'getAttribute').and.returnValue(this.newValue); - - InputSetter.setInput.call(this.inputSetter, this.config, this.selectedItem); - }); - - it('should not set the value of the input', function () { - expect(this.input.textContent).toBe('oldValue'); - }); - }); }); describe('if there is an inputAttribute', function () { |