Skip to content

Commit

Permalink
Fix brave/brave-ios#8205: Support NFT removal. Updates for NFT spam m…
Browse files Browse the repository at this point in the history 
…anagement new design (brave/brave-ios#8226)
  • Loading branch information
@nuo-xu
nuo-xu committed Oct 17, 2023
1 parent 1fedaad commit 1f48b0d
Show file tree
Hide file tree
Showing 17 changed files with 468 additions and 84 deletions.
20 changes: 20 additions & 0 deletions Sources/BraveWallet/Crypto/NFT/NFTDetailView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,26 @@ struct NFTDetailView: View {
.frame(maxWidth: .infinity, minHeight: 300)
} else {
nftImage
.overlay(alignment: .topLeading) {
if nftDetailStore.nft.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)
}
}
}
VStack(alignment: .leading, spacing: 8) {
Text(nftDetailStore.nft.nftTokenTitle)
Expand Down
87 changes: 67 additions & 20 deletions Sources/BraveWallet/Crypto/NFT/NFTView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ struct NFTView: View {
@State private var isShowingNFTDiscoveryAlert: Bool = false
@State private var isShowingAddCustomNFT: Bool = false
@State private var isNFTDiscoveryEnabled: Bool = false
@State private var nftToBeRemoved: NFTAssetViewModel?

@Environment(\.buySendSwapDestination)
private var buySendSwapDestination: Binding<BuySendSwapDestination?>
Expand Down Expand Up @@ -205,17 +206,47 @@ struct NFTView: View {
.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)
}
}
}
.contextMenu {
Button(action: {
nftStore.updateNFTStatus(nft.token, visible: isHiddenNFT(nft.token), isSpam: false)
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)
}
}) {
Label(isHiddenNFT(nft.token) ? Strings.Wallet.nftUnhide : Strings.recentSearchHide, braveSystemImage: isHiddenNFT(nft.token) ? "leo.eye.on" : "leo.eye.off")
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: {
nftStore.updateNFTStatus(nft.token, visible: isSpamNFT(nft.token), isSpam: !isSpamNFT(nft.token))
nftToBeRemoved = nft
}) {
Label(isSpamNFT(nft.token) ? Strings.Wallet.nftUnspam : Strings.Wallet.nftMoveToSpam, braveSystemImage: "leo.disable.outline")
Label(Strings.Wallet.nftRemoveFromWallet, braveSystemImage: "leo.trash")
}
}
}
Expand Down Expand Up @@ -301,6 +332,38 @@ struct NFTView: View {
}
)
)
.background(
WalletPromptView(
isPresented: Binding(
get: { nftToBeRemoved != nil },
set: { if !$0 { nftToBeRemoved = nil } }
),
primaryButton: .init(
title: Strings.Wallet.manageSiteConnectionsConfirmAlertRemove,
action: { _ in
guard let nft = nftToBeRemoved else { return }
nftStore.updateNFTStatus(nft.token, visible: false, isSpam: nft.token.isSpam, isDeletedByUser: true)
nftToBeRemoved = nil
}
),
secondaryButton: .init(
title: Strings.CancelString,
action: { _ in
nftToBeRemoved = nil
}
),
showCloseButton: false,
content: {
VStack(spacing: 16) {
Text(Strings.Wallet.nftRemoveFromWalletAlertTitle)
.font(.headline)
.foregroundColor(Color(.bravePrimary))
Text(Strings.Wallet.nftRemoveFromWalletAlertDescription)
.font(.footnote)
.foregroundStyle(Color(.secondaryBraveLabel))
}
})
)
.sheet(isPresented: $isShowingAddCustomNFT) {
AddCustomAssetView(
networkStore: networkStore,
Expand All @@ -321,22 +384,6 @@ struct NFTView: View {
}
}
}

private func isSpamNFT(_ nft: BraveWallet.BlockchainToken) -> Bool {
if nftStore.displayType == .spam {
return true
} else {
return nft.isSpam
}
}

private func isHiddenNFT(_ nft: BraveWallet.BlockchainToken) -> Bool {
if nftStore.displayType == .spam {
return false
} else {
return !nft.visible
}
}
}

