Improve label selection performance

Improves labels selection performance by using local data prop to
maintain selection state on UI.
Fixes bug where rapidly selecting label multiple times causes same
label to show up twice.

app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_button.vue

@@ -27,9 +27,9 @@ export default {
     class="labels-select-dropdown-button js-dropdown-button w-100 text-left"
     @click="handleButtonClick"
   >
-    <span class="dropdown-toggle-text" :class="{ 'flex-fill': isDropdownVariantStandalone }">{{
-      dropdownButtonText
-    }}</span>
+    <span class="dropdown-toggle-text flex-fill">
+      {{ dropdownButtonText }}
+    </span>
     <gl-icon name="chevron-down" class="pull-right" />
   </gl-button>
 </template>

app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_labels_view.vue

 <script>
 import { mapState, mapGetters, mapActions } from 'vuex';
-import { GlLoadingIcon, GlButton, GlIcon, GlSearchBoxByType, GlLink } from '@gitlab/ui';
+import { GlLoadingIcon, GlButton, GlSearchBoxByType, GlLink } from '@gitlab/ui';
 
 import { UP_KEY_CODE, DOWN_KEY_CODE, ENTER_KEY_CODE, ESC_KEY_CODE } from '~/lib/utils/keycodes';
 
+import LabelItem from './label_item.vue';
+
 export default {
   components: {
     GlLoadingIcon,
     GlButton,
-    GlIcon,
     GlSearchBoxByType,
     GlLink,
+    LabelItem,
   },
   data() {
     return {
@@ -60,11 +62,6 @@ export default {
       'updateSelectedLabels',
       'toggleDropdownContents',
     ]),
-    getDropdownLabelBoxStyle(label) {
-      return {
-        backgroundColor: label.color,
-      };
-    },
     isLabelSelected(label) {
       return this.selectedLabelsList.includes(label.id);
     },
@@ -138,24 +135,19 @@ export default {
         @click="toggleDropdownContents"
       />
     </div>
-    <div class="dropdown-input">
+    <div class="dropdown-input" @click.stop="() => {}">
       <gl-search-box-by-type v-model="searchKey" :autofocus="true" />
     </div>
-    <div v-if="!labelsFetchInProgress" ref="labelsListContainer" class="dropdown-content">
+    <div v-show="!labelsFetchInProgress" ref="labelsListContainer" class="dropdown-content">
       <ul class="list-unstyled mb-0">
         <li v-for="(label, index) in visibleLabels" :key="label.id" class="d-block text-left">
-          <gl-link
-            class="d-flex align-items-baseline text-break-word label-item"
-            :class="{ 'is-focused': index === currentHighlightItem }"
-            @click="handleLabelClick(label)"
-          >
-            <gl-icon v-show="label.set" name="mobile-issue-close" class="mr-2 align-self-center" />
-            <span v-show="!label.set" class="mr-3 pr-2"></span>
-            <span class="dropdown-label-box" :style="getDropdownLabelBoxStyle(label)"></span>
-            <span>{{ label.title }}</span>
-          </gl-link>
+          <label-item
+            :label="label"
+            :highlight="index === currentHighlightItem"
+            @clickLabel="handleLabelClick(label)"
+          />
         </li>
-        <li v-if="!visibleLabels.length" class="p-2 text-center">
+        <li v-show="!visibleLabels.length" class="p-2 text-center">
           {{ __('No matching results') }}
         </li>
       </ul>
@@ -170,9 +162,9 @@ export default {
           >
         </li>
         <li>
-          <gl-link :href="labelsManagePath" class="d-flex flex-row text-break-word label-item">
-            {{ footerManageLabelTitle }}
-          </gl-link>
+          <gl-link :href="labelsManagePath" class="d-flex flex-row text-break-word label-item">{{
+            footerManageLabelTitle
+          }}</gl-link>
         </li>
       </ul>
     </div>

app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/label_item.vue

+<script>
+import { GlIcon, GlLink } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlIcon,
+    GlLink,
+  },
+  props: {
+    label: {
+      type: Object,
+      required: true,
+    },
+    highlight: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  data() {
+    return {
+      isSet: this.label.set,
+    };
+  },
+  computed: {
+    labelBoxStyle() {
+      return {
+        backgroundColor: this.label.color,
+      };
+    },
+  },
+  methods: {
+    handleClick() {
+      this.isSet = !this.isSet;
+      this.$emit('clickLabel', this.label);
+    },
+  },
+};
+</script>
+
+<template>
+  <gl-link
+    class="d-flex align-items-baseline text-break-word label-item"
+    :class="{ 'is-focused': highlight }"
+    @click="handleClick"
+  >
+    <gl-icon v-show="isSet" name="mobile-issue-close" class="mr-2 align-self-center" />
+    <span v-show="!isSet" data-testid="no-icon" class="mr-3 pr-2"></span>
+    <span class="dropdown-label-box" data-testid="label-color-box" :style="labelBoxStyle"></span>
+    <span>{{ label.title }}</span>
+  </gl-link>
+</template>

app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/store/mutations.js

@@ -58,29 +58,13 @@ export default {
   },
 
   [types.UPDATE_SELECTED_LABELS](state, { labels }) {
-    // Iterate over all the labels and update
-    // `set` prop value to represent their current state.
-    const labelIds = labels.map(label => label.id);
-    state.labels = state.labels.reduce((allLabels, label) => {
-      if (labelIds.includes(label.id)) {
-        allLabels.push({
-          ...label,
-          touched: true,
-          set: !label.set,
-        });
-      } else {
-        // In case multiselect is not allowed
-        // we unselect any existing selected label
-        const unchangedLabel = state.allowMultiselect
-          ? label
-          : {
-              ...label,
-              touched: true,
-              set: false,
-            };
-        allLabels.push(unchangedLabel);
+    // Find the label to update from all the labels
+    // and change `set` prop value to represent their current state.
+    const labelId = labels.pop()?.id;
+    const candidateLabel = state.labels.find(label => labelId === label.id);
+    if (candidateLabel) {
+      candidateLabel.touched = true;
+      candidateLabel.set = !candidateLabel.set;
     }
-      return allLabels;
-    }, []);
   },
 };

ee/app/assets/javascripts/epic/components/sidebar_items/sidebar_labels.vue

@@ -101,7 +101,22 @@ export default {
       }
     },
     handleUpdateSelectedLabels(labels) {
-      this.updateEpicLabels(labels);
+      // Iterate over selection and check if labels which were
+      // either selected or removed aren't leading to same selection
+      // as current one, as then we don't want to make network call
+      // since nothing has changed.
+      const anyLabelUpdated = labels.some(label => {
+        // Find this label in existing selection.
+        const existingLabel = this.epicContext.labels.find(l => l.id === label.id);
+
+        // Check either of the two following conditions;
+        // 1. A label that's not currently applied is being applied.
+        // 2. A label that's already applied is being removed.
+        return (!existingLabel && label.set) || (existingLabel && !label.set);
+      });
+
+      // Only proceed with action if there are any label updates to be done.
+      if (anyLabelUpdated) this.updateEpicLabels(labels);
     },
   },
 };

ee/spec/frontend/epic/components/sidebar_items/sidebar_labels_spec.js

@@ -117,6 +117,62 @@ describe('SidebarLabelsComponent', () => {
         expect(wrapper.vm.epicContext.labels[0].id).toBe(mockLabels[0].id);
       });
     });
