Address feedback

9 files changed, 26 insertions, 32 deletions

diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee
index 4b8e4bb490d..7c7334e9e40 100644
--- a/app/assets/javascripts/gl_dropdown.js.coffee
+++ b/app/assets/javascripts/gl_dropdown.js.coffee
@@ -491,7 +491,9 @@ class GitLabDropdown
 
   addInput: (fieldName, value)->
     # Create hidden input for form
-    $input = $("<input type='hidden' name='#{fieldName}' value='#{value}' />")
+    $input = $('<input>').attr('type', 'hidden')
+                         .attr('name', fieldName)
+                        .val(value)
 
     if @options.inputId?
       $input.attr('id', @options.inputId)
diff --git a/app/assets/javascripts/issues-bulk-assignment.js.coffee b/app/assets/javascripts/issues-bulk-assignment.js.coffee
index 40751c81fd2..16d023dd391 100644
--- a/app/assets/javascripts/issues-bulk-assignment.js.coffee
+++ b/app/assets/javascripts/issues-bulk-assignment.js.coffee
@@ -13,7 +13,7 @@ class @IssuableBulkActions
     @container.find selector
 
   bindEvents: ->
-    @form.on 'submit', @onFormSubmit.bind(@)
+    @form.off('submit').on('submit', @onFormSubmit.bind(@))
 
   onFormSubmit: (e) ->
     e.preventDefault()
diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee
index eb249e24f67..ec74dfaae1a 100644
--- a/app/assets/javascripts/labels_select.js.coffee
+++ b/app/assets/javascripts/labels_select.js.coffee
@@ -199,6 +199,9 @@ class @LabelsSelect
             callback data
 
         renderRow: (label, instance) ->
+          $li = $('<li>')
+          $a  = $('<a href="#">')
+
           selectedClass = []
           removesAll = label.id is 0 or not label.id?
 
@@ -238,21 +241,15 @@ class @LabelsSelect
             colorEl = ''
 
           # We need to identify which items are actually labels
-          labelIdHtml = labelClass = ''
-
           if label.id
-            # Add label id only for labels
-            labelIdHtml = "data-label-id='#{label.id}'"
+            selectedClass.push('label-item')
+            $a.attr('data-label-id', label.id)
 
-            # Add class only for labels
-            labelClass = 'label-item'
+          $a.addClass(selectedClass.join(' '))
+            .html("#{colorEl} #{_.escape(label.title)}")
 
-          "<li>
-            <a href='#' #{labelIdHtml} class='#{labelClass} #{selectedClass.join(' ')}'>
-              #{colorEl}
-              #{_.escape(label.title)}
-            </a>
-          </li>"
+          # Return generated html
+          $li.html($a).prop('outerHTML')
         persistWhenHide: $dropdown.data('persistWhenHide')
         search:
           fields: ['title']
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index b1ff95f6208..37b93f63145 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -96,10 +96,4 @@ module IssuablesHelper
       issuable.open? ? :opened : :closed
     end
   end
-
-  def label_ids(issuable)
-    return nil if !issuable.labels.any?
-    issuable.labels.pluck :id
-  end
-
 end
diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml
index c22f5e150e9..79b14819865 100644
--- a/app/views/projects/issues/_issue.html.haml
+++ b/app/views/projects/issues/_issue.html.haml
@@ -1,4 +1,4 @@
-%li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue), data: { labels: label_ids(issue), id: issue.id } }
+%li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue), data: { labels: issue.label_ids, id: issue.id } }
   - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project)
     .issue-check
       = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue"
diff --git a/app/views/projects/labels/_label.html.haml b/app/views/projects/labels/_label.html.haml
index a36535458f7..950f6e284b1 100644
--- a/app/views/projects/labels/_label.html.haml
+++ b/app/views/projects/labels/_label.html.haml
@@ -1,4 +1,4 @@
-%li{id: dom_id(label), :"data-id" => label.id}
+%li{id: dom_id(label), data: { id: label.id } } }
   = render "shared/label_row", label: label
 
   .pull-info-right
diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml
index 8758de12f42..380ab465bf4 100644
--- a/app/views/shared/issuable/_filter.html.haml
+++ b/app/views/shared/issuable/_filter.html.haml
@@ -46,7 +46,7 @@
             = dropdown_tag("Milestone", options: { title: "Assign milestone", toggle_class: 'js-milestone-select js-extra-options js-filter-submit js-filter-bulk-update', filter: true, dropdown_class: "dropdown-menu-selectable dropdown-menu-milestone", placeholder: "Search milestones", data: { show_no: true, field_name: "update[milestone_id]", project_id: @project.id, milestones: namespace_project_milestones_path(@project.namespace, @project, :json), use_id: true } })
 
           .filter-item.inline.labels-filter
