Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ABW-3569, ABW-3586] Asset transfer & Transaction review Presenting section UI updates #1214

Merged
merged 13 commits into from
Jul 17, 2024
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

extension OnLedgerEntity.NonFungibleToken {
public init(resourceAddress: ResourceAddress, nftID: NonFungibleLocalId, nftData: NFTData?) throws {
public init(resourceAddress: ResourceAddress, nftID: NonFungibleLocalId, nftData: NFTData?) {
self.init(
id: NonFungibleGlobalID(
resourceAddress: resourceAddress,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public struct ResourceBalanceView: View {
public let isSelected: Bool?
public let action: (() -> Void)?

public enum Appearance: Equatable {
public enum Appearance: Sendable, Equatable {
case standard
case compact(border: Bool)

Expand Down Expand Up @@ -241,8 +241,8 @@ extension ResourceBalanceView {
VStack(alignment: .leading, spacing: .medium3) {
FungibleView(
thumbnail: .lsu(viewState.icon),
caption1: viewState.title,
caption2: viewState.validatorName,
caption1: viewState.title ?? "-",
caption2: viewState.validatorName ?? "-",
danvleju-rdx marked this conversation as resolved.
Show resolved Hide resolved
fallback: nil,
amount: viewState.amount,
compact: compact,
Expand Down Expand Up @@ -273,8 +273,8 @@ extension ResourceBalanceView {
VStack(alignment: .leading, spacing: .zero) {
FungibleView(
thumbnail: .poolUnit(viewState.poolIcon),
caption1: viewState.poolName,
caption2: viewState.dAppName.wrappedValue?.flatMap { $0 },
caption1: viewState.poolName ?? "-",
caption2: viewState.dAppName.wrappedValue?.flatMap { $0 } ?? "-",
fallback: nil,
amount: viewState.amount,
compact: compact,
Expand Down Expand Up @@ -314,8 +314,8 @@ extension ResourceBalanceView {
VStack(alignment: .leading, spacing: .zero) {
NonFungibleView(
thumbnail: .stakeClaimNFT(viewState.resourceMetadata.iconURL),
caption1: viewState.resourceMetadata.title ?? "",
caption2: viewState.validatorName ?? "",
caption1: viewState.resourceMetadata.title,
caption2: viewState.validatorName,
compact: compact
)

Expand Down Expand Up @@ -457,17 +457,15 @@ extension ResourceBalanceView {
compact: compact
)

if useSpacer {
if useSpacer, isSelected == nil {
Spacer(minLength: .small2)
}

AmountView(amount: amount, fallback: fallback, compact: compact)
.padding(.leading, isSelected != nil ? .small2 : 0)

if let isSelected {
if !useSpacer, caption1 == nil {
Spacer(minLength: .small2)
}
Spacer(minLength: .small2)
CheckmarkView(appearance: .dark, isChecked: isSelected)
}
}
Expand Down Expand Up @@ -497,8 +495,8 @@ extension ResourceBalanceView {
CaptionedThumbnailView(
type: thumbnail.type,
url: thumbnail.url,
caption1: caption1,
caption2: caption2,
caption1: caption1 ?? "-",
caption2: caption2 ?? "-",
compact: compact
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ extension TransactionReview {
networkID: networkID
)

let proofs: TransactionReviewProofs.State? = try await exctractProofs(summary.presentedProofs.map(\.resourceAddress))
let proofs: TransactionReviewProofs.State? = try await exctractProofs(summary.presentedProofs)

return Sections(
withdrawals: withdrawals,
Expand Down Expand Up @@ -344,20 +344,45 @@ extension TransactionReview {
return DappEntity(id: dAppDefinitionAddress, metadata: metadata)
}

private func exctractProofs(_ accountProofs: [ResourceAddress]) async throws -> TransactionReviewProofs.State? {
private func exctractProofs(_ accountProofs: [ResourceSpecifier]) async throws -> TransactionReviewProofs.State? {
let proofs = try await accountProofs
.uniqued()
.asyncMap(extractResourceBalanceInfo)
.flatMap { $0 }
.asyncMap(extractProofInfo)

guard !proofs.isEmpty else { return nil }

return TransactionReviewProofs.State(proofs: proofs.asIdentified())
}

private func extractProofInfo(_ address: ResourceAddress) async throws -> ProofEntity {
private func extractResourceBalanceInfo(specifier: ResourceSpecifier) async throws -> [(ResourceAddress, ResourceBalance.Details)] {
switch specifier {
case let .fungible(resourceAddress, amount):
return [(
resourceAddress,
.fungible(
.init(
isXRD: resourceAddress.isXRD,
amount: .init(nominalAmount: amount)
)
)
)]
case let .nonFungible(resourceAddress, ids):
let globalIds = ids.map { NonFungibleGlobalId(resourceAddress: resourceAddress, nonFungibleLocalId: $0) }
let tokens = try await onLedgerEntitiesClient.getNonFungibleTokenData(
.init(resource: resourceAddress, nonFungibleIds: globalIds)
)
return tokens.map { (resourceAddress, .nonFungible($0)) }
}
}

private func extractProofInfo(resourceAddress: ResourceAddress, details: ResourceBalance.Details) async throws -> ProofEntity {
try await ProofEntity(
id: address,
metadata: onLedgerEntitiesClient.getResource(address, metadataKeys: .dappMetadataKeys).metadata
resourceBalance: ResourceBalance(
resource: onLedgerEntitiesClient.getResource(resourceAddress, metadataKeys: .dappMetadataKeys),
details: details
)
)
}

Expand Down
109 changes: 59 additions & 50 deletions RadixWallet/Features/TransactionReviewFeature/TransactionReview.swift
Original file line number Diff line number Diff line change
Expand Up @@ -356,54 +356,7 @@ public struct TransactionReview: Sendable, FeatureReducer {
switch childAction {
case let .withdrawals(.delegate(.showAsset(transfer, token))),
let .deposits(.delegate(.showAsset(transfer, token))):
switch transfer.details {
case let .fungible(details):
state.destination = .fungibleTokenDetails(
.init(
resourceAddress: transfer.resource.resourceAddress,
resource: .success(transfer.resource),
ownedFungibleResource: .init(
resourceAddress: transfer.resource.resourceAddress,
atLedgerState: transfer.resource.atLedgerState,
amount: details.amount,
metadata: transfer.resource.metadata
),
isXRD: details.isXRD
)
)

case let .nonFungible(details):
state.destination = .nonFungibleTokenDetails(.init(
resourceAddress: transfer.resource.resourceAddress,
resourceDetails: .success(transfer.resource),
token: details,
ledgerState: transfer.resource.atLedgerState
))

case let .liquidStakeUnit(details):
state.destination = .lsuDetails(.init(
validator: details.validator,
stakeUnitResource: .init(resource: details.resource, amount: .init(nominalAmount: details.amount)),
xrdRedemptionValue: details.worth
))

return .none

case let .poolUnit(details):
state.destination = .poolUnitDetails(.init(resourcesDetails: details.details))

case let .stakeClaimNFT(details):
state.destination = .nonFungibleTokenDetails(.init(
resourceAddress: transfer.resource.resourceAddress,
resourceDetails: .success(transfer.resource),
token: token,
ledgerState: transfer.resource.atLedgerState,
stakeClaim: details.stakeClaimTokens.stakeClaims.first,
isClaimStakeEnabled: false
))
}

return .none
return resourceDetailsEffect(state: &state, resource: transfer.resource, details: transfer.details, nft: token)

case let .dAppsUsed(.delegate(.openDapp(dAppID))), let .contributingToPools(.delegate(.openDapp(dAppID))), let .redeemingFromPools(.delegate(.openDapp(dAppID))):
state.destination = .dApp(.init(dAppDefinitionAddress: dAppID))
Expand Down Expand Up @@ -431,6 +384,10 @@ public struct TransactionReview: Sendable, FeatureReducer {

return .none

case let .proofs(.delegate(.showAsset(proof))):
let resource = proof.resourceBalance.resource
return resourceDetailsEffect(state: &state, resource: resource, details: proof.resourceBalance.details)

case .networkFee(.delegate(.showCustomizeFees)):
guard let reviewedTransaction = state.reviewedTransaction else {
return .none
Expand Down Expand Up @@ -731,6 +688,58 @@ extension TransactionReview {
}
}
}

func resourceDetailsEffect(
state: inout State,
resource: OnLedgerEntity.Resource,
details: ResourceBalance.Details,
nft: OnLedgerEntity.NonFungibleToken? = nil
) -> Effect<Action> {
switch details {
case let .fungible(details):
state.destination = .fungibleTokenDetails(.init(
resourceAddress: resource.resourceAddress,
resource: .success(resource),
ownedFungibleResource: .init(
resourceAddress: resource.resourceAddress,
atLedgerState: resource.atLedgerState,
amount: details.amount,
metadata: resource.metadata
),
isXRD: details.isXRD
))

case let .nonFungible(details):
state.destination = .nonFungibleTokenDetails(.init(
resourceAddress: resource.resourceAddress,
resourceDetails: .success(resource),
token: details,
ledgerState: resource.atLedgerState
))

case let .liquidStakeUnit(details):
state.destination = .lsuDetails(.init(
validator: details.validator,
stakeUnitResource: .init(resource: details.resource, amount: .init(nominalAmount: details.amount)),
xrdRedemptionValue: details.worth
))

case let .poolUnit(details):
state.destination = .poolUnitDetails(.init(resourcesDetails: details.details))

case let .stakeClaimNFT(details):
state.destination = .nonFungibleTokenDetails(.init(
resourceAddress: resource.resourceAddress,
resourceDetails: .success(resource),
token: nft,
ledgerState: resource.atLedgerState,
stakeClaim: details.stakeClaimTokens.stakeClaims.first,
isClaimStakeEnabled: false
))
}

return .none
}
}

// MARK: - FailedToAddLockFee
Expand Down Expand Up @@ -783,8 +792,8 @@ extension TransactionReview {

extension TransactionReview {
public struct ProofEntity: Sendable, Identifiable, Hashable {
public let id: ResourceAddress
public let metadata: OnLedgerEntity.Metadata
public var id: ResourceBalance { resourceBalance }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we check a simpler identifier (such as a String) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure if we could use ResourceAddress since there could be multiple NFTs with the same resource address.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, we cannot use ResourceAddress. However, using ResourceBalance as the id could be quite expensive I guess on long lists.

We are already doing it on places with more elements, so I wouldn't say you need to fix it for this (that won't have too many NFTs as in the Assets View).

public let resourceBalance: ResourceBalance
}

public struct DappEntity: Sendable, Identifiable, Hashable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,42 +29,15 @@ extension TransactionReviewProofs {
.padding(.bottom, .medium2)

ForEach(viewStore.proofs) { proof in
VStack(spacing: 0) {
let metadata = proof.metadata
ProofView(thumbnail: metadata.iconURL, name: metadata.name) {
viewStore.send(.proofTapped(id: proof.id))
VStack(spacing: .medium3) {
ResourceBalanceView(proof.resourceBalance.viewState, appearance: .compact) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are adding a VStack for each proof, which is pointless. Need to revert the order, as shown in previous comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right!

viewStore.send(.proofTapped(proof))
}
.padding(.bottom, .medium3)

if proof.id != viewStore.proofs.last?.id {
Separator()
.padding(.bottom, .medium3)
}
}
}
}
}
}

struct ProofView: SwiftUI.View {
let thumbnail: URL?
let name: String?
let action: () -> Void

var body: some SwiftUI.View {
Button(action: action) {
HStack(spacing: 0) {
Thumbnail(.dapp, url: thumbnail, size: .smallest)
.padding(.trailing, .small1)

if let name {
Text(name)
.textStyle(.body1HighImportance)
.foregroundColor(.app.gray1)
}

Spacer(minLength: 0)
.padding(.bottom, .medium3)
}
Separator()
.padding(.bottom, .medium3)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ public struct TransactionReviewProofs: Sendable, FeatureReducer {

public enum ViewAction: Sendable, Equatable {
case infoTapped
case proofTapped(id: TransactionReview.ProofEntity.ID)
case proofTapped(TransactionReview.ProofEntity)
}

public enum DelegateAction: Sendable, Equatable {
case showAsset(TransactionReview.ProofEntity)
}

public init() {}
Expand All @@ -22,9 +26,8 @@ public struct TransactionReviewProofs: Sendable, FeatureReducer {
switch viewAction {
case .infoTapped:
.none
case let .proofTapped(id):
// FIXME: Handle tap
.none
case let .proofTapped(proof):
.send(.delegate(.showAsset(proof)))
}
}
}
Loading