Merge branch '254997-fix-labels-search-scroll' into 'master'

Fix Vue Labels Select dropdown keyboard scroll

See merge request gitlab-org/gitlab!43874

app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/constants.js

@@ -3,5 +3,3 @@ export const DropdownVariant = {
   Standalone: 'standalone',
   Embedded: 'embedded',
 };
-
-export const LIST_BUFFER_SIZE = 5;

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, GlSearchBoxByType, GlLink } from '@gitlab/ui';
+import {
+  GlIntersectionObserver,
+  GlLoadingIcon,
+  GlButton,
+  GlSearchBoxByType,
+  GlLink,
+} from '@gitlab/ui';
 import fuzzaldrinPlus from 'fuzzaldrin-plus';
 
 import { UP_KEY_CODE, DOWN_KEY_CODE, ENTER_KEY_CODE, ESC_KEY_CODE } from '~/lib/utils/keycodes';
-import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
 
 import LabelItem from './label_item.vue';
 
-import { LIST_BUFFER_SIZE } from './constants';
-
 export default {
-  LIST_BUFFER_SIZE,
   components: {
+    GlIntersectionObserver,
     GlLoadingIcon,
     GlButton,
     GlSearchBoxByType,
     GlLink,
-    SmartVirtualList,
     LabelItem,
   },
   data() {
@@ -46,15 +48,8 @@ export default {
       }
       return this.labels;
     },
-    showListContainer() {
-      if (this.isDropdownVariantSidebar) {
-        return !this.labelsFetchInProgress;
-      }
-
-      return true;
-    },
     showNoMatchingResultsMessage() {
-      return !this.labelsFetchInProgress && !this.visibleLabels.length;
+      return Boolean(this.searchKey) && this.visibleLabels.length === 0;
     },
   },
   watch: {
@@ -67,14 +62,12 @@ export default {
       }
     },
   },
-  mounted() {
-    this.fetchLabels();
-  },
   methods: {
     ...mapActions([
       'toggleDropdownContents',
       'toggleDropdownContentsCreateView',
       'fetchLabels',
+      'receiveLabelsSuccess',
       'updateSelectedLabels',
       'toggleDropdownContents',
     ]),
@@ -99,6 +92,17 @@ export default {
         }
       }
     },
