-
Notifications
You must be signed in to change notification settings - Fork 354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨feat(lld): add simplehash calls for ordis #7783
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
6d95b40
to
f1fe8c0
Compare
f1fe8c0
to
700691a
Compare
700691a
to
2d0c38b
Compare
2d0c38b
to
d6342b9
Compare
d6342b9
to
ccf1446
Compare
ccf1446
to
1bb85d6
Compare
1bb85d6
to
105f573
Compare
9b9d88a
to
9cd4895
Compare
9cd4895
to
1758aac
Compare
1758aac
to
5621f80
Compare
5621f80
to
13a428c
Compare
13a428c
to
d3d6f9d
Compare
fa1b52f
to
0b8b700
Compare
0b8b700
to
c8493b4
Compare
useEffect(() => { | ||
if (queryResult.hasNextPage && !queryResult.isFetchingNextPage) { | ||
queryResult.fetchNextPage(); | ||
} | ||
}, [queryResult, queryResult.hasNextPage, queryResult.isFetchingNextPage]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need useEffect
?
You try to fetch every pages without scrolling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that rare sats table doesn't load progressively but should displays everything once. And because I use the same call for Inscriptions and RareSats I need to fetch all pages but I agree not the best
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I just wanted to know if this is expected to fetch "all pages" :)
but looks good and with useInfiniteQuery
there is not a lot of solution to di it ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://tanstack.com/query/latest/docs/framework/react/reference/useInfiniteQuery
maybe you can do something with getNextPageParam
& allPages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I can do anything using allPages
@@ -220,7 +221,7 @@ const AccountPage = ({ | |||
) | |||
) : null} | |||
{isOrdinalsEnabled && account.type === "Account" && account.currency.id === "bitcoin" ? ( | |||
<OrdinalsAccount account={account} /> | |||
<OrdinalsAccount account={account as BitcoinAccount} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why force?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I need to have the bitcoinRessources that are only inside BitcoinAccount and I thought it would be cleaner to do it this way to avoid ts error. I can add a utils to check if it's a btc account by checking if the account has bitcoinResources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed for two type guards => one that checks if it's a fork of btc and one to check if that fork is a BTC account or a fork
apps/ledger-live-desktop/src/newArch/features/Collectibles/Ordinals/components/Loader.tsx
Outdated
Show resolved
Hide resolved
...er-live-desktop/src/newArch/features/Collectibles/Ordinals/components/Inscriptions/index.tsx
Outdated
Show resolved
Hide resolved
...er-live-desktop/src/newArch/features/Collectibles/Ordinals/components/Inscriptions/index.tsx
Outdated
Show resolved
Hide resolved
c8493b4
to
ebbb163
Compare
ebbb163
to
fee6331
Compare
fee6331
to
62f36bf
Compare
62f36bf
to
a1436f9
Compare
function isBitcoinAccount(account: BitcoinAccount): boolean { | ||
return account.currency.id === "bitcoin"; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be moved outside in an helper ?
also you can just do account: Account
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to common lib so I will be able to use it on LLM too
apps/ledger-live-desktop/src/newArch/features/Collectibles/Ordinals/screens/Account/index.tsx
Outdated
Show resolved
Hide resolved
apps/ledger-live-desktop/src/newArch/features/Collectibles/Ordinals/screens/Account/index.tsx
Outdated
Show resolved
Hide resolved
a1436f9
to
caf32a6
Compare
caf32a6
to
b2198d7
Compare
✨feat(ui): add simplehash calls for ordis
✅ Checklist
npx changeset
was attached.📝 Description
This is the third PR of LIVE-12479
We can use the same API to fetch inscriptions and rare sats. For this we use utxo addresses from
Account
as it's aBitcoinAccount
. After some manipulations we can separate rare sats and inscriptions to fill the two tables.Inside live-nft-react we only remove all the common sats from the object and separate inscriptions and rare sats.
There is a restructuration inside Collectibles/Hooks of the object.
Since we use tanstack I've added an error message when the API respond with an error. It can be useful particularly because it's an external API
A tooltip is added when hovering specific rare sat icon in rare sats table and inscription table. Some of the rare sats don't have description.
Important
Some inscriptions have a canvas as media. In LLD / LLM we are currently not able to display them. A solution would be to use an iFrame but this can represents some risk since the API is from a partner. A solution would be to do as it has been done during hackathon by using magicEden endpoint but not sure we have the right 👀it has been decided to prioritise safety so we will display fallback image for the moment
also utxo_size is removed of the table since no other platforms displays it and product is not sure about the utility of this information
Screen.Recording.2024-09-12.at.15.38.38.mov
❓ Context
🧐 Checklist for the PR Reviewers