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

UDC dialog for depositing and withdrawing tokens #606

Merged
merged 8 commits into from
Mar 31, 2021

Conversation

manuelwedler
Copy link
Contributor

Closes #556, Closes #195

This adds a dialog for depositing to and withdrawing from UDC.
The dialog is the UserDepositDialogComponent, which is opened from the Header. The dialog uses a another new component for letting the user input deposit / withdraw values. This is the DepositWithdrawFormComponent, that is created when the user clicks the deposit or the plan withdraw button in the dialog. Interaction with the UDC is done through the UserDepositService. I created the respective functions for that in an earlier PR.

Some changes to the UserDepositService were necessary, for example for polling the withdraw plans on-chain. There is also an observable for fetching blocks until the planned withdraw is ready, which creates a notification when the withdraw can be executed. That makes it necessary to subscribe to it from the AppComponent.

@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #606 (fb54f0f) into master (adf66f6) will increase coverage by 0.05%.
The diff coverage is 95.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #606      +/-   ##
==========================================
+ Coverage   95.87%   95.92%   +0.05%     
==========================================
  Files          88       91       +3     
  Lines        2252     2381     +129     
  Branches      307      317      +10     
==========================================
+ Hits         2159     2284     +125     
- Misses         51       55       +4     
  Partials       42       42              
Impacted Files Coverage Δ
...omponents/address-input/address-input.component.ts 100.00% <ø> (ø)
...omponents/raiden-dialog/raiden-dialog.component.ts 100.00% <ø> (ø)
...rc/app/modules/raiden-icons/raiden-icons.module.ts 100.00% <ø> (ø)
src/app/services/raiden.service.ts 94.71% <75.00%> (ø)
...t-withdraw-form/deposit-withdraw-form.component.ts 85.71% <85.71%> (ø)
src/app/app.component.ts 96.20% <100.00%> (+0.09%) ⬆️
...lance-with-symbol/balance-with-symbol.component.ts 100.00% <100.00%> (ø)
src/app/components/header/header.component.ts 100.00% <100.00%> (ø)
...er-deposit-dialog/user-deposit-dialog.component.ts 100.00% <100.00%> (ø)
src/app/services/user-deposit.service.ts 98.97% <100.00%> (+0.61%) ⬆️
... and 5 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 adf66f6...fb54f0f. Read the comment docs.

fxLayout="row"
fxLayoutAlign="center start"
[formGroup]="form"
(keyup.enter)="onEnter($event)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is keyup.enter the best event for forms? Doesn´t it have a more generic submit which would streamline how forms submission is performed? just curious, as handling key events directly for forms seems quite spartane

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could get it working without the enter event by appropriately using the form tag and setting type="submit" on the button. Thanks for the hint. I also improved this on other parts of the WebUI.

retryWhen((errors) =>
errors.pipe(switchMapTo(this.raidenService.rpcConnected$))
),
shareReplay(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a headsup: share* operators can be a little dangerous sometimes, because they only stay hot while they have at least one subscriber; since this seems to be a readonly observable, this would be just an optimization, but in places where you use it as source to actions, it´s important to keep this fact in mind. Good job here, nice usage of the operators

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't the shareReplay operator always staying hot without the refCount argument set to true?

filter((withdrawPlan) => withdrawPlan.amount.isGreaterThan(0))
);
const blockNumber$ = defer(() =>
from<Promise<number>>(this.raidenConfig.web3.eth.getBlockNumber())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: defer already from its result, no need to from inside. If doing so, it´s a nice practice to mark the function as async to ensure it returns an observable, because if for some reason it eventually returns something else like an array or undefined (for mocks), you may end up with an observable which doesn´t do what you´d expect.

Comment on lines 420 to 421
return interval(15000).pipe(
startWith(0),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick:

Suggested change
return interval(15000).pipe(
startWith(0),
return timer(0, 15000).pipe(

Copy link
Contributor

@andrevmatos andrevmatos left a comment

Choose a reason for hiding this comment

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

lgtm, some suggestions only, but overall seems solid, thanks for implementing this

@manuelwedler manuelwedler force-pushed the udc-dialog branch 2 times, most recently from ceb626b to 103ceec Compare March 31, 2021 12:46
@manuelwedler manuelwedler merged commit 97bd85b into raiden-network:master Mar 31, 2021
@manuelwedler manuelwedler deleted the udc-dialog branch March 31, 2021 13:18
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.

Deposit to and withdraw from UDC RDN token mint button
2 participants