Merge branch '208424-add-skeleton-loader-to-new-packages-list' into 'master'

Add skeleton loader to new packages list

Closes #208424

See merge request gitlab-org/gitlab!26176

ee/app/assets/javascripts/packages/list/components/packages_list.vue

@@ -34,6 +34,7 @@ import {
 import { TrackingActions } from '../../shared/constants';
 import { packageTypeToTrackCategory } from '../../shared/utils';
 import PackageTags from '../../shared/components/package_tags.vue';
+import PackagesListLoader from './packages_list_loader.vue';
 
 export default {
   components: {
@@ -47,6 +48,7 @@ export default {
     GlModal,
     GlIcon,
     PackageTags,
+    PackagesListLoader,
   },
   directives: { GlTooltip: GlTooltipDirective },
   mixins: [Tracking.mixin()],
@@ -63,6 +65,7 @@ export default {
       isGroupPage: state => state.config.isGroupPage,
       orderBy: state => state.sorting.orderBy,
       sort: state => state.sorting.sort,
+      isLoading: 'isLoading',
     }),
     ...mapGetters({ list: 'getList' }),
     currentPage: {
@@ -99,7 +102,7 @@ export default {
           key: LIST_KEY_PROJECT,
           label: LIST_LABEL_PROJECT,
           orderBy: LIST_KEY_PROJECT,
-          class: ['text-center'],
+          class: ['text-left'],
         },
         {
           key: LIST_KEY_VERSION,
@@ -183,11 +186,12 @@ export default {
 </script>
 
 <template>
-  <div class="d-flex flex-column align-items-end">
-    <slot v-if="isListEmpty" name="empty-state"></slot>
+  <div class="d-flex flex-column">
+    <slot v-if="isListEmpty && !isLoading" name="empty-state"></slot>
+
     <template v-else>
       <gl-sorting
-        class="my-3"
+        class="my-3 align-self-end"
         :text="sortText"
         :is-ascending="isSortAscending"
         @sortDirectionChange="onDirectionChange"
@@ -202,7 +206,18 @@ export default {
         </gl-sorting-item>
       </gl-sorting>
 
-      <gl-table :items="list" :fields="headerFields" :no-local-sorting="true" stacked="md">
+      <gl-table
+        :items="list"
+        :fields="headerFields"
+        :no-local-sorting="true"
+        :busy="isLoading"
+        stacked="md"
+        class="package-list-table"
+      >
+        <template #table-busy>
+          <packages-list-loader :is-group="isGroupPage" />
+        </template>
+
         <template #cell(name)="{value, item}">
           <div
             class="flex-truncate-parent d-flex align-items-center justify-content-end justify-content-md-start"

ee/app/assets/javascripts/packages/list/components/packages_list_app.vue

 <script>
 import { mapActions, mapState } from 'vuex';
-import { GlEmptyState, GlLoadingIcon } from '@gitlab/ui';
+import { GlEmptyState } from '@gitlab/ui';
 import { s__, sprintf } from '~/locale';
 import PackageList from './packages_list.vue';
 
 export default {
   components: {
     GlEmptyState,
-    GlLoadingIcon,
     PackageList,
   },
   computed: {
     ...mapState({
-      isLoading: 'isLoading',
       resourceId: state => state.config.resourceId,
       emptyListIllustration: state => state.config.emptyListIllustration,
       emptyListHelpUrl: state => state.config.emptyListHelpUrl,
+      totalItems: state => state.pagination.total,
     }),
     emptyListText() {
       return sprintf(
@@ -46,9 +45,7 @@ export default {
 </script>
 
 <template>
-  <gl-loading-icon v-if="isLoading" class="mt-2" />
   <package-list
-    v-else
     @page:changed="onPageChanged"
     @package:delete="onPackageDeleteRequest"
     @sort:changed="requestPackagesList"

ee/app/assets/javascripts/packages/list/components/packages_list_loader.vue

+<script>
+import { GlSkeletonLoader } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlSkeletonLoader,
+  },
+  props: {
+    isGroup: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    desktopShapes() {
+      return this.isGroup ? this.$options.shapes.groups : this.$options.shapes.projects;
+    },
+    desktopHeight() {
+      return this.isGroup ? 38 : 54;
+    },
+    mobileHeight() {
+      return this.isGroup ? 160 : 170;
+    },
+  },
+  shapes: {
+    groups: [
+      { type: 'rect', width: '100', height: '10', x: '0', y: '15' },
+      { type: 'rect', width: '100', height: '10', x: '195', y: '15' },
+      { type: 'rect', width: '60', height: '10', x: '475', y: '15' },
+      { type: 'rect', width: '60', height: '10', x: '675', y: '15' },
+      { type: 'rect', width: '100', height: '10', x: '900', y: '15' },
+    ],
+    projects: [
+      { type: 'rect', width: '220', height: '10', x: '0', y: '20' },
+      { type: 'rect', width: '60', height: '10', x: '305', y: '20' },
+      { type: 'rect', width: '60', height: '10', x: '535', y: '20' },
+      { type: 'rect', width: '100', height: '10', x: '760', y: '20' },
+      { type: 'rect', width: '30', height: '30', x: '970', y: '10', ref: 'button-loader' },
+    ],
+  },
+  rowsToRender: {
+    mobile: 5,
+    desktop: 20,
+  },
+};
+</script>
+
+<template>
+  <div>
+    <div class="d-xs-flex flex-column d-md-none">
+      <gl-skeleton-loader
+        v-for="index in $options.rowsToRender.mobile"
+        :key="index"
+        :width="500"
+        :height="mobileHeight"
+        preserve-aspect-ratio="xMinYMax meet"
+      >
+        <rect width="500" height="10" x="0" y="15" rx="4" />
+        <rect width="500" height="10" x="0" y="45" rx="4" />
+        <rect width="500" height="10" x="0" y="75" rx="4" />
+        <rect width="500" height="10" x="0" y="105" rx="4" />
+        <rect v-if="isGroup" width="500" height="10" x="0" y="135" rx="4" />
+        <rect v-else width="30" height="30" x="470" y="135" rx="4" />
+      </gl-skeleton-loader>
+    </div>
+
+    <div class="d-none d-md-flex flex-column">
+      <gl-skeleton-loader
+        v-for="index in $options.rowsToRender.desktop"
+        :key="index"
+        :width="1000"
+        :height="desktopHeight"
+        preserve-aspect-ratio="xMinYMax meet"
+      >
+        <component
+          :is="r.type"
+          v-for="(r, rIndex) in desktopShapes"
+          :key="rIndex"
+          rx="4"
+          v-bind="r"
+        />
+      </gl-skeleton-loader>
+    </div>
+  </div>
+</template>

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

@@ -2,3 +2,10 @@
   border: 0;
   border-left: 3px solid $white-dark;
 }
+
+.package-list-table[aria-busy='true'] {
+  td {
+    padding-bottom: 0;
+    padding-top: 0;
+  }
+}

ee/spec/frontend/packages/list/components/packages_list_app_spec.js

@@ -18,7 +18,6 @@ describe('packages_list_app', () => {
 
   const emptyListHelpUrl = 'helpUrl';
   const findListComponent = () => wrapper.find(PackageList);
-  const findLoadingComponent = () => wrapper.find(GlLoadingIcon);
 
   const mountComponent = () => {
     wrapper = shallowMount(PackageListApp, {
@@ -44,6 +43,7 @@ describe('packages_list_app', () => {
       },
     });
     store.dispatch = jest.fn();
+    mountComponent();
   });
 
   afterEach(() => {
@@ -55,48 +55,30 @@ describe('packages_list_app', () => {
     expect(wrapper.element).toMatchSnapshot();
   });
 
-  describe('when isLoading is true', () => {
-    beforeEach(() => {
-      store.state.isLoading = true;
-      mountComponent();
-    });
+  it('generate the correct empty list link', () => {
+    const emptyState = findListComponent();
+    const link = emptyState.find('a');
 
-    it('shows the loading component', () => {
-      const loader = findLoadingComponent();
-      expect(loader.exists()).toBe(true);
-    });
+    expect(link.html()).toMatchInlineSnapshot(
+      `"<a href=\\"${emptyListHelpUrl}\\" target=\\"_blank\\">publish and share your packages</a>"`,
+    );
   });
 
-  describe('when isLoading is false', () => {
-    beforeEach(() => {
-      mountComponent();
-    });
-
-    it('generate the correct empty list link', () => {
-      const emptyState = findListComponent();
-      const link = emptyState.find('a');
-
-      expect(link.html()).toMatchInlineSnapshot(
-        `"<a href=\\"${emptyListHelpUrl}\\" target=\\"_blank\\">publish and share your packages</a>"`,
-      );
-    });
-
-    it('call requestPackagesList on page:changed', () => {
-      const list = findListComponent();
-      list.vm.$emit('page:changed', 1);
-      expect(store.dispatch).toHaveBeenCalledWith('requestPackagesList', { page: 1 });
-    });
+  it('call requestPackagesList on page:changed', () => {
+    const list = findListComponent();
+    list.vm.$emit('page:changed', 1);
+    expect(store.dispatch).toHaveBeenCalledWith('requestPackagesList', { page: 1 });
+  });
 
-    it('call requestDeletePackage on package:delete', () => {
-      const list = findListComponent();
-      list.vm.$emit('package:delete', 'foo');
-      expect(store.dispatch).toHaveBeenCalledWith('requestDeletePackage', 'foo');
-    });
+  it('call requestDeletePackage on package:delete', () => {
+    const list = findListComponent();
+    list.vm.$emit('package:delete', 'foo');
+    expect(store.dispatch).toHaveBeenCalledWith('requestDeletePackage', 'foo');
+  });
 
-    it('calls requestPackagesList on sort:changed', () => {
-      const list = findListComponent();
-      list.vm.$emit('sort:changed');
-      expect(store.dispatch).toHaveBeenCalledWith('requestPackagesList');
-    });
+  it('calls requestPackagesList on sort:changed', () => {
+    const list = findListComponent();
+    list.vm.$emit('sort:changed');
+    expect(store.dispatch).toHaveBeenCalledWith('requestPackagesList');
   });
 });

ee/spec/frontend/packages/list/components/packages_list_loader_spec.js

+import { mount } from '@vue/test-utils';
+import PackagesListLoader from 'ee/packages/list/components/packages_list_loader.vue';
+
+describe('PackagesListLoader', () => {
+  let wrapper;
+
+  const createComponent = (props = {}) => {
+    wrapper = mount(PackagesListLoader, {
+      propsData: {
+        ...props,
+      },
+    });
+  };
+
+  const getShapes = () => wrapper.vm.desktopShapes;
+  const findSquareButton = () => wrapper.find({ ref: 'button-loader' });
+
+  beforeEach(createComponent);
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  describe('when used for projects', () => {
+    it('should return 5 rects with last one being a square', () => {
+      expect(getShapes()).toHaveLength(5);
+      expect(findSquareButton().exists()).toBe(true);
+    });
+  });
+
+  describe('when used for groups', () => {
+    beforeEach(() => {
+      createComponent({ isGroup: true });
+    });
+
+    it('should return 5 rects with no square', () => {
+      expect(getShapes()).toHaveLength(5);
+      expect(findSquareButton().exists()).toBe(false);
+    });
+  });
+});

ee/spec/frontend/packages/list/components/packages_list_spec.js

@@ -5,6 +5,7 @@ import Tracking from '~/tracking';
 import { mount, createLocalVue } from '@vue/test-utils';
 import PackagesList from 'ee/packages/list/components/packages_list.vue';
 import PackageTags from 'ee/packages/shared/components/package_tags.vue';
+import PackagesListLoader from 'ee/packages/list/components/packages_list_loader.vue';
 import * as SharedUtils from 'ee/packages/shared/utils';
 import { TrackingActions } from 'ee/packages/shared/constants';
 import stubChildren from 'helpers/stub_children';
@@ -16,12 +17,11 @@ localVue.use(Vuex);
 describe('packages_list', () => {
   let wrapper;
   let store;
-  let state;
-  let getListSpy;
 
   const GlSortingItem = { name: 'sorting-item-stub', template: '<div><slot></slot></div>' };
   const EmptySlotStub = { name: 'empty-slot-stub', template: '<div>bar</div>' };
 
+  const findPackagesListLoader = () => wrapper.find(PackagesListLoader);
   const findFirstActionColumn = () => wrapper.find({ ref: 'action-delete' });
   const findPackageListTable = () => wrapper.find(GlTable);
   const findPackageListSorting = () => wrapper.find(GlSorting);
@@ -32,31 +32,17 @@ describe('packages_list', () => {
   const findPackageTags = () => wrapper.findAll(PackageTags);
   const findEmptySlot = () => wrapper.find({ name: 'empty-slot-stub' });
 
-  const mountComponent = options => {
-    wrapper = mount(PackagesList, {
-      localVue,
-      store,
-      stubs: {
-        ...stubChildren(PackagesList),
-        GlTable,
-        GlSortingItem,
-      },
-      ...options,
-    });
-  };
-
-  beforeEach(() => {
-    getListSpy = jest.fn();
-    getListSpy.mockReturnValue(packageList);
-    state = {
-      packages: [...packageList],
+  const createStore = (isGroupPage, packages, isLoading) => {
+    const state = {
+      isLoading,
+      packages,
       pagination: {
         perPage: 1,
         total: 1,
         page: 1,
       },
       config: {
-        isGroupPage: false,
+        isGroupPage,
       },
       sorting: {
         orderBy: 'version',
@@ -66,22 +52,65 @@ describe('packages_list', () => {
     store = new Vuex.Store({
       state,
       getters: {
-        getList: getListSpy,
+        getList: () => packages,
       },
     });
     store.dispatch = jest.fn();
-  });
+  };
+
+  const mountComponent = ({
+    isGroupPage = false,
+    packages = packageList,
+    isLoading = false,
+    ...options
+  } = {}) => {
+    createStore(isGroupPage, packages, isLoading);
+
+    wrapper = mount(PackagesList, {
+      localVue,
+      store,
+      stubs: {
+        ...stubChildren(PackagesList),
+        GlTable,
+        GlSortingItem,
+      },
+      ...options,
+    });
+  };
 
   afterEach(() => {
     wrapper.destroy();
+    wrapper = null;
   });
 
-  describe('when is isGroupPage', () => {
+  describe('when is loading', () => {
+    beforeEach(() => {
+      mountComponent({
+        packages: [],
+        isLoading: true,
+      });
+    });
+
+    it('shows skeleton loader when loading', () => {
+      expect(findPackagesListLoader().exists()).toBe(true);
+    });
+  });
+
+  describe('when is not loading', () => {
     beforeEach(() => {
-      state.config.isGroupPage = true;
       mountComponent();
     });
 
+    it('does not show skeleton loader when not loading', () => {
+      expect(findPackagesListLoader().exists()).toBe(false);
+    });
+  });
+
+  describe('when is isGroupPage', () => {
+    beforeEach(() => {
+      mountComponent({ isGroupPage: true });
+    });
+
     it('has project field', () => {
       const projectColumn = findFirstProjectColumn();
       expect(projectColumn.exists()).toBe(true);
@@ -177,8 +206,8 @@ describe('packages_list', () => {
 
   describe('when the list is empty', () => {
     beforeEach(() => {
-      getListSpy.mockReturnValue([]);
       mountComponent({
+        packages: [],
         slots: {
           'empty-state': EmptySlotStub,
         },