feat: add prime total rewards card#5626
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR implements the Prime total rewards card and user rewards card for the Prime Leaderboard page, adding two new section components (
Confidence Score: 5/5The change is safe to merge — it adds new UI-only sections behind a wallet-connection gate with no mutations or contract interactions. All new components are well-scoped, have accompanying tests, and the previously raised issues (NaN progress bar, missing click handler) are resolved. The two remaining notes are quality/style nits on placeholder code that will be replaced when the API hook lands. useGetPrimeTotalRewards/index.ts and useGetPrimeUserRewards/index.ts — both import their output types from the UI components they feed, which is worth cleaning up before the real API hook lands. Important Files Changed
Reviews (4): Last reviewed commit: "feat: support market modal in prime lead..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <div className="h-1.5 w-18 overflow-hidden rounded-full bg-lightGrey"> | ||
| <div | ||
| className="h-full rounded-full bg-green" | ||
| style={{ width: `${Math.min(100, (rewardsCents / totalRewardsCents) * 100)}%` }} |
There was a problem hiding this comment.
totalRewardsCents is 0, the expression rewardsCents / totalRewardsCents produces either Infinity (when rewardsCents > 0) or NaN (when both are 0). Math.min(100, Infinity) resolves correctly to 100, but Math.min(100, NaN) returns NaN, making the inline style width: NaN% — an invalid CSS value that the browser silently ignores, leaving the bar collapsed.
| style={{ width: `${Math.min(100, (rewardsCents / totalRewardsCents) * 100)}%` }} | |
| style={{ | |
| width: `${totalRewardsCents > 0 ? Math.min(100, (rewardsCents / totalRewardsCents) * 100) : 0}%`, | |
| }} |
There was a problem hiding this comment.
Fixed — added a zero-total guard so the bar renders at 0% instead of producing a NaN width.
| const marketRewards = tokens | ||
| .slice(0, placeholderMarketRewardsCents.length) | ||
| .map((token, index) => ({ token, rewardsCents: placeholderMarketRewardsCents[index] })); |
There was a problem hiding this comment.
useGetTokens() returns all tokens registered in the app's token list, and .slice(0, 2) picks whichever two happen to be first — typically generic tokens like BTC/ETH that have nothing to do with Prime reward markets. The card will render the wrong token icons and symbols paired with arbitrary cent values, actively misleading users. Even as temporary placeholder data, pairing hardcoded amounts with arbitrary tokens is likely to produce a confusing UI on any real environment. Consider substituting a static fallback (e.g. hardcoded token stubs or an empty array) until the real API hook is wired up.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
These are placeholder tokens until the API hook lands; added a TODO noting they're unrelated to the real Prime markets and will be replaced.
|
@greptile review again |
cuzz-venus
force-pushed
the
feat/prime-total-rewards
branch
from
62bff1f to
4f5780c
Compare
|
@greptile review again |
cuzz-venus
force-pushed
the
feat/prime-leaderboard-table
branch
from
e6cf09b to
7b2c2d9
Compare
cuzz-venus
force-pushed
the
feat/prime-total-rewards
branch
from
7da6c3a to
2131c0f
Compare
|
@greptile review again |
Drive the user rewards card headline by Prime eligibility (rewards amount, eligible-to-supply prompt, or not-eligible prompt) and render per-market Prime APYs with the shared Apy component. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cuzz-venus
force-pushed
the
feat/prime-total-rewards
branch
from
6bd95cb to
6eef450
Compare
| const { data: getPoolsData } = useGetPools({ accountAddress }); | ||
| const [isModalOpen, setIsModalOpen] = useState(false); | ||
|
|
||
| const market = getPoolsData?.pools | ||
| .flatMap(pool => | ||
| pool.assets.map(asset => ({ asset, poolComptrollerAddress: pool.comptrollerAddress })), | ||
| ) | ||
| .find(({ asset }) => areAddressesEqual(asset.vToken.underlyingToken.address, token.address)); | ||
|
|
||
| if (!market) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
All you need from the market is poolComptrollerAddress and vToken. Since you already have this information from the parent that renders the modal, you could just pass them through props instead of having to go through all pools and assets.
| <div className="ml-1 h-1.5 w-1/4 shrink-0 overflow-hidden rounded-full bg-lightGrey"> | ||
| <div className="h-full rounded-full bg-green" style={{ width: `${progressPercentage}%` }} /> | ||
| </div> |
There was a problem hiding this comment.
We already have a component for this: https://github.com/VenusProtocol/venus-protocol-interface/blob/59c235eeb9aa0db5814952bb069cc23ca07cba99/apps/evm/src/components/ProgressBar/index.tsx
| {marketRewards.map(({ token, rewardsCents }) => { | ||
| const asset = getPoolsData?.pools | ||
| .flatMap(pool => pool.assets) | ||
| .find(poolAsset => | ||
| areAddressesEqual(poolAsset.vToken.underlyingToken.address, token.address), | ||
| ); |
There was a problem hiding this comment.
I'm generally not a fan of having inline logic inside the return statement, because it tends to complicate the code.
In this case, the parent that renders this component already has access to the asset item you're looking for here, so you could just pass it through props.
| {content ?? ( | ||
| <div className="flex items-center gap-x-3"> | ||
| <span className="flex size-10 shrink-0 items-center justify-center rounded-lg border border-[#805c4e]"> | ||
| <img src={primeLogoSrc} alt="" className="h-5" /> |
There was a problem hiding this comment.
Let's add content for this alt property
| <div className="flex items-center gap-x-3"> | ||
| {isPrime ? ( | ||
| <span className="flex size-10 shrink-0 items-center justify-center rounded-lg border border-[#805c4e]"> | ||
| <img src={primeLogoSrc} alt="" className="h-5" /> |
There was a problem hiding this comment.
let's add content for this alt property
| token: Token; | ||
| } | ||
|
|
||
| export const MarketActions: React.FC<MarketActionsProps> = ({ token }) => { |
There was a problem hiding this comment.
I'd recommend using a more explicit name for this. Maybe MarketActionsButton or something that makes it obvious we're rendering a CTA (otherwise the plural name could give the impression we're rendering a list of elements).
| import { Icon, Modal } from 'components'; | ||
| import { useTranslation } from 'libs/translations'; | ||
| import { useAccountAddress } from 'libs/wallet'; | ||
| import { OperationForm } from 'pages/Market/OperationForm'; |
There was a problem hiding this comment.
OperationForm should be moved to the containers directory, since it's now shared by two pages.
| {isModalOpen && ( | ||
| <Modal | ||
| isOpen | ||
| title={market.asset.vToken.underlyingToken.symbol} | ||
| handleClose={() => setIsModalOpen(false)} | ||
| > | ||
| <OperationForm | ||
| vToken={market.asset.vToken} | ||
| poolComptrollerAddress={market.poolComptrollerAddress} | ||
| onSubmitSuccess={() => setIsModalOpen(false)} | ||
| /> | ||
| </Modal> | ||
| )} |
There was a problem hiding this comment.
I recommend creating a MarketFormModal component (you can name it whatever you want) inside the containers directory to render this DOM, so that it can be shared with the MarketTable component (https://github.com/VenusProtocol/venus-protocol-interface/blob/088de2b0451f8a4ef5364cbaa7e3d2133b4d9bc8/apps/evm/src/containers/MarketTable/index.tsx).
This will allow keeping one source of truth and avoid issues such as one we have here which is that if the market has protection mode enabled, it won't be displayed in the title (unlike the modal used inside the MarketTable component):
| "userRewards": { | ||
| "eligibleMessage": "You are currently eligible for sharing the Prime rewards during this cycle. Supply assets below to share the rewards.", | ||
| "marketActions": "Open market actions", | ||
| "notEligibleMessage": "You are currently NOT eligible for sharing the Prime rewards during this cycle. Stake XVS to compete for Prime for next cycle.", |
There was a problem hiding this comment.
This content wasn't proofread it seems:
| "notEligibleMessage": "You are currently NOT eligible for sharing the Prime rewards during this cycle. Stake XVS to compete for Prime for next cycle.", | |
| "notEligibleMessage": "You are currently NOT eligible for sharing the Prime rewards during this cycle. Stake XVS to compete for Prime in the next cycle.", |
| export { default as lightning2 } from './lightning2'; | ||
| export { default as graph } from './graph'; | ||
| export { default as star } from './star'; | ||
| export { default as sparkle } from './sparkle'; |
There was a problem hiding this comment.
This icon isn't used anywhere.
There was a problem hiding this comment.
What do you think about moving this component to the containers directory and reusing it in the MarketTable component (https://github.com/VenusProtocol/venus-protocol-interface/blob/088de2b0451f8a4ef5364cbaa7e3d2133b4d9bc8/apps/evm/src/containers/MarketTable/index.tsx) which renders the same UI using the renderRowControl property?
| <button | ||
| type="button" | ||
| aria-label={t('primeLeaderboard.userRewards.marketActions')} | ||
| className="ml-2 shrink-0" |
There was a problem hiding this comment.
To make it a bit more lively:
| className="ml-2 shrink-0" | |
| className="ml-2 shrink-0 transition-colors cursor-pointer hover:text-white" |
2 participants
Jira ticket(s)
VPD-1333
Changes