Protected against synthetic clicks and added feature scenario

8 files changed, 22 insertions, 7 deletions

diff --git a/app/assets/javascripts/comment_type_toggle.js b/app/assets/javascripts/comment_type_toggle.js
index c175f89c4fa..df0ba86198c 100644
--- a/app/assets/javascripts/comment_type_toggle.js
+++ b/app/assets/javascripts/comment_type_toggle.js
@@ -24,7 +24,6 @@ class CommentTypeToggle {
       InputSetter: [{
         input: this.noteTypeInput,
         valueAttribute: 'data-value',
-        allowEmpty: true,
       },
       {
         input: this.submitButton,
diff --git a/app/assets/javascripts/droplab/constants.js b/app/assets/javascripts/droplab/constants.js
index a23d914772a..8883ed9aa14 100644
--- a/app/assets/javascripts/droplab/constants.js
+++ b/app/assets/javascripts/droplab/constants.js
@@ -2,10 +2,12 @@ const DATA_TRIGGER = 'data-dropdown-trigger';
 const DATA_DROPDOWN = 'data-dropdown';
 const SELECTED_CLASS = 'droplab-item-selected';
 const ACTIVE_CLASS = 'droplab-item-active';
+const IGNORE_CLASS = 'droplab-item-ignore';
 
 export {
   DATA_TRIGGER,
   DATA_DROPDOWN,
   SELECTED_CLASS,
   ACTIVE_CLASS,
+  IGNORE_CLASS,
 };
diff --git a/app/assets/javascripts/droplab/drop_down.js b/app/assets/javascripts/droplab/drop_down.js
index 9588921ebcd..1fb4d63923c 100644
--- a/app/assets/javascripts/droplab/drop_down.js
+++ b/app/assets/javascripts/droplab/drop_down.js
@@ -1,7 +1,7 @@
 /* eslint-disable */
 
 import utils from './utils';
-import { SELECTED_CLASS } from './constants';
+import { SELECTED_CLASS, IGNORE_CLASS } from './constants';
 
 var DropDown = function(list) {
   this.currentIndex = 0;
@@ -36,6 +36,7 @@ Object.assign(DropDown.prototype, {
 
   clickEvent: function(e) {
     if (e.target.tagName === 'UL') return;
+    if (e.target.classList.contains(IGNORE_CLASS)) return;
 
     var selected = utils.closest(e.target, 'LI');
     if (!selected) return;
diff --git a/app/assets/javascripts/droplab/plugins/input_setter.js b/app/assets/javascripts/droplab/plugins/input_setter.js
index 0736d525f40..d01fbc5830d 100644
--- a/app/assets/javascripts/droplab/plugins/input_setter.js
+++ b/app/assets/javascripts/droplab/plugins/input_setter.js
@@ -35,9 +35,6 @@ const InputSetter = {
     const newValue = selectedItem.getAttribute(config.valueAttribute);
     const inputAttribute = config.inputAttribute;
 
-    console.log('newValue', newValue, 'allowEmpty', config.allowEmpty);
-    if (!newValue && !config.allowEmpty) 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/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss
index 7767826b033..b87e712c763 100644
--- a/app/assets/stylesheets/framework/dropdowns.scss
+++ b/app/assets/stylesheets/framework/dropdowns.scss
@@ -564,3 +564,7 @@
     color: $gl-text-color-secondary;
   }
 }
+
+.droplab-item-ignore {
+  pointer-events: none;
+}
diff --git a/app/views/projects/notes/_comment_button.html.haml b/app/views/projects/notes/_comment_button.html.haml
index 6bb55f04b6e..29cf5825292 100644
--- a/app/views/projects/notes/_comment_button.html.haml
+++ b/app/views/projects/notes/_comment_button.html.haml
@@ -16,7 +16,7 @@
             %p
               Add a general comment to this #{noteable_name}.
 
-      %li.divider
+      %li.divider.droplab-item-ignore
 
       %li#discussion{ data: { value: 'DiscussionNote', 'submit-text' => 'Start discussion', 'close-text' => "Start discussion & close #{noteable_name}", 'reopen-text' => "Start discussion & reopen #{noteable_name}" } }
         %a{ href: '#' }
diff --git a/spec/javascripts/droplab/constants_spec.js b/spec/javascripts/droplab/constants_spec.js
index 35239e4fb8e..fd153a49fcd 100644
--- a/spec/javascripts/droplab/constants_spec.js
+++ b/spec/javascripts/droplab/constants_spec.js
@@ -26,4 +26,10 @@ describe('constants', function () {
       expect(constants.ACTIVE_CLASS).toBe('droplab-item-active');
     });
   });
+
+  describe('IGNORE_CLASS', function () {
+    it('should be `droplab-item-ignore`', function() {
+      expect(constants.IGNORE_CLASS).toBe('droplab-item-ignore');
+    });
+  });
 });
diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb
index 1a061ef069e..bb4542b1683 100644
--- a/spec/support/features/discussion_comments_shared_example.rb
+++ b/spec/support/features/discussion_comments_shared_example.rb
@@ -73,9 +73,15 @@ 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
+    it 'clicking the ul padding or divider should not change the text' do
       find(menu_selector).trigger 'click'
 
+      expect(page).to have_selector menu_selector
+      expect(find(dropdown_selector)).to have_content 'Comment'
+
+      find("#{menu_selector} .divider").trigger 'click'
+
+      expect(page).to have_selector menu_selector
       expect(find(dropdown_selector)).to have_content 'Comment'
     end