feat(hackathon-surplus): do not show modal when too small amounts #2857
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
alfetopito
changed the title
| ): boolean | null { | ||
| if (fiatAmount) { | ||
| // When there's a fiat amount, use that to decide whether to display the modal | ||
| return Number(fiatAmount.toFixed(3)) > MIN_FIAT_SURPLUS_VALUE_MODAL |
There was a problem hiding this comment.
why converting to number? is not enough to compare directly the underling number without doing the toFixed?
There was a problem hiding this comment.
toFixed returns a string.
fiatAmount is a Fraction so it cannot be directly compared with a number afaict.
See https://docs.uniswap.org/sdk/core/reference/classes/Fraction#greaterthan
There was a problem hiding this comment.
you can compare a fraction and a fraction, 0.01 is 1/100 :P
There was a problem hiding this comment.
anyways, merging to close the release
There was a problem hiding this comment.
Well, that's true...
If you mind I can create a new PR addressing this.
Let me know
| if (showSurplus === false && orderId) { | ||
| removeOrderId(orderId) | ||
| } | ||
| }, [orderId, removeOrderId, showSurplus]) |
There was a problem hiding this comment.
Does it make sense to add to remove later? doesn't it make sense to not add in the queue in the first place?
There was a problem hiding this comment.
The problem is, by the time we are adding it, the FIAT estimation is not available and has to be fetched/calculated.
It's a whole other can of worms to have it at that stage
There was a problem hiding this comment.
okay, i though could be the case, NVM
| if (showSurplus === false && orderId) { | ||
| removeOrderId(orderId) | ||
| } | ||
| }, [orderId, removeOrderId, showSurplus]) |
There was a problem hiding this comment.
okay, i though could be the case, NVM
Summary
Will point to the right place once ready
Fixes #2847 and #2848
Show the modal according to the rules (slightly different from the issue descriptions)
Revert old behaviour for confirmation modal when surplus is not displayed
To Test
With FIAT
1but close the confirmation modal4but close the confirmation modalWithout FIAT
7but close the confirmation modal10but close the confirmation modal