Merge branch '43643-fix-mr-label-filtering' into 'master'

Enable filtering MR list based on clicked label in MR sidebar

Closes #43643

See merge request gitlab-org/gitlab-ce!17390

4 files changed, 83 insertions, 12 deletions

diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js
index 89a246f56cf..9b46bbf83da 100644
--- a/app/assets/javascripts/labels_select.js
+++ b/app/assets/javascripts/labels_select.js
@@ -21,7 +21,7 @@ export default class LabelsSelect {
     }
 
     $els.each(function(i, dropdown) {
-      var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelHTMLTemplate, labelNoneHTMLTemplate, labelUrl, namespacePath, projectPath, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip, initialSelected, $toggleText, fieldName, useId, propertyName, showMenuAbove, $container, $dropdownContainer;
+      var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelUrl, namespacePath, projectPath, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip, initialSelected, $toggleText, fieldName, useId, propertyName, showMenuAbove, $container, $dropdownContainer;
       $dropdown = $(dropdown);
       $dropdownContainer = $dropdown.closest('.labels-filter');
       $toggleText = $dropdown.find('.dropdown-toggle-text');
@@ -53,13 +53,6 @@ export default class LabelsSelect {
         .map(function () {
           return this.value;
         }).get();
-      if (issueUpdateURL != null) {
-        issueURLSplit = issueUpdateURL.split('/');
-      }
-      if (issueUpdateURL) {
-        labelHTMLTemplate = _.template('<% _.each(labels, function(label){ %> <a href="<%- ["",issueURLSplit[1], issueURLSplit[2],""].join("/") %>issues?label_name[]=<%- encodeURIComponent(label.title) %>"> <span class="label has-tooltip color-label" title="<%- label.description %>" style="background-color: <%- label.color %>; color: <%- label.text_color %>;"> <%- label.title %> </span> </a> <% }); %>');
-        labelNoneHTMLTemplate = '<span class="no-value">None</span>';
-      }
       const handleClick = options.handleClick;
 
       $sidebarLabelTooltip.tooltip();
@@ -91,14 +84,17 @@ export default class LabelsSelect {
             $loading.fadeOut();
             $dropdown.trigger('loaded.gl.dropdown');
             $selectbox.hide();
-            data.issueURLSplit = issueURLSplit;
+            data.issueUpdateURL = issueUpdateURL;
             labelCount = 0;
-            if (data.labels.length) {
-              template = labelHTMLTemplate(data);
+            if (data.labels.length && issueUpdateURL) {
+              template = LabelsSelect.getLabelTemplate({
+                labels: data.labels,
+                issueUpdateURL,
+              });
               labelCount = data.labels.length;
             }
             else {
-              template = labelNoneHTMLTemplate;
+              template = '<span class="no-value">None</span>';
             }
             $value.removeAttr('style').html(template);
             $sidebarCollapsedValue.text(labelCount);
@@ -418,6 +414,26 @@ export default class LabelsSelect {
     this.bindEvents();
   }
 
+  static getLabelTemplate(tplData) {
+    // We could use ES6 template string here
+    // and properly indent markup for readability
+    // but that also introduces unintended white-space
+    // so best approach is to use traditional way of
+    // concatenation
+    // see: http://2ality.com/2016/05/template-literal-whitespace.html#joining-arrays
+    const tpl = _.template([
+      '<% _.each(labels, function(label){ %>',
+      '<a href="<%- issueUpdateURL.slice(0, issueUpdateURL.lastIndexOf("/")) %>?label_name[]=<%- encodeURIComponent(label.title) %>">',
+      '<span class="label has-tooltip color-label" title="<%- label.description %>" style="background-color: <%- label.color %>; color: <%- label.text_color %>;">',
+      '<%- label.title %>',
+      '</span>',
+      '</a>',
+      '<% }); %>',
+    ].join(''));
+
+    return tpl(tplData);
+  }
+
   bindEvents() {
     return $('body').on('change', '.selected_issue', this.onSelectCheckboxIssue);
   }
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index 0cf67734237..4c9732c26d9 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -103,6 +103,7 @@
 .issuable-show-labels {
   a {
     margin-bottom: 5px;
+    margin-right: 5px;
     display: inline-block;
 
     .color-label {
@@ -116,6 +117,12 @@
   }
 
   &.has-labels {
+    // this font size is a fix to
+    // prevent unintended spacing between labels
+    // which shows up when rendering markup has white-space
+    // characters present.
+    // see: https://css-tricks.com/fighting-the-space-between-inline-block-elements/#article-header-id-3
+    font-size: 0;
     margin-bottom: -5px;
   }
 }
diff --git a/changelogs/unreleased/43643-fix-mr-label-filtering.yml b/changelogs/unreleased/43643-fix-mr-label-filtering.yml
new file mode 100644
index 00000000000..32a44aef243
--- /dev/null
+++ b/changelogs/unreleased/43643-fix-mr-label-filtering.yml
@@ -0,0 +1,5 @@
+---
+title: Enable filtering MR list based on clicked label in MR sidebar
+merge_request: 17390
+author:
+type: fixed
diff --git a/spec/javascripts/labels_select_spec.js b/spec/javascripts/labels_select_spec.js
new file mode 100644
index 00000000000..b8f7b1dc855
--- /dev/null
+++ b/spec/javascripts/labels_select_spec.js
@@ -0,0 +1,43 @@
+import LabelsSelect from '~/labels_select';
+
+const mockUrl = '/foo/bar/url';
+
+const mockLabels = [
+  {
+    id: 26,
+    title: 'Foo Label',
+    description: 'Foobar',
+    color: '#BADA55',
+    text_color: '#FFFFFF',
+  },
+];
+
+describe('LabelsSelect', () => {
+  describe('getLabelTemplate', () => {
+    const label = mockLabels[0];
+    let $labelEl;
+
+    beforeEach(() => {
+      $labelEl = $(LabelsSelect.getLabelTemplate({
+        labels: mockLabels,
+        issueUpdateURL: mockUrl,
+      }));
+    });
+
+    it('generated label item template has correct label URL', () => {
+      expect($labelEl.attr('href')).toBe('/foo/bar?label_name[]=Foo%20Label');
+    });
+
+    it('generated label item template has correct label title', () => {
+      expect($labelEl.find('span.label').text()).toBe(label.title);
+    });
+
+    it('generated label item template has label description as title attribute', () => {
+      expect($labelEl.find('span.label').attr('title')).toBe(label.description);
+    });
+
+    it('generated label item template has correct label styles', () => {
+      expect($labelEl.find('span.label').attr('style')).toBe(`background-color: ${label.color}; color: ${label.text_color};`);
+    });
+  });
+});