Commit 12676b0e authored by Coung Ngo's avatar Coung Ngo Committed by Jacques Erasmus

Fix minor UX issues with "group by" within Iteration Report

parent 13b260da
<script> <script>
import { mapState } from 'vuex'; import { mapGetters, mapState } from 'vuex';
import DropdownContentsCreateView from './dropdown_contents_create_view.vue'; import DropdownContentsCreateView from './dropdown_contents_create_view.vue';
import DropdownContentsLabelsView from './dropdown_contents_labels_view.vue'; import DropdownContentsLabelsView from './dropdown_contents_labels_view.vue';
...@@ -18,6 +18,7 @@ export default { ...@@ -18,6 +18,7 @@ export default {
}, },
computed: { computed: {
...mapState(['showDropdownContentsCreateView']), ...mapState(['showDropdownContentsCreateView']),
...mapGetters(['isDropdownVariantSidebar']),
dropdownContentsView() { dropdownContentsView() {
if (this.showDropdownContentsCreateView) { if (this.showDropdownContentsCreateView) {
return 'dropdown-contents-create-view'; return 'dropdown-contents-create-view';
...@@ -25,11 +26,8 @@ export default { ...@@ -25,11 +26,8 @@ export default {
return 'dropdown-contents-labels-view'; return 'dropdown-contents-labels-view';
}, },
directionStyle() { directionStyle() {
if (this.renderOnTop) { const bottom = this.isDropdownVariantSidebar ? '3rem' : '2rem';
return { bottom: '100%' }; return this.renderOnTop ? { bottom } : {};
}
return {};
}, },
}, },
}; };
...@@ -37,7 +35,7 @@ export default { ...@@ -37,7 +35,7 @@ export default {
<template> <template>
<div <div
class="labels-select-dropdown-contents w-100 mt-1 mb-3 py-2 rounded-top rounded-bottom position-absolute" class="labels-select-dropdown-contents gl-w-full gl-my-2 gl-py-3 gl-rounded-base gl-absolute"
data-qa-selector="labels_dropdown_content" data-qa-selector="labels_dropdown_content"
:style="directionStyle" :style="directionStyle"
> >
......
...@@ -83,12 +83,13 @@ export default { ...@@ -83,12 +83,13 @@ export default {
const highlightedLabel = this.$refs.labelsListContainer.querySelector('.is-focused'); const highlightedLabel = this.$refs.labelsListContainer.querySelector('.is-focused');
if (highlightedLabel) { if (highlightedLabel) {
const rect = highlightedLabel.getBoundingClientRect(); const container = this.$refs.labelsListContainer.getBoundingClientRect();
if (rect.bottom > this.$refs.labelsListContainer.clientHeight) { const label = highlightedLabel.getBoundingClientRect();
highlightedLabel.scrollIntoView(false);
} if (label.bottom > container.bottom) {
if (rect.top < 0) { this.$refs.labelsListContainer.scrollTop += label.bottom - container.bottom;
highlightedLabel.scrollIntoView(); } else if (label.top < container.top) {
this.$refs.labelsListContainer.scrollTop -= container.top - label.top;
} }
} }
}, },
......
...@@ -22,7 +22,7 @@ export default { ...@@ -22,7 +22,7 @@ export default {
const { label, highlight, isLabelSet } = props; const { label, highlight, isLabelSet } = props;
const labelColorBox = h('span', { const labelColorBox = h('span', {
class: 'dropdown-label-box', class: 'dropdown-label-box gl-flex-shrink-0 gl-top-0 gl-mr-3',
style: { style: {
backgroundColor: label.color, backgroundColor: label.color,
}, },
...@@ -33,7 +33,7 @@ export default { ...@@ -33,7 +33,7 @@ export default {
const checkedIcon = h(GlIcon, { const checkedIcon = h(GlIcon, {
class: { class: {
'mr-2 align-self-center': true, 'gl-mr-3 gl-flex-shrink-0': true,
hidden: !isLabelSet, hidden: !isLabelSet,
}, },
props: { props: {
...@@ -43,7 +43,7 @@ export default { ...@@ -43,7 +43,7 @@ export default {
const noIcon = h('span', { const noIcon = h('span', {
class: { class: {
'mr-3 pr-2': true, 'gl-mr-5 gl-pr-3': true,
hidden: isLabelSet, hidden: isLabelSet,
}, },
attrs: { attrs: {
...@@ -56,7 +56,7 @@ export default { ...@@ -56,7 +56,7 @@ export default {
const labelLink = h( const labelLink = h(
GlLink, GlLink,
{ {
class: 'd-flex align-items-baseline text-break-word label-item', class: 'gl-display-flex gl-align-items-center label-item gl-text-black-normal',
on: { on: {
click: () => { click: () => {
listeners.clickLabel(label); listeners.clickLabel(label);
...@@ -70,8 +70,8 @@ export default { ...@@ -70,8 +70,8 @@ export default {
'li', 'li',
{ {
class: { class: {
'd-block': true, 'gl-display-block': true,
'text-left': true, 'gl-text-left': true,
'is-focused': highlight, 'is-focused': highlight,
}, },
}, },
......
...@@ -268,7 +268,7 @@ export default { ...@@ -268,7 +268,7 @@ export default {
this.$emit('toggleCollapse'); this.$emit('toggleCollapse');
}, },
setContentIsOnViewport(showDropdownContents) { setContentIsOnViewport(showDropdownContents) {
if (!this.isDropdownVariantEmbedded || !showDropdownContents) { if (!showDropdownContents) {
this.contentIsOnViewport = true; this.contentIsOnViewport = true;
return; return;
...@@ -276,8 +276,7 @@ export default { ...@@ -276,8 +276,7 @@ export default {
this.$nextTick(() => { this.$nextTick(() => {
if (this.$refs.dropdownContents) { if (this.$refs.dropdownContents) {
const offset = { top: 100 }; this.contentIsOnViewport = isInViewport(this.$refs.dropdownContents.$el);
this.contentIsOnViewport = isInViewport(this.$refs.dropdownContents.$el, offset);
} }
}); });
}, },
...@@ -313,6 +312,7 @@ export default { ...@@ -313,6 +312,7 @@ export default {
<dropdown-contents <dropdown-contents
v-show="dropdownButtonVisible && showDropdownContents" v-show="dropdownButtonVisible && showDropdownContents"
ref="dropdownContents" ref="dropdownContents"
:render-on-top="!contentIsOnViewport"
/> />
</template> </template>
<template v-if="isDropdownVariantStandalone || isDropdownVariantEmbedded"> <template v-if="isDropdownVariantStandalone || isDropdownVariantEmbedded">
......
...@@ -6,8 +6,8 @@ import { ...@@ -6,8 +6,8 @@ import {
GlButton, GlButton,
GlLabel, GlLabel,
GlLink, GlLink,
GlLoadingIcon,
GlPagination, GlPagination,
GlSkeletonLoader,
GlTable, GlTable,
GlTooltipDirective, GlTooltipDirective,
} from '@gitlab/ui'; } from '@gitlab/ui';
...@@ -18,12 +18,15 @@ import { Namespace } from '../constants'; ...@@ -18,12 +18,15 @@ import { Namespace } from '../constants';
import iterationIssuesQuery from '../queries/iteration_issues.query.graphql'; import iterationIssuesQuery from '../queries/iteration_issues.query.graphql';
import iterationIssuesWithLabelFilterQuery from '../queries/iteration_issues_with_label_filter.query.graphql'; import iterationIssuesWithLabelFilterQuery from '../queries/iteration_issues_with_label_filter.query.graphql';
const skeletonLoaderLimit = 5;
const states = { const states = {
opened: 'opened', opened: 'opened',
closed: 'closed', closed: 'closed',
}; };
export default { export default {
skeletonLoaderLimit,
fields: [ fields: [
{ {
key: 'title', key: 'title',
...@@ -50,8 +53,8 @@ export default { ...@@ -50,8 +53,8 @@ export default {
GlButton, GlButton,
GlLabel, GlLabel,
GlLink, GlLink,
GlLoadingIcon,
GlPagination, GlPagination,
GlSkeletonLoader,
GlTable, GlTable,
}, },
directives: { directives: {
...@@ -252,7 +255,11 @@ export default { ...@@ -252,7 +255,11 @@ export default {
</gl-badge> </gl-badge>
</div> </div>
<gl-loading-icon v-show="$apollo.queries.issues.loading" class="gl-my-9" size="md" /> <div v-show="$apollo.queries.issues.loading">
<div v-for="n in $options.skeletonLoaderLimit" :key="n" class="gl-mx-5 gl-my-6">
<gl-skeleton-loader />
</div>
</div>
<gl-table <gl-table
v-show="shouldShowTable" v-show="shouldShowTable"
:items="issues.list" :items="issues.list"
......
...@@ -135,8 +135,8 @@ export default { ...@@ -135,8 +135,8 @@ export default {
<gl-tabs> <gl-tabs>
<gl-tab title="Issues"> <gl-tab title="Issues">
<template #title> <template #title>
<span>{{ __('Issues') }}</span {{ __('Issues') }}
><gl-badge class="gl-ml-2" variant="neutral">{{ issueCount }}</gl-badge> <gl-badge class="gl-ml-2" size="sm" variant="muted">{{ issueCount }}</gl-badge>
</template> </template>
<div class="card gl-bg-gray-10 gl-display-flex gl-flex-direction-row gl-flex-wrap gl-px-4"> <div class="card gl-bg-gray-10 gl-display-flex gl-flex-direction-row gl-flex-wrap gl-px-4">
...@@ -153,9 +153,9 @@ export default { ...@@ -153,9 +153,9 @@ export default {
<div <div
v-if="shouldShowFilterByLabel" v-if="shouldShowFilterByLabel"
class="gl-display-flex gl-align-items-center gl-flex-basis-half gl-white-space-nowrap gl-my-3" class="gl-display-flex gl-align-items-center gl-flex-basis-half gl-my-3"
> >
<label class="gl-mb-0 gl-mr-2">{{ __('Filter by label') }}</label> <label class="gl-white-space-nowrap gl-mb-0 gl-mr-2">{{ __('Filter by label') }}</label>
<labels-select <labels-select
:allow-label-create="false" :allow-label-create="false"
:allow-label-edit="true" :allow-label-edit="true"
......
...@@ -32,7 +32,6 @@ ...@@ -32,7 +32,6 @@
width: 300px !important; width: 300px !important;
min-height: none; min-height: none;
max-height: none; max-height: none;
margin-bottom: $gl-spacing-scale-6 !important;
a:not(.btn) { a:not(.btn) {
@include gl-reset-color; @include gl-reset-color;
......
---
title: Fix minor UX issues with "group by" within iteration report
merge_request: 59522
author:
type: fixed
...@@ -4,8 +4,8 @@ import { ...@@ -4,8 +4,8 @@ import {
GlBadge, GlBadge,
GlButton, GlButton,
GlLabel, GlLabel,
GlLoadingIcon,
GlPagination, GlPagination,
GlSkeletonLoader,
GlTable, GlTable,
} from '@gitlab/ui'; } from '@gitlab/ui';
import { mount, shallowMount } from '@vue/test-utils'; import { mount, shallowMount } from '@vue/test-utils';
...@@ -31,7 +31,7 @@ describe('Iterations report issues', () => { ...@@ -31,7 +31,7 @@ describe('Iterations report issues', () => {
const findGlBadge = () => wrapper.findComponent(GlBadge); const findGlBadge = () => wrapper.findComponent(GlBadge);
const findGlButton = () => wrapper.findComponent(GlButton); const findGlButton = () => wrapper.findComponent(GlButton);
const findGlLabel = () => wrapper.findComponent(GlLabel); const findGlLabel = () => wrapper.findComponent(GlLabel);
const findGlLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findGlSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoader);
const findGlPagination = () => wrapper.findComponent(GlPagination); const findGlPagination = () => wrapper.findComponent(GlPagination);
const findGlTable = () => wrapper.findComponent(GlTable); const findGlTable = () => wrapper.findComponent(GlTable);
...@@ -64,14 +64,14 @@ describe('Iterations report issues', () => { ...@@ -64,14 +64,14 @@ describe('Iterations report issues', () => {
loading: true, loading: true,
}); });
expect(findGlLoadingIcon().exists()).toBe(true); expect(findGlSkeletonLoader().exists()).toBe(true);
expect(findGlTable().isVisible()).toBe(false); expect(findGlTable().isVisible()).toBe(false);
}); });
it('shows iterations list when not loading', () => { it('shows iterations list when not loading', () => {
mountComponent({ loading: false, mountFunction: mount }); mountComponent({ loading: false, mountFunction: mount });
expect(findGlLoadingIcon().isVisible()).toBe(false); expect(findGlSkeletonLoader().isVisible()).toBe(false);
expect(findGlTable().exists()).toBe(true); expect(findGlTable().exists()).toBe(true);
expect(wrapper.text()).toContain('No issues found'); expect(wrapper.text()).toContain('No issues found');
}); });
......
...@@ -40,7 +40,7 @@ RSpec.describe 'List issue resource label events', :js do ...@@ -40,7 +40,7 @@ RSpec.describe 'List issue resource label events', :js do
labels.each { |label| click_link label } labels.each { |label| click_link label }
click_on 'Edit' send_keys(:escape)
wait_for_requests wait_for_requests
end end
end end
......
import { shallowMount, createLocalVue } from '@vue/test-utils'; import { shallowMount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex'; import Vuex from 'vuex';
import { DropdownVariant } from '~/vue_shared/components/sidebar/labels_select_vue/constants';
import DropdownContents from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_contents.vue'; import DropdownContents from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_contents.vue';
import labelsSelectModule from '~/vue_shared/components/sidebar/labels_select_vue/store'; import labelsSelectModule from '~/vue_shared/components/sidebar/labels_select_vue/store';
import { mockConfig } from './mock_data'; import { mockConfig } from './mock_data';
...@@ -50,13 +50,20 @@ describe('DropdownContent', () => { ...@@ -50,13 +50,20 @@ describe('DropdownContent', () => {
describe('template', () => { describe('template', () => {
it('renders component container element with class `labels-select-dropdown-contents` and no styles', () => { it('renders component container element with class `labels-select-dropdown-contents` and no styles', () => {
expect(wrapper.attributes('class')).toContain('labels-select-dropdown-contents'); expect(wrapper.attributes('class')).toContain('labels-select-dropdown-contents');
expect(wrapper.attributes('style')).toBe(undefined); expect(wrapper.attributes('style')).toBeUndefined();
}); });
it('renders component container element with styles when `renderOnTop` is true', () => { describe('when `renderOnTop` is true', () => {
wrapper = createComponent(mockConfig, { renderOnTop: true }); it.each`
variant | expected
${DropdownVariant.Sidebar} | ${'bottom: 3rem'}
${DropdownVariant.Standalone} | ${'bottom: 2rem'}
${DropdownVariant.Embedded} | ${'bottom: 2rem'}
`('renders upward for $variant variant', ({ variant, expected }) => {
wrapper = createComponent({ ...mockConfig, variant }, { renderOnTop: true });
expect(wrapper.attributes('style')).toContain('bottom: 100%'); expect(wrapper.attributes('style')).toContain(expected);
});
}); });
}); });
}); });
...@@ -3,6 +3,7 @@ import Vuex from 'vuex'; ...@@ -3,6 +3,7 @@ import Vuex from 'vuex';
import { isInViewport } from '~/lib/utils/common_utils'; import { isInViewport } from '~/lib/utils/common_utils';
import DropdownValueCollapsed from '~/vue_shared/components/sidebar/labels_select/dropdown_value_collapsed.vue'; import DropdownValueCollapsed from '~/vue_shared/components/sidebar/labels_select/dropdown_value_collapsed.vue';
import { DropdownVariant } from '~/vue_shared/components/sidebar/labels_select_vue/constants';
import DropdownButton from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_button.vue'; import DropdownButton from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_button.vue';
import DropdownContents from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_contents.vue'; import DropdownContents from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_contents.vue';
import DropdownTitle from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_title.vue'; import DropdownTitle from '~/vue_shared/components/sidebar/labels_select_vue/dropdown_title.vue';
...@@ -190,40 +191,33 @@ describe('LabelsSelectRoot', () => { ...@@ -190,40 +191,33 @@ describe('LabelsSelectRoot', () => {
}); });
describe('sets content direction based on viewport', () => { describe('sets content direction based on viewport', () => {
it('does not set direction when `state.variant` is not "embedded"', async () => { describe.each(Object.values(DropdownVariant))(
createComponent(); 'when labels variant is "%s"',
({ variant }) => {
wrapper.vm.$store.dispatch('toggleDropdownContents'); beforeEach(() => {
wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state); createComponent({ ...mockConfig, variant });
await wrapper.vm.$nextTick; wrapper.vm.$store.dispatch('toggleDropdownContents');
});
expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(false);
});
describe('when `state.variant` is "embedded"', () => {
beforeEach(() => {
createComponent({ ...mockConfig, variant: 'embedded' });
wrapper.vm.$store.dispatch('toggleDropdownContents');
});
it('set direction when out of viewport', () => { it('set direction when out of viewport', () => {
isInViewport.mockImplementation(() => false); isInViewport.mockImplementation(() => false);
wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state); wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state);
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(true); expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(true);
});
}); });
});
it('does not set direction when inside of viewport', () => { it('does not set direction when inside of viewport', () => {
isInViewport.mockImplementation(() => true); isInViewport.mockImplementation(() => true);
wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state); wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state);
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(false); expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(false);
});
}); });
}); },
}); );
}); });
}); });
......
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