feat: add balance logic in implicit account creation wizard #8075

evavirseda · 2024-02-20T09:52:32Z

Summary

Closes #8053
...

Changelog

- Please write a list of changes

Testing

Platforms

Please select any platforms where your changes have been tested.

Desktop
- MacOS
- Linux
- Windows

Instructions

Please describe the specific instructions, configurations, and/or test cases necessary to test and verify that your changes work as intended.

...

Checklist

Please tick the following boxes that are relevant to your changes.

I have followed the contribution guidelines for this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or modified tests that prove my changes work as intended
I have verified that new and existing unit tests pass locally with my changes
I have verified that my latest changes pass CI workflows for testing and linting
I have made corresponding changes to the documentation

…t-management-list-ui

…ress and disable the password input

packages/desktop/views/dashboard/account-management/AccountManagement.svelte

…nt-info packages/desktop/views/dashboard/account-management/AccountManagement.svelte

…nt-info

packages/desktop/views/dashboard/account-management/AccountManagement.svelte

…w' and 'feat/add-account-info-logic' of github.com:iotaledger/firefly into feat/update-logic-for-activate-account-in-details-view

…w' and 'develop-iota2.0' of github.com:iotaledger/firefly into feat/update-logic-for-activate-account-in-details-view

…count-in-details-view

…w' and 'develop-iota2.0' of github.com:iotaledger/firefly into feat/update-logic-for-activate-account-in-details-view

… into feat/add-balance-logic-to-implicit-wizard-and-management

packages/desktop/components/AccountManagementDetails.svelte

...views/dashboard/wallet/views/implicit-account-creation-multistep/FundConfirmationView.svelte

cpl121

Nice 👌🏼

marc2332 · 2024-02-21T14:36:42Z

packages/desktop/components/AccountManagementDetails.svelte

@@ -54,10 +54,21 @@
    $: accountId = isAccountOutput(selectedOutput) ? (selectedOutput?.output as AccountOutput)?.accountId : null
    $: address = accountId ? getBech32AddressFromAddressTypes(new AccountAddress(accountId)) : null
    $: isMainAccount = accountId && accountId === $selectedWalletMainAccountId
+    $: balance = getAccountBalance(selectedOutput)


You should probably pass isImplicitAccount to getAccountBalance, so if isImplicitAccount changes for any reason, the balance will be updated, right?

i'm a bit confused about why it might be necessary to pass it. If an implicitAccount is created through handleActivateAccount, it's not like it would spontaneously change, right?
sorry, but I don't see the situation in which this would happen... 🙈
could you help me understand this better? 🙏 maybe i'm wrong..

Yes, I agree with Eva, the moment the implicitOutput stops being implicit the selectedOutput will also change

I prefer explicit reactive dependencies over assumptions, if L53 changes in the future, isImplicitAccount might start changing its value because of another factor than selectedOutput, and balance might get out of sync if you forget to update the logic of getAccountBalance. I don't see how passing isImplicitAccount would be a problem

Okeey, makes sense 👌🏼

marc2332 · 2024-02-21T14:38:45Z

...views/dashboard/wallet/views/implicit-account-creation-multistep/FundConfirmationView.svelte

+    $: formattedWalletBalance = walletBalance
+        ? formatTokenAmountBestMatch(Number(walletBalance.baseCoin?.available), baseCoin?.metadata)
+        : '-'


this makes more sense, right?

Suggested change

$: formattedWalletBalance = walletBalance

? formatTokenAmountBestMatch(Number(walletBalance.baseCoin?.available), baseCoin?.metadata)

: '-'

$: formattedWalletBalance = walletBalance?.baseCoin?.available

? formatTokenAmountBestMatch(Number(walletBalance.baseCoin.available), baseCoin?.metadata)

: '-'

…t-wizard-and-management packages/desktop/views/dashboard/wallet/views/implicit-account-creation-multistep/FundConfirmationView.svelte

…t-wizard-and-management

marc2332 · 2024-02-23T08:20:51Z

packages/desktop/components/AccountManagementDetails.svelte

@@ -54,10 +54,21 @@
    $: accountId = isAccountOutput(selectedOutput) ? (selectedOutput?.output as AccountOutput)?.accountId : null
    $: address = accountId ? getBech32AddressFromAddressTypes(new AccountAddress(accountId)) : null
    $: isMainAccount = accountId && accountId === $selectedWalletMainAccountId
+    $: balance = getAccountBalance(selectedOutput)


I prefer explicit reactive dependencies over assumptions, if L53 changes in the future, isImplicitAccount might start changing its value because of another factor than selectedOutput, and balance might get out of sync if you forget to update the logic of getAccountBalance. I don't see how passing isImplicitAccount would be a problem

marc2332

Unrelated to this PR but I don't see why

firefly/packages/desktop/components/AccountManagementDetails.svelte

Line 123 in 3038ff4

listBlockKeysFeature(selectedOutput)

is being called on an onMount, what if the output changes?

marc2332

🆒

evavirseda added 30 commits February 6, 2024 12:14

feat: add account management router

40512e3

feat: update icon

b6a15f2

feat: add account list

b6ceba1

Merge branch 'develop-iota2.0' into feat/add-account-management-router

094f722

Merge branch 'develop-iota2.0' into feat/add-acount-management-list-ui

45e6f80

Merge branch 'develop-iota2.0' into feat/add-account-management-router

97969ac

Merge branch 'feat/add-account-management-router' into feat/add-acoun…

00136ce

…t-management-list-ui

fix: display as isBusy if hasImplicitAccountCreationTransactionInProg…

96d9438

…ress and disable the password input

feat: format and truncate address

ebd0372

feat: cleanup code

b7a6c67

feat: add features flag

222e941

feat: add account management details ui

c92a0cb

feat: add locales

