feat: APP-368 add user balance error in buy credits form #2501
Conversation
✅ Deploy Preview for regen-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Relevant files: |
@r41ph you can also simply have the base branch for this PR be feat-APP-204-buy-credits so the diff is more readable (at least until my PR gets merged) |
Got it, thanks! I've changed the base now |
| selectedSellOrder?: UISellOrderInfo; | ||
| askDenom?: string; |
There was a problem hiding this comment.
what about refactoring this to only take askDenom as param?
There was a problem hiding this comment.
I didn't do that because this hook is used in BuyCreditsModal. Thought it'd be better to refactor it once we have deleted this buying flow.
There was a problem hiding this comment.
it's only used once, so it would be really a small refactor:
const userBalance = useFetchUserBalance({ askDenom: selectedSellOrder?.askDenom });
| @@ -99,11 +102,15 @@ export function ChooseCreditsForm({ | |||
|
|
|||
| const [spendingCap, setSpendingCap] = useState(0); | |||
| const [creditsAvailable, setCreditsAvailable] = useState(0); | |||
| // A big number here avoids the NOT_ENOUGH_BALANCE error on rendering | |||
| // the form until the actual user balance is fetched | |||
| const [userBalance, setUserBalance] = useState(10101010101010101); | |||
There was a problem hiding this comment.
this is a bit hacky, what about displaying a loading spinner instead of the form while we fetch the balance?
There was a problem hiding this comment.
Since the data we are fetching is related to validation for an error users may never encounter, I thought it'd be better to not block the UI from rendering, so users can interact with the form straight away. But yes, a loading animation is a more conventional solution.
| @@ -3,3 +3,4 @@ import { msg } from '@lingui/macro'; | |||
| export const MAX_AMOUNT = msg`Amount cannot exceed`; | |||
| export const MAX_CREDITS = msg`Credits cannot exceed`; | |||
| export const POSITIVE_NUMBER = msg`Must be positive`; | |||
| export const NOT_ENOUGH_BALANCE = msg`You don’t have enough`; | |||
There was a problem hiding this comment.
aren't we supposed to display You don’t have enough ${displayDenom}?
There was a problem hiding this comment.
There was a problem hiding this comment.
oh I see, that assumes that all errors for this field will have the displayDenom at the end of the message.
I guess that works for now but that's not very reusable and also it gives less context for translating the content which could result in wrong translations.
Description
https://regennetwork.atlassian.net/browse/APP-368
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
How to test
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...