feat/PRO-2845/transaction-history-new#209
Conversation
Deploying x with
|
| Latest commit: |
d5a1071
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b7eff6dd.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-2845-transaction-hi.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-2845/transaction-history-new)
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
Just a few minor points but other than that it's looking good!
src/types/api.ts
Outdated
| incoming: Transaction[]; | ||
| }; | ||
|
|
||
| export type Transaction = { |
There was a problem hiding this comment.
Can you rename this to FlairTransaction please? The reason for this is because this data type is specific to Flair and we will have different shapes of transactions from Etherscan, for example. In that scenario there would be an additional type called EtherscanTransaction and it would be easier to differentiate between the two 🙏
| const allChainsOptions = [ | ||
| { | ||
| chainId: 1, | ||
| chainName: 'Ethereum', | ||
| }, | ||
| { | ||
| chainId: 137, | ||
| chainName: 'Polygon', | ||
| }, | ||
| { | ||
| chainId: 8453, | ||
| chainName: 'Base', | ||
| }, | ||
| { | ||
| chainId: 100, | ||
| chainName: 'Gnosis', | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Does this belong in utils/blockchain.ts? Looks useful to be used elsewhere
There was a problem hiding this comment.
Yeah fair enough, I moved it to utils
| ? '#8A77FF' | ||
| : '#e2ddff4d' |
There was a problem hiding this comment.
Should these be moved to the theme?
There was a problem hiding this comment.
Theme is for styledComponents and I used tailwind for this component, but I can add this to tailwind config yes
|
|
||
| const sortedTransactions = allTransactions.sort( | ||
| (a, b) => b.timestamp - a.timestamp | ||
| const transactions = (history as TransactionHistory).results; |
There was a problem hiding this comment.
May be worth checking here that .results exists before continuing
There was a problem hiding this comment.
Yes good point, added a '?' to check
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes