From 49bc1cd6eb7b90b34c3f2f18f27743ccf2c4338d Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Mon, 18 Jul 2016 17:43:00 +0100
Subject: [PATCH] Updated how the label toggle gets the text Fixed some issues
 based on self-review

---
 .../javascripts/labels_select.js.coffee       | 74 ++++---------------
 .../javascripts/milestone_select.js.coffee    |  6 +-
 app/helpers/labels_helper.rb                  |  2 +-
 app/helpers/milestones_helper.rb              |  2 +-
 .../shared/issuable/_label_dropdown.html.haml |  2 +-
 5 files changed, 21 insertions(+), 65 deletions(-)

diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee
index 2a497c2e804..5e09823b2d9 100644
--- a/app/assets/javascripts/labels_select.js.coffee
+++ b/app/assets/javascripts/labels_select.js.coffee
@@ -25,11 +25,9 @@ class @LabelsSelect
       $newLabelError = $('.js-label-error')
       $colorPreview = $('.js-dropdown-label-color-preview')
       $newLabelCreateButton = $('.js-new-label-btn')
-      selectedLabels = []
-
-      $("input[name='#{$dropdown.data('field-name')}']").each ->
-        title = $(this).data('title')
-        selectedLabels.push($(this).data('title')) if title
+      fieldName = $dropdown.data('field-name')
+      useId = $dropdown.hasClass('js-issuable-form-dropdown') or $dropdown.hasClass('js-filter-bulk-update')
+      propertyName = if useId then "id" else "title"
 
       $newLabelError.hide()
       $loading = $block.find('.block-loading').fadeOut()
@@ -125,7 +123,7 @@ class @LabelsSelect
       saveLabelData = ->
         selected = $dropdown
           .closest('.selectbox')
-          .find("input[name='#{$dropdown.data('field-name')}']")
+          .find("input[name='#{fieldName}']")
           .map(->
             @value
           ).get()
@@ -278,63 +276,21 @@ class @LabelsSelect
           fields: ['title']
         selectable: true
         filterable: true
-        toggleLabel: (selected, $el, glDropdownInstance) ->
-          # When comes from a triggered event handle it VERY differently
-          if selected instanceof jQuery.Event
-            $dropdownParent = $dropdown.closest '.labels-filter'
-            $labelInputs = $dropdownParent.find "input[name='#{@fieldName}']"
-            numberSelectedLabels = $labelInputs.length
-            firstLabel = _.pluck($labelInputs, 'value')[0]
-
-            if numberSelectedLabels is 1
-                firstLabel
-            else if numberSelectedLabels > 1
-              "#{firstLabel} +#{numberSelectedLabels - 1} more"
-            else
-              defaultLabel
-          # when clicking on a dropdown option
-          else
-            # Return when clicking "No Label"
-            return if selected.id is 0
-            return 'Any Label' if selected.isAny is true
-
-            if glDropdownInstance?
-              $dropdownParent = glDropdownInstance.dropdown.closest '.issuable-form-select-holder, .labels-filter'
-            else
-              $dropdownParent = $()
+        toggleLabel: (selected, el, glDropdown) ->
+          if glDropdown?
+            selectedIds = $("input[name='#{fieldName}']").map(-> $(this).val()).get()
 
-            $labelInputs = $dropdownParent.find "input[name='#{@fieldName}']"
+            selected = _.filter glDropdown.fullData, (label) ->
+              selectedIds.indexOf("#{label[propertyName]}") >= 0 if label[propertyName]?
 
-            # Find the label by its attribute according the dropdown settings
-            if $dropdown.hasClass('js-issuable-form-dropdown') or $dropdown.hasClass('js-filter-bulk-update')
-              # When settings labels to a issuable we find the label for its ID
-              whereQuery = { id: parseInt $labelInputs.first().val() }
+            if selected.length is 1
+              selected[0].title
+            else if selected.length > 1
+              "#{selected[0].title} +#{selected.length - 1} more"
             else
-              # When filtering issuables we find the label for its title
-              whereQuery = { title: $labelInputs.first().val() }
-
-            firstLabel = _.findWhere glDropdownInstance.fullData, whereQuery
-
-            # Better rely on inputs since filtering may returns invalid number of active labels
-            numberSelectedLabels = $labelInputs.length
-
-            # If we are adding a label
-            if $el.is '.is-active'
-              if numberSelectedLabels is 1
-                selected.title
-              else
-                "#{selected.title} +#{numberSelectedLabels - 1} more"
-
-            # otherwise we are removing a label
-            else
-              if numberSelectedLabels is 1
-                firstLabel.title
-              else if numberSelectedLabels > 1
-                "#{firstLabel.title} +#{numberSelectedLabels - 1} more"
-              else
-                defaultLabel
+              defaultLabel
         defaultLabel: defaultLabel
-        fieldName: $dropdown.data('field-name')
+        fieldName: fieldName
         id: (label) ->
           if $dropdown.hasClass('js-issuable-form-dropdown')
             if label.id is 0
diff --git a/app/assets/javascripts/milestone_select.js.coffee b/app/assets/javascripts/milestone_select.js.coffee
index 0d71f3b623a..56e9a18e7ff 100644
--- a/app/assets/javascripts/milestone_select.js.coffee
+++ b/app/assets/javascripts/milestone_select.js.coffee
@@ -70,8 +70,8 @@ class @MilestoneSelect
         search:
           fields: ['title']
         selectable: true
-        toggleLabel: (selected, el, e, added) ->
-          if selected and 'id' of selected and added
+        toggleLabel: (selected, el, e) ->
+          if selected and 'id' of selected and $(el).hasClass('is-active')
             selected.title
           else
             defaultLabel
@@ -80,7 +80,7 @@ class @MilestoneSelect
         text: (milestone) ->
           _.escape(milestone.title)
         id: (milestone) ->
-          if !useId and not $dropdown.is('.js-issuable-form-dropdown')
+          if not useId and not $dropdown.is('.js-issuable-form-dropdown')
             milestone.name
           else
             milestone.id
diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 5acfae753b9..b9f3d6c75c2 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -116,7 +116,7 @@ module LabelsHelper
 
   def labels_filter_path
     project = @target_project || @project
-    if @project
+    if project
       namespace_project_labels_path(project.namespace, project, :json)
     else
       dashboard_labels_path(:json)
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index bc87ac32e3e..90f83f5fde5 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -48,7 +48,7 @@ module MilestonesHelper
 
   def milestones_filter_dropdown_path
     project = @target_project || @project
-    if @project
+    if project
       namespace_project_milestones_path(project.namespace, project, :json)
     else
       dashboard_milestones_path(:json)
diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml
index b52fa6fa8c2..666372b95a6 100644
--- a/app/views/shared/issuable/_label_dropdown.html.haml
+++ b/app/views/shared/issuable/_label_dropdown.html.haml
@@ -22,7 +22,7 @@
   - selected.each do |label|
     - id = label.try(:id) || label
     - title = label.try(:title) || label
-    = hidden_field_tag data_options[:field_name], useId ? id : title, id: nil, data: { title: title }
+    = hidden_field_tag data_options[:field_name], useId ? id : title, id: nil
 .dropdown
   %button.dropdown-menu-toggle.js-label-select.js-multiselect{class: classes.join(' '), type: "button", data: dropdown_data}
     %span.dropdown-toggle-text{ class: ("is-default" if selected.nil? || selected.to_a.size == 0) }
-- 
GitLab