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

Feature/dapp switch eth balance according to account #2273

Conversation

weilbith
Copy link
Contributor

@weilbith weilbith commented Oct 19, 2020

Fixes #2224

Short description
In addition to the origin issue, I agreed with @taleldayekh that showing both ETH balances on the main account screen is another helpful addition for the user. This is ofc only the case if a Raiden account is in use.

Definition of Done

  • Steps to manually test the change have been documented
  • Acceptance criteria are met
  • Change has been manually tested by the reviewer (dApp)

Steps to manually test the change (dApp)

  1. Connect to dApp with Raiden account enabled
  2. Navigate to the account screen
  3. Make sure that there are now two ETH balances shown
  4. Navigate to the UDC screen and click withdraw
  5. Make sure that the shown ETH balance relates to the Raiden account (probably zero if you haven't transferred something)
  6. Repeat this without using a Raiden account and make sure the expected changes in the UI are there

The AccountContent view is displaying a single ETH balance. It gives no
information from which account this balance is. Moreover does this
balance depend on if the Raiden account has a balance or not.
To improve this view and do not confuse the user there are two
improvements. First should the balance not switch dependent on if there
is a Raiden account balance or not, but rather when a Raiden account is
in use or not. But actually it is more informative for the user to just
show both balances. In case no Raiden account is used, the main account
balance is sufficient.
The UDC screen in the account router shows an ETH balance during the
withdrawal procedure. The relevant balance depends on if a Raiden
account is used. So far it always shows the main account balance. This
can be confusing, when the Raiden account has no balance, the
transaction to withdrawal fails, but the UI displays a positive (and
high enough) ETH balance. Therefore this balance must always relate to
the correct account that is relevant for this transaction.
@weilbith weilbith requested a review from taleldayekh October 19, 2020 12:07
@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #2273 into master will increase coverage by 36.79%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #2273       +/-   ##
===========================================
+ Coverage   58.00%   94.79%   +36.79%     
===========================================
  Files          61      156       +95     
  Lines        4460     6264     +1804     
  Branches      936     1160      +224     
===========================================
+ Hits         2587     5938     +3351     
+ Misses       1860      261     -1599     
- Partials       13       65       +52     
Flag Coverage Δ
#dapp 91.85% <50.00%> (?)
#sdk 95.98% <ø> (?)
#sdk_e2e ?
#sdk_integration 68.58% <ø> (?)
#sdk_unit 85.56% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...den-dapp/src/components/account/AccountContent.vue 88.00% <ø> (ø)
raiden-dapp/src/views/account/UDC.vue 95.65% <0.00%> (ø)
raiden-dapp/src/store/index.ts 98.05% <100.00%> (ø)
raiden-ts/src/polyfills.ts 68.42% <0.00%> (-31.58%) ⬇️
raiden-dapp/src/views/account/WithdrawalRoute.vue 100.00% <0.00%> (ø)
...ponents/account/backup-state/UploadStateDialog.vue 85.29% <0.00%> (ø)
raiden-dapp/src/router/routes.ts 100.00% <0.00%> (ø)
...aiden-dapp/src/components/tokens/TokenListItem.vue 100.00% <0.00%> (ø)
raiden-dapp/src/components/TokenInformation.vue 77.77% <0.00%> (ø)
... and 130 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6047659...beb9208. Read the comment docs.

Copy link
Contributor

@taleldayekh taleldayekh left a comment

Choose a reason for hiding this comment

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

Great, thanks @weilbith 👍

@weilbith
Copy link
Contributor Author

We don't understand what CodeCov wants to tell us here. Looks pretty broken. The changed/added tests are pretty good. Therefore this gets merged now.

@weilbith weilbith merged commit aa524a2 into raiden-network:master Oct 19, 2020
@weilbith weilbith deleted the feature/dapp-switch-eth-balance-according-to-account branch October 19, 2020 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UDC withdrawal dialog displays main account ETH when subkey account is used
2 participants