#if DEBUG
Expand Down
2 changes: 1 addition & 1 deletion Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
let tokens: [BraveWallet.BlockchainToken]
let sortOrder: Int
}
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: networksForAccount, includingSpam: true)
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: networksForAccount, includingUserDeleted: true)
let allTokens = await blockchainRegistry.allTokens(in: networksForAccountCoin).flatMap(\.tokens)
var updatedUserAssets: [AssetViewModel] = []
var updatedUserNFTs: [NFTAssetViewModel] = []
Expand Down
2 changes: 1 addition & 1 deletion Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ class AssetDetailStore: ObservableObject, WalletObserverStore {
) async -> [TransactionSummary] {
guard case let .blockchainToken(token) = assetDetailType
else { return [] }
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingSpam: true).flatMap { $0.tokens }
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens }
let allTokens = await blockchainRegistry.allTokens(network.chainId, coin: network.coin)
let allTransactions = await withTaskGroup(of: [BraveWallet.TransactionInfo].self) { @MainActor group -> [BraveWallet.TransactionInfo] in
for account in keyring.accountInfos {
Expand Down
37 changes: 22 additions & 15 deletions Sources/BraveWallet/Crypto/Stores/NFTStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
case .visible:
return userNFTs.filter(\.token.visible)
case .hidden:
return userNFTs.filter { !$0.token.visible && !$0.token.isSpam }
case .spam:
return userNFTs.filter(\.token.isSpam)
return userNFTs.filter { !$0.token.visible }
}
}
/// All User Accounts
Expand Down Expand Up @@ -75,7 +73,6 @@ public class NFTStore: ObservableObject, WalletObserverStore {
enum NFTDisplayType: Int, CaseIterable, Identifiable {
case visible
case hidden
case spam

var id: Int {
rawValue
Expand All @@ -84,11 +81,9 @@ public class NFTStore: ObservableObject, WalletObserverStore {
var dropdownTitle: String {
switch self {
case .visible:
return Strings.Wallet.nftsTitle
return Strings.Wallet.nftCollected
case .hidden:
return Strings.Wallet.nftHidden
case .spam:
return Strings.Wallet.nftSpam
}
}

Expand All @@ -98,16 +93,14 @@ public class NFTStore: ObservableObject, WalletObserverStore {
return Strings.Wallet.nftPageEmptyTitle
case .hidden:
return Strings.Wallet.nftInvisiblePageEmptyTitle
case .spam:
return Strings.Wallet.nftSpamPageEmptyTitle
}
}

var emptyDescription: String? {
switch self {
case .visible:
return Strings.Wallet.nftPageEmptyDescription
case .hidden, .spam:
case .hidden:
return nil
}
}
Expand Down Expand Up @@ -344,17 +337,21 @@ public class NFTStore: ObservableObject, WalletObserverStore {
selectedAccounts: [BraveWallet.AccountInfo],
simpleHashSpamNFTs: [NetworkAssets]
) -> [NetworkAssets] {
// all user marked deleted NFTs
let allUserMarkedDeletedNFTs = assetManager.getAllUserDeletedNFTs()
// all spam NFTs marked by user
let allUserMarkedSpamNFTs = assetManager.getAllUserNFTs(networks: selectedNetworks, isSpam: true)
// filter out any spam NFTs from `simpleHashSpamNFTs` that are marked
// not-spam by user
// not-spam or deleted by user
var updatedSimpleHashSpamNFTs: [NetworkAssets] = []
for simpleHashSpamNFTsOnNetwork in simpleHashSpamNFTs {
let userMarkedNotSpamTokensOnNetwork = assetManager.getAllUserNFTs(networks: [simpleHashSpamNFTsOnNetwork.network], isSpam: false).flatMap(\.tokens)
let filteredSimpleHashSpamTokens = simpleHashSpamNFTsOnNetwork.tokens.filter { simpleHashSpamToken in
!userMarkedNotSpamTokensOnNetwork.contains { token in
return !userMarkedNotSpamTokensOnNetwork.contains { token in
token.id == simpleHashSpamToken.id
}
} && !allUserMarkedDeletedNFTs.contains(where: { deletedNFT in
deletedNFT.contractAddress == simpleHashSpamToken.contractAddress && deletedNFT.chainId == simpleHashSpamToken.chainId && deletedNFT.tokenId == simpleHashSpamToken.tokenId
})
}
updatedSimpleHashSpamNFTs.append(NetworkAssets(network: simpleHashSpamNFTsOnNetwork.network, tokens: filteredSimpleHashSpamTokens, sortOrder: simpleHashSpamNFTsOnNetwork.sortOrder))
}
Expand Down Expand Up @@ -387,8 +384,18 @@ public class NFTStore: ObservableObject, WalletObserverStore {
walletService.setNftDiscoveryEnabled(true)
}

func updateNFTStatus(_ token: BraveWallet.BlockchainToken, visible: Bool, isSpam: Bool) {
assetManager.updateUserAsset(for: token, visible: visible, isSpam: isSpam) { [weak self] in
func updateNFTStatus(
_ token: BraveWallet.BlockchainToken,
visible: Bool,
isSpam: Bool,
isDeletedByUser: Bool
) {
assetManager.updateUserAsset(
for: token,
visible: visible,
isSpam: isSpam,
isDeletedByUser: isDeletedByUser
) { [weak self] in
guard let self else { return }
let selectedAccounts = self.filters.accounts.filter(\.isSelected).map(\.model)
let selectedNetworks = self.filters.networks.filter(\.isSelected).map(\.model)
Expand Down
6 changes: 3 additions & 3 deletions Sources/BraveWallet/Crypto/Stores/TransactionConfirmationStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ public class TransactionConfirmationStore: ObservableObject, WalletObserverStore
let transactionNetworks: [BraveWallet.NetworkInfo] = Set(allTxs.map(\.chainId))
.compactMap { chainId in allNetworks.first(where: { $0.chainId == chainId }) }
for network in transactionNetworks {
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingSpam: true).flatMap { $0.tokens }
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens }
await fetchAssetRatios(for: userAssets)
}
await fetchUnknownTokens(for: unapprovedTxs)
Expand Down Expand Up @@ -307,7 +307,7 @@ public class TransactionConfirmationStore: ObservableObject, WalletObserverStore
return
}
let allTokens = await blockchainRegistry.allTokens(network.chainId, coin: coin) + tokenInfoCache.map(\.value)
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingSpam: true).flatMap { $0.tokens }
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens }
let solEstimatedTxFee: UInt64? = solEstimatedTxFeeCache[transaction.id]

if transaction.isEIP1559Transaction {
Expand Down Expand Up @@ -431,7 +431,7 @@ public class TransactionConfirmationStore: ObservableObject, WalletObserverStore
guard let network = allNetworks.first(where: { $0.chainId == BraveWallet.MainnetChainId }) else {
return
}
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingSpam: true).flatMap { $0.tokens }
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens }
let allTokens = await blockchainRegistry.allTokens(network.chainId, coin: network.coin)
let unknownTokenContractAddresses = mainnetTransactions.flatMap(\.tokenContractAddresses)
.filter { contractAddress in
Expand Down
2 changes: 1 addition & 1 deletion Sources/BraveWallet/Crypto/Stores/TransactionDetailsStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class TransactionDetailsStore: ObservableObject, WalletObserverStore {
let keringId = BraveWallet.KeyringId.keyringId(for: coin, on: transaction.chainId)
let keyring = await keyringService.keyringInfo(keringId)
var allTokens: [BraveWallet.BlockchainToken] = await blockchainRegistry.allTokens(network.chainId, coin: network.coin) + tokenInfoCache.map(\.value)
let userAssets: [BraveWallet.BlockchainToken] = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingSpam: true).flatMap { $0.tokens }
let userAssets: [BraveWallet.BlockchainToken] = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens }
let unknownTokenContractAddresses = transaction.tokenContractAddresses
.filter { contractAddress in
!userAssets.contains(where: { $0.contractAddress(in: network).caseInsensitiveCompare(contractAddress) == .orderedSame })
Expand Down
2 changes: 1 addition & 1 deletion Sources/BraveWallet/Crypto/Stores/TransactionsActivityStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
let allTransactions = await txService.allTransactions(
networksForCoin: networksForCoin, for: allKeyrings
).filter { $0.txStatus != .rejected }
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworksAllCoins, includingSpam: true).flatMap(\.tokens)
let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworksAllCoins, includingUserDeleted: true).flatMap(\.tokens)
let allTokens = await blockchainRegistry.allTokens(
in: allNetworksAllCoins
).flatMap(\.tokens)
Expand Down
10 changes: 5 additions & 5 deletions Sources/BraveWallet/Crypto/Stores/UserAssetsStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class AssetStore: ObservableObject, Equatable, WalletObserverStore {
@Published var token: BraveWallet.BlockchainToken
@Published var isVisible: Bool {
didSet {
assetManager.updateUserAsset(for: token, visible: isVisible, isSpam: false, completion: nil)
assetManager.updateUserAsset(for: token, visible: isVisible, isSpam: false, isDeletedByUser: false, completion: nil)
}
}
var network: BraveWallet.NetworkInfo
Expand Down Expand Up @@ -137,7 +137,7 @@ public class UserAssetsStore: ObservableObject, WalletObserverStore {
}
}
let networks: [BraveWallet.NetworkInfo] = self.networkFilters.filter(\.isSelected).map(\.model)
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: networks, includingSpam: true)
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: networks, includingUserDeleted: false)
var allTokens = await self.blockchainRegistry.allTokens(in: networks)
// Filter `allTokens` to remove any tokens existing in `allUserAssets`. This is possible for ERC721 tokens in the registry without a `tokenId`, which requires the user to add as a custom token
let allUserTokens = allUserAssets.flatMap(\.tokens)
Expand Down Expand Up @@ -183,7 +183,7 @@ public class UserAssetsStore: ObservableObject, WalletObserverStore {
_ asset: BraveWallet.BlockchainToken,
completion: @escaping (_ success: Bool) -> Void
) {
if assetManager.getUserAsset(asset) != nil {
if let existedAsset = assetManager.getUserAsset(asset), !existedAsset.isDeletedByUser {
completion(false)
} else {
assetManager.addUserAsset(asset) { [weak self] in
Expand Down Expand Up @@ -233,7 +233,7 @@ public class UserAssetsStore: ObservableObject, WalletObserverStore {

@MainActor func allAssets() async -> [AssetViewModel] {
let allNetworks = await rpcService.allNetworksForSupportedCoins()
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworks, includingSpam: true)
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworks, includingUserDeleted: false)
// Filter `allTokens` to remove any tokens existing in `allUserAssets`. This is possible for ERC721 tokens in the registry without a `tokenId`, which requires the user to add as a custom token
let allUserTokens = allUserAssets.flatMap(\.tokens)
let allBlockchainTokens = await blockchainRegistry.allTokens(in: allNetworks)
Expand Down Expand Up @@ -262,7 +262,7 @@ public class UserAssetsStore: ObservableObject, WalletObserverStore {

@MainActor func allNFTMetadata() async -> [String: NFTMetadata] {
let allNetworks = await rpcService.allNetworksForSupportedCoins()
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworks, includingSpam: true)
let allUserAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworks, includingUserDeleted: true)
// Filter `allTokens` to remove any tokens existing in `allUserAssets`. This is possible for ERC721 tokens in the registry without a `tokenId`, which requires the user to add as a custom token
let allUserTokens = allUserAssets.flatMap(\.tokens)

Expand Down
7 changes: 7 additions & 0 deletions Sources/BraveWallet/Extensions/WalletColors.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,4 +250,11 @@ enum WalletV2Design {
static let passwordMediumYellow = UIColor(rgb: 0xbd9600)

static let passwordStrongGreen = UIColor(rgb: 0x31803e)

static let spamNFTLabelBackground = UIColor(
red: 1,
green: 209 / 255,
blue: 207 / 255,
alpha: 1
)
}
Loading

0 comments on commit 1f48b0d

Please sign in to comment.