Yet more NFT UX cleanups (#13435)

* yet more nft ux cleanups

app/_locales/en/messages.json

@@ -1640,6 +1640,9 @@
   "loading": {
     "message": "Loading..."
   },
+  "loadingNFTs": {
+    "message": "Loading NFTs..."
+  },
   "loadingTokens": {
     "message": "Loading Tokens..."
   },

app/scripts/controllers/preferences.js

@@ -134,16 +134,10 @@ export default class PreferencesController {
   /**
    * Setter for the `useCollectibleDetection` property
    *
-   * @param {boolean} val - Whether or not the user prefers to autodetect collectibles.
+   * @param {boolean} useCollectibleDetection - Whether or not the user prefers to autodetect collectibles.
    */
-  setUseCollectibleDetection(val) {
+  setUseCollectibleDetection(useCollectibleDetection) {
-    const { openSeaEnabled } = this.store.getState();
+    this.store.updateState({ useCollectibleDetection });
-    if (val && !openSeaEnabled) {
-      throw new Error(
-        'useCollectibleDetection cannot be enabled if openSeaEnabled is false',
-      );
-    }
-    this.store.updateState({ useCollectibleDetection: val });
   }
 
   /**

ui/components/app/collectibles-items/collectibles-items.js

@@ -17,7 +17,11 @@ import {
 } from '../../../helpers/constants/design-system';
 import { ENVIRONMENT_TYPE_POPUP } from '../../../../shared/constants/app';
 import { getEnvironmentType } from '../../../../app/scripts/lib/util';
-import { getIpfsGateway } from '../../../selectors';
+import {
+  getCurrentChainId,
+  getIpfsGateway,
+  getSelectedAddress,
+} from '../../../selectors';
 import { ASSET_ROUTE } from '../../../helpers/constants/routes';
 import { getAssetImageURL } from '../../../helpers/utils/util';
 import { updateCollectibleDropDownState } from '../../../store/actions';
@@ -39,22 +43,39 @@ export default function CollectiblesItems({
   const collectionsKeys = Object.keys(collections);
   const collectiblesDropdownState = useSelector(getCollectiblesDropdownState);
   const previousCollectionKeys = usePrevious(collectionsKeys);
+  const selectedAddress = useSelector(getSelectedAddress);
+  const chainId = useSelector(getCurrentChainId);
 
   useEffect(() => {
     if (
-      !Object.keys(collectiblesDropdownState).length &&
+      chainId !== undefined &&
-      previousCollectionKeys !== collectionsKeys
+      selectedAddress !== undefined &&
+      previousCollectionKeys !== collectionsKeys &&
+      (collectiblesDropdownState?.[selectedAddress]?.[chainId] === undefined ||
+        Object.keys(collectiblesDropdownState?.[selectedAddress]?.[chainId])
+          .length === 0)
     ) {
       const initState = {};
       collectionsKeys.forEach((key) => {
         initState[key] = true;
       });
-      dispatch(updateCollectibleDropDownState(initState));
+
+      const newCollectibleDropdownState = {
+        ...collectiblesDropdownState,
+        [selectedAddress]: {
+          ...collectiblesDropdownState?.[selectedAddress],
+          [chainId]: initState,
+        },
+      };
+
+      dispatch(updateCollectibleDropDownState(newCollectibleDropdownState));
     }
   }, [
     collectionsKeys,
     previousCollectionKeys,
     collectiblesDropdownState,
+    selectedAddress,
+    chainId,
     dispatch,
   ]);
 
@@ -84,6 +105,22 @@ export default function CollectiblesItems({
     );
   };
 
+  const updateCollectibleDropDownStateKey = (key, isExpanded) => {
+    const currentAccountCollectibleDropdownState =
+      collectiblesDropdownState[selectedAddress][chainId];
+
+    const newCurrentAccountState = {
+      ...currentAccountCollectibleDropdownState,
+      [key]: !isExpanded,
+    };
+
+    collectiblesDropdownState[selectedAddress][
+      chainId
+    ] = newCurrentAccountState;
+
+    dispatch(updateCollectibleDropDownState(collectiblesDropdownState));
+  };
+
   const renderCollection = ({
     collectibles,
     collectionName,
@@ -95,17 +132,13 @@ export default function CollectiblesItems({
       return null;
     }
 
-    const isExpanded = collectiblesDropdownState[key];
+    const isExpanded =
+      collectiblesDropdownState[selectedAddress]?.[chainId]?.[key];
     return (
       <div className="collectibles-items__collection" key={`collection-${key}`}>
         <button
           onClick={() => {
-            dispatch(
+            updateCollectibleDropDownStateKey(key, isExpanded);
-              updateCollectibleDropDownState({
-                ...collectiblesDropdownState,
-                [key]: !isExpanded,
-              }),
-            );
           }}
           className="collectibles-items__collection-wrapper"
         >

ui/components/app/collectibles-items/index.scss

@@ -26,7 +26,6 @@
       background: var(--ui-4);
       color: var(--ui-white);
       text-align: center;
-      line-height: 1;
     }
 
     &-item-wrapper {

ui/components/app/collectibles-tab/collectibles-tab.js

@@ -1,8 +1,7 @@
-import React, { useEffect, useState } from 'react';
+import React from 'react';
 import PropTypes from 'prop-types';
 import { useDispatch, useSelector } from 'react-redux';
 import { useHistory } from 'react-router-dom';
-import { isEqual } from 'lodash';
 import Box from '../../ui/box';
 import Button from '../../ui/button';
 import Typography from '../../ui/typography/typography';
@@ -18,22 +17,16 @@ import {
   ALIGN_ITEMS,
 } from '../../../helpers/constants/design-system';
 import { useI18nContext } from '../../../hooks/useI18nContext';
-import {
+import { getCollectiblesDetectionNoticeDismissed } from '../../../ducks/metamask/metamask';
-  getCollectibles,
-  getCollectibleContracts,
-  getCollectiblesDetectionNoticeDismissed,
-} from '../../../ducks/metamask/metamask';
 import { getIsMainnet, getUseCollectibleDetection } from '../../../selectors';
 import { EXPERIMENTAL_ROUTE } from '../../../helpers/constants/routes';
 import {
   checkAndUpdateAllCollectiblesOwnershipStatus,
   detectCollectibles,
 } from '../../../store/actions';
-import { usePrevious } from '../../../hooks/usePrevious';
+import { useCollectiblesCollections } from '../../../hooks/useCollectiblesCollections';
 
 export default function CollectiblesTab({ onAddNFT }) {
-  const collectibles = useSelector(getCollectibles);
-  const collectibleContracts = useSelector(getCollectibleContracts);
   const useCollectibleDetection = useSelector(getUseCollectibleDetection);
   const isMainnet = useSelector(getIsMainnet);
   const collectibleDetectionNoticeDismissed = useSelector(
@@ -42,46 +35,12 @@ export default function CollectiblesTab({ onAddNFT }) {
   const history = useHistory();
   const t = useI18nContext();
   const dispatch = useDispatch();
-  const [collections, setCollections] = useState({});
-  const [previouslyOwnedCollection, setPreviouslyOwnedCollection] = useState({
-    collectionName: 'Previously Owned',
-    collectibles: [],
-  });
 
-  const prevCollectibles = usePrevious(collectibles);
+  const {
-  useEffect(() => {
+    collectiblesLoading,
-    const getCollections = () => {
+    collections,
-      const newCollections = {};
+    previouslyOwnedCollection,
-      const newPreviouslyOwnedCollections = {
+  } = useCollectiblesCollections();
-        collectionName: 'Previously Owned',
-        collectibles: [],
-      };
-
-      collectibles.forEach((collectible) => {
-        if (collectible?.isCurrentlyOwned === false) {
-          newPreviouslyOwnedCollections.collectibles.push(collectible);
-        } else if (newCollections[collectible.address]) {
-          newCollections[collectible.address].collectibles.push(collectible);
-        } else {
-          const collectionContract = collectibleContracts.find(
-            ({ address }) => address === collectible.address,
-          );
-          newCollections[collectible.address] = {
-            collectionName: collectionContract?.name || collectible.name,
-            collectionImage:
-              collectionContract?.logo || collectible.collectionImage,
-            collectibles: [collectible],
-          };
-        }
-      });
-      setCollections(newCollections);
-      setPreviouslyOwnedCollection(newPreviouslyOwnedCollections);
-    };
-
-    if (!isEqual(prevCollectibles, collectibles)) {
-      getCollections();
-    }
-  }, [collectibles, prevCollectibles, collectibleContracts]);
 
   const onEnableAutoDetect = () => {
     history.push(EXPERIMENTAL_ROUTE);
@@ -94,6 +53,10 @@ export default function CollectiblesTab({ onAddNFT }) {
     checkAndUpdateAllCollectiblesOwnershipStatus();
   };
 
+  if (collectiblesLoading) {
+    return <div className="collectibles-tab__loading">{t('loadingNFTs')}</div>;
+  }
+
   return (
     <div className="collectibles-tab">
       {Object.keys(collections).length > 0 ||

ui/components/app/collectibles-tab/collectibles-tab.test.js

@@ -180,11 +180,13 @@ describe('Collectible Items', () => {
   const setCollectiblesDetectionNoticeDismissedStub = jest.fn();
   const getStateStub = jest.fn();
   const checkAndUpdateAllCollectiblesOwnershipStatusStub = jest.fn();
+  const updateCollectibleDropDownStateStub = jest.fn();
   setBackgroundConnection({
     setCollectiblesDetectionNoticeDismissed: setCollectiblesDetectionNoticeDismissedStub,
     detectCollectibles: detectCollectiblesStub,
     getState: getStateStub,
     checkAndUpdateAllCollectiblesOwnershipStatus: checkAndUpdateAllCollectiblesOwnershipStatusStub,
+    updateCollectibleDropDownState: updateCollectibleDropDownStateStub,
   });
   const historyPushMock = jest.fn();
 

ui/components/app/collectibles-tab/index.scss

@@ -5,4 +5,12 @@
       font-size: 0.875rem;
     }
   }
+
+  &__loading {
+    display: flex;
+    height: 250px;
+    align-items: center;
+    justify-content: center;
+    padding: 30px;
+  }
 }

ui/hooks/useCollectiblesCollections.js

@@ -0,0 +1,80 @@
+import { useEffect, useState } from 'react';
+import { useSelector } from 'react-redux';
+import { isEqual } from 'lodash';
+import {
+  getCollectibles,
+  getCollectibleContracts,
+} from '../ducks/metamask/metamask';
+import { getCurrentChainId, getSelectedAddress } from '../selectors';
+import { usePrevious } from './usePrevious';
+
+export function useCollectiblesCollections() {
+  const [collections, setCollections] = useState({});
+  const [previouslyOwnedCollection, setPreviouslyOwnedCollection] = useState({
+    collectionName: 'Previously Owned',
+    collectibles: [],
+  });
+  const collectibles = useSelector(getCollectibles);
+  const [collectiblesLoading, setCollectiblesLoading] = useState(
+    () => collectibles?.length >= 0,
+  );
+  const selectedAddress = useSelector(getSelectedAddress);
+  const chainId = useSelector(getCurrentChainId);
+  const collectibleContracts = useSelector(getCollectibleContracts);
+  const prevCollectibles = usePrevious(collectibles);
+  const prevChainId = usePrevious(chainId);
+  const prevSelectedAddress = usePrevious(selectedAddress);
+  useEffect(() => {
+    const getCollections = () => {
+      setCollectiblesLoading(true);
+      if (selectedAddress === undefined || chainId === undefined) {
+        return;
+      }
+      const newCollections = {};
+      const newPreviouslyOwnedCollections = {
+        collectionName: 'Previously Owned',
+        collectibles: [],
+      };
+
+      collectibles.forEach((collectible) => {
+        if (collectible?.isCurrentlyOwned === false) {
+          newPreviouslyOwnedCollections.collectibles.push(collectible);
+        } else if (newCollections[collectible.address]) {
+          newCollections[collectible.address].collectibles.push(collectible);
+        } else {
+          const collectionContract = collectibleContracts.find(
+            ({ address }) => address === collectible.address,
+          );
+          newCollections[collectible.address] = {
+            collectionName: collectionContract?.name || collectible.name,
+            collectionImage:
+              collectionContract?.logo || collectible.collectionImage,
+            collectibles: [collectible],
+          };
+        }
+      });
+      setCollections(newCollections);
+      setPreviouslyOwnedCollection(newPreviouslyOwnedCollections);
+      setCollectiblesLoading(false);
+    };
+
+    if (
+      !isEqual(prevCollectibles, collectibles) ||
+      !isEqual(prevSelectedAddress, selectedAddress) ||
+      !isEqual(prevChainId, chainId)
+    ) {
+      getCollections();
+    }
+  }, [
+    collectibles,
+    prevCollectibles,
+    collectibleContracts,
+    setCollectiblesLoading,
+    chainId,
+    prevChainId,
+    selectedAddress,
+    prevSelectedAddress,
+  ]);
+
+  return { collectiblesLoading, collections, previouslyOwnedCollection };
+}