+    /**
+     * We want to remove loaded labels to ensure component
+     * fetches fresh set of labels every time when shown.
+     */
+    handleComponentDisappear() {
+      this.receiveLabelsSuccess([]);
+    },
+    handleCreateLabelClick() {
+      this.receiveLabelsSuccess([]);
+      this.toggleDropdownContentsCreateView();
+    },
     /**
      * This method enables keyboard navigation support for
      * the dropdown.
@@ -135,12 +139,8 @@ export default {
 </script>
 
 <template>
+  <gl-intersection-observer @appear="fetchLabels" @disappear="handleComponentDisappear">
     <div class="labels-select-contents-list js-labels-list" @keydown="handleKeyDown">
-    <gl-loading-icon
-      v-if="labelsFetchInProgress"
-      class="labels-fetch-loading position-absolute gl-display-flex gl-align-items-center w-100 h-100"
-      size="md"
-    />
       <div
         v-if="isDropdownVariantSidebar || isDropdownVariantEmbedded"
         class="dropdown-title gl-display-flex gl-align-items-center gl-pt-0 gl-pb-3!"
@@ -160,35 +160,29 @@ export default {
         <gl-search-box-by-type
           v-model="searchKey"
           :autofocus="true"
+          :disabled="labelsFetchInProgress"
           data-qa-selector="dropdown_input_field"
         />
       </div>
-    <div
-      v-show="showListContainer"
-      ref="labelsListContainer"
-      class="dropdown-content"
-      data-testid="dropdown-content"
-    >
-      <smart-virtual-list
-        :length="visibleLabels.length"
-        :remain="$options.LIST_BUFFER_SIZE"
-        :size="$options.LIST_BUFFER_SIZE"
-        wclass="list-unstyled mb-0"
-        wtag="ul"
-        class="h-100"
-      >
-        <li v-for="(label, index) in visibleLabels" :key="label.id" class="d-block text-left">
+      <div ref="labelsListContainer" class="dropdown-content" data-testid="dropdown-content">
+        <gl-loading-icon
+          v-if="labelsFetchInProgress"
+          class="labels-fetch-loading gl-align-items-center w-100 h-100"
+          size="md"
+        />
+        <ul v-else class="list-unstyled mb-0">
           <label-item
+            v-for="(label, index) in visibleLabels"
+            :key="label.id"
             :label="label"
             :is-label-set="label.set"
             :highlight="index === currentHighlightItem"
             @clickLabel="handleLabelClick(label)"
           />
-        </li>
           <li v-show="showNoMatchingResultsMessage" class="gl-p-3 gl-text-center">
             {{ __('No matching results') }}
           </li>
-      </smart-virtual-list>
+        </ul>
       </div>
       <div
         v-if="isDropdownVariantSidebar || isDropdownVariantEmbedded"
@@ -199,7 +193,7 @@ export default {
           <li v-if="allowLabelCreate">
             <gl-link
               class="gl-display-flex w-100 flex-row text-break-word label-item"
-            @click="toggleDropdownContentsCreateView"
+              @click="handleCreateLabelClick"
             >
               {{ footerCreateLabelTitle }}
             </gl-link>
@@ -215,4 +209,5 @@ export default {
         </ul>
       </div>
     </div>
+  </gl-intersection-observer>
 </template>

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

 <script>
-import { GlIcon, GlLink } from '@gitlab/ui';
+import { GlLink, GlIcon } from '@gitlab/ui';
 
 export default {
-  components: {
-    GlIcon,
-    GlLink,
-  },
+  functional: true,
   props: {
     label: {
       type: Object,
@@ -21,46 +18,65 @@ export default {
       default: false,
     },
   },
-  data() {
-    return {
-      isSet: this.isLabelSet,
-    };
+  render(h, { props, listeners }) {
+    const { label, highlight, isLabelSet } = props;
+
+    const labelColorBox = h('span', {
+      class: 'dropdown-label-box',
+      style: {
+        backgroundColor: label.color,
+      },
+      attrs: {
+        'data-testid': 'label-color-box',
+      },
+    });
+
+    const checkedIcon = h(GlIcon, {
+      class: {
+        'mr-2 align-self-center': true,
+        hidden: !isLabelSet,
+      },
+      props: {
+        name: 'mobile-issue-close',
+      },
+    });
+
+    const noIcon = h('span', {
+      class: {
+        'mr-3 pr-2': true,
+        hidden: isLabelSet,
+      },
+      attrs: {
+        'data-testid': 'no-icon',
       },
-  computed: {
-    labelBoxStyle() {
-      return {
-        backgroundColor: this.label.color,
-      };
+    });
+
+    const labelTitle = h('span', label.title);
+
+    const labelLink = h(
+      GlLink,
+      {
+        class: 'd-flex align-items-baseline text-break-word label-item',
+        on: {
+          click: () => {
+            listeners.clickLabel(label);
           },
         },
-  watch: {
-    /**
-     * This watcher assures that if user used
-     * `Enter` key to set/unset label, changes
-     * are reflected here too.
-     */
-    isLabelSet(value) {
-      this.isSet = value;
       },
+      [noIcon, checkedIcon, labelColorBox, labelTitle],
+    );
+
+    return h(
+      'li',
+      {
+        class: {
+          'd-block': true,
+          'text-left': true,
+          'is-focused': highlight,
         },
-  methods: {
-    handleClick() {
-      this.isSet = !this.isSet;
-      this.$emit('clickLabel', this.label);
       },
+      [labelLink],
+    );
   },
 };
 </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/labels_select_root.vue

@@ -266,7 +266,7 @@ export default {
       </dropdown-value>
       <dropdown-button v-show="dropdownButtonVisible" class="gl-mt-2" />
       <dropdown-contents
-        v-if="dropdownButtonVisible && showDropdownContents"
+        v-show="dropdownButtonVisible && showDropdownContents"
         ref="dropdownContents"
       />
     </template>

app/assets/stylesheets/framework/dropdowns.scss

@@ -1017,6 +1017,23 @@ header.header-content .dropdown-menu.frequent-items-dropdown-menu {
     }
   }
 
