frontend: improvements of Send component
#2966
Open
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
NicolaLS
changed the title
Send component
Utilize the `SendWrapper` to get the mobile channel error, and (bb01) paired status in case a bb01 is connected. This simplifies the `Send` component and better distinguishes code for bb01/bb02 stuff.
Type `coinUnit` in `IAccount` to use `CoinUnit`.
Remove the unused `account` state from the `State` type declaration
because it is never used.
Remove the `paired?` state from the `Send` `State` declaration and also
remove its usage in the return statement. It is always `undefined` which
means `hidden={paired !== false}` is always `true`. This means we can
remove it completely.
The `txProposalErrorHandling` function should **only** return errors and
not set any state. Although this isn't causing an issue right now, the
function should not return an object that is unrelated to errors in
principle. Additionally, this will be necessary for an upcoming refactor
of the `Send` component.
Return only `TProposalError` from the function but make all fields
optional. To keep the logic the same we set `proposedFee` to `undefined`
according to the same logic (ie. `{}` or `{ amountError: ... }` is
returned) but in the `Send` component not in the error handling
function.
Instead of individual variables `proposedFee`, `proposedTotal`, and `proposedAmount` we can put the result itself into the component state. Because of this, it is not possible anymore to reset individual fields to `undefined` but this is correct since there should be no "old" proposal values available in case the current proposal was invalid resulting in an `undefined` `proposalResult`. Before this commit the amount fields could show false values because they would show the last converted `fiatAmount` and the last valid `proposedAmount` since it was not reset on error. Now that we reset it it would've shown `''` but this is not a good solution which is why this commit also falls back to the last typed amount and its fiat equivalent in case `sendAll` is `true` but the proposal was invalid. We also disable the `sendAll` checkbox in case there is an `addressError` or `feeError` because it can not work if the proposal is not valid. We don't disable it if there's only `amountError` because the `amount` is not relevant if `sendAll` is `true`.
Set the `isUpdatingProposal` to `false` in the `finally` block of `validateAndDisplayFee` instead of twice in the `txProposal` funciton. This makes the component a bit easier to understand and smaller.
Improve the onChange handler of the `NoteInput` component to take a function that takes a string instead of the `ChangeEvent`. This will enable us to just pass the `setNote` setter to it when we refactor the `Send` component to be functional.
Don't reset the custom fee when the fee target changes/is set. The
backend only uses the custom fee if fee target = custom which means it
does not need to be set to be empty ('').
Like this the user can select custom, type something, change to some
other target, but as soon as he chooses custom again, what he typed
earlier is still there.
This will also enable us to only pass a `setFeeTarget` to the
sub-component instead of having to define a handler, later when we
refactor the `Send` component to functional.
NicolaLS
force-pushed
the
final-improvments-before-refactor
branch
from
9c67653
to
d437c29
Compare
|
Is it ready to be reviewed? :) @NicolaLS |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
After #2955
Refactoring the
Sendcomponent I noticed these as either improvements or required preparation for the refactor. The commits are quite small and I don't think they need to be tested individually, just test HEAD of the PR, I can also squash them all into one "Send component improvements" commit ?Please review each commit individually.
Putting the result of the proposal request into the state changes the most:
be190c6
But it also fixes a bug that exists on master right now, to reproduce:
After this commit there will never be any
proposedStuffin case the last proposal (and thus partially our current state) is invalid. To prevent this the "send all" checkbox is disabled on address/fee errors. And we fallback on the last typed amount/fiat in case we end up in the state ofsendAll = truebutproposalResult = undefined.In case you think we should reset the custom fee I can drop this:
9c67653
But it will let us remove the handler function later and only pass the state setter when we refactor
Sendto functional in case we don't reset the custom fee.