Skip to content

Commit

Permalink
Fix brave/brave-ios#8560, brave/brave-ios#8542, brave/brave-ios#8558:…
Browse files Browse the repository at this point in the history 
… Two NFT UI bugs and one caching issue (brave/brave-ios#8636)
  • Loading branch information
@nuo-xu
nuo-xu authored Jan 10, 2024
1 parent b25a3cb commit 24aac02
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 81 deletions.
25 changes: 12 additions & 13 deletions Sources/BraveWallet/Crypto/NFT/NFTDetailView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct NFTDetailView: View {
}

@ViewBuilder private var nftLogo: some View {
if let image = nftDetailStore.networkInfo.nativeTokenLogoImage, !nftDetailStore.isLoading {
if let image = nftDetailStore.networkInfo?.nativeTokenLogoImage, !nftDetailStore.isLoading {
Image(uiImage: image)
.resizable()
.frame(width: 32, height: 32)
Expand Down Expand Up @@ -103,10 +103,10 @@ struct NFTDetailView: View {
Text(nftDetailStore.nft.name)
.foregroundColor(Color(.secondaryBraveLabel))
}
.transaction { transaction in
transaction.animation = nil
transaction.disablesAnimations = true
}
}
.transaction { transaction in
transaction.animation = nil
transaction.disablesAnimations = true
}
.listRowInsets(.zero)
.listRowBackground(Color.clear)
Expand Down Expand Up @@ -135,7 +135,7 @@ struct NFTDetailView: View {
}
NFTDetailRow(title: nftDetailStore.nft.isErc721 ? Strings.Wallet.contractAddressAccessibilityLabel : Strings.Wallet.tokenMintAddress) {
Button {
if let url = nftDetailStore.networkInfo.nftBlockExplorerURL(nftDetailStore.nft) {
if let url = nftDetailStore.networkInfo?.nftBlockExplorerURL(nftDetailStore.nft) {
openWalletURL(url)
}
} label: {
Expand All @@ -147,10 +147,12 @@ struct NFTDetailView: View {
.foregroundColor(Color(.braveBlurpleTint))
}
}
NFTDetailRow(title: Strings.Wallet.nftDetailBlockchain) {
Text(nftDetailStore.networkInfo.chainName)
.font(.subheadline)
.foregroundColor(Color(.braveLabel))
if let networkInfo = nftDetailStore.networkInfo {
NFTDetailRow(title: Strings.Wallet.nftDetailBlockchain) {
Text(networkInfo.chainName)
.font(.subheadline)
.foregroundColor(Color(.braveLabel))
}
}
NFTDetailRow(title: Strings.Wallet.nftDetailTokenStandard) {
Text(nftDetailStore.nft.isErc721 ? Strings.Wallet.nftDetailERC721 : Strings.Wallet.nftDetailSPL)
Expand Down Expand Up @@ -208,9 +210,6 @@ struct NFTDetailView: View {
isPresentingRemoveAlert = false
}
})
.onAppear {
nftDetailStore.update()
}
.background(Color(UIColor.braveGroupedBackground).ignoresSafeArea())
.navigationBarTitle(nftDetailStore.nft.nftDetailTitle)
.toolbar {
Expand Down
24 changes: 16 additions & 8 deletions Sources/BraveWallet/Crypto/NFT/NFTView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ struct NFTView: View {
}
.padding(.horizontal)
.frame(maxWidth: .infinity, alignment: .leading)
.transaction { transaction in
transaction.animation = nil
transaction.disablesAnimations = true
}
}

private var addCustomAssetButton: some View {
Expand Down Expand Up @@ -296,24 +300,28 @@ struct NFTView: View {
NavigationLink(
isActive: Binding(
get: { selectedNFTViewModel != nil },
set: { if !$0 { selectedNFTViewModel = nil } }
set: {
if !$0 {
if let viewModel = selectedNFTViewModel {
cryptoStore.closeNFTDetailStore(for: viewModel.token)
}
selectedNFTViewModel = nil
}
}
),
destination: {
if let nftViewModel = selectedNFTViewModel {
if let selectedNFTViewModel {
NFTDetailView(
keyringStore: keyringStore,
nftDetailStore: cryptoStore.nftDetailStore(for: nftViewModel.token, nftMetadata: nftViewModel.nftMetadata, owner: nftStore.owner(for: nftViewModel.token)),
nftDetailStore: cryptoStore.nftDetailStore(for: selectedNFTViewModel.token, nftMetadata: selectedNFTViewModel.nftMetadata, owner: nftStore.owner(for: selectedNFTViewModel.token)),
buySendSwapDestination: buySendSwapDestination,
onNFTMetadataRefreshed: { nftMetadata in
nftStore.updateNFTMetadataCache(for: nftViewModel.token, metadata: nftMetadata)
onNFTMetadataRefreshed: { nftMetadata in
nftStore.updateNFTMetadataCache(for: selectedNFTViewModel.token, metadata: nftMetadata)
},
onNFTStatusUpdated: {
nftStore.update()
}
)
.onDisappear {
cryptoStore.closeNFTDetailStore(for: nftViewModel.token)
}
}
},
label: {
Expand Down
2 changes: 1 addition & 1 deletion Sources/BraveWallet/Crypto/NFTImageView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ struct LoadingNFTView: View {
.preference(key: SizePreferenceKey.self, value: geometryProxy.size)
}
)
.frame(height: viewSize.width)
.aspectRatio(1, contentMode: .fit)
.onPreferenceChange(SizePreferenceKey.self) { newSize in
viewSize = newSize
}
Expand Down
4 changes: 3 additions & 1 deletion Sources/BraveWallet/Crypto/Stores/CryptoStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
assetRatioService: assetRatioService,
blockchainRegistry: blockchainRegistry,
ipfsApi: ipfsApi,
userAssetManager: userAssetManager
userAssetManager: userAssetManager,
txService: txService
)
self.transactionsActivityStore = .init(
keyringService: keyringService,
Expand Down Expand Up @@ -527,6 +528,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
assetManager: userAssetManager,
keyringService: keyringService,
rpcService: rpcService,
txService: txService,
ipfsApi: ipfsApi,
nft: nft,
nftMetadata: nftMetadata,
Expand Down
98 changes: 67 additions & 31 deletions Sources/BraveWallet/Crypto/Stores/NFTDetailStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,24 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
private let assetManager: WalletUserAssetManagerType
private let keyringService: BraveWalletKeyringService
private let rpcService: BraveWalletJsonRpcService
private let txService: BraveWalletTxService
private let ipfsApi: IpfsAPI
private var txServiceObserver: TxServiceObserver?
@Published var owner: BraveWallet.AccountInfo?
@Published var nft: BraveWallet.BlockchainToken
@Published var isLoading: Bool = false
@Published var nftMetadata: NFTMetadata?
@Published var networkInfo: BraveWallet.NetworkInfo = .init()
@Published var networkInfo: BraveWallet.NetworkInfo?

var isObserving: Bool = false
var isObserving: Bool {
txServiceObserver != nil
}

init(
assetManager: WalletUserAssetManagerType,
keyringService: BraveWalletKeyringService,
rpcService: BraveWalletJsonRpcService,
txService: BraveWalletTxService,
ipfsApi: IpfsAPI,
nft: BraveWallet.BlockchainToken,
nftMetadata: NFTMetadata?,
Expand All @@ -90,10 +95,30 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
self.assetManager = assetManager
self.keyringService = keyringService
self.rpcService = rpcService
self.txService = txService
self.ipfsApi = ipfsApi
self.nft = nft
self.nftMetadata = nftMetadata?.httpfyIpfsUrl(ipfsApi: ipfsApi)
self.owner = owner

self.setupObservers()

self.update()
}

func setupObservers() {
self.txServiceObserver = TxServiceObserver(
txService: txService,
_onTransactionStatusChanged: { [weak self] txInfo in
if txInfo.txStatus == .confirmed, txInfo.isSend, (txInfo.coin == .eth || txInfo.coin == .sol) {
self?.updateOwner()
}
}
)
}

func tearDown() {
txServiceObserver = nil
}

func update() {
Expand All @@ -104,35 +129,7 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
}

if owner == nil {
let accounts = await keyringService.allAccounts().accounts
let nftBalances: [String: Int] = await withTaskGroup(
of: [String: Int].self,
body: { @MainActor [rpcService, networkInfo, nft] group in
for account in accounts where account.coin == nft.coin {
group.addTask { @MainActor in
let balanceForToken = await rpcService.balance(
for: nft,
in: account,
network: networkInfo
)
return [account.address: Int(balanceForToken ?? 0)]
}
}
return await group.reduce(into: [String: Int](), { partialResult, new in
for key in new.keys {
partialResult[key] = new[key]
}
})
}
)
if let address = nftBalances.first(where: { address, balance in
balance > 0
})?.key,
let account = accounts.first(where: { accountInfo in
accountInfo.address.caseInsensitiveCompare(address) == .orderedSame
}) {
owner = account
}
updateOwner()
}

if nftMetadata == nil {
Expand Down Expand Up @@ -162,4 +159,43 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
completion()
}
}

var updateOwnerTask: Task<(), Never>?
private func updateOwner() {
updateOwnerTask?.cancel()
updateOwnerTask = Task { @MainActor in
guard let network = networkInfo else { return }
let accounts = await keyringService.allAccounts().accounts
let nftBalances: [String: Int] = await withTaskGroup(
of: [String: Int].self,
body: { @MainActor [rpcService, nft] group in
for account in accounts where account.coin == nft.coin {
group.addTask { @MainActor in
let balanceForToken = await rpcService.balance(
for: nft,
in: account,
network: network
)
return [account.address: Int(balanceForToken ?? 0)]
}
}
return await group.reduce(into: [String: Int](), { partialResult, new in
for key in new.keys {
partialResult[key] = new[key]
}
})
}
)
if let address = nftBalances.first(where: { address, balance in
balance > 0
})?.key,
let account = accounts.first(where: { accountInfo in
accountInfo.address.caseInsensitiveCompare(address) == .orderedSame
}) {
owner = account
} else {
owner = nil
}
}
}
}
26 changes: 19 additions & 7 deletions Sources/BraveWallet/Crypto/Stores/NFTStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,11 @@ public class NFTStore: ObservableObject, WalletObserverStore {
private let blockchainRegistry: BraveWalletBlockchainRegistry
private let ipfsApi: IpfsAPI
private let assetManager: WalletUserAssetManagerType
private let txService: BraveWalletTxService
private var rpcServiceObserver: JsonRpcServiceObserver?
private var keyringServiceObserver: KeyringServiceObserver?
private var walletServiveObserber: WalletServiceObserver?
private var walletServiveObserver: WalletServiceObserver?
private var txServiceObserver: TxServiceObserver?

/// Cancellable for the last running `update()` Task.
private var updateTask: Task<(), Never>?
Expand All @@ -159,7 +161,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
}

var isObserving: Bool {
rpcServiceObserver != nil && keyringServiceObserver != nil && walletServiveObserber != nil
rpcServiceObserver != nil && keyringServiceObserver != nil && walletServiveObserver != nil && txServiceObserver != nil
}

var isShowingNFTEmptyState: Bool {
Expand All @@ -180,7 +182,8 @@ public class NFTStore: ObservableObject, WalletObserverStore {
assetRatioService: BraveWalletAssetRatioService,
blockchainRegistry: BraveWalletBlockchainRegistry,
ipfsApi: IpfsAPI,
userAssetManager: WalletUserAssetManagerType
userAssetManager: WalletUserAssetManagerType,
txService: BraveWalletTxService
) {
self.keyringService = keyringService
self.rpcService = rpcService
Expand All @@ -189,6 +192,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
self.blockchainRegistry = blockchainRegistry
self.ipfsApi = ipfsApi
self.assetManager = userAssetManager
self.txService = txService

self.setupObservers()

Expand All @@ -207,7 +211,8 @@ public class NFTStore: ObservableObject, WalletObserverStore {
func tearDown() {
rpcServiceObserver = nil
keyringServiceObserver = nil
walletServiveObserber = nil
walletServiveObserver = nil
txServiceObserver = nil

userAssetsStore.tearDown()
}
Expand All @@ -231,7 +236,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
self?.update()
}
)
self.walletServiveObserber = WalletServiceObserver(
self.walletServiveObserver = WalletServiceObserver(
walletService: walletService,
_onNetworkListChanged: { [weak self] in
// A network was added or removed, `update()` will update `allNetworks`.
Expand All @@ -245,14 +250,21 @@ public class NFTStore: ObservableObject, WalletObserverStore {
// assets update will be called via `CryptoStore`
}
)
self.txServiceObserver = TxServiceObserver(
txService: txService, _onTransactionStatusChanged: { [weak self] txInfo in
if txInfo.txStatus == .confirmed, txInfo.isSend, (txInfo.coin == .eth || txInfo.coin == .sol) {
self?.update(forceUpdateNFTBalances: true)
}
}
)

userAssetsStore.setupObservers()
}

/// Cache of NFT balances for each account tokenBalances: [token.contractAddress]
private var nftBalancesCache: [String: [String: Int]] = [:]

func update() {
func update(forceUpdateNFTBalances: Bool = false) {
self.updateTask?.cancel()
self.updateTask = Task { @MainActor in
self.allAccounts = await keyringService.allAccounts().accounts
Expand Down Expand Up @@ -295,7 +307,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
of: [String: Int].self,
body: { @MainActor group in
for account in allAccounts where account.coin == nft.coin {
if let cachedBalance = nftBalancesCache[nft.id]?[account.address] { // cached balance
if !forceUpdateNFTBalances, let cachedBalance = nftBalancesCache[nft.id]?[account.address] { // cached balance
return [account.address: cachedBalance]
} else { // no balance for this account
group.addTask { @MainActor in
Expand Down
Loading

0 comments on commit 24aac02

Please sign in to comment.