+  li {
+    &:hover,
+    &.is-focused {
+      .label-item {
+        @include dropdown-item-hover;
+
+        text-decoration: none;
+      }
+    }
+  }
+
+  .labels-select-dropdown-button {
+    .gl-button-text {
+      width: 100%;
+    }
+  }
+
   .labels-select-dropdown-contents {
     min-height: $dropdown-min-height;
     max-height: 330px;
@@ -1050,13 +1067,6 @@ header.header-content .dropdown-menu.frequent-items-dropdown-menu {
 
   .label-item {
     padding: 8px 20px;
-
-    &:hover,
-    &.is-focused {
-      @include dropdown-item-hover;
-
-      text-decoration: none;
-    }
   }
 
   .color-input-container {

changelogs/unreleased/254997-fix-labels-search-scroll.yml

+---
+title: Fix Vue Labels Select dropdown keyboard scroll
+merge_request: 43874
+author:
+type: fixed

ee/app/assets/javascripts/epic/components/epic_sidebar.vue

@@ -149,6 +149,7 @@ export default {
       <sidebar-todo
         v-show="sidebarCollapsed && isUserSignedIn"
         :sidebar-collapsed="sidebarCollapsed"
+        data-testid="todo"
       />
       <sidebar-date-picker
         v-show="!sidebarCollapsed"
@@ -167,6 +168,7 @@ export default {
         :date-from-milestones="startDateTimeFromMilestones"
         :selected-date="startDateTime"
         :is-date-invalid="isDateInvalid"
+        data-testid="start-date"
         block-class="start-date"
         @toggleCollapse="toggleSidebar({ sidebarCollapsed })"
         @toggleDateType="changeStartDateType"
@@ -188,6 +190,7 @@ export default {
         :date-from-milestones="dueDateTimeFromMilestones"
         :selected-date="dueDateTime"
         :is-date-invalid="isDateInvalid"
+        data-testid="due-date"
         block-class="due-date"
         @toggleDateType="changeDueDateType"
         @saveDate="saveDueDate"
@@ -199,9 +202,13 @@ export default {
         :max-date="dueDateForCollapsedSidebar"
         @toggleCollapse="toggleSidebar({ sidebarCollapsed })"
       />
-      <sidebar-labels :can-update="canUpdate" :sidebar-collapsed="sidebarCollapsed" />
+      <sidebar-labels
+        :can-update="canUpdate"
+        :sidebar-collapsed="sidebarCollapsed"
+        data-testid="labels-select"
+      />
       <div v-if="allowSubEpics" class="block ancestors">
-        <ancestors-tree :ancestors="ancestors" :is-fetching="false" />
+        <ancestors-tree :ancestors="ancestors" :is-fetching="false" data-testid="ancestors" />
       </div>
 
       <confidential-issue-sidebar
@@ -216,7 +223,7 @@ export default {
           @toggleSidebar="toggleSidebar({ sidebarCollapsed })"
         />
       </div>
-      <sidebar-subscription :sidebar-collapsed="sidebarCollapsed" />
+      <sidebar-subscription :sidebar-collapsed="sidebarCollapsed" data-testid="subscribe" />
     </div>
   </aside>
 </template>

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

@@ -22,6 +22,7 @@ describe('SidebarLabelsComponent', () => {
       propsData: { canUpdate: false, sidebarCollapsed: false },
       store,
       stubs: {
+        LabelsSelectVue: true,
         GlLabel: true,
       },
     });

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, GlSearchBoxByType, GlLink } from '@gitlab/ui';
+import {
+  GlIntersectionObserver,
+  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 SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
 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';
 
@@ -88,20 +93,25 @@ describe('DropdownContentsLabelsView', () => {
       });
     });
 
-    describe('showListContainer', () => {
+    describe('showNoMatchingResultsMessage', () => {
       it.each`
-        variant          | loading  | showList
-        ${'sidebar'}     | ${false} | ${true}
-        ${'sidebar'}     | ${true}  | ${false}
-        ${'not-sidebar'} | ${true}  | ${true}
-        ${'not-sidebar'} | ${false} | ${true}
+        searchKey | labels        | labelsDescription | returnValue
+        ${''}     | ${[]}         | ${'empty'}        | ${false}
+        ${'bug'}  | ${[]}         | ${'empty'}        | ${true}
+        ${''}     | ${mockLabels} | ${'not empty'}    | ${false}
+        ${'bug'}  | ${mockLabels} | ${'not empty'}    | ${false}
       `(
-        'returns $showList if `state.variant` is "$variant" and `labelsFetchInProgress` is $loading',
-        ({ variant, loading, showList }) => {
-          createComponent({ ...mockConfig, variant });
-          wrapper.vm.$store.state.labelsFetchInProgress = loading;
+        'returns $returnValue when searchKey is "$searchKey" and visibleLabels is $labelsDescription',
+        async ({ searchKey, labels, returnValue }) => {
+          wrapper.setData({
+            searchKey,
+          });
+
+          wrapper.vm.$store.dispatch('receiveLabelsSuccess', labels);
+
+          await wrapper.vm.$nextTick();
 
-          expect(wrapper.vm.showListContainer).toBe(showList);
+          expect(wrapper.vm.showNoMatchingResultsMessage).toBe(returnValue);
         },
       );
     });
@@ -118,6 +128,28 @@ describe('DropdownContentsLabelsView', () => {
       });
     });
 
+    describe('handleComponentDisappear', () => {
+      it('calls action `receiveLabelsSuccess` with empty array', () => {
+        jest.spyOn(wrapper.vm, 'receiveLabelsSuccess');
+
+        wrapper.vm.handleComponentDisappear();
+
+        expect(wrapper.vm.receiveLabelsSuccess).toHaveBeenCalledWith([]);
+      });
+    });
+
+    describe('handleCreateLabelClick', () => {
+      it('calls actions `receiveLabelsSuccess` with empty array and `toggleDropdownContentsCreateView`', () => {
+        jest.spyOn(wrapper.vm, 'receiveLabelsSuccess');
+        jest.spyOn(wrapper.vm, 'toggleDropdownContentsCreateView');
+
+        wrapper.vm.handleCreateLabelClick();
+
+        expect(wrapper.vm.receiveLabelsSuccess).toHaveBeenCalledWith([]);
+        expect(wrapper.vm.toggleDropdownContentsCreateView).toHaveBeenCalled();
+      });
+    });
+
     describe('handleKeyDown', () => {
       it('decreases `currentHighlightItem` value by 1 when Up arrow key is pressed', () => {
         wrapper.setData({
@@ -226,8 +258,8 @@ describe('DropdownContentsLabelsView', () => {
   });
 
   describe('template', () => {
-    it('renders component container element with class `labels-select-contents-list`', () => {
-      expect(wrapper.attributes('class')).toContain('labels-select-contents-list');
+    it('renders gl-intersection-observer as component root', () => {
+      expect(wrapper.find(GlIntersectionObserver).exists()).toBe(true);
     });
 
     it('renders gl-loading-icon component when `labelsFetchInProgress` prop is true', () => {
@@ -272,15 +304,11 @@ describe('DropdownContentsLabelsView', () => {
       expect(searchInputEl.attributes('autofocus')).toBe('true');
     });
 
-    it('renders smart-virtual-list element', () => {
-      expect(wrapper.find(SmartVirtualList).exists()).toBe(true);
-    });
-
     it('renders label elements for all labels', () => {
       expect(wrapper.findAll(LabelItem)).toHaveLength(mockLabels.length);
     });
 
-    it('renders label element with "is-focused" when value of `currentHighlightItem` is more than -1', () => {
+    it('renders label element with `highlight` set to true when value of `currentHighlightItem` is more than -1', () => {
       wrapper.setData({
         currentHighlightItem: 0,
       });
@@ -288,7 +316,7 @@ describe('DropdownContentsLabelsView', () => {
       return wrapper.vm.$nextTick(() => {
         const labelItemEl = findDropdownContent().find(LabelItem);
 
-        expect(labelItemEl.props('highlight')).toBe(true);
+        expect(labelItemEl.attributes('highlight')).toBe('true');
       });
     });
 
@@ -310,9 +338,12 @@ describe('DropdownContentsLabelsView', () => {
 
       return wrapper.vm.$nextTick(() => {
         const dropdownContent = findDropdownContent();
+        const loadingIcon = findLoadingIcon();
 
         expect(dropdownContent.exists()).toBe(true);
-        expect(dropdownContent.isVisible()).toBe(false);
+        expect(dropdownContent.isVisible()).toBe(true);
+        expect(loadingIcon.exists()).toBe(true);
+        expect(loadingIcon.isVisible()).toBe(true);
       });
     });
 

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

@@ -6,11 +6,15 @@ import { mockRegularLabel } from './mock_data';
 
 const mockLabel = { ...mockRegularLabel, set: true };
 
-const createComponent = ({ label = mockLabel, highlight = true } = {}) =>
+const createComponent = ({
+  label = mockLabel,
+  isLabelSet = mockLabel.set,
+  highlight = true,
+} = {}) =>
   shallowMount(LabelItem, {
     propsData: {
       label,
-      isLabelSet: label.set,
+      isLabelSet,
       highlight,
     },
   });
@@ -26,94 +30,44 @@ describe('LabelItem', () => {
     wrapper.destroy();
   });
 
-  describe('computed', () => {
-    describe('labelBoxStyle', () => {
-      it('returns an object containing `backgroundColor` based on `label` prop', () => {
-        expect(wrapper.vm.labelBoxStyle).toEqual(
-          expect.objectContaining({
-            backgroundColor: mockLabel.color,
-          }),
-        );
-      });
-    });
-  });
-
-  describe('watchers', () => {
-    describe('isLabelSet', () => {
-      it('sets value of `isLabelSet` to `isSet` data prop', () => {
-        expect(wrapper.vm.isSet).toBe(true);
-
-        wrapper.setProps({
-          isLabelSet: false,
-        });
-
-        return wrapper.vm.$nextTick(() => {
-          expect(wrapper.vm.isSet).toBe(false);
-        });
-      });
-    });
-  });
-
-  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([mockLabel]);
-      });
-    });
-  });
-
   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({
+    it('renders component root with class `is-focused` when `highlight` prop is true', () => {
+      const wrapperTemp = createComponent({
         highlight: true,
       });
 
-      return wrapper.vm.$nextTick(() => {
-        expect(wrapper.find(GlLink).classes()).toContain('is-focused');
-      });
+      expect(wrapperTemp.classes()).toContain('is-focused');
+
+      wrapperTemp.destroy();
     });
 
-    it('renders visible gl-icon component when `isSet` prop is true', () => {
-      wrapper.setData({
-        isSet: true,
+    it('renders visible gl-icon component when `isLabelSet` prop is true', () => {
+      const wrapperTemp = createComponent({
+        isLabelSet: true,
       });
 
-      return wrapper.vm.$nextTick(() => {
-        const iconEl = wrapper.find(GlIcon);
+      const iconEl = wrapperTemp.find(GlIcon);
 
       expect(iconEl.isVisible()).toBe(true);
       expect(iconEl.props('name')).toBe('mobile-issue-close');
-      });
+
+      wrapperTemp.destroy();
     });
 
-    it('renders visible span element as placeholder instead of gl-icon when `isSet` prop is false', () => {
-      wrapper.setData({
-        isSet: false,
+    it('renders visible span element as placeholder instead of gl-icon when `isLabelSet` prop is false', () => {
+      const wrapperTemp = createComponent({
+        isLabelSet: false,
       });
 
-      return wrapper.vm.$nextTick(() => {
-        const placeholderEl = wrapper.find('[data-testid="no-icon"]');
+      const placeholderEl = wrapperTemp.find('[data-testid="no-icon"]');
 
       expect(placeholderEl.isVisible()).toBe(true);
-      });
+
+      wrapperTemp.destroy();
     });
 
     it('renders label color element', () => {