Add new line with wallet name on Transaction Confirmation #866

ghost · 2018-07-30T12:43:28Z

Referencing #862

OlegGordiichuk · 2018-07-30T14:15:16Z

@BloodyPixy could you pls add image of the new UI?

OlegGordiichuk · 2018-07-30T14:20:19Z

Trust/Transfer/ViewModels/ConfirmPaymentDetailsViewModel.swift

+    }
+
+    var currentWalletName: String {
+        return keystore.recentlyUsedWallet?.info.name


We could use keystore.mainWallet instead of the keystore.recentlyUsedWallet it will increase readability of the code. Also could we avoid of "??" it will dramatically increase complication time.

keystore.mainWallet is also optional, so we won't avoid '??', but I agree regarding clarity and will use it instead.

ghost · 2018-07-30T14:44:39Z

@OlegGordiichuk

vikmeup · 2018-07-30T14:50:38Z

@BloodyPixy Ideally it should be: Wallet #3 (0x123...123)

Use your judgment

vikmeup · 2018-07-30T14:51:43Z

@vikmeup Wallet Title => Wallet

ghost · 2018-07-30T14:54:26Z

@vikmeup , I did it this way, because the sender address (current wallet) is included in 'Sender' field. I suggest merging sender and wallet name then. It will look more clear.

vikmeup · 2018-07-30T14:55:31Z

@BloodyPixy For sure! It should be just one field

ghost · 2018-07-30T14:59:13Z

@vikmeup let do it like this:

From Wallet
Wallet Name (address)

vikmeup · 2018-07-30T15:01:27Z

@BloodyPixy 👍 nice!

ghost · 2018-07-30T15:22:20Z

@OlegGordiichuk keystore.mainWallet is not changed, when changing the wallet in settings. As I understand, there is only one mainWallet, at least until it is removed and substituted for another. We need different behavior, so I will use keystore.recentlyUsedWallet. At least, that what is done in other places.

ghost · 2018-07-30T15:29:05Z

Updated appearance

vikmeup

let's just use From as before and avoid introducing new words

Trust/Transfer/ViewModels/ConfirmPaymentDetailsViewModel.swift

vikmeup · 2018-08-01T15:31:20Z

Trust/Transfer/ViewModels/ConfirmPaymentDetailsViewModel.swift

+    var currentWalletDescriptionString: String {
+        return currentWalletName + " " + ("(\(currentWalletAddress))")
+    }
+
    var paymentFromTitle: String {


You should use paymentFromTitle property

vikmeup · 2018-08-01T15:31:37Z

Trust/Transfer/ViewModels/ConfirmPaymentDetailsViewModel.swift

+    }
+
+    private var currentWallet: WalletInfo? {
+        return keystore.recentlyUsedWallet ?? keystore.wallets.first


pass current wallet and then use it here.

…tion * master: Add option to skip import main wallet Disable AuthenticateUserCoordinatorTests Version bump Use valueToSend() Add RPCServer as new parameter of the NonFungibleTokenObject. So now we could open nftokens. (#872) Fix an issue with adding extra ether to each transaction Enable deletion of the main wallet Average USD field update on QR code scanning. (#861)

vikmeup · 2018-08-02T16:52:43Z

Trust/Localization/en.lproj/Localizable.strings

@@ -212,7 +212,7 @@
 "configureTransaction.dataField.label.title" = "Data (Optional). %@";
 "Nonce" = "Nonce";
 "transaction.recipient.label.title" = "Recipient";
-"transaction.sender.label.title" = "Sender";
+"transaction.sender.label.title" = "From";


do not change this.

vikmeup · 2018-08-02T16:52:56Z

Trust/Transfer/ViewModels/ConfirmPaymentDetailsViewModel.swift

    }

    var currentWalletDescriptionString: String {
        return currentWalletName + " " + ("(\(currentWalletAddress))")
    }

    var paymentFromTitle: String {
-        return NSLocalizedString("transaction.sender.label.title", value: "Sender", comment: "")
+        return NSLocalizedString("transaction.sender.label.title", value: "From", comment: "")


same here. Use R.string.localized

vikmeup · 2018-08-02T17:35:12Z

Trust/Localization/de.lproj/Localizable.strings

@@ -216,6 +216,7 @@
 "Are you sure you would like to delete?" = "Sind Sie sicher, dass Sie löschen möchten?";
 "confirmPayment.amount.label.title" = "Betrag";
 "confirmPayment.dapp.label.title" = "DApp";
+"confirmPayment.walletTitle.label.title" = "Aus Brieftasche";


can we remove this? do not pull strings unless

vikmeup · 2018-08-02T17:35:27Z

Trust/Localization/en.lproj/Localizable.strings

 "Are you sure you would like to delete?" = "Are you sure you would like to delete?";
 "confirmPayment.amount.label.title" = "Amount";
 "confirmPayment.dapp.label.title" = "DApp";
+"confirmPayment.walletTitle.label.title" = "From Wallet";


vikmeup · 2018-08-02T18:20:25Z

Trust/Transfer/ViewModels/ConfirmPaymentDetailsViewModel.swift

+        return session.account.address.description
+    }
+
+    var currentWalletDescriptionString: String {


can you combine all in one string? no need to create separate variables.

maybe one test case?

vikmeup · 2018-08-02T23:27:54Z

TrustTests/Transfer/ViewModels/ConfirmPaymentDetailsViewModelTests.swift

+    func testActionButtonTitleOnSignAndSend() {
+        let bigInt = BigInt("11274902618710000000000")!
+
+        let transaction = PreviewTransaction(


make factory for PreviewTransaction, so you don't need to specify so much

…ansactions * master: Add new line with wallet name on Transaction Confirmation (#866) Mark wallet as main when imported Add link to help center (#876) # Conflicts: # Trust.xcodeproj/project.pbxproj

Add new line with wallet name on Transaction Confirmation

1c2f273

OlegGordiichuk reviewed Jul 30, 2018

View reviewed changes

Changge wallet info used for currentWalletTitle to improve clarity.

3177511

Change visual appearance of sender wallet view.

120024d

vikmeup reviewed Aug 1, 2018

View reviewed changes

Trust/Transfer/ViewModels/ConfirmPaymentDetailsViewModel.swift Outdated Show resolved Hide resolved

vikmeup reviewed Aug 1, 2018

View reviewed changes

taraspasichnyk added 2 commits August 2, 2018 11:16

Change title From Wallet -> From. Avoid passing Keystore.

a7472fe

vikmeup reviewed Aug 2, 2018

View reviewed changes

taraspasichnyk added 3 commits August 2, 2018 20:38

Use localizable string instead

a486862

Remove useless localized string

2979f4f

Change Sender -> From

fa7a02d

vikmeup approved these changes Aug 2, 2018

View reviewed changes

taraspasichnyk added 2 commits August 2, 2018 21:28

Remove variables, combined in other string variable.

025e6ab

Add unit test for format of string in ConfirmPaymentDetailsViewModel

630b5cb

vikmeup approved these changes Aug 2, 2018

View reviewed changes

vikmeup added 2 commits August 2, 2018 21:00

Add factory to PreviewTransaction

c01b524

Impove ConfirmPaymentDetailsViewModelTests

9ca25df

Shorten address

9ff1b74

vikmeup merged commit 411554e into master Aug 3, 2018

vikmeup deleted the feature/862-wallet-name-on-confirm-transaction branch August 3, 2018 04:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new line with wallet name on Transaction Confirmation #866

Add new line with wallet name on Transaction Confirmation #866

ghost commented Jul 30, 2018

OlegGordiichuk commented Jul 30, 2018

OlegGordiichuk Jul 30, 2018

ghost Jul 30, 2018

ghost commented Jul 30, 2018

vikmeup commented Jul 30, 2018

vikmeup commented Jul 30, 2018

ghost commented Jul 30, 2018

vikmeup commented Jul 30, 2018

ghost commented Jul 30, 2018

vikmeup commented Jul 30, 2018

ghost commented Jul 30, 2018 •

edited by ghost

Loading

ghost commented Jul 30, 2018

vikmeup left a comment •

edited

Loading

vikmeup Aug 1, 2018

vikmeup Aug 1, 2018

vikmeup Aug 2, 2018

vikmeup Aug 2, 2018

vikmeup Aug 2, 2018

vikmeup Aug 2, 2018

vikmeup Aug 2, 2018

vikmeup Aug 2, 2018

Add new line with wallet name on Transaction Confirmation #866

Add new line with wallet name on Transaction Confirmation #866

Conversation

ghost commented Jul 30, 2018

OlegGordiichuk commented Jul 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jul 30, 2018

vikmeup commented Jul 30, 2018

vikmeup commented Jul 30, 2018

ghost commented Jul 30, 2018

vikmeup commented Jul 30, 2018

ghost commented Jul 30, 2018

vikmeup commented Jul 30, 2018

ghost commented Jul 30, 2018 • edited by ghost Loading

ghost commented Jul 30, 2018

vikmeup left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jul 30, 2018 •

edited by ghost

Loading

vikmeup left a comment •

edited

Loading