feat: SIP-10 balances on mobile #820
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #820 +/- ##
==========================================
+ Coverage 29.15% 29.53% +0.37%
==========================================
Files 205 206 +1
Lines 8159 8204 +45
Branches 560 567 +7
==========================================
+ Hits 2379 2423 +44
- Misses 5780 5781 +1
|
pete-watters
force-pushed
the
feat/LEA-1832/stacks-sip10
branch
3 times, most recently
from
728e6d5 to
4810561
Compare
pete-watters
changed the base branch from
feat/LEA-1726/sip10-balances
to
dev
| import { BitcoinAccountIdentifier, getRunesBalancesService } from '@leather.io/services'; | ||
|
|
||
| export function useRunesTotalBalance() { | ||
| // TODO LEA-1982: check with Alex once we have xpub-based runes endpoint from BIS |
There was a problem hiding this comment.
@alexp3y I just stubbed out this Runes code, we can revisit it once the BIS endpoint is ready and probably refactor this
pete-watters
force-pushed
the
feat/LEA-1832/stacks-sip10
branch
from
4810561 to
737562a
Compare
| @@ -55,6 +57,7 @@ export function createSip10BalancesService( | |||
| return { | |||
| usd: totalUsdBalance, | |||
| addressBalances, | |||
| aggregateBalances: getAggregateSip10Balances(addressBalances), | |||
There was a problem hiding this comment.
@alexp3y I added aggregateBalances as that's what we need on the homepage.
We don't use addressBalances from this service but it could be useful? I'm not sure. I think it has a different query cache than account specific balances anyway?
There was a problem hiding this comment.
Nice! Yeah having an aggregateBalances field here makes perfect sense. Definitely more so than the addressBalances array, which I thought could be useful but in retrospect it does stretch the responsibility of the call and bloats the query cache unnecessarily. If we're not going to use it then it might be best to just remove.
|
|
||
| import { TokenBalance } from '../token-balance'; | ||
|
|
||
| const sip10MaxDisplay = 3; |
There was a problem hiding this comment.
I'm adding this hard cap of 3 SIP-10 tokens until we figure out what we want to do here
|
|
||
| // TODO LEA-1726: handle balance loading & error states | ||
| if (data.state !== 'success') return; | ||
| return data.value?.runes.map((balance: any, index: any) => ( |
There was a problem hiding this comment.
isn't it typed such that values is definitely defined when state !== 'success'?
| @@ -25,6 +27,9 @@ export function AllAccountBalances() { | |||
| <> | |||
| <BitcoinBalance /> | |||
| <StacksBalance /> | |||
| <Sip10Balance /> | |||
There was a problem hiding this comment.
@kyranjamie : do you have some good ideas about how to handle this best? I asked about it in the design channel.
In the accounts widget, we only have space for 5 tokens as we want to show other widgets also. For now I have prioritised:
- BTX
- STX
- 3 others SIPs
We load tokens from different places so if we only want to show 5 and prioritise some it gets messy quickly. At the moment users will also have no way to see all of their SIP-10s as work on that is ongoing.
I added a hard cap of 3 SIP-10's in the interim but there is no sorting there yet. This will also not work if there are 0 balances for STX or BTC as we will still only show 3 SIP-10's 😢
@alexp3y maybe we could use a balance wrapper service to wrap and curate the balances?
That would move us away from this coding style though of separate balance components which we favour
| @@ -55,6 +57,7 @@ export function createSip10BalancesService( | |||
| return { | |||
| usd: totalUsdBalance, | |||
| addressBalances, | |||
| aggregateBalances: getAggregateSip10Balances(addressBalances), | |||
There was a problem hiding this comment.
Nice! Yeah having an aggregateBalances field here makes perfect sense. Definitely more so than the addressBalances array, which I thought could be useful but in retrospect it does stretch the responsibility of the call and bloats the query cache unnecessarily. If we're not going to use it then it might be best to just remove.
This PR:
I didn't add the icons yet as Tigran will add update icon component to accept URLs LEA-1909
For the
Tokens Widgetwe have a hard cap to show only 5 tokens so I added some code to cap it:This is OK for now but we need to revisit this. You can see a demo here:
SIP-10-less.mp4
I did more testing before this cap to make sure we are showing the correct aggregate balances on the homepage:
SIP-10.mp4