Added issuables_helper.js, tidied up droplab-dropdown JS, fixed filtered search specs

10 files changed, 96 insertions, 98 deletions

diff --git a/app/assets/javascripts/helpers/issuables_helper.js b/app/assets/javascripts/helpers/issuables_helper.js
new file mode 100644
index 00000000000..68fd8c74bea
--- /dev/null
+++ b/app/assets/javascripts/helpers/issuables_helper.js
@@ -0,0 +1,21 @@
+import CloseReopenReportToggle from '../close_reopen_report_toggle';
+
+export default function initCloseReopenReport() {
+  const container = document.querySelector('.js-issuable-close-dropdown');
+
+  if (!container) return undefined;
+
+  const dropdownTrigger = container.querySelector('.js-issuable-close-toggle');
+  const dropdownList = container.querySelector('.js-issuable-close-menu');
+  const button = container.querySelector('.js-issuable-close-button');
+
+  const closeReopenReportToggle = new CloseReopenReportToggle({
+    dropdownTrigger,
+    dropdownList,
+    button,
+  });
+
+  closeReopenReportToggle.initDroplab();
+
+  return closeReopenReportToggle;
+}
diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js
index 91e696f5320..912da6ed6c1 100644
--- a/app/assets/javascripts/issue.js
+++ b/app/assets/javascripts/issue.js
@@ -6,7 +6,7 @@ import '~/lib/utils/text_utility';
 import './flash';
 import './task_list';
 import CreateMergeRequestDropdown from './create_merge_request_dropdown';
