Fix UI flash when sorting collection items (#16511)

* Optionaly keep prev list value in EntityListLoader

* Use prop-types in EntityListLoader

* Fix UI flash when sorting collection items

* Fix empty state displayed before items are fetched

* Alias condition statement

* Unify shouldUpdatePrevList condition

* Fix react/display-name

* Rename keepPreviousList —> keepListWhileLoading

* Swap list and previous list internally

* Replace `_.isEqual` check with plain `!==`

frontend/src/metabase/collections/containers/CollectionContent.jsx

@@ -123,8 +123,13 @@ function CollectionContent({ collection, collectionId, isAdmin, isRoot }) {
   };
 
   return (
-    <Search.ListLoader query={pinnedQuery} wrapped>
-      {({ list: pinnedItems }) => {
+    <Search.ListLoader
+      query={pinnedQuery}
+      loadingAndErrorWrapper={false}
+      keepListWhileLoading
+      wrapped
+    >
+      {({ list: pinnedItems = [], loading: loadingPinnedItems }) => {
         const hasPinnedItems = pinnedItems.length > 0;
 
         return (
@@ -150,8 +155,17 @@ function CollectionContent({ collection, collectionId, isAdmin, isRoot }) {
                 onCopy={handleCopy}
               />
 
-              <Search.ListLoader query={unpinnedQuery} wrapped>
-                {({ list: unpinnedItems, metadata }) => {
+              <Search.ListLoader
+                query={unpinnedQuery}
+                loadingAndErrorWrapper={false}
+                keepListWhileLoading
+                wrapped
+              >
+                {({
+                  list: unpinnedItems = [],
+                  metadata = {},
+                  loading: loadingUnpinnedItems,
+                }) => {
                   const hasPagination = metadata.total > PAGE_SIZE;
 
                   const unselected = [...pinnedItems, ...unpinnedItems].filter(
@@ -163,7 +177,11 @@ function CollectionContent({ collection, collectionId, isAdmin, isRoot }) {
                     toggleAll(unselected);
                   };
 
-                  if (!hasPinnedItems && unpinnedItems.length === 0) {
+                  const loading = loadingPinnedItems || loadingUnpinnedItems;
+                  const isEmpty =
+                    !loading && !hasPinnedItems && unpinnedItems.length === 0;
+
+                  if (isEmpty) {
                     return (
                       <Box mt="120px">
                         <CollectionEmptyState />

frontend/src/metabase/entities/containers/EntityListLoader.jsx

-/* eslint-disable react/prop-types */
 import React from "react";
+import PropTypes from "prop-types";
 import { connect } from "react-redux";
 import _ from "underscore";
 import { createSelector } from "reselect";
@@ -9,27 +9,44 @@ import entityType from "./EntityType";
 import paginationState from "metabase/hoc/PaginationState";
 import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
 
-export type Props = {
-  entityType?: string,
-  entityQuery?: ?{ [key: string]: any },
-  reload?: boolean,
-  wrapped?: boolean,
-  debounced?: boolean,
-  loadingAndErrorWrapper: boolean,
-  selectorName?: string,
-  children: (props: RenderProps) => ?React.Element,
+const propTypes = {
+  entityType: PropTypes.string,
+  entityQuery: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
+  reload: PropTypes.bool,
+  wrapped: PropTypes.bool,
+  debounced: PropTypes.bool,
+  loadingAndErrorWrapper: PropTypes.bool,
+  keepListWhileLoading: PropTypes.bool,
+  selectorName: PropTypes.string,
+  children: PropTypes.func,
+
+  // via entityType HOC
+  entityDef: PropTypes.object,
+
+  // via react-redux connect
+  list: PropTypes.arrayOf(PropTypes.object),
+  metadata: PropTypes.object,
+  loading: PropTypes.bool,
+  loaded: PropTypes.bool,
+  fetched: PropTypes.bool,
+  error: PropTypes.any,
+  allLoading: PropTypes.bool,
+  allLoaded: PropTypes.bool,
+  allFetched: PropTypes.bool,
+  allError: PropTypes.bool,
+  dispatch: PropTypes.func,
 };
 
-export type RenderProps = {
-  list: ?(any[]),
-  fetched: boolean,
-  loading: boolean,
-  error: ?any,
-  reload: () => void,
+const defaultProps = {
+  loadingAndErrorWrapper: true,
+  keepListWhileLoading: false,
+  reload: false,
+  wrapped: false,
+  debounced: false,
 };
 
 // props that shouldn't be passed to children in order to properly stack
-const CONSUMED_PROPS: string[] = [
+const CONSUMED_PROPS = [
   "entityType",
   "entityQuery",
   // "reload", // Masked by `reload` function. Should we rename that?
@@ -96,19 +113,12 @@ const getMemoizedEntityQuery = createMemoizedSelector(
     allError: error || (allError == null ? null : allError),
   };
 })
-export default class EntityListLoader extends React.Component {
-  props: Props;
-
-  static defaultProps = {
-    loadingAndErrorWrapper: true,
-    reload: false,
-    wrapped: false,
-    debounced: false,
+class EntityListLoader extends React.Component {
+  state = {
+    previousList: [],
   };
 
-  _getWrappedList: ?(props: Props) => any;
-
-  constructor(props: Props) {
+  constructor(props) {
     super(props);
 
     this._getWrappedList = createSelector(
@@ -118,7 +128,7 @@ export default class EntityListLoader extends React.Component {
     );
   }
 
-  maybeDebounce(f: any, ...args: any) {
+  maybeDebounce(f, ...args) {
     if (this.props.debounced) {
       return _.debounce(f, ...args);
     } else {
@@ -129,7 +139,7 @@ export default class EntityListLoader extends React.Component {
   fetchList = this.maybeDebounce(
     async (
       { fetchList, entityQuery, pageSize, onChangeHasMorePages },
-      options?: any,
+      options,
     ) => {
       const result = await fetchList(entityQuery, options);
 
@@ -147,7 +157,7 @@ export default class EntityListLoader extends React.Component {
     this.fetchList(this.props, { reload: this.props.reload });
   }
 
-  UNSAFE_componentWillReceiveProps(nextProps: Props) {
+  UNSAFE_componentWillReceiveProps(nextProps) {
     if (!_.isEqual(nextProps.entityQuery, this.props.entityQuery)) {
       // entityQuery changed, reload
       this.fetchList(nextProps, { reload: nextProps.reload });
@@ -159,16 +169,44 @@ export default class EntityListLoader extends React.Component {
     }
   }
 
-  renderChildren = () => {
-    let { children, entityDef, wrapped, list, reload, ...props } = this.props; // eslint-disable-line no-unused-vars
+  componentDidUpdate(prevProps) {
+    const { keepListWhileLoading } = this.props;
+    const { previousList } = this.state;
+
+    const shouldUpdatePrevList =
+      keepListWhileLoading &&
+      Array.isArray(prevProps.list) &&
+      previousList !== prevProps.list;
 
-    if (wrapped) {
-      list = this._getWrappedList(this.props);
+    if (shouldUpdatePrevList) {
+      this.setState({ previousList: prevProps.list });
     }
+  }
+
+  renderChildren = () => {
+    const {
+      children,
+      entityDef,
+      wrapped,
+      list: currentList,
+      loading,
+      reload, // eslint-disable-line no-unused-vars
+      keepListWhileLoading,
+      ...props
+    } = this.props;
+    const { previousList } = this.state;
+
+    const finalList =
+      keepListWhileLoading && loading ? previousList : currentList;
+
+    const list = wrapped
+      ? this._getWrappedList({ ...this.props, list: finalList })
+      : finalList;
 
     return children({
       ..._.omit(props, ...CONSUMED_PROPS),
       list,
+      loading,
       // alias the entities name:
       [entityDef.nameMany]: list,
       reload: this.reload,
@@ -192,11 +230,14 @@ export default class EntityListLoader extends React.Component {
   };
 }
 
-export const entityListLoader = (ellProps: Props) =>
-  // eslint-disable-line react/display-name
-  (ComposedComponent: any) =>
-    // eslint-disable-next-line react/display-name
-    (props: Props) => (
+EntityListLoader.propTypes = propTypes;
+EntityListLoader.defaultProps = defaultProps;
+
+export default EntityListLoader;
+
+export const entityListLoader = ellProps => ComposedComponent => {
+  function WrappedComponent(props) {
+    return (
       <EntityListLoader {...props} {...ellProps}>
         {childProps => (
           <ComposedComponent
@@ -206,3 +247,7 @@ export const entityListLoader = (ellProps: Props) =>
         )}
       </EntityListLoader>
     );
+  }
+  WrappedComponent.displayName = ComposedComponent.displayName;
+  return WrappedComponent;
+};