-            = render "shared/issuable/label_dropdown", classes: ' js-filter-bulk-update js-multiselect ', show_create: false, show_footer: false, extra_options: false, filter_submit: false, show_footer: false, data_options: { persist_when_hide: "true", field_name: "update[label_ids][]", show_no: false, show_any: false, use_id: true }
+            = render "shared/issuable/label_dropdown", classes: ['js-filter-bulk-update', 'js-multiselect'], show_create: false, show_footer: false, extra_options: false, filter_submit: false, show_footer: false, data_options: { persist_when_hide: "true", field_name: "update[label_ids][]", show_no: false, show_any: false, use_id: true }
 
           = hidden_field_tag 'update[issues_ids]', []
           = hidden_field_tag :state_event, params[:state_event]
diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml
index b095fd19629..d34d28f6736 100644
--- a/app/views/shared/issuable/_label_dropdown.html.haml
+++ b/app/views/shared/issuable/_label_dropdown.html.haml
@@ -3,23 +3,23 @@
 - filter_submit = local_assigns.fetch(:filter_submit, true)
 - show_footer = local_assigns.fetch(:show_footer, true)
 - data_options = local_assigns.fetch(:data_options, {})
-- classes = local_assigns.fetch(:classes, '')
+- classes = local_assigns.fetch(:classes, [])
 - dropdown_data = {toggle: 'dropdown', field_name: 'label_name[]', show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}
 - dropdown_data.merge!(data_options)
-- classes << ' js-extra-options ' if extra_options
-- classes << ' js-filter-submit ' if filter_submit
+- classes << 'js-extra-options' if extra_options
+- classes << 'js-filter-submit' if filter_submit
 
 - if params[:label_name].present?
   - if params[:label_name].respond_to?('any?')
     - params[:label_name].each do |label|
       = hidden_field_tag "label_name[]", label, id: nil
 .dropdown
-  %button.dropdown-menu-toggle.js-label-select.js-multiselect{class: classes, type: "button", data: dropdown_data}
+  %button.dropdown-menu-toggle.js-label-select.js-multiselect{class: classes.join(' '), type: "button", data: dropdown_data}
     %span.dropdown-toggle-text
       = h(multi_label_name(params[:label_name], "Label"))
     = icon('chevron-down')
   .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable
-    = render partial: "shared/issuable/label_page_default", locals: { title: "Filter by label", show_footer: show_footer }
-    - if can? current_user, :admin_label, @project and @project and show_create
+    = render partial: "shared/issuable/label_page_default", locals: { title: "Filter by label", show_footer: show_footer, show_create: show_create }
+    - if show_create and @project and can?(current_user, :admin_label, @project)
       = render partial: "shared/issuable/label_page_create"
     = dropdown_loading
diff --git a/app/views/shared/issuable/_label_page_default.html.haml b/app/views/shared/issuable/_label_page_default.html.haml
index e5be3751d2f..935a0193a2d 100644
--- a/app/views/shared/issuable/_label_page_default.html.haml
+++ b/app/views/shared/issuable/_label_page_default.html.haml
@@ -1,4 +1,5 @@
 - title = local_assigns.fetch(:title, 'Assign labels')
+- show_create = local_assigns.fetch(:show_create, true)
 - show_footer = local_assigns.fetch(:show_footer, true)
 - filter_placeholder = local_assigns.fetch(:filter_placeholder, 'Search labels')
 .dropdown-page-one
@@ -8,14 +9,14 @@
   - if @project and show_footer
     = dropdown_footer do
       %ul.dropdown-footer-list
-        - if can? current_user, :admin_label, @project
+        - if can?(current_user, :admin_label, @project)
           %li
             %a.dropdown-toggle-page{href: "#"}
               Create new
         %li
           = link_to namespace_project_labels_path(@project.namespace, @project), :"data-is-link" => true do
-            - if can? current_user, :admin_label, @project
+            - if show_create && @project && can?(current_user, :admin_label, @project)
               Manage labels
             - else
               View labels
-  = dropdown_loading
\ No newline at end of file
+  = dropdown_loading