feat: Sort/Import Tokens in Extension (#27184)

## **Description** ### This PR adds Token sorting to the Asset List page, and also moves Token importing to the top of the Token List. A few of the main changes introduced: 1. Include `NativeToken` in `TokenList` component to be included in sorting logic, and treated (as far as sorting is concerned) as any other token in the list 2. Intoduce a `tokenSortConfig` into state that keeps track of the sort order, the key being sorted by, and the direction of the sort order. Also includes an action to update this state. 3. Introduce a `useEffect` that subscribes to `tokenSortConfig` as well as a few other application state variables to update and sort tokenList when appropriate. 2. Clean up `asset-list` component, and move some of it's relevant code into the `useAccountTotalFiatBalance` **Acceptance Criteria:** 1. Tokens should be sorted by default by declining balance 2. Sort controls should sort tokens alphabetically, and by decreasing fiat token balance 3. Sort order should persist through refresh 4. Sort order should persist after app is closed and reopened 5. When a token gets imported, it should be included in the sort list, in the correct order in the list **A couple of disclaimers. There are still (at least) two bugs that I discovered that were not caught by tests:** 1. ~~When toggling preferred currency setting, Native Token sorted incorrectly by decreasing fiat balance~~ ✅ fixed 2. ~~When switching between accounts, token list does not update~~ ✅ fixed [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27184?quickstart=1) ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMASSETS-356 ## **Manual testing steps** 1. Go to AssetList page, and click dropdown and select option to sort by 2. Tokens should sort, and remain sorted through refresh and application close/open (it is in state) 4. Importing tokens should import them into the sort order ## **Screenshots/Recordings** https://github.com/user-attachments/assets/8ecca5e4-093f-4651-946e-31c612795427 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
MetaMask · Oct 8, 2024 · b34484e · b34484e
1 parent b5b8b8f
commit b34484e
Show file tree

Hide file tree

Showing 55 changed files with 1,486 additions and 239 deletions.
diff --git a/.storybook/test-data.js b/.storybook/test-data.js
@@ -677,6 +677,11 @@ const state = {
     currentLocale: 'en',
     preferences: {
       showNativeTokenAsMainBalance: true,
+      tokenSortConfig: {
+        key: 'token-sort-key',
+        order: 'dsc',
+        sortCallback: 'stringNumeric',
+      },
     },
     incomingTransactionsPreferences: {
       [CHAIN_IDS.MAINNET]: true,

diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json
diff --git a/app/scripts/controllers/metametrics.js b/app/scripts/controllers/metametrics.js
@@ -868,6 +868,8 @@ export default class MetaMetricsController {
         metamaskState.participateInMetaMetrics,
       [MetaMetricsUserTrait.HasMarketingConsent]:
         metamaskState.dataCollectionForMarketing,
+      [MetaMetricsUserTrait.TokenSortPreference]:
+        metamaskState.tokenSortConfig?.key || '',
     };
 
     if (!previousUserTraits) {

diff --git a/app/scripts/controllers/metametrics.test.js b/app/scripts/controllers/metametrics.test.js
@@ -1122,6 +1122,11 @@ describe('MetaMetricsController', function () {
             },
           },
         },
+        tokenSortConfig: {
+          key: 'token-sort-key',
+          order: 'dsc',
+          sortCallback: 'stringNumeric',
+        },
       });
 
       expect(traits).toStrictEqual({
@@ -1153,6 +1158,7 @@ describe('MetaMetricsController', function () {
         ///: BEGIN:ONLY_INCLUDE_IF(petnames)
         [MetaMetricsUserTrait.PetnameAddressCount]: 3,
         ///: END:ONLY_INCLUDE_IF
+        [MetaMetricsUserTrait.TokenSortPreference]: 'token-sort-key',
       });
     });
 
@@ -1181,6 +1187,11 @@ describe('MetaMetricsController', function () {
         useNftDetection: false,
         theme: 'default',
         useTokenDetection: true,
+        tokenSortConfig: {
+          key: 'token-sort-key',
+          order: 'dsc',
+          sortCallback: 'stringNumeric',
+        },
         showNativeTokenAsMainBalance: true,
       });
 
@@ -1208,6 +1219,11 @@ describe('MetaMetricsController', function () {
         useNftDetection: false,
         theme: 'default',
         useTokenDetection: true,
+        tokenSortConfig: {
+          key: 'token-sort-key',
+          order: 'dsc',
+          sortCallback: 'stringNumeric',
+        },
         showNativeTokenAsMainBalance: false,
       });
 
@@ -1245,6 +1261,11 @@ describe('MetaMetricsController', function () {
         useNftDetection: true,
         theme: 'default',
         useTokenDetection: true,
+        tokenSortConfig: {
+          key: 'token-sort-key',
+          order: 'dsc',
+          sortCallback: 'stringNumeric',
+        },
         showNativeTokenAsMainBalance: true,
       });
 
@@ -1267,6 +1288,11 @@ describe('MetaMetricsController', function () {
         useNftDetection: true,
         theme: 'default',
         useTokenDetection: true,
+        tokenSortConfig: {
+          key: 'token-sort-key',
+          order: 'dsc',
+          sortCallback: 'stringNumeric',
+        },
         showNativeTokenAsMainBalance: true,
       });
       expect(updatedTraits).toStrictEqual(null);

diff --git a/app/scripts/controllers/preferences-controller.ts b/app/scripts/controllers/preferences-controller.ts
@@ -106,6 +106,11 @@ export type Preferences = {
   showMultiRpcModal: boolean;
   isRedesignedConfirmationsDeveloperEnabled: boolean;
   showConfirmationAdvancedDetails: boolean;
+  tokenSortConfig: {
+    key: string;
+    order: string;
+    sortCallback: string;
+  };
   shouldShowAggregatedBalancePopover: boolean;
 };
 
@@ -237,6 +242,11 @@ export default class PreferencesController {
         showMultiRpcModal: false,
         isRedesignedConfirmationsDeveloperEnabled: false,
         showConfirmationAdvancedDetails: false,
+        tokenSortConfig: {
+          key: 'tokenFiatAmount',
+          order: 'dsc',
+          sortCallback: 'stringNumeric',
+        },
         shouldShowAggregatedBalancePopover: true, // by default user should see popover;
       },
       // ENS decentralized website resolution

diff --git a/app/scripts/migrations/130.test.ts b/app/scripts/migrations/130.test.ts
@@ -0,0 +1,91 @@
+import { migrate, version } from './130';
+
+const oldVersion = 129;
+
+describe(`migration #${version}`, () => {
+  it('updates the version metadata', async () => {
+    const oldStorage = {
+      meta: { version: oldVersion },
+      data: {},
+    };
+    const newStorage = await migrate(oldStorage);
+    expect(newStorage.meta).toStrictEqual({ version });
+  });
+  describe(`migration #${version}`, () => {
+    it('updates the preferences with a default tokenSortConfig', async () => {
+      const oldStorage = {
+        meta: { version: oldVersion },
+        data: {
+          PreferencesController: {
+            preferences: {},
+          },
+        },
+      };
+      const expectedData = {
+        PreferencesController: {
+          preferences: {
+            tokenSortConfig: {
+              key: 'tokenFiatAmount',
+              order: 'dsc',
+              sortCallback: 'stringNumeric',
+            },
+          },
+        },
+      };
+      const newStorage = await migrate(oldStorage);
+
+      expect(newStorage.data).toStrictEqual(expectedData);
+    });
+
+    it('does nothing if the preferences already has a tokenSortConfig', async () => {
+      const oldStorage = {
+        meta: { version: oldVersion },
+        data: {
+          PreferencesController: {
+            preferences: {
+              tokenSortConfig: {
+                key: 'fooKey',
+                order: 'foo',
+                sortCallback: 'fooCallback',
+              },
+            },
+          },
+        },
+      };
+
+      const newStorage = await migrate(oldStorage);
+
+      expect(newStorage.data).toStrictEqual(oldStorage.data);
+    });
+
+    it('does nothing to other preferences if they exist without a tokenSortConfig', async () => {
+      const oldStorage = {
+        meta: { version: oldVersion },
+        data: {
+          PreferencesController: {
+            preferences: {
+              existingPreference: true,
+            },
+          },
+        },
+      };
+
+      const expectedData = {
+        PreferencesController: {
+          preferences: {
+            existingPreference: true,
+            tokenSortConfig: {
+              key: 'tokenFiatAmount',
+              order: 'dsc',
+              sortCallback: 'stringNumeric',
+            },
+          },
+        },
+      };
+
+      const newStorage = await migrate(oldStorage);
+
+      expect(newStorage.data).toStrictEqual(expectedData);
+    });
+  });
+});
diff --git a/app/scripts/migrations/130.ts b/app/scripts/migrations/130.ts
@@ -0,0 +1,44 @@
+import { hasProperty, isObject } from '@metamask/utils';
+import { cloneDeep } from 'lodash';
+
+type VersionedData = {
+  meta: { version: number };
+  data: Record<string, unknown>;
+};
+export const version = 130;
+/**
+ * This migration adds a tokenSortConfig to the user's preferences
+ *
+ *
+ * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist.
+ * @param originalVersionedData.meta - State metadata.
+ * @param originalVersionedData.meta.version - The current state version.
+ * @param originalVersionedData.data - The persisted MetaMask state, keyed by controller.
+ * @returns Updated versioned MetaMask extension state.
+ */
+export async function migrate(
+  originalVersionedData: VersionedData,
+): Promise<VersionedData> {
+  const versionedData = cloneDeep(originalVersionedData);
+  versionedData.meta.version = version;
+  transformState(versionedData.data);
+  return versionedData;
+}
+function transformState(
+  state: Record<string, unknown>,
+): Record<string, unknown> {
+  if (
+    hasProperty(state, 'PreferencesController') &&
+    isObject(state.PreferencesController) &&
+    hasProperty(state.PreferencesController, 'preferences') &&
+    isObject(state.PreferencesController.preferences) &&
+    !state.PreferencesController.preferences.tokenSortConfig
+  ) {
+    state.PreferencesController.preferences.tokenSortConfig = {
+      key: 'tokenFiatAmount',
+      order: 'dsc',
+      sortCallback: 'stringNumeric',
+    };
+  }
+  return state;
+}
diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js
@@ -149,6 +149,7 @@ const migrations = [
   require('./127'),
   require('./128'),
   require('./129'),
+  require('./130'),
 ];
 
 export default migrations;
diff --git a/shared/constants/metametrics.ts b/shared/constants/metametrics.ts
@@ -472,6 +472,10 @@ export enum MetaMetricsUserTrait {
    * Identified when the user selects a currency from settings
    */
   CurrentCurrency = 'current_currency',
+  /**
+   * Identified when the user changes token sort order on asset-list
+   */
+  TokenSortPreference = 'token_sort_preference',
 }
 
 /**
@@ -630,6 +634,7 @@ export enum MetaMetricsEventName {
   TokenScreenOpened = 'Token Screen Opened',
   TokenAdded = 'Token Added',
   TokenRemoved = 'Token Removed',
+  TokenSortPreference = 'Token Sort Preference',
   NFTRemoved = 'NFT Removed',
   TokenDetected = 'Token Detected',
   TokenHidden = 'Token Hidden',

diff --git a/test/data/mock-state.json b/test/data/mock-state.json
@@ -372,7 +372,12 @@
       "showFiatInTestnets": false,
       "showNativeTokenAsMainBalance": true,
       "showTestNetworks": true,
-      "smartTransactionsOptInStatus": false
+      "smartTransactionsOptInStatus": false,
+      "tokenSortConfig": {
+        "key": "tokenFiatAmount",
+        "order": "dsc",
+        "sortCallback": "stringNumeric"
+      }
     },
     "ensResolutionsByAddress": {},
     "isAccountMenuOpen": false,

diff --git a/test/e2e/default-fixture.js b/test/e2e/default-fixture.js
@@ -215,6 +215,11 @@ function defaultFixture(inputChainId = CHAIN_IDS.LOCALHOST) {
           showMultiRpcModal: false,
           isRedesignedConfirmationsDeveloperEnabled: false,
           showConfirmationAdvancedDetails: false,
+          tokenSortConfig: {
+            key: 'tokenFiatAmount',
+            order: 'dsc',
+            sortCallback: 'stringNumeric',
+          },
           shouldShowAggregatedBalancePopover: true,
         },
         selectedAddress: '0x5cfe73b6021e818b776b421b1c4db2474086a7e1',

diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js
@@ -77,6 +77,11 @@ function onboardingFixture() {
           showMultiRpcModal: false,
           isRedesignedConfirmationsDeveloperEnabled: false,
           showConfirmationAdvancedDetails: false,
+          tokenSortConfig: {
+            key: 'tokenFiatAmount',
+            order: 'dsc',
+            sortCallback: 'stringNumeric',
+          },
           shouldShowAggregatedBalancePopover: true,
         },
         useExternalServices: true,

diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json
@@ -215,7 +215,7 @@
       "isRedesignedConfirmationsDeveloperEnabled": "boolean",
       "redesignedConfirmationsEnabled": true,
       "redesignedTransactionsEnabled": "boolean",
-      "showMultiRpcModal": "boolean",
+      "tokenSortConfig": "object",
       "shouldShowAggregatedBalancePopover": "boolean"
     },
     "ipfsGateway": "string",

diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json
@@ -37,6 +37,7 @@
       "isRedesignedConfirmationsDeveloperEnabled": "boolean",
       "redesignedConfirmationsEnabled": true,
       "redesignedTransactionsEnabled": "boolean",
+      "tokenSortConfig": "object",
       "showMultiRpcModal": "boolean",
       "shouldShowAggregatedBalancePopover": "boolean"
     },

diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json
@@ -80,11 +80,11 @@
     },
     "NetworkController": {
       "selectedNetworkClientId": "string",
+      "networkConfigurations": "object",
       "networksMetadata": {
         "networkConfigurationId": { "EIPS": {}, "status": "available" }
       },
-      "providerConfig": "object",
-      "networkConfigurations": "object"
+      "providerConfig": "object"
     },
     "OnboardingController": {
       "completedOnboarding": true,
@@ -114,8 +114,9 @@
         "smartTransactionsOptInStatus": false,
         "showNativeTokenAsMainBalance": true,
         "petnamesEnabled": true,
-        "showConfirmationAdvancedDetails": false,
         "isRedesignedConfirmationsDeveloperEnabled": "boolean",
+        "showConfirmationAdvancedDetails": false,
+        "tokenSortConfig": "object",
         "showMultiRpcModal": "boolean",
         "shouldShowAggregatedBalancePopover": "boolean"
       },

diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json
@@ -80,11 +80,11 @@
     },
     "NetworkController": {
       "selectedNetworkClientId": "string",
+      "networkConfigurations": "object",
       "networksMetadata": {
         "networkConfigurationId": { "EIPS": {}, "status": "available" }
       },
-      "providerConfig": "object",
-      "networkConfigurations": "object"
+      "providerConfig": "object"
     },
     "OnboardingController": {
       "completedOnboarding": true,
@@ -114,8 +114,9 @@
         "smartTransactionsOptInStatus": false,
         "showNativeTokenAsMainBalance": true,
         "petnamesEnabled": true,
-        "showConfirmationAdvancedDetails": false,
         "isRedesignedConfirmationsDeveloperEnabled": "boolean",
+        "showConfirmationAdvancedDetails": false,
+        "tokenSortConfig": "object",
         "showMultiRpcModal": "boolean",
         "shouldShowAggregatedBalancePopover": "boolean"
       },