Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Updates to Preview Branch (stars) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
there is no deployment if master is not destination? I really liked how you broke these into smaller prs but this now kind of loses that benefit and makes it hard to review again |
jbojcic1
left a comment
jbojcic1
left a comment
There was a problem hiding this comment.
how does one get "star accounts"?
| export function useGetAccountFromLocation(select?: { type?: AccountType }) { | ||
| const [searchParams] = useSearchParams(); | ||
| const accountId = searchParams.get('accountId'); | ||
| const { data: accounts } = useAccounts({ type: select?.type }); |
There was a problem hiding this comment.
we already have useAccount hook. can we use that?
There was a problem hiding this comment.
have you analysed the new queries to see if the query plan is still efficient (we need to be careful with this query as this table will have a lot of data)?
| from wallet.transactions t | ||
| where t.user_id = p_user_id | ||
| and t.state in ('PENDING', 'COMPLETED', 'REVERSED') | ||
| and (p_account_id is null or t.account_id = p_account_id) -- Apply account filter if provided |
There was a problem hiding this comment.
e.g. I think this or might not be a good thing for the performance
| const PAGE_SIZE = 25; | ||
|
|
||
| export function useTransactions() { | ||
| export function useTransactions(select?: { accountId?: string }) { |
There was a problem hiding this comment.
did we call params like this (ones not used in the select part of the use query) select in other places too? I would maybe call it filter instead
|
|
||
| const result = useInfiniteQuery({ | ||
| queryKey: [allTransactionsQueryKey], | ||
| queryKey: [allTransactionsQueryKey, select], |
There was a problem hiding this comment.
so query key will now be ['all-transactions', '[uuid]']. I wonder if we should make it ['all-transactions', 'account', '[uuid]'] instead to make it clear it's not a transaction id. or ['account-transactions', '[uuid]']
| */ | ||
| export function SelectableWalletCard({ | ||
| account, | ||
| isSelected, |
There was a problem hiding this comment.
if we pass index and selectedCardIndex do we need isSelected?
| }; | ||
|
|
||
| // Calculate Y position based on state | ||
| let yOffset: number; |
There was a problem hiding this comment.
I would extract this logic to function called getYOffset maybe. or one called getTransition that gets all transition styles
| const transition = `all ${ANIMATION_DURATION}ms ${EASE_IN_OUT}`; | ||
|
|
||
| return ( | ||
| // biome-ignore lint/a11y/useSemanticElements: div needed for absolute positioning wrapper |
There was a problem hiding this comment.
why is this needed? becuase you used role button on div? why didn't you just use button if you want that role?
| const [customDesignPath, setCustomDesignPath] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| loadCardAsset(account.mintUrl).then(setCustomDesignPath); |
There was a problem hiding this comment.
so while this is being loaded we show default element. how does the transition look once this is loaded? does the content jump?
| loadCardAsset(account.mintUrl).then(setCustomDesignPath); | ||
| }, [account.mintUrl]); | ||
|
|
||
| const cardName = account.wallet.cachedMintInfo.name ?? account.name; |
There was a problem hiding this comment.
why are we picking this over acc name?
https://fake.agi.cash |
but how does user gets those? is the idea that they add it via regular account add flow? |
That's a question for @bobscully3, but yea one way is to add via regular account flow. This is a little awkward atm because we don't show star accounts in the account list so after adding you don't see the account until going over to the loyalty view. The other way to get star accounts is to receive ecash from from them (via nfc card for example). If the mint can only settle internally, then when receiving ecash the only option will be to add the account and swap to claim |
|
we will want to have a way to search and add merchant mints in the future that is separate from the regular mint flow. i have some mockups that i can show on our call tomorrow. |
|
@gudnuf can this be closed? Can you also go through all your PRs and close irrelevant ones? |
|
replaced by #758... closing |
3 participants
depends on #659 and #658
Adds a new /cards route that renders all of the user's star accounts. We dynamically load all the card designs based on the account's mint url. When a card is selected from the stack there is an animation the slides the card up while and balance, send, receive, and tx history appear.
This PR is currently open to master so that we can have the deployment.
There are 4 commits to this PR. The first 3 are needed to make the UI work.
This is the new cards ui:
