GIX-1890: Add Transaction Fee confirm screen #3327
Conversation
|
New UI:
|
|
@dskloetd please review |
There was a problem hiding this comment.
In the screenshot, the transaction fee is larger than the entire amount disbursed.
How does this work? Where is the transaction fee subtracted from?
And should we prevent this or at least warn against it?
There was a problem hiding this comment.
Yes, I have another branch that solves that.
| @@ -26,8 +26,8 @@ | |||
| $snsProjectMainAccountStore?.identifier ?? "" | |||
| ); | |||
|
|
|||
| let token: Token | undefined; | |||
| $: token = $tokensStore[$selectedUniverseIdStore.toText()]?.token; | |||
| let token: IcrcTokenMetadata | undefined; | |||
There was a problem hiding this comment.
We are still getting token from the $tokensStore. How can it be that the type is different now?
There was a problem hiding this comment.
IcrcTokenMetadata extends Token
| on:nnsDisburseMaturity={disburseMaturity} | ||
| on:nnsClose | ||
| /> | ||
| {#if nonNullish(token)} |
There was a problem hiding this comment.
Why is this needed now but not before?
There was a problem hiding this comment.
Beucase it was passing only the tokenSymbol and falling back to "".
| @@ -77,8 +84,9 @@ describe("SnsDisburseMaturityModal", () => { | |||
| await po.clickNextButton(); | |||
|
|
|||
| expect(await po.getConfirmPercentage()).toBe("50%"); | |||
| expect(await po.getConfirmTokens()).toBe("0.48-0.53 ICP"); | |||
There was a problem hiding this comment.
This looks like a bug? Are you fixing a bug at the same time in this PR?
There was a problem hiding this comment.
It was not a bug, but the setup of the test was wrong.
When I set it for the transaction fee, this surfaced. I didn't fee it was big enough to deserve its own PR.
|
|
||
| async getTransactionFee(): Promise<string> { | ||
| return ( | ||
| await this.getNeuronConfirmActionScreenPo().getText("token-value-label") |
There was a problem hiding this comment.
Is this relying on there being only 1 token amount in the modal?
Might be better to explicitly refer to the fee.
There was a problem hiding this comment.
Yes. I also thought so, I didn't want to add anything else to this PR, but I'll add a testId prop to the AmountDisplay
|
The fee is not deducted, it's only used as a minimum. Therefore, there is no need to show it in the confirmation screen. |
PENDING
Confirm: The fee is not subtracted from the maturity. Instead, it just used as an arbitrary minimum to mint the tokens.
Motivation
Add transaction fee in the confirmation screen when disbursing maturity.
Changes
IcrcTokenMetadataas prop instead of just thetokenSymbolinDisburseMaturityModal.DisburseMaturityModalconfirmation screen.Tests
"should display summary information in the last step".Todos
Not necessary. It's covered by the disburse maturity changelog entry.