+
+    describe('handleUpdateSelectedLabels', () => {
+      const updatingLabel = {
+        id: 1,
+        title: 'Foo Label',
+        description: 'Foobar',
+        color: '#BADA55',
+        text_color: '#FFFFFF',
+      };
+
+      beforeEach(() => {
+        jest.spyOn(wrapper.vm, 'updateEpicLabels').mockImplementation();
+      });
+
+      it('calls action `updateEpicLabels` when there is a label to apply', () => {
+        store.state.labels = mockLabels;
+        const appliedLabel = {
+          ...updatingLabel,
+          set: true,
+        };
+
+        wrapper.vm.handleUpdateSelectedLabels([appliedLabel]);
+
+        expect(wrapper.vm.updateEpicLabels).toHaveBeenCalledWith(
+          expect.arrayContaining([appliedLabel]),
+        );
+      });
+
+      it('calls action `updateEpicLabels` when there is a label to remove', () => {
+        const removedLabel = {
+          ...updatingLabel,
+          set: false,
+        };
+
+        store.state.labels = [...mockLabels, removedLabel];
+
+        wrapper.vm.handleUpdateSelectedLabels([removedLabel]);
+
+        expect(wrapper.vm.updateEpicLabels).toHaveBeenCalledWith(
+          expect.arrayContaining([removedLabel]),
+        );
+      });
+
+      it('does not call action `updateEpicLabels` when there are no labels to apply or remove', () => {
+        const appliedLabel = {
+          ...updatingLabel,
+          set: true,
+        };
+
+        store.state.labels = [...mockLabels, appliedLabel];
+
+        wrapper.vm.handleUpdateSelectedLabels([appliedLabel]);
+
+        expect(wrapper.vm.updateEpicLabels).not.toHaveBeenCalled();
+      });
+    });
   });
 
   describe('template', () => {

spec/frontend/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_labels_view_spec.js

 import Vuex from 'vuex';
 import { shallowMount, createLocalVue } from '@vue/test-utils';
 
-import { GlButton, GlLoadingIcon, GlIcon, GlSearchBoxByType, GlLink } from '@gitlab/ui';
+import { GlButton, GlLoadingIcon, GlSearchBoxByType, GlLink } from '@gitlab/ui';
 import { UP_KEY_CODE, DOWN_KEY_CODE, ENTER_KEY_CODE, ESC_KEY_CODE } from '~/lib/utils/keycodes';
 import DropdownContentsLabelsView from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_contents_labels_view.vue';
+import LabelItem from '~/vue_shared/components/sidebar/labels_select_vue/label_item.vue';
 
 import defaultState from '~/vue_shared/components/sidebar/labels_select_vue/store/state';
 import mutations from '~/vue_shared/components/sidebar/labels_select_vue/store/mutations';
@@ -78,16 +79,6 @@ describe('DropdownContentsLabelsView', () => {
   });
 
   describe('methods', () => {
-    describe('getDropdownLabelBoxStyle', () => {
-      it('returns an object containing `backgroundColor` based on provided `label` param', () => {
-        expect(wrapper.vm.getDropdownLabelBoxStyle(mockRegularLabel)).toEqual(
-          expect.objectContaining({
-            backgroundColor: mockRegularLabel.color,
-          }),
-        );
-      });
-    });
-
     describe('isLabelSelected', () => {
       it('returns true when provided `label` param is one of the selected labels', () => {
         expect(wrapper.vm.isLabelSelected(mockRegularLabel)).toBe(true);
@@ -234,16 +225,7 @@ describe('DropdownContentsLabelsView', () => {
     });
 
     it('renders label elements for all labels', () => {
-      const labelsEl = wrapper.findAll('.dropdown-content li');
-      const labelItemEl = labelsEl.at(0).find(GlLink);
-
-      expect(labelsEl.length).toBe(mockLabels.length);
-      expect(labelItemEl.exists()).toBe(true);
-      expect(labelItemEl.find(GlIcon).props('name')).toBe('mobile-issue-close');
-      expect(labelItemEl.find('.dropdown-label-box').attributes('style')).toBe(
-        'background-color: rgb(186, 218, 85);',
-      );
-      expect(labelItemEl.find(GlLink).text()).toContain(mockLabels[0].title);
+      expect(wrapper.findAll(LabelItem)).toHaveLength(mockLabels.length);
     });
 
     it('renders label element with "is-focused" when value of `currentHighlightItem` is more than -1', () => {
@@ -253,9 +235,9 @@ describe('DropdownContentsLabelsView', () => {
 
       return wrapper.vm.$nextTick(() => {
         const labelsEl = wrapper.findAll('.dropdown-content li');
-        const labelItemEl = labelsEl.at(0).find(GlLink);
+        const labelItemEl = labelsEl.at(0).find(LabelItem);
 
-        expect(labelItemEl.attributes('class')).toContain('is-focused');
+        expect(labelItemEl.props('highlight')).toBe(true);
       });
     });
 
@@ -267,7 +249,7 @@ describe('DropdownContentsLabelsView', () => {
       return wrapper.vm.$nextTick(() => {
         const noMatchEl = wrapper.find('.dropdown-content li');
 
-        expect(noMatchEl.exists()).toBe(true);
+        expect(noMatchEl.isVisible()).toBe(true);
         expect(noMatchEl.text()).toContain('No matching results');
       });
     });

spec/frontend/vue_shared/components/sidebar/labels_select_vue/label_item_spec.js

+import { shallowMount } from '@vue/test-utils';
+
+import { GlIcon, GlLink } from '@gitlab/ui';
+import LabelItem from '~/vue_shared/components/sidebar/labels_select_vue/label_item.vue';
+import { mockRegularLabel } from './mock_data';
+
+const createComponent = ({ label = mockRegularLabel, highlight = true } = {}) =>
+  shallowMount(LabelItem, {
+    propsData: {
+      label,
+      highlight,
+    },
+  });
+
+describe('LabelItem', () => {
+  let wrapper;
+
+  beforeEach(() => {
+    wrapper = createComponent();
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('computed', () => {
+    describe('labelBoxStyle', () => {
+      it('returns an object containing `backgroundColor` based on `label` prop', () => {
+        expect(wrapper.vm.labelBoxStyle).toEqual(
+          expect.objectContaining({
+            backgroundColor: mockRegularLabel.color,
+          }),
+        );
+      });
+    });
+  });
+
+  describe('methods', () => {
+    describe('handleClick', () => {
+      it('sets value of `isSet` data prop to opposite of its current value', () => {
+        wrapper.setData({
+          isSet: true,
+        });
+
+        wrapper.vm.handleClick();
+        expect(wrapper.vm.isSet).toBe(false);
+        wrapper.vm.handleClick();
+        expect(wrapper.vm.isSet).toBe(true);
+      });
+
+      it('emits event `clickLabel` on component with `label` prop as param', () => {
+        wrapper.vm.handleClick();
+
+        expect(wrapper.emitted('clickLabel')).toBeTruthy();
+        expect(wrapper.emitted('clickLabel')[0]).toEqual([mockRegularLabel]);
+      });
+    });
+  });
+
+  describe('template', () => {
+    it('renders gl-link component', () => {
+      expect(wrapper.find(GlLink).exists()).toBe(true);
+    });
+
+    it('renders gl-link component with class `is-focused` when `highlight` prop is true', () => {
+      wrapper.setProps({
+        highlight: true,
+      });
+
+      return wrapper.vm.$nextTick(() => {
+        expect(wrapper.find(GlLink).classes()).toContain('is-focused');
+      });
+    });
+
+    it('renders visible gl-icon component when `isSet` prop is true', () => {
+      wrapper.setData({
+        isSet: true,
+      });
+
+      return wrapper.vm.$nextTick(() => {
+        const iconEl = wrapper.find(GlIcon);
+
+        expect(iconEl.isVisible()).toBe(true);
+        expect(iconEl.props('name')).toBe('mobile-issue-close');
+      });
+    });
+
+    it('renders visible span element as placeholder instead of gl-icon when `isSet` prop is false', () => {
+      wrapper.setData({
+        isSet: false,
+      });
+
+      return wrapper.vm.$nextTick(() => {
+        const placeholderEl = wrapper.find('[data-testid="no-icon"]');
+
+        expect(placeholderEl.isVisible()).toBe(true);
+      });
+    });
+
+    it('renders label color element', () => {
+      const colorEl = wrapper.find('[data-testid="label-color-box"]');
+
+      expect(colorEl.exists()).toBe(true);
+      expect(colorEl.attributes('style')).toBe('background-color: rgb(186, 218, 85);');
+    });
+
+    it('renders label title', () => {
+      expect(wrapper.text()).toContain(mockRegularLabel.title);
+    });
+  });
+});

spec/frontend/vue_shared/components/sidebar/labels_select_vue/store/mutations_spec.js

@@ -155,29 +155,12 @@ describe('LabelsSelect Mutations', () => {
   describe(`${types.UPDATE_SELECTED_LABELS}`, () => {
     const labels = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }];
 
-    it('updates `state.labels` to include `touched` and `set` props based on provided `labels` param when `state.allowMultiselect` is `true`', () => {
-      const updatedLabelIds = [2, 4];
-      const state = {
-        labels,
-        allowMultiselect: true,
-      };
-      mutations[types.UPDATE_SELECTED_LABELS](state, { labels });
-
-      state.labels.forEach(label => {
-        if (updatedLabelIds.includes(label.id)) {
-          expect(label.touched).toBe(true);
-          expect(label.set).toBe(true);
-        }
-      });
-    });
-
-    it('updates `state.labels` to include `touched` and `set` props based on provided `labels` param when `state.allowMultiselect` is `false`', () => {
+    it('updates `state.labels` to include `touched` and `set` props based on provided `labels` param', () => {
       const updatedLabelIds = [2];
       const state = {
         labels,
-        allowMultiselect: false,
       };
-      mutations[types.UPDATE_SELECTED_LABELS](state, { labels });
+      mutations[types.UPDATE_SELECTED_LABELS](state, { labels: [{ id: 2 }] });
 
       state.labels.forEach(label => {
         if (updatedLabelIds.includes(label.id)) {