-
Notifications
You must be signed in to change notification settings - Fork 9
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
Token transfer assets elements #502
Token transfer assets elements #502
Conversation
...eatures/AssetTransferFeature/Components/AssetTransferMessage/AssetTransferMessage+View.swift
Outdated
Show resolved
Hide resolved
Sources/Features/AssetTransferFeature/Components/ChooseAccount/ChooseAccount+Reducer.swift
Outdated
Show resolved
Hide resolved
...re/Components/TransferAccountList/ReceivingAccount/Asset/FungibleResourceAsset+Reducer.swift
Outdated
Show resolved
Hide resolved
...re/Components/TransferAccountList/ReceivingAccount/Asset/FungibleResourceAsset+Reducer.swift
Show resolved
Hide resolved
...re/Components/TransferAccountList/ReceivingAccount/Asset/FungibleResourceAsset+Reducer.swift
Show resolved
Hide resolved
...re/Components/TransferAccountList/ReceivingAccount/Asset/FungibleResourceAsset+Reducer.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly related to this PR:
Now with your nice RoundedCorners implementation, we should get rid of the bad old version that Nikola added. I polished it up before, but it's time to get rid of it. The only good thing was that it used UIRectEdge, and we're probably always interested in pairs of corners.
I would extend UIRectCorner with bottom
and top
, switch the order of the arguments in your modifier, remove the label for corners
and then get rid of .bottom
- and and .topRoundedCorners
. That would put it inline with .padding.
Something like
func roundedCorners(_ corners: UIRectCorner = .allCorners, fillColor: Color = .app.white, strokeColor: Color = .clear) -> some View
Also not sure what self.modifier
... does.
Looks great. I would probably put the total sum error message below the box, and also ideally make the box border red. |
...ature/Components/TransferAccountList/ReceivingAccount/Asset/FungibleResourceAsset+View.swift
Outdated
Show resolved
Hide resolved
...nents/TransferAccountList/ReceivingAccount/Asset/NonFungibleResourceAsset+Reducer+View.swift
Outdated
Show resolved
Hide resolved
...nents/TransferAccountList/ReceivingAccount/Asset/NonFungibleResourceAsset+Reducer+View.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
...ansferFeature/Components/TransferAccountList/ReceivingAccount/Asset/ResourceAsset+View.swift
Show resolved
Hide resolved
...ansferFeature/Components/TransferAccountList/ReceivingAccount/Asset/ResourceAsset+View.swift
Show resolved
Hide resolved
...ansferFeature/Components/TransferAccountList/ReceivingAccount/ReceivingAccount+Reducer.swift
Outdated
Show resolved
Hide resolved
...tTransferFeature/Components/TransferAccountList/ReceivingAccount/ReceivingAccount+View.swift
Show resolved
Hide resolved
...tTransferFeature/Components/TransferAccountList/ReceivingAccount/ReceivingAccount+View.swift
Outdated
Show resolved
Hide resolved
...ansferFeature/Components/TransferAccountList/ReceivingAccount/ReceivingAccount+Reducer.swift
Show resolved
Hide resolved
...atures/AssetTransferFeature/Components/TransferAccountList/TransferAccountList+Reducer.swift
Outdated
Show resolved
Hide resolved
...atures/AssetTransferFeature/Components/TransferAccountList/TransferAccountList+Reducer.swift
Outdated
Show resolved
Hide resolved
...atures/AssetTransferFeature/Components/TransferAccountList/TransferAccountList+Reducer.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent overall, just some minor comments, mostly about unifying with views from TransactionReview
Jira ticket: paste link here
Description
Asset elements for the token transfer.
Screenshot