a241985

feat: enable features

2772f39

feat: update accountManagement

4fa8d55

Merge branch 'develop-iota2.0' into feat/add-acount-management-list-ui

3fba6a5

packages/desktop/views/dashboard/account-management/AccountManagement.svelte

Merge branch 'feat/add-acount-management-list-ui' into feat/add-accou…

7741032

…nt-info packages/desktop/views/dashboard/account-management/AccountManagement.svelte

feat: use real data

01d151e

feat: fix wrong merge

b922eec

Merge branch 'develop-iota2.0' into feat/add-acount-management-list-ui

6c40111

feat: add isImplicitAccountAccountOutput function

c01b709

Merge branch 'feat/add-acount-management-list-ui' into feat/add-accou…

2b55cb6

…nt-info packages/desktop/views/dashboard/account-management/AccountManagement.svelte

fix import

d73eb3d

Merge branch 'feat/add-acount-management-list-ui' into feat/add-accou…

2d5e4d9

…nt-info

fix: improve function

1e6710d

Merge branch 'feat/add-acount-management-list-ui' into feat/add-accou…

c74de66

…nt-info

feat: cleanup

826ac45

feat: cleanup

152171e

feat: cleanup

d7f94ed

Merge branch 'feat/add-account-info' into feat/add-account-info-logic

687d022

packages/desktop/views/dashboard/account-management/AccountManagement.svelte

cpl121 and others added 5 commits February 16, 2024 12:59

Merge branches 'feat/update-logic-for-activate-account-in-details-vie…

a66daea

…w' and 'feat/add-account-info-logic' of github.com:iotaledger/firefly into feat/update-logic-for-activate-account-in-details-view

Merge branches 'feat/update-logic-for-activate-account-in-details-vie…

487b4ae

…w' and 'develop-iota2.0' of github.com:iotaledger/firefly into feat/update-logic-for-activate-account-in-details-view

fixes

b72e992

feat: update styles

edc0089

feat: add balance logic in implicit account creation wizard

ca008f6

evavirseda linked an issue Feb 20, 2024 that may be closed by this pull request

[Task]: Add balance logic in implicit account creation wizard #8053

Closed

evavirseda and others added 4 commits February 20, 2024 11:26

Merge branch 'develop-iota2.0' into feat/update-logic-for-activate-ac…

9af8f1e

…count-in-details-view

Merge branches 'feat/update-logic-for-activate-account-in-details-vie…

b43ed6f

…w' and 'develop-iota2.0' of github.com:iotaledger/firefly into feat/update-logic-for-activate-account-in-details-view

fixes

36c1819

Merge branch 'feat/update-logic-for-activate-account-in-details-view'…

83a4d60

… into feat/add-balance-logic-to-implicit-wizard-and-management

cpl121 requested changes Feb 20, 2024

View reviewed changes

fix: add todo comment and show -

875727f

evavirseda requested a review from cpl121 February 21, 2024 10:47

fix: link issue

3038ff4

cpl121 approved these changes Feb 21, 2024

View reviewed changes

cpl121 requested a review from marc2332 February 21, 2024 12:53

marc2332 requested changes Feb 21, 2024

View reviewed changes

minor fix

49f3c8e

Base automatically changed from feat/update-logic-for-activate-account-in-details-view to develop-iota2.0 February 22, 2024 16:20

Merge branch 'develop-iota2.0' into feat/add-balance-logic-to-implici…

a3dbfe4

…t-wizard-and-management packages/desktop/views/dashboard/wallet/views/implicit-account-creation-multistep/FundConfirmationView.svelte

evavirseda requested a review from marc2332 February 23, 2024 07:45

Merge branch 'develop-iota2.0' into feat/add-balance-logic-to-implici…

f196ff5

…t-wizard-and-management

marc2332 requested changes Feb 23, 2024

View reviewed changes

evavirseda added 2 commits February 23, 2024 09:44

fix

d4c931b

fix

cec129e

evavirseda requested a review from marc2332 February 23, 2024 08:45

marc2332 approved these changes Feb 23, 2024

View reviewed changes

cpl121 merged commit f3a42fa into develop-iota2.0 Feb 23, 2024
3 of 4 checks passed

cpl121 deleted the feat/add-balance-logic-to-implicit-wizard-and-management branch February 23, 2024 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add balance logic in implicit account creation wizard #8075

feat: add balance logic in implicit account creation wizard #8075

evavirseda commented Feb 20, 2024

cpl121 left a comment

marc2332 Feb 21, 2024

evavirseda Feb 21, 2024 •

edited

Loading

cpl121 Feb 22, 2024

marc2332 Feb 23, 2024 •

edited

Loading

cpl121 Feb 23, 2024

marc2332 Feb 21, 2024

marc2332 Feb 23, 2024 •

edited

Loading

marc2332 left a comment

marc2332 left a comment

feat: add balance logic in implicit account creation wizard #8075

feat: add balance logic in implicit account creation wizard #8075

Conversation

evavirseda commented Feb 20, 2024

Summary

Changelog

Testing

Platforms

Instructions

Checklist

cpl121 left a comment

Choose a reason for hiding this comment

marc2332 Feb 21, 2024

Choose a reason for hiding this comment

evavirseda Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

cpl121 Feb 22, 2024

Choose a reason for hiding this comment

marc2332 Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

cpl121 Feb 23, 2024

Choose a reason for hiding this comment

marc2332 Feb 21, 2024

Choose a reason for hiding this comment

marc2332 Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

marc2332 left a comment

Choose a reason for hiding this comment

marc2332 left a comment

Choose a reason for hiding this comment

evavirseda Feb 21, 2024 •

edited

Loading

marc2332 Feb 23, 2024 •

edited

Loading

marc2332 Feb 23, 2024 •

edited

Loading