Finish up feature spec

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);

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;

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'

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 () {