Skip to content

G
gitlab-ce
Commit b060e23f authored by Kushal Pandya's avatar Kushal Pandya
Browse files
Options

Fix labels select dropdown search scroll 

Avoid using VirtualList for rendering labels list while
performing search to prevent janky scroll.
parent f9e98deb
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 344 additions and 354 deletions
app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/constants.js
View file @ b060e23f
......@@ -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
View file @ b060e23f
<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,84 +139,75 @@ export default {
</script>
<template>
<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!"
data-testid="dropdown-title"
>
<span class="flex-grow-1">{{ labelsListTitle }}</span>
<gl-button
:aria-label="__('Close')"
variant="link"
size="small"
class="dropdown-header-button gl-p-0!"
icon="close"
@click="toggleDropdownContents"
/>
</div>
<div class="dropdown-input" @click.stop="() => {}">
<gl-search-box-by-type
v-model="searchKey"
:autofocus="true"
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"
<gl-intersection-observer @appear="fetchLabels" @disappear="handleComponentDisappear">
<div class="labels-select-contents-list js-labels-list" @keydown="handleKeyDown">
<div
v-if="isDropdownVariantSidebar || isDropdownVariantEmbedded"
class="dropdown-title gl-display-flex gl-align-items-center gl-pt-0 gl-pb-3!"
data-testid="dropdown-title"
>
<li v-for="(label, index) in visibleLabels" :key="label.id" class="d-block text-left">
<span class="flex-grow-1">{{ labelsListTitle }}</span>
<gl-button
:aria-label="__('Close')"
variant="link"
size="small"
class="dropdown-header-button gl-p-0!"
icon="close"
@click="toggleDropdownContents"
/>
</div>
<div class="dropdown-input" @click.stop="() => {}">
<gl-search-box-by-type
v-model="searchKey"
:autofocus="true"
:disabled="labelsFetchInProgress"
data-qa-selector="dropdown_input_field"
/>
</div>
<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>
</div>
<div
v-if="isDropdownVariantSidebar || isDropdownVariantEmbedded"
class="dropdown-footer"
data-testid="dropdown-footer"
>
<ul class="list-unstyled">
<li v-if="allowLabelCreate">
<gl-link
class="gl-display-flex w-100 flex-row text-break-word label-item"
@click="toggleDropdownContentsCreateView"
>
{{ footerCreateLabelTitle }}
</gl-link>
</li>
<li>
<gl-link
:href="labelsManagePath"
class="gl-display-flex flex-row text-break-word label-item"
>
{{ footerManageLabelTitle }}
</gl-link>
</li>
</ul>
<li v-show="showNoMatchingResultsMessage" class="gl-p-3 gl-text-center">
{{ __('No matching results') }}
</li>
</ul>
</div>
<div
v-if="isDropdownVariantSidebar || isDropdownVariantEmbedded"
class="dropdown-footer"
data-testid="dropdown-footer"
>
<ul class="list-unstyled">
<li v-if="allowLabelCreate">
<gl-link
class="gl-display-flex w-100 flex-row text-break-word label-item"
@click="handleCreateLabelClick"
>
{{ footerCreateLabelTitle }}
</gl-link>
</li>
<li>
<gl-link
:href="labelsManagePath"
class="gl-display-flex flex-row text-break-word label-item"
>
{{ footerManageLabelTitle }}
</gl-link>
</li>
</ul>
</div>
</div>
</div>
</gl-intersection-observer>
</template>
app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/label_item.vue
View file @ b060e23f
<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,
};
},
computed: {
labelBoxStyle() {
return {
backgroundColor: this.label.color,
};
},
},
watch: {
/**
* This watcher assures that if user used
* `Enter` key to set/unset label, changes
* are reflected here too.
*/
isLabelSet(value) {
this.isSet = value;
},
},
methods: {
handleClick() {
this.isSet = !this.isSet;
this.$emit('clickLabel', this.label);
},
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',
},
});
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);
},
},
},
[noIcon, checkedIcon, labelColorBox, labelTitle],
);
return h(
'li',
{
class: {
'd-block': true,
'text-left': true,
'is-focused': highlight,
},
},
[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
View file @ b060e23f
......@@ -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
View file @ b060e23f
......@@ -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 0 → 100644
View file @ b060e23f
---
title: Fix Vue Labels Select dropdown keyboard scroll
merge_request: 43874
author:
type: fixed
ee/app/assets/javascripts/epic/components/epic_sidebar.vue
View file @ b060e23f
......@@ -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
View file @ b060e23f
......@@ -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
View file @ b060e23f
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,
});
expect(wrapper.vm.showListContainer).toBe(showList);
wrapper.vm.$store.dispatch('receiveLabelsSuccess', labels);
await wrapper.vm.$nextTick();
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
View file @ b060e23f
......@@ -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');
});
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);
});
expect(placeholderEl.isVisible()).toBe(true);
wrapperTemp.destroy();
});
it('renders label color element', () => {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7