-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Feature/dapp improve display and input components #2920
Feature/dapp improve display and input components #2920
Conversation
This allows to specify an optional label for the amount display component. Furthermore it can be set to be full width. This makes basically only sense in combination with a label. The display amount usage in the transfer component had to be adopted due to these changes. The unit tests of the amount display were extended, but also refactored to align with the most recent standards.
This allows to specify an optional label for the address display component. Furthermore it can be set to be full width. This makes basically only sense in combination with a label. The display amount usage in the transfer component had to be adopted due to these changes.
Depending on where the address input component is used, the excluded addresses are defined differently. The according error message when a user types one of those addresses should reflect the specific case.
No CHANGELOG entry necessary for this. No new feature or bugfix for the user. |
Codecov Report
@@ Coverage Diff @@
## master #2920 +/- ##
==========================================
- Coverage 93.77% 93.07% -0.71%
==========================================
Files 116 199 +83
Lines 6348 8015 +1667
Branches 1140 1339 +199
==========================================
+ Hits 5953 7460 +1507
- Misses 340 458 +118
- Partials 55 97 +42
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼
As he address input component can be configured to exclude a list of addresses that are not allowed to be used, this ads the option to restrict the input to a fixed list of allowed addresses. A practical use case is for example the selection of a channel partner to make a deposit to. The unit tests of the amount input component are a nightmare. But that became too big to easily improve them now.
When it fails to parse a balance for a token, instead of failing with an uncaught error (e.g. amount display) it falls back to zero. This lead automatically leads to all actions to fail due to a non-positive amount.
The default for an empty amount input might be set to zero. Depending on the decimals of the targeted token, this should switch between `0` and `0.00`. This was formerly done by other components using the amount input component. This got hereby simplified and keep it within this component.
All feedback addressed. I'll will squash the fixup commits now. |
64a8c4c
to
db64faf
Compare
I'll wait for the CI to confirm and then I'll merge. Afterwards I'll rebase #2921 on top of it. |
This PR got split out of a bigger PR that will come afterwards. It basically includes a couple a smaller improvements which will be used later.