Feature/dapp quick pay route #2930
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #2930 +/- ##
==========================================
- Coverage 93.37% 93.32% -0.05%
==========================================
Files 203 207 +4
Lines 8598 8812 +214
Branches 1356 1381 +25
==========================================
+ Hits 8028 8224 +196
- Misses 473 487 +14
- Partials 97 101 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
I need to add a CHANGELOG entry. |
taleldayekh
approved these changes
Sep 10, 2021
There was a problem hiding this comment.
Went through this together with @weilbith and looks very good, thx! 👍🏼
|
@taleldayekh thanks for the nice review. I always appreciate that you take the time to do it together and discuss the details. Also thanks to always improve my user translations! 🙏🏾 |
|
I addressed all feedback. I'm very positive that I don't need an in-between CI confirmation. Therefore I'll go ahead and squash the fixups. Then I'll wait for CI to finally merge it. |
weilbith
force-pushed
the
feature/dapp-quick-pay-route
branch
from
196fc1b to
3b6b449
Compare
|
Sorry, I messed something up... So much to I'm optimistic. |
weilbith
force-pushed
the
feature/dapp-quick-pay-route
branch
from
3b6b449 to
d7d257c
Compare
|
@weilbith can you rebase on top of master on next run, please? some dependencies updates (including jest/test related stuff) got merged. Hopefully it won't impact actual code nor conflict, but it'll be better to have that tested before merging. Thanks |
weilbith
force-pushed
the
feature/dapp-quick-pay-route
branch
from
d7d257c to
67ca27a
Compare
Depending on which of the values of the form are editable, this makes the first input auto focused. While the amount input already had the autofocus property implemented, the address input did not and was in need of an extension.
This allows the parent component of a channel action form to be update when then input value changes. The advantages of such a single event are that it is debounced and limited to valid inputs per default, as well as it does not require to force the usage of synced properties and thereby make it more optional.
As the amount input component can be configured to not exceed a maximum amount, this adds the feature to require a minimum amount. This feature gets used/forwarded inside the channel action form.
When the Raiden service gets started, a hook is placed to prevent the unloading of the page. This can be removed again once the service disconnects again.
There is the use case that the SDK should automatically do a transfer. This includes the whole detection if a direct transfer is possible or not, getting the best path finding service and the cheapest route for a mediated transfer. This does not work if the dApps Raiden service requires a value for the path parameter.
Actions which have just a single step look kinda awkward if the progress card displays this step. Though it still makes sense to have a step for example for the description with the hints for the user what to do.
Actions do emit an event when they completed successfully or failed. This adds another event to signal that the actions has been started. This is helpful for the parent component to act accordingly.
While the amount input expects to work on a plain string, the amount display requires the BigNumber value. In the channel action form both components got feed with the same value for the amount. This does not work and leads to errors. But the form component is already of this type difference and has a computed property for it. Thereby the fix was very simple.
This is a quite common error, especially for transfers to new Raiden nodes. Even for us developers this happens from time to time in our test setups. Then we have to waste time to find the original error behind the else very generic error message. This shows the actual cause and helps the user to solve the issue.
This includes actions which kinda contain of multiple actions, or rather combine multiple actions which also exist standalone. This allows to deposit more tokens into a channel and then do a direct transfer through the same channel. The same works with first opening a new channel and then a direct transfer. This is very useful for straight forward transfers without user interactions and a clear visual progress without interruptions.
The quick pay route is meant to do transfers without much effort my the user. The URL must contain all information do to such a transfer. Then no matter if the user has ever used Raiden, this specific token network or what his connectivity and liquidity is, without one or two clicks he can do a transfer. Please checkout issue raiden-network#2882 for all the details of how the different cases look like and how the decision tree must be implemented. After all it is mostly just a smart combination of all the nice features that have been refactored/added to the dApp recently. This does not yet handle too low UDC deposits to pay for mediated transfer route requests.
This adds two features. First it also allows the user to close the dialog early when it was successful and it is not necessary to wait for the timeout. Second this adds a new event when the dialog gets closed by the user.
The transfer action was handled a little special in contrast to the other actions. This included to show the progress inline. This got unified in combination with a better error visualization. All actions show their progress in a dialog now. The transfer action just has some empty translations to make it less obtrusively as intended before already. This allows to re-use the nice looking error visualization of the action progress card in the dialog. It provides more information and looks better. Before this was hidden by a second error dialog on top of it. This change did also allow to get rid of the extra functionality to hide and display again the action message (which was only for the inline progress of the transfer action). Though there is still an error dialog remaining to handle all errors which are not caused by actions themselves. For the moment this is only the case for invalid query parameters. Moreover does the changing of query parameters during an active action not cancel this action anymore. Furthermore do action errors take precedence. Finally the closing of the action progress dialog will also cause the direction. This solves a missing feature where the user was stuck for failed actions, waiting for ever to get redirected. Second this allows the user to get quicker redirected in case of a success by closing the dialog manually. This became possible by a recent extension of the action mixin.
|
Rebase as requested by Andre... |
weilbith
force-pushed
the
feature/dapp-quick-pay-route
branch
from
67ca27a to
b5a8c16
Compare
|
Okay, apparently this did break something... |
|
The Jest update changed the behavior of the |
The `toContain` function has changed and does not automatically adapt the types anymore.
|
I don't know why the |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thank you for submitting this pull request :)
Fixes #2882
Short description
The most exciting feature for the dApp since the connection manager 😬
All the previous refactoring enabled to do such nice thing like this.
Definition of Done
Steps to manually test the change (dApp)