-import CloseReopenReportToggle from './close_reopen_report_toggle';
+import initCloseReopenReport from './helpers/issuables_helper';
 
 class Issue {
   constructor() {
@@ -29,12 +29,10 @@ class Issue {
     Issue.initMergeRequests();
     Issue.initRelatedBranches();
 
-    this.initCloseReopenReport();
+    this.closeButtons = $('a.btn-close');
+    this.reopenButtons = $('a.btn-reopen');
 
-    if (!this.closeReopenReportToggle) {
-      this.closeButtons = $('a.btn-close');
-      this.reopenButtons = $('a.btn-reopen');
-    }
+    this.initCloseReopenReport();
 
     if (Issue.createMrDropdownWrap) {
       this.createMergeRequestDropdown = new CreateMergeRequestDropdown(Issue.createMrDropdownWrap);
@@ -100,21 +98,10 @@ class Issue {
   }
 
   initCloseReopenReport() {
-    const container = document.querySelector('.js-issuable-close-dropdown');
-
-    if (!container) return;
-
-    const dropdownTrigger = container.querySelector('.js-issuable-close-toggle');
-    const dropdownList = container.querySelector('.js-issuable-close-menu');
-    const button = container.querySelector('.js-issuable-close-button');
+    this.closeReopenReportToggle = initCloseReopenReport();
 
-    this.closeReopenReportToggle = new CloseReopenReportToggle({
-      dropdownTrigger,
-      dropdownList,
-      button,
-    });
-
-    this.closeReopenReportToggle.initDroplab();
+    this.closeButtons = this.closeButtons.not('.issuable-close-button');
+    this.reopenButtons = this.reopenButtons.not('.issuable-close-button');
   }
 
   disableCloseReopenButton($button, shouldDisable) {
@@ -126,12 +113,9 @@ class Issue {
   }
 
   toggleCloseReopenButton(isClosed) {
-    if (this.closeReopenReportToggle) {
-      this.closeReopenReportToggle.updateButton(isClosed);
-    } else {
-      this.closeButtons.toggleClass('hidden', isClosed);
-      this.reopenButtons.toggleClass('hidden', !isClosed);
-    }
+    if (this.closeReopenReportToggle) this.closeReopenReportToggle.updateButton(isClosed);
+    this.closeButtons.toggleClass('hidden', isClosed);
+    this.reopenButtons.toggleClass('hidden', !isClosed);
   }
 
   static submitNoteForm(form) {
diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js
index 10860d1363f..938c7446a06 100644
--- a/app/assets/javascripts/merge_request.js
+++ b/app/assets/javascripts/merge_request.js
@@ -4,7 +4,7 @@
 import 'vendor/jquery.waitforimages';
 import './task_list';
 import './merge_request_tabs';
-import CloseReopenReportToggle from './close_reopen_report_toggle';
+import initCloseReopenReport from './helpers/issuables_helper';
 
 (function() {
   this.MergeRequest = (function() {
@@ -26,7 +26,7 @@ import CloseReopenReportToggle from './close_reopen_report_toggle';
       this.initTabs();
       this.initMRBtnListeners();
       this.initCommitMessageListeners();
-      this.initCloseReopenReport();
+      this.closeReopenReportToggle = initCloseReopenReport();
 
       if ($("a.btn-close").length) {
         this.taskList = new gl.TaskList({
@@ -127,24 +127,6 @@ import CloseReopenReportToggle from './close_reopen_report_toggle';
       $el.text(gl.text.addDelimiter(count));
     };
 
-    MergeRequest.prototype.initCloseReopenReport = function () {
-      const container = document.querySelector('.js-issuable-close-dropdown');
-
-      if (!container) return;
-
-      const dropdownTrigger = container.querySelector('.js-issuable-close-toggle');
-      const dropdownList = container.querySelector('.js-issuable-close-menu');
-      const button = container.querySelector('.js-issuable-close-button');
-
-      this.closeReopenReportToggle = new CloseReopenReportToggle({
-        dropdownTrigger,
-        dropdownList,
-        button,
-      });
-
-      this.closeReopenReportToggle.initDroplab();
-    };
-
     return MergeRequest;
   })();
 }).call(window);
diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss
index c9986441406..dc4ed42544f 100644
--- a/app/assets/stylesheets/framework/dropdowns.scss
+++ b/app/assets/stylesheets/framework/dropdowns.scss
@@ -308,6 +308,7 @@
 
   li {
     padding: $gl-btn-padding $gl-btn-padding 2px;
+    cursor: pointer;
 
     > a,
     > button {
diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss
index c848fd76298..8cc29e4614b 100644
--- a/app/assets/stylesheets/framework/filters.scss
+++ b/app/assets/stylesheets/framework/filters.scss
@@ -70,6 +70,7 @@
 
   .input-token {
     max-width: 200px;
+    padding: 0;
 
     &:hover,
     &:focus {
@@ -83,7 +84,6 @@
     flex: 1;
     -webkit-flex: 1;
     max-width: inherit;
-    padding: 0;
   }
 }
 
@@ -165,6 +165,12 @@
 
 .droplab-dropdown li.filtered-search-token {
   padding: 0;
+  
+  &:hover,
+  &:focus {
+    background-color: inherit;
+    color: inherit;
+  }
 }
 
 .filtered-search-term {
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index 2f35fb94a3d..56a4b53ed61 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -805,8 +805,22 @@
   @include transition(border-color, color);
 }
 
-.issuable-close-dropdown .dropdown-menu {
-  min-width: 270px;
-  left: auto;
-  right: 0;
+.issuable-close-dropdown {
+  .dropdown-menu {
+    min-width: 270px;
+    left: auto;
+    right: 0;
+  }
+
+  .description {
+    margin-bottom: 10px;
+
+    .text {
+      margin: 0;
+    }
+  }
+
+  .dropdown-toggle > .icon {
+    margin: 0 3px;
+  }
 }
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 4161a6a1896..d0c518f81f7 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -263,8 +263,6 @@ module IssuablesHelper
       issue_url(issuable, *options)
     when MergeRequest
       merge_request_url(issuable, *options)
-    else
-      raise TypeError.new('unknown issuable type')
     end
   end
 
@@ -274,8 +272,6 @@ module IssuablesHelper
       issue_button_visibility(issuable, closed)
     when MergeRequest
       merge_request_button_visibility(issuable, closed)
-    else
-      raise TypeError.new('unknown issuable type')
     end
   end
 
@@ -285,8 +281,6 @@ module IssuablesHelper
       ''
     when MergeRequest
       'put'
-    else
-      raise TypeError.new('unknown issuable type')
     end
   end
 
@@ -323,8 +317,6 @@ module IssuablesHelper
         issue_template_names
       when MergeRequest
         merge_request_template_names
-      else
-        raise 'Unknown issuable type!'
       end
   end
 
diff --git a/app/views/shared/issuable/_close_reopen_report_toggle.html.haml b/app/views/shared/issuable/_close_reopen_report_toggle.html.haml
index 0480880118a..6756a7f17fd 100644
--- a/app/views/shared/issuable/_close_reopen_report_toggle.html.haml
+++ b/app/views/shared/issuable/_close_reopen_report_toggle.html.haml
@@ -21,10 +21,9 @@
       %button.btn.btn-transparent
         = icon('check', class: 'icon')
         .description
-          %strong
+          %strong.title
             Close
             = display_issuable_type
-          %p
 
     %li.reopen-item{ class: "#{issuable_button_visibility(issuable, false) || 'droplab-item-selected'}",
                      data: { text: "Reopen #{display_issuable_type}", url: reopen_issuable_url(issuable),
@@ -32,10 +31,9 @@
       %button.btn.btn-transparent
         = icon('check', class: 'icon')
         .description
-          %strong
+          %strong.title
             Reopen
             = display_issuable_type
-          %p
 
       %li.divider.droplab-item-ignore
 
@@ -44,8 +42,8 @@
         %button.btn.btn-transparent
           = icon('check', class: 'icon')
           .description
-            %strong Report abuse
-            %p
+            %strong.title Report abuse
+            %p.text
               Report
               = display_issuable_type.pluralize
               that are abusive, inappropriate or spam.
diff --git a/spec/features/issuables/close_reopen_report_toggle_spec.rb b/spec/features/issuables/close_reopen_report_toggle_spec.rb
index 38e8ca3eb36..361f14e4366 100644
--- a/spec/features/issuables/close_reopen_report_toggle_spec.rb
+++ b/spec/features/issuables/close_reopen_report_toggle_spec.rb
@@ -1,45 +1,45 @@
 require 'spec_helper'
 
-shared_examples 'an issuable close/reopen/report toggle' do
-  let(:container) { find('.issuable-close-dropdown') }
-  let(:human_model_name) { issuable.model_name.human.downcase }
+describe 'Issuables Close/Reopen/Report toggle', :feature do
+  let(:user) { create(:user) }
 
-  it 'shows toggle' do
-    expect(page).to have_link("Close #{human_model_name}")
-    expect(page).to have_selector('.issuable-close-dropdown')
-  end
+  shared_examples 'an issuable close/reopen/report toggle' do
+    let(:container) { find('.issuable-close-dropdown') }
+    let(:human_model_name) { issuable.model_name.human.downcase }
 
-  it 'opens a dropdown when toggle is clicked' do
-    container.find('.dropdown-toggle').click
-
-    expect(container).to have_selector('.dropdown-menu')
-    expect(container).to have_content("Close #{human_model_name}")
-    expect(container).to have_content('Report abuse')
-    expect(container).to have_content("Report #{human_model_name.pluralize} that are abusive, inappropriate or spam.")
-    expect(container).to have_selector('.close-item.droplab-item-selected')
-    expect(container).to have_selector('.report-item')
-    expect(container).not_to have_selector('.report-item.droplab-item-selected')
-    expect(container).not_to have_selector('.reopen-item')
-  end
+    it 'shows toggle' do
+      expect(page).to have_link("Close #{human_model_name}")
+      expect(page).to have_selector('.issuable-close-dropdown')
+    end
+
+    it 'opens a dropdown when toggle is clicked' do
+      container.find('.dropdown-toggle').click
+
+      expect(container).to have_selector('.dropdown-menu')
+      expect(container).to have_content("Close #{human_model_name}")
+      expect(container).to have_content('Report abuse')
+      expect(container).to have_content("Report #{human_model_name.pluralize} that are abusive, inappropriate or spam.")
+      expect(container).to have_selector('.close-item.droplab-item-selected')
+      expect(container).to have_selector('.report-item')
+      expect(container).not_to have_selector('.report-item.droplab-item-selected')
+      expect(container).not_to have_selector('.reopen-item')
+    end
 
-  it 'changes the button when an item is selected' do
-    button = container.find('.issuable-close-button')
+    it 'changes the button when an item is selected' do
+      button = container.find('.issuable-close-button')
 
-    container.find('.dropdown-toggle').click
-    container.find('.report-item').click
+      container.find('.dropdown-toggle').click
+      container.find('.report-item').click
 
-    expect(container).not_to have_selector('.dropdown-menu')
-    expect(button).to have_content('Report abuse')
+      expect(container).not_to have_selector('.dropdown-menu')
+      expect(button).to have_content('Report abuse')
 
-    container.find('.dropdown-toggle').click
-    container.find('.close-item').click
+      container.find('.dropdown-toggle').click
+      container.find('.close-item').click
 
-    expect(button).to have_content("Close #{human_model_name}")
+      expect(button).to have_content("Close #{human_model_name}")
+    end
   end
-end
-
-describe 'Issuables Close/Reopen/Report toggle', :feature do
-  let(:user) { create(:user) }
 
   context 'on an issue' do
     let(:project) { create(:empty_project) }
diff --git a/spec/features/issues/filtered_search/visual_tokens_spec.rb b/spec/features/issues/filtered_search/visual_tokens_spec.rb
index 08360bfa641..e40cfd09601 100644
--- a/spec/features/issues/filtered_search/visual_tokens_spec.rb
+++ b/spec/features/issues/filtered_search/visual_tokens_spec.rb
@@ -133,7 +133,7 @@ describe 'Visual tokens', js: true, feature: true do
   describe 'editing milestone token' do
     before do
       input_filtered_search('milestone:%10.0 author:none', submit: false)
-      first('.tokens-container .filtered-search-token').double_click
+      first('.tokens-container .filtered-search-token').click
       first('#js-dropdown-milestone .filter-dropdown .filter-dropdown-item')
     end