Merge branch '299238-fix-minor-ux-issues-with-group-by-within-iteration-report' into 'master'

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

See merge request gitlab-org/gitlab!59522

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

 <script>
-import { mapState } from 'vuex';
+import { mapGetters, mapState } from 'vuex';
 
 import DropdownContentsCreateView from './dropdown_contents_create_view.vue';
 import DropdownContentsLabelsView from './dropdown_contents_labels_view.vue';
@@ -18,6 +18,7 @@ export default {
   },
   computed: {
     ...mapState(['showDropdownContentsCreateView']),
+    ...mapGetters(['isDropdownVariantSidebar']),
     dropdownContentsView() {
       if (this.showDropdownContentsCreateView) {
         return 'dropdown-contents-create-view';
@@ -25,11 +26,8 @@ export default {
       return 'dropdown-contents-labels-view';
     },
     directionStyle() {
-      if (this.renderOnTop) {
-        return { bottom: '100%' };
-      }
-
-      return {};
+      const bottom = this.isDropdownVariantSidebar ? '3rem' : '2rem';
+      return this.renderOnTop ? { bottom } : {};
     },
   },
 };
@@ -37,7 +35,7 @@ export default {
 
 <template>
   <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"
     :style="directionStyle"
   >

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

@@ -83,12 +83,13 @@ export default {
       const highlightedLabel = this.$refs.labelsListContainer.querySelector('.is-focused');
 
       if (highlightedLabel) {
-        const rect = highlightedLabel.getBoundingClientRect();
-        if (rect.bottom > this.$refs.labelsListContainer.clientHeight) {
-          highlightedLabel.scrollIntoView(false);
-        }
-        if (rect.top < 0) {
-          highlightedLabel.scrollIntoView();
+        const container = this.$refs.labelsListContainer.getBoundingClientRect();
+        const label = highlightedLabel.getBoundingClientRect();
+
+        if (label.bottom > container.bottom) {
+          this.$refs.labelsListContainer.scrollTop += label.bottom - container.bottom;
+        } else if (label.top < container.top) {
+          this.$refs.labelsListContainer.scrollTop -= container.top - label.top;
         }
       }
     },

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

@@ -22,7 +22,7 @@ export default {
     const { label, highlight, isLabelSet } = props;
 
     const labelColorBox = h('span', {
-      class: 'dropdown-label-box',
+      class: 'dropdown-label-box gl-flex-shrink-0 gl-top-0 gl-mr-3',
       style: {
         backgroundColor: label.color,
       },
@@ -33,7 +33,7 @@ export default {
 
     const checkedIcon = h(GlIcon, {
       class: {
-        'mr-2 align-self-center': true,
+        'gl-mr-3 gl-flex-shrink-0': true,
         hidden: !isLabelSet,
       },
       props: {
@@ -43,7 +43,7 @@ export default {
 
     const noIcon = h('span', {
       class: {
-        'mr-3 pr-2': true,
+        'gl-mr-5 gl-pr-3': true,
         hidden: isLabelSet,
       },
       attrs: {
@@ -56,7 +56,7 @@ export default {
     const labelLink = h(
       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: {
           click: () => {
             listeners.clickLabel(label);
@@ -70,8 +70,8 @@ export default {
       'li',
       {
         class: {
-          'd-block': true,
-          'text-left': true,
+          'gl-display-block': true,
+          'gl-text-left': true,
           'is-focused': highlight,
         },
       },

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

@@ -268,7 +268,7 @@ export default {
       this.$emit('toggleCollapse');
     },
     setContentIsOnViewport(showDropdownContents) {
-      if (!this.isDropdownVariantEmbedded || !showDropdownContents) {
+      if (!showDropdownContents) {
         this.contentIsOnViewport = true;
 
         return;
@@ -276,8 +276,7 @@ export default {
 
       this.$nextTick(() => {
         if (this.$refs.dropdownContents) {
-          const offset = { top: 100 };
-          this.contentIsOnViewport = isInViewport(this.$refs.dropdownContents.$el, offset);
+          this.contentIsOnViewport = isInViewport(this.$refs.dropdownContents.$el);
         }
       });
     },
@@ -313,6 +312,7 @@ export default {
       <dropdown-contents
         v-show="dropdownButtonVisible && showDropdownContents"
         ref="dropdownContents"
+        :render-on-top="!contentIsOnViewport"
       />
     </template>
     <template v-if="isDropdownVariantStandalone || isDropdownVariantEmbedded">

ee/app/assets/javascripts/iterations/components/iteration_report_issues.vue

@@ -6,8 +6,8 @@ import {
   GlButton,
   GlLabel,
   GlLink,
-  GlLoadingIcon,
   GlPagination,
+  GlSkeletonLoader,
   GlTable,
   GlTooltipDirective,
 } from '@gitlab/ui';
@@ -18,12 +18,15 @@ import { Namespace } from '../constants';
 import iterationIssuesQuery from '../queries/iteration_issues.query.graphql';
 import iterationIssuesWithLabelFilterQuery from '../queries/iteration_issues_with_label_filter.query.graphql';
 
+const skeletonLoaderLimit = 5;
+
 const states = {
   opened: 'opened',
   closed: 'closed',
 };
 
 export default {
+  skeletonLoaderLimit,
   fields: [
     {
       key: 'title',
@@ -50,8 +53,8 @@ export default {
     GlButton,
     GlLabel,
     GlLink,
-    GlLoadingIcon,
     GlPagination,
+    GlSkeletonLoader,
     GlTable,
   },
   directives: {
@@ -252,7 +255,11 @@ export default {
       </gl-badge>
     </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
       v-show="shouldShowTable"
       :items="issues.list"

ee/app/assets/javascripts/iterations/components/iteration_report_tabs.vue

@@ -135,8 +135,8 @@ export default {
   <gl-tabs>
     <gl-tab title="Issues">
       <template #title>
-        <span>{{ __('Issues') }}</span
-        ><gl-badge class="gl-ml-2" variant="neutral">{{ issueCount }}</gl-badge>
+        {{ __('Issues') }}
+        <gl-badge class="gl-ml-2" size="sm" variant="muted">{{ issueCount }}</gl-badge>
       </template>
 
       <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 {
 
         <div
           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
             :allow-label-create="false"
             :allow-label-edit="true"

ee/app/assets/stylesheets/pages/issuable_form.scss

@@ -32,7 +32,6 @@
     width: 300px !important;
     min-height: none;
     max-height: none;
-    margin-bottom: $gl-spacing-scale-6 !important;
 
     a:not(.btn) {
       @include gl-reset-color;

ee/changelogs/unreleased/299238-fix-minor-ux-issues-with-group-by-within-iteration-report.yml

+---
+title: Fix minor UX issues with "group by" within iteration report
+merge_request: 59522
+author:
+type: fixed

ee/spec/frontend/iterations/components/iteration_report_issues_spec.js

@@ -4,8 +4,8 @@ import {
   GlBadge,
   GlButton,
   GlLabel,
-  GlLoadingIcon,
   GlPagination,
+  GlSkeletonLoader,
   GlTable,
 } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
@@ -31,7 +31,7 @@ describe('Iterations report issues', () => {
   const findGlBadge = () => wrapper.findComponent(GlBadge);
   const findGlButton = () => wrapper.findComponent(GlButton);
   const findGlLabel = () => wrapper.findComponent(GlLabel);
-  const findGlLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
+  const findGlSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoader);
   const findGlPagination = () => wrapper.findComponent(GlPagination);
   const findGlTable = () => wrapper.findComponent(GlTable);
 
@@ -64,14 +64,14 @@ describe('Iterations report issues', () => {
       loading: true,
     });
 
-    expect(findGlLoadingIcon().exists()).toBe(true);
+    expect(findGlSkeletonLoader().exists()).toBe(true);
     expect(findGlTable().isVisible()).toBe(false);
   });
 
   it('shows iterations list when not loading', () => {
     mountComponent({ loading: false, mountFunction: mount });
 
-    expect(findGlLoadingIcon().isVisible()).toBe(false);
+    expect(findGlSkeletonLoader().isVisible()).toBe(false);
     expect(findGlTable().exists()).toBe(true);
     expect(wrapper.text()).toContain('No issues found');
   });

spec/features/issues/resource_label_events_spec.rb

@@ -40,7 +40,7 @@ RSpec.describe 'List issue resource label events', :js do
 
         labels.each { |label| click_link label }
 
-        click_on 'Edit'
+        send_keys(:escape)
         wait_for_requests
       end
     end

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

 import { shallowMount, createLocalVue } from '@vue/test-utils';
 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 labelsSelectModule from '~/vue_shared/components/sidebar/labels_select_vue/store';
 
 import { mockConfig } from './mock_data';
@@ -50,13 +50,20 @@ describe('DropdownContent', () => {
   describe('template', () => {
     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('style')).toBe(undefined);
+      expect(wrapper.attributes('style')).toBeUndefined();
     });
 
-    it('renders component container element with styles when `renderOnTop` is true', () => {
-      wrapper = createComponent(mockConfig, { renderOnTop: true });
+    describe('when `renderOnTop` is 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);
+      });
     });
   });
 });

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

@@ -3,6 +3,7 @@ import Vuex from 'vuex';
 
 import { isInViewport } from '~/lib/utils/common_utils';
 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 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';
@@ -190,40 +191,33 @@ describe('LabelsSelectRoot', () => {
     });
 
     describe('sets content direction based on viewport', () => {
-      it('does not set direction when `state.variant` is not "embedded"', async () => {
-        createComponent();
-
-        wrapper.vm.$store.dispatch('toggleDropdownContents');
-        wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state);
-        await wrapper.vm.$nextTick;
-
-        expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(false);
-      });
-
-      describe('when `state.variant` is "embedded"', () => {
-        beforeEach(() => {
-          createComponent({ ...mockConfig, variant: 'embedded' });
-          wrapper.vm.$store.dispatch('toggleDropdownContents');
-        });
+      describe.each(Object.values(DropdownVariant))(
+        'when labels variant is "%s"',
+        ({ variant }) => {
+          beforeEach(() => {
+            createComponent({ ...mockConfig, variant });
+            wrapper.vm.$store.dispatch('toggleDropdownContents');
+          });
 
-        it('set direction when out of viewport', () => {
-          isInViewport.mockImplementation(() => false);
-          wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state);
+          it('set direction when out of viewport', () => {
+            isInViewport.mockImplementation(() => false);
+            wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state);
 
-          return wrapper.vm.$nextTick().then(() => {
-            expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(true);
+            return wrapper.vm.$nextTick().then(() => {
+              expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(true);
+            });
           });
-        });
 
-        it('does not set direction when inside of viewport', () => {
-          isInViewport.mockImplementation(() => true);
-          wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state);
+          it('does not set direction when inside of viewport', () => {
+            isInViewport.mockImplementation(() => true);
+            wrapper.vm.setContentIsOnViewport(wrapper.vm.$store.state);
 
-          return wrapper.vm.$nextTick().then(() => {
-            expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(false);
+            return wrapper.vm.$nextTick().then(() => {
+              expect(wrapper.find(DropdownContents).props('renderOnTop')).toBe(false);
+            });
           });
-        });
-      });
+        },
+      );
     });
   });