Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
addressed code review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@nuo-xu
nuo-xu committed Oct 23, 2023
1 parent cbd7d4d commit 68f866a
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 84 deletions.
77 changes: 32 additions & 45 deletions Sources/BraveWallet/Crypto/NFT/NFTView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
37 changes: 3 additions & 34 deletions Sources/BraveWallet/Crypto/Portfolio/PortfolioAssetsView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
}
31 changes: 31 additions & 0 deletions Sources/BraveWallet/Crypto/Portfolio/PortfolioView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions Sources/BraveWallet/Crypto/Stores/NFTStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
15 changes: 10 additions & 5 deletions Sources/BraveWallet/Crypto/WalletDisclosureGroup.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -44,18 +45,22 @@ struct WalletDisclosureGroup<Label: View, Content: View>: View {
if isExpanded {
Divider()
.padding(.top, 6)
.padding(.horizontal, 8)
.padding(.horizontal, isNFTGroup ? nil : 8)
content()
.padding(.horizontal)
}
}
// 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
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 @@ -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
)
}

0 comments on commit 68f866a

Please sign in to comment.