feat: earn transaction feed items #5414
Conversation
# Conflicts: # locales/base/translation.json
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5414 +/- ##
==========================================
- Coverage 86.27% 86.26% -0.02%
==========================================
Files 753 754 +1
Lines 30700 30785 +85
Branches 5238 5257 +19
==========================================
+ Hits 26487 26557 +70
- Misses 3986 4001 +15
Partials 227 227
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| transaction: EarnWithdraw | EarnDeposit | EarnClaimReward | ||
| } | ||
|
|
||
| export default function EarnFeedItem({ transaction }: Props) { |
There was a problem hiding this comment.
Why not make 3 separate components (deposit, withdraw, reward claim)?
There was a problem hiding this comment.
We could, I just thought this would lead to a lot of duplicate code across the components.
There was a problem hiding this comment.
I guess these are similar enough to be in a single component, but not a fan of the multiple conditional rendering inside title and subtitle, how about something like
const TX_TYPE_TO_STRINGS = {
'EarnDeposit': {
title: t('earnFlow.transactionFeed.earnDepositTitle'),
subtitle: t('...'),
}
..
}
and then you can do TX_TYPE_TO_STRINGS[transaction.__typename].title / subtitle in the JSX
or you can also do a switch case similar to AmountDisplay
There was a problem hiding this comment.
Used a switch in be4243d!
| transaction: EarnWithdraw | EarnDeposit | EarnClaimReward | ||
| } | ||
|
|
||
| export default function EarnFeedItem({ transaction }: Props) { |
There was a problem hiding this comment.
I guess these are similar enough to be in a single component, but not a fan of the multiple conditional rendering inside title and subtitle, how about something like
const TX_TYPE_TO_STRINGS = {
'EarnDeposit': {
title: t('earnFlow.transactionFeed.earnDepositTitle'),
subtitle: t('...'),
}
..
}
and then you can do TX_TYPE_TO_STRINGS[transaction.__typename].title / subtitle in the JSX
or you can also do a switch case similar to AmountDisplay
| ...EarnDepositItem | ||
| ...EarnWithdrawItem | ||
| ...EarnClaimRewardItem |
There was a problem hiding this comment.
one side effect of starting to query these items is that, even if the gate is off, we'll still be getting and displaying earn txs. Maybe that's ok, but just wanted to call it out. cc @silasbw
There was a problem hiding this comment.
In testing we are already displaying them currently just as swaps and a payment received.
Example from Main
|
|
||
| function Description({ transaction }: DescriptionProps) { | ||
| const { t } = useTranslation() | ||
| const { providerName } = getDynamicConfigParams( |
There was a problem hiding this comment.
probably a rare edge case, but in case there are multiple providers in the future, and someone is on the app version that gets the name from the dynamic config, and have earn txs from a different provider, they'll see the Aave name.. maybe one option would be to just filter by provider id (hardcoded aave-v3) for now to be extra safe?
There was a problem hiding this comment.
I think we'll have a similar issue with the bottom sheet so I think its fine for now and when we have multiple providers we can have a more robust solution.
# Conflicts: # locales/base/translation.json # src/analytics/Events.tsx # src/analytics/Properties.tsx # src/analytics/docs.ts
Description
Adds the
EarnFeedItem.tsx, types and associated GraphQL queries to populate the transaction feed.Screenshots
Test plan
Related issues
Backwards compatibility
Yes
Network scalability
If a new NetworkId and/or Network are added in the future, the changes in this PR will: