From e9563562ebfb397dca81f5a9c14975cc2bcfd6b1 Mon Sep 17 00:00:00 2001
From: Nuo Xu <nuoxu.nx@gmail.com>
Date: Wed, 18 Oct 2023 21:21:01 -0400
Subject: [PATCH 1/6] add nft grouping by none/accounts/networks

---
 .../Crypto/FiltersDisplaySettingsView.swift   |  19 +-
 Sources/BraveWallet/Crypto/NFT/NFTView.swift  | 230 ++++++++------
 .../Crypto/Stores/AccountActivityStore.swift  |   2 +
 .../BraveWallet/Crypto/Stores/NFTStore.swift  | 292 +++++++++++++-----
 .../Crypto/Stores/PortfolioStore.swift        |  23 +-
 5 files changed, 398 insertions(+), 168 deletions(-)

diff --git a/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift b/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift
index ca1adf8bab2..54f33ff67a3 100644
--- a/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift
+++ b/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift
@@ -105,6 +105,8 @@ struct FiltersDisplaySettingsView: View {
   @State var isHidingUnownedNFTs: Bool
   /// If we are showing the network logo on NFTs. Default is true.
   @State var isShowingNFTNetworkLogo: Bool
+  /// If we should disable `Hide Unowned`
+  @State var isHidingUnownedNFTsDisabled: Bool
   
   /// All accounts and if they are currently selected. Default is all accounts selected.
   @State var accounts: [Selectable<BraveWallet.AccountInfo>]
@@ -152,8 +154,9 @@ struct FiltersDisplaySettingsView: View {
     self._groupBy = State(initialValue: filters.groupBy)
     self._sortOrder = State(initialValue: filters.sortOrder)
     self._isHidingSmallBalances = State(initialValue: filters.isHidingSmallBalances)
-    self._isHidingUnownedNFTs = State(initialValue: filters.isHidingUnownedNFTs)
+    self._isHidingUnownedNFTs = State(initialValue: filters.groupBy == .accounts ? true : filters.isHidingUnownedNFTs)
     self._isShowingNFTNetworkLogo = State(initialValue: filters.isShowingNFTNetworkLogo)
+    self._isHidingUnownedNFTsDisabled = State(initialValue: filters.groupBy == .accounts ? true : false)
     self._accounts = State(initialValue: filters.accounts)
     self._networks = State(initialValue: filters.networks)
     self.isNFTFilters = isNFTFilters
@@ -167,6 +170,9 @@ struct FiltersDisplaySettingsView: View {
       ScrollView {
         LazyVStack(spacing: 0) {
           if isNFTFilters {
+            groupByRow
+              .padding(.vertical, rowPadding)
+            
             showNFTNetworkLogo
               .padding(.vertical, rowPadding)
             
@@ -194,6 +200,16 @@ struct FiltersDisplaySettingsView: View {
         }
         .padding(.horizontal)
       }
+      .onChange(of: groupBy, perform: { newValue in
+        if isNFTFilters {
+          if newValue == .accounts {
+            isHidingUnownedNFTs = true
+            isHidingUnownedNFTsDisabled = true
+          } else {
+            isHidingUnownedNFTsDisabled = false
+          }
+        }
+      })
       .background(Color(uiColor: WalletV2Design.containerBackground))
       .safeAreaInset(edge: .bottom, content: {
         saveChangesContainer
@@ -268,6 +284,7 @@ struct FiltersDisplaySettingsView: View {
       )
     }
     .tint(Color(.braveBlurpleTint))
+    .disabled(isHidingUnownedNFTsDisabled)
   }
   
   private var showNFTNetworkLogo: some View {
diff --git a/Sources/BraveWallet/Crypto/NFT/NFTView.swift b/Sources/BraveWallet/Crypto/NFT/NFTView.swift
index 474809c1ff1..9c99c649949 100644
--- a/Sources/BraveWallet/Crypto/NFT/NFTView.swift
+++ b/Sources/BraveWallet/Crypto/NFT/NFTView.swift
@@ -21,6 +21,7 @@ struct NFTView: View {
   @State private var isShowingAddCustomNFT: Bool = false
   @State private var isNFTDiscoveryEnabled: Bool = false
   @State private var nftToBeRemoved: NFTAssetViewModel?
+  @State private var groupToggleState: [NFTGroupViewModel.ID: Bool] = [:]
   
   @Environment(\.buySendSwapDestination)
   private var buySendSwapDestination: Binding<BuySendSwapDestination?>
@@ -108,27 +109,6 @@ struct NFTView: View {
     AssetButton(braveSystemName: "leo.filter.settings", action: {
       isPresentingFiltersDisplaySettings = true
     })
-    .sheet(isPresented: $isPresentingFiltersDisplaySettings) {
-      FiltersDisplaySettingsView(
-        filters: nftStore.filters,
-        isNFTFilters: true,
-        networkStore: networkStore,
-        save: { filters in
-          nftStore.saveFilters(filters)
-        }
-      )
-      .osAvailabilityModifiers({ view in
-        if #available(iOS 16, *) {
-          view
-            .presentationDetents([
-              .fraction(0.6),
-              .large
-            ])
-        } else {
-          view
-        }
-      })
-    }
   }
   
   private var nftHeaderView: some View {
@@ -182,89 +162,142 @@ struct NFTView: View {
     return attributedString
   }
   
-  @ViewBuilder var nftGridsView: some View {
-    if nftStore.displayNFTs.isEmpty {
-      emptyView
-        .listRowBackground(Color(.clear))
-    } else {
-      LazyVGrid(columns: nftGrids) {
-        ForEach(nftStore.displayNFTs) { nft in
-          Button(action: {
-            selectedNFTViewModel = nft
-          }) {
-            VStack(alignment: .leading, spacing: 4) {
-              nftImage(nft)
-                .padding(.bottom, 8)
-              Text(nft.token.nftTokenTitle)
-                .font(.callout.weight(.medium))
-                .foregroundColor(Color(.braveLabel))
+  /// Builds the grids of NFTs without any grouping or expandable / collapse behaviour.
+  @ViewBuilder private func nftGridsPlainView(_ group: NFTGroupViewModel) -> some View {
+    LazyVGrid(columns: nftGrids) {
+      ForEach(group.assets) { nft in
+        Button(action: {
+          selectedNFTViewModel = nft
+        }) {
+          VStack(alignment: .leading, spacing: 4) {
+            nftImage(nft)
+              .padding(.bottom, 8)
+            Text(nft.token.nftTokenTitle)
+              .font(.callout.weight(.medium))
+              .foregroundColor(Color(.braveLabel))
+              .multilineTextAlignment(.leading)
+            if !nft.token.symbol.isEmpty {
+              Text(nft.token.symbol)
+                .font(.caption)
+                .foregroundColor(Color(.secondaryBraveLabel))
                 .multilineTextAlignment(.leading)
-              if !nft.token.symbol.isEmpty {
-                Text(nft.token.symbol)
-                  .font(.caption)
-                  .foregroundColor(Color(.secondaryBraveLabel))
-                  .multilineTextAlignment(.leading)
-              }
             }
-            .overlay(alignment: .topLeading) {
-              if nft.token.isSpam {
-                HStack(spacing: 4) {
-                  Text(Strings.Wallet.nftSpam)
-                    .padding(.vertical, 4)
-                    .padding(.leading, 6)
-                    .foregroundColor(Color(.braveErrorLabel))
-                  Image(braveSystemName: "leo.warning.triangle-outline")
-                    .padding(.vertical, 4)
-                    .padding(.trailing, 6)
-                    .foregroundColor(Color(.braveErrorBorder))
-                }
-                .font(.system(size: 13).weight(.semibold))
-                .background(
-                  Color(uiColor: WalletV2Design.spamNFTLabelBackground)
-                    .cornerRadius(4)
-                )
-                .padding(12)
+          }
+          .overlay(alignment: .topLeading) {
+            if nft.token.isSpam {
+              HStack(spacing: 4) {
+                Text(Strings.Wallet.nftSpam)
+                  .padding(.vertical, 4)
+                  .padding(.leading, 6)
+                  .foregroundColor(Color(.braveErrorLabel))
+                Image(braveSystemName: "leo.warning.triangle-outline")
+                  .padding(.vertical, 4)
+                  .padding(.trailing, 6)
+                  .foregroundColor(Color(.braveErrorBorder))
               }
+              .font(.system(size: 13).weight(.semibold))
+              .background(
+                Color(uiColor: WalletV2Design.spamNFTLabelBackground)
+                  .cornerRadius(4)
+              )
+              .padding(12)
             }
           }
-          .contextMenu {
-            Button(action: {
-              if nft.token.visible { // a collected visible NFT, mark as hidden
-                nftStore.updateNFTStatus(nft.token, visible: false, isSpam: false, isDeletedByUser: false)
-              } else { // either a hidden NFT or a junk NFT, mark as visible
-                nftStore.updateNFTStatus(nft.token, visible: true, isSpam: false, isDeletedByUser: false)
-              }
-            }) {
-              if nft.token.visible { // a collected visible NFT
-                Label(Strings.recentSearchHide, braveSystemImage: "leo.eye.off")
-              } else if nft.token.isSpam { // a spam NFT
-                Label(Strings.Wallet.nftUnspam, braveSystemImage: "leo.disable.outline")
-              } else { // a hidden but not spam NFT
-                Label(Strings.Wallet.nftUnhide, braveSystemImage: "leo.eye.on")
-              }
+        }
+        .contextMenu {
+          Button(action: {
+            if nft.token.visible { // a collected visible NFT, mark as hidden
+              nftStore.updateNFTStatus(nft.token, visible: false, isSpam: false, isDeletedByUser: false)
+            } else { // either a hidden NFT or a junk NFT, mark as visible
+              nftStore.updateNFTStatus(nft.token, visible: true, isSpam: false, isDeletedByUser: false)
             }
-            Button(action: {
-              nftToBeRemoved = nft
-            }) {
-              Label(Strings.Wallet.nftRemoveFromWallet, braveSystemImage: "leo.trash")
+          }) {
+            if nft.token.visible { // a collected visible NFT
+              Label(Strings.recentSearchHide, braveSystemImage: "leo.eye.off")
+            } else if nft.token.isSpam { // a spam NFT
+              Label(Strings.Wallet.nftUnspam, braveSystemImage: "leo.disable.outline")
+            } else { // a hidden but not spam NFT
+              Label(Strings.Wallet.nftUnhide, braveSystemImage: "leo.eye.on")
             }
           }
+          Button(action: {
+            nftToBeRemoved = nft
+          }) {
+            Label(Strings.Wallet.nftRemoveFromWallet, braveSystemImage: "leo.trash")
+          }
         }
       }
-      .padding(.horizontal)
-      VStack(spacing: 16) {
-        Divider()
-        editUserAssetsButton
+    }
+  }
+  
+  /// Builds the expandable /  collapseable section content for a given group.
+  @ViewBuilder private func groupedNFTSection(_ group: NFTGroupViewModel) -> some View {
+    if group.assets.isEmpty {
+      EmptyView()
+    } else {
+      WalletDisclosureGroup(
+        isExpanded: Binding(
+          get: { groupToggleState[group.id, default: true] },
+          set: { groupToggleState[group.id] = $0 }
+        ),
+        content: {
+          nftGridsPlainView(group)
+        },
+        label: {
+          if case let .account(account) = group.groupType {
+            AddressView(address: account.address) {
+              groupHeader(for: group)
+            }
+          } else {
+            groupHeader(for: group)
+          }
+        }
+      )
+    }
+  }
+  
+  /// Builds the in-section header for an NFTGroupViewModel that is shown in expanded and non-expanded state. Not used for ungrouped assets.
+  private func groupHeader(for group: NFTGroupViewModel) -> some View {
+    VStack(spacing: 0) {
+      HStack {
+        if case let .network(networkInfo) = group.groupType {
+          NetworkIcon(network: networkInfo, length: 32)
+        } else if case let .account(accountInfo) = group.groupType {
+          Blockie(address: accountInfo.address, shape: .rectangle)
+            .frame(width: 32, height: 32)
+            .clipShape(RoundedRectangle(cornerRadius: 4))
+        }
+        VStack(alignment: .leading) {
+          Text(group.title)
+            .font(.callout.weight(.semibold))
+            .foregroundColor(Color(WalletV2Design.textPrimary))
+          if let description = group.description {
+            Text(description)
+              .font(.footnote)
+              .foregroundColor(Color(WalletV2Design.textSecondary))
+          }
+        }
+        .multilineTextAlignment(.leading)
       }
-      .padding(.top, 20)
+      .padding(.vertical, 4)
     }
   }
   
   var body: some View {
-    VStack(spacing: 16) {
+    LazyVStack(spacing: 16) {
       nftHeaderView
-      
-      nftGridsView
+      if nftStore.isShowingNFTEmptyState {
+        emptyView
+      } else {
+        ForEach(nftStore.displayNFTGroups) { group in
+          if group.groupType == .none {
+            nftGridsPlainView(group)
+              .padding(.horizontal)
+          } else {
+            groupedNFTSection(group)
+          }
+        }
+      }
     }
     .background(
       NavigationLink(
@@ -375,6 +408,27 @@ struct NFTView: View {
         cryptoStore.updateAssets()
       }
     }
+    .sheet(isPresented: $isPresentingFiltersDisplaySettings) {
+      FiltersDisplaySettingsView(
+        filters: nftStore.filters,
+        isNFTFilters: true,
+        networkStore: networkStore,
+        save: { filters in
+          nftStore.saveFilters(filters)
+        }
+      )
+      .osAvailabilityModifiers({ view in
+        if #available(iOS 16, *) {
+          view
+            .presentationDetents([
+              .fraction(0.6),
+              .large
+            ])
+        } else {
+          view
+        }
+      })
+    }
     .onAppear {
       Task {
         isNFTDiscoveryEnabled = await nftStore.isNFTDiscoveryEnabled()
diff --git a/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift b/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift
index 2316205145c..3f63ced6c6b 100644
--- a/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift
@@ -150,6 +150,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
           if token.isErc721 || token.isNft {
             updatedUserNFTs.append(
               NFTAssetViewModel(
+                groupType: .none,
                 token: token,
                 network: networkAssets.network,
                 balanceForAccounts: [:]
@@ -225,6 +226,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
           if token.isErc721 || token.isNft {
             updatedUserNFTs.append(
               NFTAssetViewModel(
+                groupType: .none,
                 token: token,
                 network: networkAssets.network,
                 balanceForAccounts: [account.address: Int(totalBalances[token.assetBalanceId] ?? 0)],
diff --git a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
index 59dcbbf81a6..d9a4bf99f64 100644
--- a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
@@ -7,7 +7,18 @@ import Foundation
 import BraveCore
 import Preferences
 
+struct NFTGroupViewModel: WalletAssetGroupViewModel, Equatable, Identifiable {
+  typealias ViewModel = NFTAssetViewModel
+  
+  var groupType: AssetGroupType
+  var assets: [NFTAssetViewModel]
+  var id: String {
+    "\(groupType.id) \(title)"
+  }
+}
+
 struct NFTAssetViewModel: Identifiable, Equatable {
+  let groupType: AssetGroupType
   var token: BraveWallet.BlockchainToken
   var network: BraveWallet.NetworkInfo
   /// Balance for the NFT for each account address. The key is the account address.
@@ -24,13 +35,25 @@ struct NFTAssetViewModel: Identifiable, Equatable {
 }
 
 public class NFTStore: ObservableObject, WalletObserverStore {
-  /// The NFTs grouped by enum `NFTDisplayType` displayed in `NFTView`
-  var displayNFTs: [NFTAssetViewModel] {
+  /// The current displayed NFT groups
+  var displayNFTGroups: [NFTGroupViewModel] {
     switch displayType {
     case .visible:
-      return userNFTs.filter(\.token.visible)
+      return userNFTGroups.map {
+        NFTGroupViewModel(
+          groupType: $0.groupType,
+          assets: $0.assets.filter(\.token.visible)
+        )
+      }
     case .hidden:
-      return userNFTs.filter { !$0.token.visible }
+      return userNFTGroups.map {
+        NFTGroupViewModel(
+          groupType: $0.groupType,
+          assets: $0.assets.filter { nft in
+            !nft.token.visible
+          }
+        )
+      }
     }
   }
   /// All User Accounts
@@ -109,7 +132,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
   /// Current group to display
   @Published var displayType: NFTDisplayType = .visible
   /// View model for all NFT include visible, hidden and spams
-  @Published private(set) var userNFTs: [NFTAssetViewModel] = []
+  @Published private(set) var userNFTGroups: [NFTGroupViewModel] = []
   
   private let keyringService: BraveWalletKeyringService
   private let rpcService: BraveWalletJsonRpcService
@@ -133,6 +156,13 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     rpcServiceObserver != nil && keyringServiceObserver != nil && walletServiveObserber != nil
   }
   
+  var isShowingNFTEmptyState: Bool {
+    if filters.groupBy == .none, let noneGroup = displayNFTGroups.first {
+      return noneGroup.assets.isEmpty
+    }
+    return displayNFTGroups.isEmpty
+  }
+  
   public init(
     keyringService: BraveWalletKeyringService,
     rpcService: BraveWalletJsonRpcService,
@@ -235,63 +265,72 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         simpleHashSpamNFTs: simpleHashSpamNFTs
       )
       
-      let allNetworkAssets = userVisibleAssets + userHiddenAssets + unionedSpamNFTs
-      userNFTs = buildAssetViewModels(allUserAssets: allNetworkAssets)
-        .optionallyFilterUnownedNFTs(
-          isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
-          selectedAccounts: selectedAccounts
-        )
+      var allNetworkAssets: [NetworkAssets] = []
+      for networkAssets in userVisibleAssets {
+        let hiddenAssets = userHiddenAssets.first(where: {
+          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
+        })?.tokens ?? []
+        let spamNFTs = unionedSpamNFTs.first(where: {
+          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
+        })?.tokens ?? []
+        allNetworkAssets.append(.init(network: networkAssets.network, tokens: networkAssets.tokens + hiddenAssets + spamNFTs, sortOrder: networkAssets.sortOrder))
+      }
+      userNFTGroups = buildNFTGroupModels(
+        groupBy: filters.groupBy,
+        allUserAssets: allNetworkAssets,
+        selectedAccounts: selectedAccounts,
+        selectedNetworks: selectedNetworks
+      )
       
       var allTokens: [BraveWallet.BlockchainToken] = []
       for networkAssets in [userVisibleAssets, userHiddenAssets, unionedSpamNFTs] {
         allTokens.append(contentsOf: networkAssets.flatMap(\.tokens))
       }
       let allNFTs = allTokens.filter { $0.isNft || $0.isErc721 }
-      // if we're not hiding unowned or grouping by account, balance isn't needed
-      if filters.isHidingUnownedNFTs {
-        // fetch balance for all NFTs
-        let allAccounts = filters.accounts.map(\.model)
-        nftBalancesCache = await withTaskGroup(
-          of: [String: [String: Int]].self,
-          body: { @MainActor [nftBalancesCache, rpcService] group in
-            for nft in allNFTs { // for each NFT
-              guard let networkForNFT = allNetworks.first(where: { $0.chainId == nft.chainId }) else {
-                continue
-              }
-              group.addTask { @MainActor in
-                let updatedBalances = await withTaskGroup(
-                  of: [String: Int].self,
-                  body: { @MainActor group in
-                    for account in allAccounts where account.coin == nft.coin {
-                      group.addTask { @MainActor in
-                        let balanceForToken = await rpcService.balance(
-                          for: nft,
-                          in: account,
-                          network: networkForNFT
-                        )
-                        return [account.address: Int(balanceForToken ?? 0)]
-                      }
+      // fetch balance for all NFTs
+      let allAccounts = filters.accounts.map(\.model)
+      nftBalancesCache = await withTaskGroup(
+        of: [String: [String: Int]].self,
+        body: { @MainActor [nftBalancesCache, rpcService] group in
+          for nft in allNFTs { // for each NFT
+            guard let networkForNFT = allNetworks.first(where: { $0.chainId == nft.chainId }) else {
+              continue
+            }
+            group.addTask { @MainActor in
+              let updatedBalances = await withTaskGroup(
+                of: [String: Int].self,
+                body: { @MainActor group in
+                  for account in allAccounts where account.coin == nft.coin {
+                    group.addTask { @MainActor in
+                      let balanceForToken = await rpcService.balance(
+                        for: nft,
+                        in: account,
+                        network: networkForNFT
+                      )
+                      return [account.address: Int(balanceForToken ?? 0)]
                     }
-                    return await group.reduce(into: [String: Int](), { partialResult, new in
-                      partialResult.merge(with: new)
-                    })
+                  }
+                  return await group.reduce(into: [String: Int](), { partialResult, new in
+                    partialResult.merge(with: new)
                   })
-                var tokenBalances = nftBalancesCache[nft.id] ?? [:]
-                tokenBalances.merge(with: updatedBalances)
-                return [nft.id: tokenBalances]
-              }
+                })
+              var tokenBalances = nftBalancesCache[nft.id] ?? [:]
+              tokenBalances.merge(with: updatedBalances)
+              return [nft.id: tokenBalances]
             }
-            return await group.reduce(into: [String: [String: Int]](), { partialResult, new in
-              partialResult.merge(with: new)
-            })
+          }
+          return await group.reduce(into: [String: [String: Int]](), { partialResult, new in
+            partialResult.merge(with: new)
           })
-      }
+        })
+      
       guard !Task.isCancelled else { return }
-      userNFTs = buildAssetViewModels(allUserAssets: allNetworkAssets)
-        .optionallyFilterUnownedNFTs(
-          isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
-          selectedAccounts: selectedAccounts
-        )
+      userNFTGroups = buildNFTGroupModels(
+        groupBy: filters.groupBy,
+        allUserAssets: allNetworkAssets,
+        selectedAccounts: selectedAccounts,
+        selectedNetworks: selectedNetworks
+      )
       
       // fetch nft metadata for all NFTs
       let allMetadata = await rpcService.fetchNFTMetadata(tokens: allNFTs, ipfsApi: ipfsApi)
@@ -299,36 +338,93 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         metadataCache[key] = value
       }
       guard !Task.isCancelled else { return }
-      userNFTs = buildAssetViewModels(allUserAssets: allNetworkAssets)
-        .optionallyFilterUnownedNFTs(
-          isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
-          selectedAccounts: selectedAccounts
-        )
+      userNFTGroups = buildNFTGroupModels(
+        groupBy: filters.groupBy,
+        allUserAssets: allNetworkAssets,
+        selectedAccounts: selectedAccounts,
+        selectedNetworks: selectedNetworks
+      )
     }
   }
   
   func updateNFTMetadataCache(for token: BraveWallet.BlockchainToken, metadata: NFTMetadata) {
     metadataCache[token.id] = metadata
-    if let index = userNFTs.firstIndex(where: { $0.token.id == token.id }),
-       var updatedViewModel = userNFTs[safe: index] {
-      updatedViewModel.nftMetadata = metadata
-      userNFTs[index] = updatedViewModel
+    var updatedGroups: [NFTGroupViewModel] = []
+    for group in userNFTGroups {
+      if let index = group.assets.firstIndex(where: { $0.token.id == token.id }) {
+        var newAssets = group.assets
+        newAssets[index].nftMetadata = metadata
+        updatedGroups.append(.init(groupType: group.groupType, assets: newAssets))
+      } else {
+        updatedGroups.append(group)
+      }
     }
+    userNFTGroups = updatedGroups
   }
   
   private func buildAssetViewModels(
+    for groupType: AssetGroupType,
     allUserAssets: [NetworkAssets]
   ) -> [NFTAssetViewModel] {
-    allUserAssets.flatMap { networkAssets in
-      networkAssets.tokens.compactMap { token in
-        guard token.isErc721 || token.isNft else { return nil }
-        return NFTAssetViewModel(
-          token: token,
-          network: networkAssets.network,
-          balanceForAccounts: nftBalancesCache[token.id] ?? [:],
-          nftMetadata: metadataCache[token.id]
-        )
+    let selectedAccounts = self.filters.accounts.filter(\.isSelected).map(\.model)
+    switch groupType {
+    case .none:
+      return allUserAssets.flatMap { networkAssets in
+        networkAssets.tokens.compactMap { token in
+          guard token.isErc721 || token.isNft else { return nil }
+          return NFTAssetViewModel(
+            groupType: groupType,
+            token: token,
+            network: networkAssets.network,
+            balanceForAccounts: nftBalancesCache[token.id] ?? [:],
+            nftMetadata: metadataCache[token.id]
+          )
+        }
+      }
+      .optionallyFilterUnownedNFTs(
+        isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
+        selectedAccounts: selectedAccounts
+      )
+    case let .network(network):
+      guard let networkAssets = allUserAssets
+        .first(where: { $0.network.chainId == network.chainId && $0.network.coin == network.coin }) else {
+        return []
       }
+      return networkAssets.tokens
+        .filter { $0.isErc721 || $0.isNft }
+        .map { token in
+          NFTAssetViewModel(
+            groupType: groupType,
+            token: token,
+            network: networkAssets.network,
+            balanceForAccounts: nftBalancesCache[token.id] ?? [:],
+            nftMetadata: metadataCache[token.id]
+          )
+        }
+        .optionallyFilterUnownedNFTs(
+          isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
+          selectedAccounts: selectedAccounts
+        )
+    case let .account(account):
+      return allUserAssets
+        .filter { $0.network.coin == account.coin && $0.network.supportedKeyrings.contains(account.accountId.keyringId.rawValue as NSNumber)
+        }
+        .flatMap { networkAssets in
+          networkAssets.tokens.compactMap { token in
+            guard token.isErc721 || token.isNft else { return nil }
+            return NFTAssetViewModel(
+              groupType: groupType,
+              token: token,
+              network: networkAssets.network,
+              balanceForAccounts: nftBalancesCache[token.id] ?? [:],
+              nftMetadata: metadataCache[token.id]
+            )
+          }
+        }
+        .optionallyFilterUnownedNFTs(
+          isHidingUnownedNFTs: true, // by default hiding unowned NFTs when `Group By` is `Accounts`
+          selectedAccounts: selectedAccounts
+        )
     }
   }
   
@@ -376,6 +472,53 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     return computedSpamNFTs
   }
   
+  private func buildNFTGroupModels(
+    groupBy: GroupBy,
+    allUserAssets: [NetworkAssets],
+    selectedAccounts: [BraveWallet.AccountInfo],
+    selectedNetworks: [BraveWallet.NetworkInfo]
+  ) -> [NFTGroupViewModel] {
+    let groups: [NFTGroupViewModel]
+    switch filters.groupBy {
+    case .none:
+      let assets = buildAssetViewModels(
+        for: .none,
+        allUserAssets: allUserAssets
+      )
+      return [
+        .init(
+          groupType: .none,
+          assets: assets
+        )
+      ]
+    case .accounts:
+      groups = selectedAccounts.map { account in
+        let groupType: AssetGroupType = .account(account)
+        let assets = buildAssetViewModels(
+          for: .account(account),
+          allUserAssets: allUserAssets
+        )
+        return NFTGroupViewModel(
+          groupType: groupType,
+          assets: assets
+        )
+      }
+    case .networks:
+      groups = selectedNetworks.map { network in
+        let groupType: AssetGroupType = .network(network)
+        let assets = buildAssetViewModels(
+          for: .network(network),
+          allUserAssets: allUserAssets
+        )
+        return NFTGroupViewModel(
+          groupType: groupType,
+          assets: assets
+        )
+      }
+    }
+    return groups
+  }
+  
   @MainActor func isNFTDiscoveryEnabled() async -> Bool {
     await walletService.nftDiscoveryEnabled()
   }
@@ -407,11 +550,12 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         simpleHashSpamNFTs: simpleHashSpamNFTs
       )
       let allNetworkAssets = userVisibleAssets + userHiddenAssets + spamNFTs
-      userNFTs = buildAssetViewModels(allUserAssets: allNetworkAssets)
-        .optionallyFilterUnownedNFTs(
-          isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
-          selectedAccounts: selectedAccounts
-        )
+      userNFTGroups = buildNFTGroupModels(
+        groupBy: filters.groupBy,
+        allUserAssets: allNetworkAssets,
+        selectedAccounts: selectedAccounts,
+        selectedNetworks: selectedNetworks
+      )
     }
   }
 }
diff --git a/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift b/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
index dc880581eec..b97031fd62b 100644
--- a/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
@@ -26,10 +26,14 @@ public enum AssetGroupType: Equatable, Identifiable {
   }
 }
 
-public struct AssetGroupViewModel: Identifiable, Equatable {
-  let groupType: AssetGroupType
-  let assets: [AssetViewModel]
-  
+/// A protocol for both fungible and non-fungible asset group's view model
+protocol WalletAssetGroupViewModel {
+  associatedtype ViewModel
+  var groupType: AssetGroupType { get }
+  var assets: [ViewModel] { get }
+}
+
+extension WalletAssetGroupViewModel {
   var title: String {
     switch groupType {
     case .none:
@@ -40,6 +44,7 @@ public struct AssetGroupViewModel: Identifiable, Equatable {
       return account.name
     }
   }
+  
   var description: String? {
     switch groupType {
     case .none, .network:
@@ -48,6 +53,15 @@ public struct AssetGroupViewModel: Identifiable, Equatable {
       return account.address.truncatedAddress
     }
   }
+}
+
+public struct AssetGroupViewModel: WalletAssetGroupViewModel, Identifiable, Equatable {
+  typealias ViewModel = AssetViewModel
+  
+  public var groupType: AssetGroupType
+  public var assets: [AssetViewModel]
+  public var id: String { "\(groupType.id) \(title)" }
+  
   var totalFiatValue: Double {
     assets.reduce(0) { partialResult, asset in
       let balance: Double
@@ -61,7 +75,6 @@ public struct AssetGroupViewModel: Identifiable, Equatable {
       return partialResult + assetValue
     }
   }
-  public var id: String { "\(groupType.id) \(title)" }
 }
 
 public struct AssetViewModel: Identifiable, Equatable {

From 286ef22ccba710ea3e2c25e7b4aaf9fe0d1547d1 Mon Sep 17 00:00:00 2001
From: Nuo Xu <nuoxu.nx@gmail.com>
Date: Thu, 19 Oct 2023 11:23:00 -0400
Subject: [PATCH 2/6] fix unit tests.

---
 .../BraveWallet/Crypto/Stores/NFTStore.swift  |  14 +-
 Tests/BraveWalletTests/NFTStoreTests.swift    | 476 +++++++++++++++---
 2 files changed, 430 insertions(+), 60 deletions(-)

diff --git a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
index d9a4bf99f64..a46528530ab 100644
--- a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
@@ -385,6 +385,9 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
         selectedAccounts: selectedAccounts
       )
+      .optionallySort(shouldSort: true) { first, second in
+        first.token.symbol < second.token.symbol
+      }
     case let .network(network):
       guard let networkAssets = allUserAssets
         .first(where: { $0.network.chainId == network.chainId && $0.network.coin == network.coin }) else {
@@ -405,13 +408,17 @@ public class NFTStore: ObservableObject, WalletObserverStore {
           isHidingUnownedNFTs: filters.isHidingUnownedNFTs,
           selectedAccounts: selectedAccounts
         )
+        .optionallySort(shouldSort: true) { first, second in
+          first.token.symbol < second.token.symbol
+        }
     case let .account(account):
       return allUserAssets
         .filter { $0.network.coin == account.coin && $0.network.supportedKeyrings.contains(account.accountId.keyringId.rawValue as NSNumber)
         }
         .flatMap { networkAssets in
           networkAssets.tokens.compactMap { token in
-            guard token.isErc721 || token.isNft else { return nil }
+            // we need to exclude any NFT that THIS account does not own (balance is not 1)
+            guard token.isErc721 || token.isNft, let balance = nftBalancesCache[token.id]?[account.address], balance > 0 else { return nil }
             return NFTAssetViewModel(
               groupType: groupType,
               token: token,
@@ -422,9 +429,12 @@ public class NFTStore: ObservableObject, WalletObserverStore {
           }
         }
         .optionallyFilterUnownedNFTs(
-          isHidingUnownedNFTs: true, // by default hiding unowned NFTs when `Group By` is `Accounts`
+          isHidingUnownedNFTs: true,
           selectedAccounts: selectedAccounts
         )
+        .optionallySort(shouldSort: true) { first, second in
+          first.token.symbol < second.token.symbol
+        }
     }
   }
   
diff --git a/Tests/BraveWalletTests/NFTStoreTests.swift b/Tests/BraveWalletTests/NFTStoreTests.swift
index f6e1fa93c18..74b505846fb 100644
--- a/Tests/BraveWalletTests/NFTStoreTests.swift
+++ b/Tests/BraveWalletTests/NFTStoreTests.swift
@@ -163,12 +163,14 @@ class NFTStoreTests: XCTestCase {
     resetFilters()
   }
   private func resetFilters() {
+    Preferences.Wallet.groupByFilter.reset()
     Preferences.Wallet.isHidingUnownedNFTsFilter.reset()
     Preferences.Wallet.isShowingNFTNetworkLogoFilter.reset()
     Preferences.Wallet.nonSelectedAccountsFilter.reset()
     Preferences.Wallet.nonSelectedNetworksFilter.reset()
   }
   
+  // MARK: Group By `None`
   func testUpdate() async {
     let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
       .previewToken.copy(asVisibleAsset: true),
@@ -196,33 +198,35 @@ class NFTStoreTests: XCTestCase {
       ipfsApi: TestIpfsAPI(),
       userAssetManager: mockAssetManager
     )
-    
     // test that `update()` will assign new value to `userNFTs` publisher
     let userVisibleNFTsException = expectation(description: "update-userVisibleNFTs")
-    XCTAssertTrue(store.userNFTs.isEmpty)  // Initial state
-    store.$userNFTs
+    XCTAssertTrue(store.userNFTGroups.isEmpty)  // Initial state
+    store.$userNFTGroups
       .dropFirst()
       .collect(3)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { userVisibleNFTsException.fulfill() }
-        XCTAssertEqual(userNFTs.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
-        guard let lastUpdatedUserNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 1)
+        guard let visibleNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.visible) else {
           XCTFail("Unexpected test result")
           return
         }
-        let visibleNFTs = lastUpdatedUserNFTs.filter(\.token.visible)
         XCTAssertEqual(visibleNFTs.count, 3)
-        XCTAssertEqual(visibleNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
-        XCTAssertEqual(visibleNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockERC721Metadata.imageURLString)
-        XCTAssertEqual(visibleNFTs[safe: 0]?.nftMetadata?.name, self.mockERC721Metadata.name)
-        XCTAssertEqual(visibleNFTs[safe: 0]?.nftMetadata?.description, self.mockERC721Metadata.description)
-        XCTAssertEqual(visibleNFTs[safe: 1]?.token.symbol, mockEthUserAssets[safe: 3]?.symbol)
-        XCTAssertNil(visibleNFTs[safe: 1]?.nftMetadata)
-        
-        XCTAssertEqual(visibleNFTs[safe: 2]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
-        XCTAssertEqual(visibleNFTs[safe: 2]?.nftMetadata?.imageURLString, self.mockSolMetadata.imageURLString)
-        XCTAssertEqual(visibleNFTs[safe: 2]?.nftMetadata?.name, self.mockSolMetadata.name)
-        XCTAssertEqual(visibleNFTs[safe: 2]?.nftMetadata?.description, self.mockSolMetadata.description)
+        XCTAssertEqual(visibleNFTs[safe: 0]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(visibleNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockSolMetadata.imageURLString)
+        XCTAssertEqual(visibleNFTs[safe: 0]?.nftMetadata?.name, self.mockSolMetadata.name)
+        XCTAssertEqual(visibleNFTs[safe: 0]?.nftMetadata?.description, self.mockSolMetadata.description)
+        XCTAssertEqual(visibleNFTs[safe: 1]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(visibleNFTs[safe: 1]?.nftMetadata?.imageURLString, self.mockERC721Metadata.imageURLString)
+        XCTAssertEqual(visibleNFTs[safe: 1]?.nftMetadata?.name, self.mockERC721Metadata.name)
+        XCTAssertEqual(visibleNFTs[safe: 1]?.nftMetadata?.description, self.mockERC721Metadata.description)
+        XCTAssertEqual(visibleNFTs[safe: 2]?.token.symbol, mockEthUserAssets[safe: 3]?.symbol)
+        XCTAssertNil(visibleNFTs[safe: 2]?.nftMetadata)
       }.store(in: &cancellables)
     
     store.update()
@@ -233,17 +237,22 @@ class NFTStoreTests: XCTestCase {
     
     // MARK: Network Filter Test
     let networksExpectation = expectation(description: "update-networks")
-    store.$userNFTs
+    store.$userNFTGroups
       .dropFirst()
       .collect(2)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { networksExpectation.fulfill() }
-        XCTAssertEqual(userNFTs.count, 2) // empty nfts, populated nfts
-        guard let lastUpdatedUserNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 2) // empty nfts, populated nfts
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
           XCTFail("Unexpected test result")
           return
         }
-        let visibleNFTs = lastUpdatedUserNFTs.filter(\.token.visible)
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 1)
+        guard let visibleNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.visible) else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        
         XCTAssertEqual(visibleNFTs.count, 2)
         XCTAssertEqual(visibleNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
         XCTAssertEqual(visibleNFTs[safe: 1]?.token.symbol, mockEthUserAssets[safe: 3]?.symbol)
@@ -265,20 +274,24 @@ class NFTStoreTests: XCTestCase {
     
     // MARK: Hiding Unowned Filter Test
     let hidingUnownedExpectation = expectation(description: "update-hidingUnowned")
-    store.$userNFTs
+    store.$userNFTGroups
       .dropFirst()
       .collect(3)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { hidingUnownedExpectation.fulfill() }
-        XCTAssertEqual(userNFTs.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
-        guard let lastUpdatedUserNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 1)
+        guard let visibleNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.visible) else {
           XCTFail("Unexpected test result")
           return
         }
-        let visibleNFTs = lastUpdatedUserNFTs.filter(\.token.visible)
         XCTAssertEqual(visibleNFTs.count, 2)
-        XCTAssertEqual(visibleNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
-        XCTAssertEqual(visibleNFTs[safe: 1]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(visibleNFTs[safe: 0]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(visibleNFTs[safe: 1]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
       }.store(in: &cancellables)
     store.saveFilters(.init(
       groupBy: defaultFilters.groupBy,
@@ -294,17 +307,21 @@ class NFTStoreTests: XCTestCase {
     
     // MARK: Accounts Filter Test
     let accountsExpectation = expectation(description: "update-accounts")
-    store.$userNFTs
+    store.$userNFTGroups
       .dropFirst()
       .collect(2)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { accountsExpectation.fulfill() }
-        XCTAssertEqual(userNFTs.count, 2) // empty nfts, populated nfts
-        guard let lastUpdatedNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 2) // empty nfts, populated nfts
+        guard let lastUpdatedNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedNFTGroups.count, 1)
+        guard let visibleNFTs = lastUpdatedNFTGroups.first?.assets.filter(\.token.visible) else {
           XCTFail("Unexpected test result")
           return
         }
-        let visibleNFTs = lastUpdatedNFTs.filter(\.token.visible)
         XCTAssertEqual(visibleNFTs.count, 1)
         XCTAssertEqual(visibleNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
       }.store(in: &cancellables)
@@ -323,6 +340,7 @@ class NFTStoreTests: XCTestCase {
     cancellables.removeAll()
   }
   
+  // MARK: Group By `None`
   func testUpdateForInvisibleGroup() async {
     let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
       .previewToken.copy(asVisibleAsset: true),
@@ -361,19 +379,28 @@ class NFTStoreTests: XCTestCase {
       userAssetManager: mockAssetManager
     )
     
+    // MARK: Group By: None
     // test that `update()` will assign new value to `userInvisibleNFTs` publisher
     let userHiddenNFTsException = expectation(description: "update-userInvisibleNFTs")
-    store.$userNFTs
+    store.$userNFTGroups
       .dropFirst()
       .collect(3)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { userHiddenNFTsException.fulfill() }
-        XCTAssertEqual(userNFTs.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
-        guard let lastUpdatedNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedNFTGroups.count, 1)
+        guard let onlyGroupAssets = lastUpdatedNFTGroups.first?.assets  else {
           XCTFail("Unexpected test result")
           return
         }
-        let hiddenNFTs = lastUpdatedNFTs.filter { !$0.token.visible && !$0.token.isSpam }
+        let hiddenNFTs = onlyGroupAssets.filter {
+          !$0.token.visible && !$0.token.isSpam
+        }
+        
         XCTAssertEqual(hiddenNFTs.count, 1)
         XCTAssertEqual(hiddenNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 4]?.symbol)
         XCTAssertEqual(hiddenNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockERC721Metadata.imageURLString)
@@ -384,7 +411,8 @@ class NFTStoreTests: XCTestCase {
     cancellables.removeAll()
   }
   
-  func testUpdateSpamGrupOnlyFromSimpleHash() async {
+  // MARK: Group By `None`
+  func testUpdateSpamGroupOnlyFromSimpleHash() async {
     let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
       .previewToken.copy(asVisibleAsset: true),
       .mockUSDCToken.copy(asVisibleAsset: false), // Verify non-visible assets not displayed #6386
@@ -411,6 +439,7 @@ class NFTStoreTests: XCTestCase {
       completion("", metadata, .success, "")
     }
 
+    // MARK: Group By: None
     // setup store
     let store = NFTStore(
       keyringService: keyringService,
@@ -424,17 +453,21 @@ class NFTStoreTests: XCTestCase {
 
     // test that `update()` will assign new value to `userNFTs` publisher
     let userSpamNFTsException = expectation(description: "update-userInvisibleNFTs1")
-    store.$userNFTs
+    store.$userNFTGroups
       .dropFirst()
       .collect(3)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { userSpamNFTsException.fulfill() }
-        XCTAssertEqual(userNFTs.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
-        guard let lastUpdatedUserNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 1)
+        guard let spamNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.isSpam) else {
           XCTFail("Unexpected test result")
           return
         }
-        let spamNFTs = lastUpdatedUserNFTs.filter(\.token.isSpam)
         XCTAssertEqual(spamNFTs.count, 1)
         XCTAssertEqual(spamNFTs[safe: 0]?.token.symbol, self.spamEthNFT.symbol)
         XCTAssertEqual(spamNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockERC721Metadata.imageURLString)
@@ -446,7 +479,8 @@ class NFTStoreTests: XCTestCase {
     cancellables.removeAll()
   }
   
-  func testUpdateSpamGrupFromSimpleHashAndUserMarked() async {
+  // MARK: Group By `None`
+  func testUpdateSpamGroupFromSimpleHashAndUserMarked() async {
     let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
       .previewToken.copy(asVisibleAsset: true),
       .mockUSDCToken.copy(asVisibleAsset: false), // Verify non-visible assets not displayed #6386
@@ -486,17 +520,21 @@ class NFTStoreTests: XCTestCase {
     
     // test that `update()` will assign new value to `userNFTs` publisher
     let userSpamNFTsException = expectation(description: "update-userSpamNFTsException2")
-    store.$userNFTs
+    store.$userNFTGroups
       .dropFirst()
       .collect(3)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { userSpamNFTsException.fulfill() }
-        XCTAssertEqual(userNFTs.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
-        guard let lastUpdatedUserNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 1)
+        guard let spamNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.isSpam) else {
           XCTFail("Unexpected test result")
           return
         }
-        let spamNFTs = lastUpdatedUserNFTs.filter(\.token.isSpam)
         XCTAssertEqual(spamNFTs.count, 2)
         XCTAssertEqual(spamNFTs[safe: 0]?.token.symbol, BraveWallet.BlockchainToken.mockSolanaNFTToken.symbol)
         XCTAssertEqual(spamNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockSolMetadata.imageURLString)
@@ -511,7 +549,8 @@ class NFTStoreTests: XCTestCase {
     cancellables.removeAll()
   }
   
-  func testUpdateSpamGrupDuplicationFromSimpleHashAndUserMarked() async {
+  // MARK: Group By `None`
+  func testUpdateSpamGroupDuplicationFromSimpleHashAndUserMarked() async {
     let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
       .previewToken.copy(asVisibleAsset: true),
       .mockUSDCToken.copy(asVisibleAsset: false), // Verify non-visible assets not displayed #6386
@@ -552,17 +591,21 @@ class NFTStoreTests: XCTestCase {
     
     // test that `update()` will assign new value to `userSpamNFTs` publisher
     let userSpamNFTsException = expectation(description: "update-userSpamNFTsException2")
-    store.$userNFTs
+    store.$userNFTGroups
       .dropFirst()
       .collect(3)
-      .sink { userNFTs in
+      .sink { userNFTGroups in
         defer { userSpamNFTsException.fulfill() }
-        XCTAssertEqual(userNFTs.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
-        guard let lastUpdatedUserNFTs = userNFTs.last else {
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 1)
+        guard let spamNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.isSpam) else {
           XCTFail("Unexpected test result")
           return
         }
-        let spamNFTs = lastUpdatedUserNFTs.filter(\.token.isSpam)
         XCTAssertEqual(spamNFTs.count, 1)
         XCTAssertEqual(spamNFTs[safe: 0]?.token.symbol, self.spamEthNFT.symbol)
         XCTAssertEqual(spamNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockERC721Metadata.imageURLString)
@@ -573,4 +616,321 @@ class NFTStoreTests: XCTestCase {
     await fulfillment(of: [userSpamNFTsException], timeout: 1)
     cancellables.removeAll()
   }
+  
+  // MARK: Group by `Accounts` with `displayType`: `visible`
+  func testUpdateGroupByAccountsVisibleNFTs() async {
+    let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
+      .previewToken.copy(asVisibleAsset: true),
+      .mockUSDCToken.copy(asVisibleAsset: false),
+      .mockERC721NFTToken,
+      unownedEthNFT,
+      invisibleEthNFT
+    ]
+    let mockSolUserAssets: [BraveWallet.BlockchainToken] = [
+      BraveWallet.NetworkInfo.mockSolana.nativeToken.copy(asVisibleAsset: true),
+      .mockSpdToken.copy(asVisibleAsset: false),
+      .mockSolanaNFTToken
+    ]
+    
+    // setup test services
+    let (keyringService, rpcService, walletService, assetRatioService, mockAssetManager) = setupServices(mockEthUserAssets: mockEthUserAssets, mockSolUserAssets: mockSolUserAssets)
+    
+    // setup store
+    let store = NFTStore(
+      keyringService: keyringService,
+      rpcService: rpcService,
+      walletService: walletService,
+      assetRatioService: assetRatioService,
+      blockchainRegistry: BraveWallet.TestBlockchainRegistry(),
+      ipfsApi: TestIpfsAPI(),
+      userAssetManager: mockAssetManager
+    )
+    
+    let defaultFilters = store.filters
+
+    let groupByAccountVisibleExpectation = expectation(description: "groupByAccountVisibleExpectation")
+    store.$userNFTGroups
+      .dropFirst()
+      .collect(3)
+      .sink { userNFTGroups in
+        defer { groupByAccountVisibleExpectation.fulfill() }
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 3)
+        guard
+          let groupOneVisibleNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.visible),
+          let groupTwoVisibleNFTs = lastUpdatedUserNFTGroups[safe: 1]?.assets.filter(\.token.visible),
+          let groupThreeVisibleNFTs = lastUpdatedUserNFTGroups[safe: 2]?.assets.filter(\.token.visible)
+        else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(groupOneVisibleNFTs.count, 1)
+        XCTAssertEqual(groupOneVisibleNFTs[safe: 0]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(groupTwoVisibleNFTs.count, 1)
+        XCTAssertEqual(groupTwoVisibleNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
+        XCTAssertTrue(groupThreeVisibleNFTs.isEmpty)
+      }.store(in: &cancellables)
+    store.saveFilters(.init(
+      groupBy: .accounts,
+      sortOrder: defaultFilters.sortOrder,
+      isHidingSmallBalances: defaultFilters.isHidingSmallBalances,
+      isHidingUnownedNFTs: true,
+      isShowingNFTNetworkLogo: defaultFilters.isShowingNFTNetworkLogo,
+      accounts: defaultFilters.accounts,
+      networks: defaultFilters.networks
+    ))
+    await fulfillment(of: [groupByAccountVisibleExpectation], timeout: 1)
+    cancellables.removeAll()
+  }
+  // MARK: Group by `Accounts` with `displayType`: `hidden`
+  func testUpdateGroupByAccountsHiddenNFTs() async {
+    let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
+      .previewToken.copy(asVisibleAsset: true),
+      .mockUSDCToken.copy(asVisibleAsset: false),
+      .mockERC721NFTToken.copy(asVisibleAsset: false),
+      unownedEthNFT,
+      invisibleEthNFT
+    ]
+    let mockSolUserAssets: [BraveWallet.BlockchainToken] = [
+      BraveWallet.NetworkInfo.mockSolana.nativeToken.copy(asVisibleAsset: true),
+      .mockSpdToken.copy(asVisibleAsset: false),
+      .mockSolanaNFTToken.copy(asVisibleAsset: false)
+    ]
+    
+    // setup test services
+    let (keyringService, rpcService, walletService, assetRatioService, mockAssetManager) = setupServices(mockEthUserAssets: mockEthUserAssets, mockSolUserAssets: mockSolUserAssets)
+    rpcService._erc721Metadata = { contractAddress, tokenId, chainId, completion in
+      let metadata = """
+      {
+        "image": "mock.image.url",
+        "name": "mock nft name",
+        "description": "mock nft description"
+      }
+      """
+      completion("", metadata, .success, "")
+    }
+    
+    // setup store
+    let store = NFTStore(
+      keyringService: keyringService,
+      rpcService: rpcService,
+      walletService: walletService,
+      assetRatioService: assetRatioService,
+      blockchainRegistry: BraveWallet.TestBlockchainRegistry(),
+      ipfsApi: TestIpfsAPI(),
+      userAssetManager: mockAssetManager
+    )
+    
+    let defaultFilters = store.filters
+    
+    let groupByAccountHiddenNFTsException = expectation(description: "groupByAccountHiddenNFTsException")
+    store.$userNFTGroups
+      .dropFirst()
+      .collect(3)
+      .sink { userNFTGroups in
+        defer { groupByAccountHiddenNFTsException.fulfill() }
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedNFTGroups.count, 3)
+        guard
+          let groupOne = lastUpdatedNFTGroups.first,
+          let groupTwo = lastUpdatedNFTGroups[safe: 1],
+          let groupThree = lastUpdatedNFTGroups[safe: 2]
+        else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        let groupOneHiddenNFTs = groupOne.assets.filter {
+          !$0.token.visible && !$0.token.isSpam
+        }
+        let groupTwoHiddenNFTs = groupTwo.assets.filter {
+          !$0.token.visible && !$0.token.isSpam
+        }
+        let groupThreeHiddenNFTs = groupThree.assets.filter {
+          !$0.token.visible && !$0.token.isSpam
+        }
+        XCTAssertEqual(groupOneHiddenNFTs.count, 1)
+        XCTAssertEqual(groupOneHiddenNFTs[safe: 0]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(groupOneHiddenNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockSolMetadata.imageURLString)
+        XCTAssertEqual(groupOneHiddenNFTs[safe: 0]?.nftMetadata?.name, self.mockSolMetadata.name)
+        XCTAssertEqual(groupTwoHiddenNFTs.count, 1)
+        XCTAssertEqual(groupTwoHiddenNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(groupTwoHiddenNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockERC721Metadata.imageURLString)
+        XCTAssertEqual(groupTwoHiddenNFTs[safe: 0]?.nftMetadata?.name, self.mockERC721Metadata.name)
+        XCTAssertEqual(groupThreeHiddenNFTs.count, 0)
+      }.store(in: &cancellables)
+    store.saveFilters(.init(
+      groupBy: .accounts,
+      sortOrder: defaultFilters.sortOrder,
+      isHidingSmallBalances: defaultFilters.isHidingSmallBalances,
+      isHidingUnownedNFTs: true,
+      isShowingNFTNetworkLogo: defaultFilters.isShowingNFTNetworkLogo,
+      accounts: defaultFilters.accounts,
+      networks: defaultFilters.networks
+    ))
+    await fulfillment(of: [groupByAccountHiddenNFTsException], timeout: 1)
+    cancellables.removeAll()
+  }
+  
+  // MARK: Group by `Networks` with `displayType`: `visible`
+  func testUpdateGroupByNetworksVisibleNFTs() async {
+    let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
+      .previewToken.copy(asVisibleAsset: true),
+      .mockUSDCToken.copy(asVisibleAsset: false),
+      .mockERC721NFTToken,
+      unownedEthNFT,
+      invisibleEthNFT
+    ]
+    let mockSolUserAssets: [BraveWallet.BlockchainToken] = [
+      BraveWallet.NetworkInfo.mockSolana.nativeToken.copy(asVisibleAsset: true),
+      .mockSpdToken.copy(asVisibleAsset: false),
+      .mockSolanaNFTToken
+    ]
+    
+    // setup test services
+    let (keyringService, rpcService, walletService, assetRatioService, mockAssetManager) = setupServices(mockEthUserAssets: mockEthUserAssets, mockSolUserAssets: mockSolUserAssets)
+    
+    // setup store
+    let store = NFTStore(
+      keyringService: keyringService,
+      rpcService: rpcService,
+      walletService: walletService,
+      assetRatioService: assetRatioService,
+      blockchainRegistry: BraveWallet.TestBlockchainRegistry(),
+      ipfsApi: TestIpfsAPI(),
+      userAssetManager: mockAssetManager
+    )
+    
+    let defaultFilters = store.filters
+    
+    let groupByNetworkVisibleExpectation = expectation(description: "groupByNetworkVisibleExpectation")
+    store.$userNFTGroups
+      .dropFirst()
+      .collect(3)
+      .sink { userNFTGroups in
+        defer { groupByNetworkVisibleExpectation.fulfill() }
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedUserNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedUserNFTGroups.count, 2)
+        guard
+          let groupOneVisibleNFTs = lastUpdatedUserNFTGroups.first?.assets.filter(\.token.visible),
+          let groupTwoVisibleNFTs = lastUpdatedUserNFTGroups.last?.assets.filter(\.token.visible)
+        else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(groupOneVisibleNFTs.count, 1)
+        XCTAssertEqual(groupOneVisibleNFTs[safe: 0]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(groupTwoVisibleNFTs.count, 1)
+        XCTAssertEqual(groupTwoVisibleNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
+      }.store(in: &cancellables)
+    store.saveFilters(.init(
+      groupBy: .networks,
+      sortOrder: defaultFilters.sortOrder,
+      isHidingSmallBalances: defaultFilters.isHidingSmallBalances,
+      isHidingUnownedNFTs: true,
+      isShowingNFTNetworkLogo: defaultFilters.isShowingNFTNetworkLogo,
+      accounts: defaultFilters.accounts,
+      networks: defaultFilters.networks
+    ))
+    await fulfillment(of: [groupByNetworkVisibleExpectation], timeout: 1)
+    cancellables.removeAll()
+  }
+  // MARK: Group by `Networks` with `displayType`: `hidden`
+  func testUpdateGroupByNetworksHiddenNFTs() async {
+    let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
+      .previewToken.copy(asVisibleAsset: true),
+      .mockUSDCToken.copy(asVisibleAsset: false),
+      .mockERC721NFTToken.copy(asVisibleAsset: false),
+      unownedEthNFT,
+      invisibleEthNFT
+    ]
+    let mockSolUserAssets: [BraveWallet.BlockchainToken] = [
+      BraveWallet.NetworkInfo.mockSolana.nativeToken.copy(asVisibleAsset: true),
+      .mockSpdToken.copy(asVisibleAsset: false),
+      .mockSolanaNFTToken.copy(asVisibleAsset: false)
+    ]
+    
+    // setup test services
+    let (keyringService, rpcService, walletService, assetRatioService, mockAssetManager) = setupServices(mockEthUserAssets: mockEthUserAssets, mockSolUserAssets: mockSolUserAssets)
+    rpcService._erc721Metadata = { contractAddress, tokenId, chainId, completion in
+      let metadata = """
+      {
+        "image": "mock.image.url",
+        "name": "mock nft name",
+        "description": "mock nft description"
+      }
+      """
+      completion("", metadata, .success, "")
+    }
+    
+    // setup store
+    let store = NFTStore(
+      keyringService: keyringService,
+      rpcService: rpcService,
+      walletService: walletService,
+      assetRatioService: assetRatioService,
+      blockchainRegistry: BraveWallet.TestBlockchainRegistry(),
+      ipfsApi: TestIpfsAPI(),
+      userAssetManager: mockAssetManager
+    )
+    
+    let defaultFilters = store.filters
+    
+    let groupByAccountHiddenNFTsException = expectation(description: "groupByAccountHiddenNFTsException")
+    store.$userNFTGroups
+      .dropFirst()
+      .collect(3)
+      .sink { userNFTGroups in
+        defer { groupByAccountHiddenNFTsException.fulfill() }
+        XCTAssertEqual(userNFTGroups.count, 3) // empty nfts, populated w/ balance nfts, populated w/ metadata
+        guard let lastUpdatedNFTGroups = userNFTGroups.last else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        XCTAssertEqual(lastUpdatedNFTGroups.count, 2)
+        guard
+          let groupOne = lastUpdatedNFTGroups.first,
+          let groupTwo = lastUpdatedNFTGroups.last
+        else {
+          XCTFail("Unexpected test result")
+          return
+        }
+        let groupOneHiddenNFTs = groupOne.assets.filter {
+          !$0.token.visible && !$0.token.isSpam
+        }
+        let groupTwoHiddenNFTs = groupTwo.assets.filter {
+          !$0.token.visible && !$0.token.isSpam
+        }
+        XCTAssertEqual(groupOneHiddenNFTs.count, 1)
+        XCTAssertEqual(groupOneHiddenNFTs[safe: 0]?.token.symbol, mockSolUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(groupOneHiddenNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockSolMetadata.imageURLString)
+        XCTAssertEqual(groupOneHiddenNFTs[safe: 0]?.nftMetadata?.name, self.mockSolMetadata.name)
+        XCTAssertEqual(groupTwoHiddenNFTs.count, 1)
+        XCTAssertEqual(groupTwoHiddenNFTs[safe: 0]?.token.symbol, mockEthUserAssets[safe: 2]?.symbol)
+        XCTAssertEqual(groupTwoHiddenNFTs[safe: 0]?.nftMetadata?.imageURLString, self.mockERC721Metadata.imageURLString)
+        XCTAssertEqual(groupTwoHiddenNFTs[safe: 0]?.nftMetadata?.name, self.mockERC721Metadata.name)
+      }.store(in: &cancellables)
+    store.saveFilters(.init(
+      groupBy: .networks,
+      sortOrder: defaultFilters.sortOrder,
+      isHidingSmallBalances: defaultFilters.isHidingSmallBalances,
+      isHidingUnownedNFTs: true,
+      isShowingNFTNetworkLogo: defaultFilters.isShowingNFTNetworkLogo,
+      accounts: defaultFilters.accounts,
+      networks: defaultFilters.networks
+    ))
+    await fulfillment(of: [groupByAccountHiddenNFTsException], timeout: 1)
+    cancellables.removeAll()
+  }
 }

From 14667d21375bff15621c1efb77059b6f20306fae Mon Sep 17 00:00:00 2001
From: Nuo Xu <nuoxu.nx@gmail.com>
Date: Thu, 19 Oct 2023 13:00:46 -0400
Subject: [PATCH 3/6] fixed. Hide/unhide NFTs in groups then change display
 type collected/hidden doesnt load correctly.

---
 Sources/BraveWallet/Crypto/Stores/NFTStore.swift | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
index a46528530ab..21064ba8e25 100644
--- a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
@@ -554,12 +554,21 @@ public class NFTStore: ObservableObject, WalletObserverStore {
       let selectedNetworks = self.filters.networks.filter(\.isSelected).map(\.model)
       let userVisibleAssets = self.assetManager.getAllUserAssetsInNetworkAssetsByVisibility(networks: selectedNetworks, visible: true)
       let userHiddenAssets = self.assetManager.getAllUserAssetsInNetworkAssetsByVisibility(networks: selectedNetworks, visible: false)
-      let spamNFTs = computeSpamNFTs(
+      let unionedSpamNFTs = computeSpamNFTs(
         selectedNetworks: selectedNetworks,
         selectedAccounts: selectedAccounts,
         simpleHashSpamNFTs: simpleHashSpamNFTs
       )
-      let allNetworkAssets = userVisibleAssets + userHiddenAssets + spamNFTs
+      var allNetworkAssets: [NetworkAssets] = []
+      for networkAssets in userVisibleAssets {
+        let hiddenAssets = userHiddenAssets.first(where: {
+          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
+        })?.tokens ?? []
+        let spamNFTs = unionedSpamNFTs.first(where: {
+          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
+        })?.tokens ?? []
+        allNetworkAssets.append(.init(network: networkAssets.network, tokens: networkAssets.tokens + hiddenAssets + spamNFTs, sortOrder: networkAssets.sortOrder))
+      }
       userNFTGroups = buildNFTGroupModels(
         groupBy: filters.groupBy,
         allUserAssets: allNetworkAssets,

From e91f8af21e3b58ed7a9b2d0543a342e5ad59463f Mon Sep 17 00:00:00 2001
From: Nuo Xu <nuoxu.nx@gmail.com>
Date: Fri, 20 Oct 2023 17:06:14 -0400
Subject: [PATCH 4/6] addressed code review comments.

---
 Sources/BraveWallet/Crypto/NFT/NFTView.swift  | 77 ++++++++-----------
 .../Portfolio/PortfolioAssetsView.swift       | 37 +--------
 .../Crypto/Portfolio/PortfolioView.swift      | 31 ++++++++
 .../BraveWallet/Crypto/Stores/NFTStore.swift  |  5 ++
 .../Crypto/Stores/PortfolioStore.swift        |  1 +
 .../Crypto/WalletDisclosureGroup.swift        | 15 ++--
 .../BraveWallet/Extensions/WalletColors.swift |  7 ++
 7 files changed, 89 insertions(+), 84 deletions(-)

diff --git a/Sources/BraveWallet/Crypto/NFT/NFTView.swift b/Sources/BraveWallet/Crypto/NFT/NFTView.swift
index 9c99c649949..e3b6027dbb1 100644
--- a/Sources/BraveWallet/Crypto/NFT/NFTView.swift
+++ b/Sources/BraveWallet/Crypto/NFT/NFTView.swift
@@ -113,28 +113,40 @@ struct NFTView: View {
   
   private var nftHeaderView: some View {
     HStack {
-      Text(Strings.Wallet.assetsTitle)
-        .font(.title3.weight(.semibold))
-        .foregroundColor(Color(braveSystemName: .textPrimary))
+      Menu {
+        Picker("", selection: $nftStore.displayType) {
+          ForEach(NFTStore.NFTDisplayType.allCases) { type in
+            Text(type.dropdownTitle)
+              .foregroundColor(Color(.secondaryBraveLabel))
+              .tag(type)
+          }
+        }
+        .pickerStyle(.inline)
+      } label: {
+        HStack(spacing: 12) {
+          Text(nftStore.displayType.dropdownTitle)
+            .font(.subheadline.weight(.semibold))
+          Text("\(nftStore.totalDisplayNFTCounter)")
+            .padding(.horizontal, 8)
+            .padding(.vertical, 4)
+            .font(.caption2.weight(.semibold))
+            .background(
+              Color(uiColor: WalletV2Design.displayNFTCounterColor)
+                .cornerRadius(4)
+            )
+          Image(braveSystemName: "leo.carat.down")
+            .font(.subheadline.weight(.semibold))
+        }
+        .foregroundColor(Color(.braveBlurpleTint))
+      }
       if nftStore.isLoadingDiscoverAssets && isNFTDiscoveryEnabled {
         ProgressView()
           .padding(.leading, 5)
       }
       Spacer()
-      Picker(selection: $nftStore.displayType) {
-        ForEach(NFTStore.NFTDisplayType.allCases) { type in
-          Text(type.dropdownTitle)
-            .foregroundColor(Color(.secondaryBraveLabel))
-            .tag(type)
-        }
-      } label: {
-        Text(nftStore.displayType.dropdownTitle)
-          .font(.footnote)
-          .foregroundColor(Color(.braveLabel))
-      }
-      filtersButton
-        .padding(.trailing, 10)
       addCustomAssetButton
+        .padding(.trailing, 10)
+      filtersButton
     }
     .padding(.horizontal)
     .frame(maxWidth: .infinity, alignment: .leading)
@@ -236,53 +248,28 @@ struct NFTView: View {
       EmptyView()
     } else {
       WalletDisclosureGroup(
+        isNFTGroup: true,
         isExpanded: Binding(
           get: { groupToggleState[group.id, default: true] },
           set: { groupToggleState[group.id] = $0 }
         ),
         content: {
           nftGridsPlainView(group)
+            .padding(.top)
         },
         label: {
           if case let .account(account) = group.groupType {
             AddressView(address: account.address) {
-              groupHeader(for: group)
+              PortfolioAssetGroupHeaderView(group: group)
             }
           } else {
-            groupHeader(for: group)
+            PortfolioAssetGroupHeaderView(group: group)
           }
         }
       )
     }
   }
   
-  /// Builds the in-section header for an NFTGroupViewModel that is shown in expanded and non-expanded state. Not used for ungrouped assets.
-  private func groupHeader(for group: NFTGroupViewModel) -> some View {
-    VStack(spacing: 0) {
-      HStack {
-        if case let .network(networkInfo) = group.groupType {
-          NetworkIcon(network: networkInfo, length: 32)
-        } else if case let .account(accountInfo) = group.groupType {
-          Blockie(address: accountInfo.address, shape: .rectangle)
-            .frame(width: 32, height: 32)
-            .clipShape(RoundedRectangle(cornerRadius: 4))
-        }
-        VStack(alignment: .leading) {
-          Text(group.title)
-            .font(.callout.weight(.semibold))
-            .foregroundColor(Color(WalletV2Design.textPrimary))
-          if let description = group.description {
-            Text(description)
-              .font(.footnote)
-              .foregroundColor(Color(WalletV2Design.textSecondary))
-          }
-        }
-        .multilineTextAlignment(.leading)
-      }
-      .padding(.vertical, 4)
-    }
-  }
-  
   var body: some View {
     LazyVStack(spacing: 16) {
       nftHeaderView
diff --git a/Sources/BraveWallet/Crypto/Portfolio/PortfolioAssetsView.swift b/Sources/BraveWallet/Crypto/Portfolio/PortfolioAssetsView.swift
index ef748929425..967fff09487 100644
--- a/Sources/BraveWallet/Crypto/Portfolio/PortfolioAssetsView.swift
+++ b/Sources/BraveWallet/Crypto/Portfolio/PortfolioAssetsView.swift
@@ -166,6 +166,7 @@ struct PortfolioAssetsView: View {
   /// Builds the expandable/collapseable (expanded by default) section content for a given group.
   @ViewBuilder private func groupedAssetsSection(for group: AssetGroupViewModel) -> some View {
     WalletDisclosureGroup(
+      isNFTGroup: false,
       isExpanded: Binding(
         get: { groupToggleState[group.id, default: true] },
         set: { isExpanded in
@@ -196,44 +197,12 @@ struct PortfolioAssetsView: View {
       label: {
         if case let .account(account) = group.groupType {
           AddressView(address: account.address) {
-            groupHeader(for: group)
+            PortfolioAssetGroupHeaderView(group: group)
           }
         } else {
-          groupHeader(for: group)
+          PortfolioAssetGroupHeaderView(group: group)
         }
       }
     )
   }
-  
-  /// Builds the in-section header for an AssetGroupViewModel that is shown in expanded and non-expanded state. Not used for ungrouped assets.
-  private func groupHeader(for group: AssetGroupViewModel) -> some View {
-    VStack(spacing: 0) {
-      HStack {
-        if case let .network(networkInfo) = group.groupType {
-          NetworkIcon(network: networkInfo, length: 32)
-        } else if case let .account(accountInfo) = group.groupType {
-          Blockie(address: accountInfo.address, shape: .rectangle)
-            .frame(width: 32, height: 32)
-            .clipShape(RoundedRectangle(cornerRadius: 4))
-        }
-        VStack(alignment: .leading) {
-          Text(group.title)
-            .font(.callout.weight(.semibold))
-            .foregroundColor(Color(WalletV2Design.textPrimary))
-          if let description = group.description {
-            Text(description)
-              .font(.footnote)
-              .foregroundColor(Color(WalletV2Design.textSecondary))
-          }
-        }
-        .multilineTextAlignment(.leading)
-        Spacer()
-        Text(isShowingBalances.value ? portfolioStore.currencyFormatter.string(from: NSNumber(value: group.totalFiatValue)) ?? "" : "****")
-          .font(.callout.weight(.semibold))
-          .foregroundColor(Color(WalletV2Design.textPrimary))
-          .multilineTextAlignment(.trailing)
-      }
-      .padding(.vertical, 4)
-    }
-  }
 }
diff --git a/Sources/BraveWallet/Crypto/Portfolio/PortfolioView.swift b/Sources/BraveWallet/Crypto/Portfolio/PortfolioView.swift
index 5d6172597b2..645428a7bf2 100644
--- a/Sources/BraveWallet/Crypto/Portfolio/PortfolioView.swift
+++ b/Sources/BraveWallet/Crypto/Portfolio/PortfolioView.swift
@@ -91,6 +91,37 @@ struct PortfolioView: View {
   }
 }
 
+/// Builds the in-section header for `Assets`/`NFT` that is shown in expanded and non-expanded state. Not used for ungrouped assets.
+struct PortfolioAssetGroupHeaderView: View {
+  let group: any WalletAssetGroupViewModel
+  
+  var body: some View {
+    VStack(spacing: 0) {
+      HStack {
+        if case let .network(networkInfo) = group.groupType {
+          NetworkIcon(network: networkInfo, length: 32)
+        } else if case let .account(accountInfo) = group.groupType {
+          Blockie(address: accountInfo.address, shape: .rectangle)
+            .frame(width: 32, height: 32)
+            .clipShape(RoundedRectangle(cornerRadius: 4))
+        }
+        VStack(alignment: .leading) {
+          Text(group.title)
+            .font(.callout.weight(.semibold))
+            .foregroundColor(Color(WalletV2Design.textPrimary))
+          if let description = group.description {
+            Text(description)
+              .font(.footnote)
+              .foregroundColor(Color(WalletV2Design.textSecondary))
+          }
+        }
+        .multilineTextAlignment(.leading)
+      }
+      .padding(.vertical, 4)
+    }
+  }
+}
+
 #if DEBUG
 struct PortfolioViewController_Previews: PreviewProvider {
   static var previews: some View {
diff --git a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
index 21064ba8e25..8328d96339a 100644
--- a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
@@ -163,6 +163,10 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     return displayNFTGroups.isEmpty
   }
   
+  var totalDisplayNFTCounter: Int {
+    displayNFTGroups.reduce(0) { $0 + $1.assets.count }
+  }
+  
   public init(
     keyringService: BraveWalletKeyringService,
     rpcService: BraveWalletJsonRpcService,
@@ -191,6 +195,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     Preferences.Wallet.isHidingUnownedNFTsFilter.observe(from: self)
     Preferences.Wallet.isShowingNFTNetworkLogoFilter.observe(from: self)
     Preferences.Wallet.nonSelectedNetworksFilter.observe(from: self)
+    Preferences.Wallet.groupByFilter.observe(from: self)
   }
   
   func tearDown() {
diff --git a/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift b/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
index b97031fd62b..d524b1a1554 100644
--- a/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
@@ -353,6 +353,7 @@ public class PortfolioStore: ObservableObject, WalletObserverStore {
     Preferences.Wallet.isHidingSmallBalancesFilter.observe(from: self)
     Preferences.Wallet.nonSelectedAccountsFilter.observe(from: self)
     Preferences.Wallet.nonSelectedNetworksFilter.observe(from: self)
+    Preferences.Wallet.groupByFilter.observe(from: self)
   }
   
   func tearDown() {
diff --git a/Sources/BraveWallet/Crypto/WalletDisclosureGroup.swift b/Sources/BraveWallet/Crypto/WalletDisclosureGroup.swift
index c8f8d4ff559..400f2c7679f 100644
--- a/Sources/BraveWallet/Crypto/WalletDisclosureGroup.swift
+++ b/Sources/BraveWallet/Crypto/WalletDisclosureGroup.swift
@@ -6,6 +6,7 @@
 import SwiftUI
 
 struct WalletDisclosureGroup<Label: View, Content: View>: View {
+  var isNFTGroup: Bool
   @Binding var isExpanded: Bool
   @ViewBuilder var content: () -> Content
   @ViewBuilder var label: () -> Label
@@ -44,7 +45,7 @@ struct WalletDisclosureGroup<Label: View, Content: View>: View {
       if isExpanded {
         Divider()
           .padding(.top, 6)
-          .padding(.horizontal, 8)
+          .padding(.horizontal, isNFTGroup ? nil : 8)
         content()
           .padding(.horizontal)
       }
@@ -52,10 +53,14 @@ struct WalletDisclosureGroup<Label: View, Content: View>: View {
     // when collapsed, padding is applied to `header`
     .padding(.vertical, isExpanded ? 6 : 0)
     .osAvailabilityModifiers {
-      if isExpanded {
-        $0.overlay {
-          RoundedRectangle(cornerRadius: 16)
-            .stroke(Color(braveSystemName: .dividerSubtle), lineWidth: 1)
+      if !isNFTGroup {
+        if isExpanded {
+          $0.overlay {
+            RoundedRectangle(cornerRadius: 16)
+              .stroke(Color(braveSystemName: .dividerSubtle), lineWidth: 1)
+          }
+        } else {
+          $0
         }
       } else {
         $0
diff --git a/Sources/BraveWallet/Extensions/WalletColors.swift b/Sources/BraveWallet/Extensions/WalletColors.swift
index b7c57b5d001..1f5e778cce1 100644
--- a/Sources/BraveWallet/Extensions/WalletColors.swift
+++ b/Sources/BraveWallet/Extensions/WalletColors.swift
@@ -257,4 +257,11 @@ enum WalletV2Design {
     blue: 207 / 255,
     alpha: 1
   )
+  
+  static let displayNFTCounterColor = UIColor(
+    red: 213 / 255,
+    green: 220 / 255,
+    blue: 1,
+    alpha: 1
+  )
 }

From 50a0166fcacc441eb5ef07f9e1fac56ebb5b072e Mon Sep 17 00:00:00 2001
From: Nuo Xu <nuoxu.nx@gmail.com>
Date: Mon, 23 Oct 2023 14:03:59 -0400
Subject: [PATCH 5/6] addressed more comments.

---
 .../Crypto/FiltersDisplaySettingsView.swift   |   2 +-
 Sources/BraveWallet/Crypto/NFT/NFTView.swift  |   4 +-
 .../BraveWallet/Crypto/Stores/NFTStore.swift  | 130 ++++++++++--------
 .../BraveWallet/Extensions/WalletColors.swift |   7 -
 4 files changed, 78 insertions(+), 65 deletions(-)

diff --git a/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift b/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift
index 54f33ff67a3..d884d25064f 100644
--- a/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift
+++ b/Sources/BraveWallet/Crypto/FiltersDisplaySettingsView.swift
@@ -156,7 +156,7 @@ struct FiltersDisplaySettingsView: View {
     self._isHidingSmallBalances = State(initialValue: filters.isHidingSmallBalances)
     self._isHidingUnownedNFTs = State(initialValue: filters.groupBy == .accounts ? true : filters.isHidingUnownedNFTs)
     self._isShowingNFTNetworkLogo = State(initialValue: filters.isShowingNFTNetworkLogo)
-    self._isHidingUnownedNFTsDisabled = State(initialValue: filters.groupBy == .accounts ? true : false)
+    self._isHidingUnownedNFTsDisabled = State(initialValue: filters.groupBy == .accounts)
     self._accounts = State(initialValue: filters.accounts)
     self._networks = State(initialValue: filters.networks)
     self.isNFTFilters = isNFTFilters
diff --git a/Sources/BraveWallet/Crypto/NFT/NFTView.swift b/Sources/BraveWallet/Crypto/NFT/NFTView.swift
index e3b6027dbb1..d5331f0d702 100644
--- a/Sources/BraveWallet/Crypto/NFT/NFTView.swift
+++ b/Sources/BraveWallet/Crypto/NFT/NFTView.swift
@@ -126,12 +126,12 @@ struct NFTView: View {
         HStack(spacing: 12) {
           Text(nftStore.displayType.dropdownTitle)
             .font(.subheadline.weight(.semibold))
-          Text("\(nftStore.totalDisplayNFTCounter)")
+          Text("\(nftStore.totalDisplayedNFTCount)")
             .padding(.horizontal, 8)
             .padding(.vertical, 4)
             .font(.caption2.weight(.semibold))
             .background(
-              Color(uiColor: WalletV2Design.displayNFTCounterColor)
+              Color(braveSystemName: .primary20)
                 .cornerRadius(4)
             )
           Image(braveSystemName: "leo.carat.down")
diff --git a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
index 8328d96339a..3f30ce1e529 100644
--- a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
@@ -163,7 +163,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     return displayNFTGroups.isEmpty
   }
   
-  var totalDisplayNFTCounter: Int {
+  var totalDisplayedNFTCount: Int {
     displayNFTGroups.reduce(0) { $0 + $1.assets.count }
   }
   
@@ -258,10 +258,24 @@ public class NFTStore: ObservableObject, WalletObserverStore {
       let selectedAccounts = filters.accounts.filter(\.isSelected).map(\.model)
       let selectedNetworks = filters.networks.filter(\.isSelected).map(\.model)
       
-      // user visible assets
-      let userVisibleAssets = assetManager.getAllUserAssetsInNetworkAssetsByVisibility(networks: selectedNetworks, visible: true)
-      // user hidden assets
-      let userHiddenAssets = assetManager.getAllUserAssetsInNetworkAssetsByVisibility(networks: selectedNetworks, visible: false)
+      // user visible NFTs
+      let userVisibleNFTs = assetManager.getAllUserAssetsInNetworkAssetsByVisibility(networks: selectedNetworks, visible: true)
+        .map { networkAssets in
+          NetworkAssets(
+            network: networkAssets.network,
+            tokens: networkAssets.tokens.filter { $0.isNft || $0.isErc721 }
+            , sortOrder: networkAssets.sortOrder
+          )
+        }
+      // user hidden NFTs
+      let userHiddenNFTs = assetManager.getAllUserAssetsInNetworkAssetsByVisibility(networks: selectedNetworks, visible: false)
+        .map { networkAssets in
+          NetworkAssets(
+            network: networkAssets.network,
+            tokens: networkAssets.tokens.filter { $0.isNft || $0.isErc721 }
+            , sortOrder: networkAssets.sortOrder
+          )
+        }
       // all spam NFTs marked by SimpleHash
       simpleHashSpamNFTs = await walletService.simpleHashSpamNFTs(for: selectedAccounts, on: selectedNetworks)
       let unionedSpamNFTs = computeSpamNFTs(
@@ -270,28 +284,22 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         simpleHashSpamNFTs: simpleHashSpamNFTs
       )
       
-      var allNetworkAssets: [NetworkAssets] = []
-      for networkAssets in userVisibleAssets {
-        let hiddenAssets = userHiddenAssets.first(where: {
-          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
-        })?.tokens ?? []
-        let spamNFTs = unionedSpamNFTs.first(where: {
-          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
-        })?.tokens ?? []
-        allNetworkAssets.append(.init(network: networkAssets.network, tokens: networkAssets.tokens + hiddenAssets + spamNFTs, sortOrder: networkAssets.sortOrder))
-      }
+      let allNetworkNFTs = generateAllNFTsInNetworks(
+        userVisibleNFTs: userVisibleNFTs,
+        userHiddenNFTs: userHiddenNFTs,
+        computedSpamNFTs: unionedSpamNFTs
+      )
       userNFTGroups = buildNFTGroupModels(
         groupBy: filters.groupBy,
-        allUserAssets: allNetworkAssets,
+        allUserNFTs: allNetworkNFTs,
         selectedAccounts: selectedAccounts,
         selectedNetworks: selectedNetworks
       )
       
-      var allTokens: [BraveWallet.BlockchainToken] = []
-      for networkAssets in [userVisibleAssets, userHiddenAssets, unionedSpamNFTs] {
-        allTokens.append(contentsOf: networkAssets.flatMap(\.tokens))
+      var allNFTs: [BraveWallet.BlockchainToken] = []
+      for networkAssets in [userVisibleNFTs, userHiddenNFTs, unionedSpamNFTs] {
+        allNFTs.append(contentsOf: networkAssets.flatMap(\.tokens))
       }
-      let allNFTs = allTokens.filter { $0.isNft || $0.isErc721 }
       // fetch balance for all NFTs
       let allAccounts = filters.accounts.map(\.model)
       nftBalancesCache = await withTaskGroup(
@@ -332,7 +340,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
       guard !Task.isCancelled else { return }
       userNFTGroups = buildNFTGroupModels(
         groupBy: filters.groupBy,
-        allUserAssets: allNetworkAssets,
+        allUserNFTs: allNetworkNFTs,
         selectedAccounts: selectedAccounts,
         selectedNetworks: selectedNetworks
       )
@@ -345,7 +353,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
       guard !Task.isCancelled else { return }
       userNFTGroups = buildNFTGroupModels(
         groupBy: filters.groupBy,
-        allUserAssets: allNetworkAssets,
+        allUserNFTs: allNetworkNFTs,
         selectedAccounts: selectedAccounts,
         selectedNetworks: selectedNetworks
       )
@@ -367,17 +375,16 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     userNFTGroups = updatedGroups
   }
   
-  private func buildAssetViewModels(
+  private func buildNFTAssetViewModels(
     for groupType: AssetGroupType,
-    allUserAssets: [NetworkAssets]
+    allUserNFTs: [NetworkAssets]
   ) -> [NFTAssetViewModel] {
     let selectedAccounts = self.filters.accounts.filter(\.isSelected).map(\.model)
     switch groupType {
     case .none:
-      return allUserAssets.flatMap { networkAssets in
-        networkAssets.tokens.compactMap { token in
-          guard token.isErc721 || token.isNft else { return nil }
-          return NFTAssetViewModel(
+      return allUserNFTs.flatMap { networkAssets in
+        networkAssets.tokens.map { token in
+          NFTAssetViewModel(
             groupType: groupType,
             token: token,
             network: networkAssets.network,
@@ -394,17 +401,16 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         first.token.symbol < second.token.symbol
       }
     case let .network(network):
-      guard let networkAssets = allUserAssets
+      guard let networkNFTs = allUserNFTs
         .first(where: { $0.network.chainId == network.chainId && $0.network.coin == network.coin }) else {
         return []
       }
-      return networkAssets.tokens
-        .filter { $0.isErc721 || $0.isNft }
+      return networkNFTs.tokens
         .map { token in
           NFTAssetViewModel(
             groupType: groupType,
             token: token,
-            network: networkAssets.network,
+            network: networkNFTs.network,
             balanceForAccounts: nftBalancesCache[token.id] ?? [:],
             nftMetadata: metadataCache[token.id]
           )
@@ -417,17 +423,17 @@ public class NFTStore: ObservableObject, WalletObserverStore {
           first.token.symbol < second.token.symbol
         }
     case let .account(account):
-      return allUserAssets
+      return allUserNFTs
         .filter { $0.network.coin == account.coin && $0.network.supportedKeyrings.contains(account.accountId.keyringId.rawValue as NSNumber)
         }
-        .flatMap { networkAssets in
-          networkAssets.tokens.compactMap { token in
+        .flatMap { networkNFTs in
+          networkNFTs.tokens.compactMap { token in
             // we need to exclude any NFT that THIS account does not own (balance is not 1)
-            guard token.isErc721 || token.isNft, let balance = nftBalancesCache[token.id]?[account.address], balance > 0 else { return nil }
+            guard let balance = nftBalancesCache[token.id]?[account.address], balance > 0 else { return nil }
             return NFTAssetViewModel(
               groupType: groupType,
               token: token,
-              network: networkAssets.network,
+              network: networkNFTs.network,
               balanceForAccounts: nftBalancesCache[token.id] ?? [:],
               nftMetadata: metadataCache[token.id]
             )
@@ -443,6 +449,24 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     }
   }
   
+  private func generateAllNFTsInNetworks(
+    userVisibleNFTs: [NetworkAssets],
+    userHiddenNFTs: [NetworkAssets],
+    computedSpamNFTs: [NetworkAssets]
+  ) -> [NetworkAssets] {
+    var allNetworkNFTs: [NetworkAssets] = []
+    for networkNFTs in userVisibleNFTs {
+      let hiddenNFTs = userHiddenNFTs.first(where: {
+        $0.network.chainId == networkNFTs.network.chainId && $0.network.coin == networkNFTs.network.coin
+      })?.tokens ?? []
+      let spamNFTs = computedSpamNFTs.first(where: {
+        $0.network.chainId == networkNFTs.network.chainId && $0.network.coin == networkNFTs.network.coin
+      })?.tokens ?? []
+      allNetworkNFTs.append(.init(network: networkNFTs.network, tokens: networkNFTs.tokens + hiddenNFTs + spamNFTs, sortOrder: networkNFTs.sortOrder))
+    }
+    return allNetworkNFTs
+  }
+  
   private func computeSpamNFTs(
     selectedNetworks: [BraveWallet.NetworkInfo],
     selectedAccounts: [BraveWallet.AccountInfo],
@@ -489,16 +513,16 @@ public class NFTStore: ObservableObject, WalletObserverStore {
   
   private func buildNFTGroupModels(
     groupBy: GroupBy,
-    allUserAssets: [NetworkAssets],
+    allUserNFTs: [NetworkAssets],
     selectedAccounts: [BraveWallet.AccountInfo],
     selectedNetworks: [BraveWallet.NetworkInfo]
   ) -> [NFTGroupViewModel] {
     let groups: [NFTGroupViewModel]
     switch filters.groupBy {
     case .none:
-      let assets = buildAssetViewModels(
+      let assets = buildNFTAssetViewModels(
         for: .none,
-        allUserAssets: allUserAssets
+        allUserNFTs: allUserNFTs
       )
       return [
         .init(
@@ -509,9 +533,9 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     case .accounts:
       groups = selectedAccounts.map { account in
         let groupType: AssetGroupType = .account(account)
-        let assets = buildAssetViewModels(
+        let assets = buildNFTAssetViewModels(
           for: .account(account),
-          allUserAssets: allUserAssets
+          allUserNFTs: allUserNFTs
         )
         return NFTGroupViewModel(
           groupType: groupType,
@@ -521,9 +545,9 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     case .networks:
       groups = selectedNetworks.map { network in
         let groupType: AssetGroupType = .network(network)
-        let assets = buildAssetViewModels(
+        let assets = buildNFTAssetViewModels(
           for: .network(network),
-          allUserAssets: allUserAssets
+          allUserNFTs: allUserNFTs
         )
         return NFTGroupViewModel(
           groupType: groupType,
@@ -564,19 +588,15 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         selectedAccounts: selectedAccounts,
         simpleHashSpamNFTs: simpleHashSpamNFTs
       )
-      var allNetworkAssets: [NetworkAssets] = []
-      for networkAssets in userVisibleAssets {
-        let hiddenAssets = userHiddenAssets.first(where: {
-          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
-        })?.tokens ?? []
-        let spamNFTs = unionedSpamNFTs.first(where: {
-          $0.network.chainId == networkAssets.network.chainId && $0.network.coin == networkAssets.network.coin
-        })?.tokens ?? []
-        allNetworkAssets.append(.init(network: networkAssets.network, tokens: networkAssets.tokens + hiddenAssets + spamNFTs, sortOrder: networkAssets.sortOrder))
-      }
+      
+      let allNetworkNFTs = generateAllNFTsInNetworks(
+        userVisibleNFTs: userVisibleAssets,
+        userHiddenNFTs: userHiddenAssets,
+        computedSpamNFTs: unionedSpamNFTs
+      )
       userNFTGroups = buildNFTGroupModels(
         groupBy: filters.groupBy,
-        allUserAssets: allNetworkAssets,
+        allUserNFTs: allNetworkNFTs,
         selectedAccounts: selectedAccounts,
         selectedNetworks: selectedNetworks
       )
diff --git a/Sources/BraveWallet/Extensions/WalletColors.swift b/Sources/BraveWallet/Extensions/WalletColors.swift
index 1f5e778cce1..b7c57b5d001 100644
--- a/Sources/BraveWallet/Extensions/WalletColors.swift
+++ b/Sources/BraveWallet/Extensions/WalletColors.swift
@@ -257,11 +257,4 @@ enum WalletV2Design {
     blue: 207 / 255,
     alpha: 1
   )
-  
-  static let displayNFTCounterColor = UIColor(
-    red: 213 / 255,
-    green: 220 / 255,
-    blue: 1,
-    alpha: 1
-  )
 }

From 69722795fe8c734ed543be06cfcf4dac5c89892b Mon Sep 17 00:00:00 2001
From: Nuo Xu <nuoxu.nx@gmail.com>
Date: Mon, 23 Oct 2023 14:58:24 -0400
Subject: [PATCH 6/6] index fixing

---
 Sources/BraveWallet/Crypto/Stores/NFTStore.swift | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
index 3f30ce1e529..662d6ab3a1c 100644
--- a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
+++ b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift
@@ -263,8 +263,8 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         .map { networkAssets in
           NetworkAssets(
             network: networkAssets.network,
-            tokens: networkAssets.tokens.filter { $0.isNft || $0.isErc721 }
-            , sortOrder: networkAssets.sortOrder
+            tokens: networkAssets.tokens.filter { $0.isNft || $0.isErc721 },
+            sortOrder: networkAssets.sortOrder
           )
         }
       // user hidden NFTs
@@ -272,8 +272,8 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         .map { networkAssets in
           NetworkAssets(
             network: networkAssets.network,
-            tokens: networkAssets.tokens.filter { $0.isNft || $0.isErc721 }
-            , sortOrder: networkAssets.sortOrder
+            tokens: networkAssets.tokens.filter { $0.isNft || $0.isErc721 },
+            sortOrder: networkAssets.sortOrder
           )
         }
       // all spam NFTs marked by SimpleHash