-
Notifications
You must be signed in to change notification settings - Fork 103
Staking Page Refactor Part I #4467
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
Conversation
packages/yoroi-extension/app/UI/features/staking/useCases/RewardsSummary/RewardsSummaryCard.tsx
Outdated
Show resolved
Hide resolved
packages/yoroi-extension/app/UI/features/staking/module/StakingContextProvider.tsx
Show resolved
Hide resolved
packages/yoroi-extension/app/UI/features/staking/useCases/RewardsSummary/RewardsSummaryCard.tsx
Outdated
Show resolved
Hide resolved
SorinC6
left a comment
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.
/check
SorinC6
left a comment
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.
/check
packages/yoroi-extension/app/UI/features/staking/module/StakingContextProvider.tsx
Show resolved
Hide resolved
loxator
left a comment
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.
Some constants are typed as null | undefined, but usually its only checked for if(x == null), why not use something like if(!x) so it takes care of both cases?
Just some small comments as I am not versed with the business logic too much, better to get a review from @yushih too!
| [initialState] | ||
| ); | ||
|
|
||
| React.useEffect(() => {}, [stores]); |
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.
empty useEffect?
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.
fixed in the next PR
| <Box> | ||
| <Box display="block"> | ||
| <Typography component="div" color="var(--yoroi-palette-gray-600)"> | ||
| Stake Pool |
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.
should this come from useStrings()?
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.
fixed in the next PR
| /* ---------- Styled components ---------- */ | ||
|
|
||
| const Accordion = styled((props: MuiAccordionProps) => ( | ||
| <MuiAccordion TransitionProps={{ timeout: { exit: 500 } }} disableGutters elevation={0} square {...props} /> | ||
| ))(() => ({ | ||
| borderBottom: `1px solid var(--yoroi-palette-gray-50)`, | ||
| '&:not(:last-child)': { | ||
| borderBottom: 0, | ||
| }, | ||
| '&:before': { | ||
| display: 'none', | ||
| }, | ||
| paddingBottom: '16px', | ||
| marginBottom: '16px', | ||
| })); | ||
|
|
||
| const AccordionDetails = styled(MuiAccordionDetails)(() => ({ | ||
| marginTop: '24px', | ||
| padding: 0, | ||
| })); | ||
|
|
||
| const AvatarImg = styled('img')({ | ||
| width: '100%', | ||
| background: 'white', | ||
| objectFit: 'scale-down', | ||
| }); | ||
|
|
||
| const AvatarWrapper = styled(Box)({ | ||
| width: '24px', | ||
| height: '24px', | ||
| minWidth: '24px', | ||
| marginRight: '12px', | ||
| borderRadius: '20px', | ||
| overflow: 'hidden', | ||
| }); | ||
|
|
||
| const ExpandMoreIcon: React.FC = () => ( | ||
| <svg width="14" height="8" viewBox="0 0 14 8" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path | ||
| fillRule="evenodd" | ||
| clipRule="evenodd" | ||
| d="M0.292893 0.292893C0.683417 -0.0976311 1.31658 -0.0976311 1.70711 0.292893L7 5.58579L12.2929 0.292893C12.6834 -0.0976311 13.3166 -0.0976311 13.7071 0.292893C14.0976 0.683417 14.0976 1.31658 13.7071 1.70711L7.70711 7.70711C7.31658 8.09763 6.68342 8.09763 6.29289 7.70711L0.292893 1.70711C-0.0976311 1.31658 -0.0976311 0.683417 0.292893 0.292893Z" | ||
| fill="#6B7384" | ||
| /> | ||
| </svg> | ||
| ); | ||
|
|
||
| const AccordionSummary = styled((props: MuiAccordionSummaryProps) => ( | ||
| <MuiAccordionSummary expandIcon={<ExpandMoreIcon />} {...props} /> | ||
| ))(() => ({ | ||
| padding: 0, | ||
| '.MuiAccordionSummary-content': { | ||
| margin: 0, | ||
| }, | ||
| '& .MuiAccordionSummary-expandIconWrapper.Mui-expanded': { | ||
| transform: 'rotate(180deg)', | ||
| }, | ||
| '& .MuiCollapse-vertical': { | ||
| transitionDuration: '0.5s', | ||
| }, | ||
| })); | ||
|
|
||
| const RewardHistoryGraph: React.FC<RewardHistoryGraphProps> = ({ graphData, onOpenRewardList }) => { | ||
| const strings = useStrings(); | ||
| const { rewardsGraphData } = graphData; | ||
| const rewardList = rewardsGraphData.items?.perEpochRewards; | ||
| const title = strings.rewardHistoryLabel; | ||
|
|
||
| return ( | ||
| <Box | ||
| p="24px" | ||
| sx={{ | ||
| display: 'flex', | ||
| flexFlow: 'column', | ||
| justifyContent: 'space-between', | ||
| }} | ||
| > | ||
| <Box | ||
| sx={{ | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'space-between', | ||
| width: '100%', | ||
| }} | ||
| > | ||
| <Typography component="div" variant="body1" fontWeight={500} color="ds.text_gray_medium"> | ||
| {title} | ||
| </Typography> | ||
| <Button | ||
| // @ts-ignore | ||
| variant="primary" | ||
| size="medium" | ||
| onClick={onOpenRewardList} | ||
| sx={{ lineHeight: '21px' }} | ||
| > | ||
| {title} | ||
| </Button> | ||
| </Box> | ||
|
|
||
| {rewardsGraphData.error && !rewardsGraphData.items && ( | ||
| <div> | ||
| <Typography variant="body2" color="ds.text_error"> | ||
| {strings.errorLabel} | ||
| </Typography> | ||
| </div> | ||
| )} | ||
|
|
||
| {!Array.isArray(rewardList) ? ( | ||
| <CircularProgress /> | ||
| ) : ( | ||
| <Box ml="-50px"> | ||
| <RewardGraphClean | ||
| epochTitle={strings.epochLabel} | ||
| stakepoolNameTitle={strings.stakepoolNameLabel} | ||
| xAxisLabel={strings.epochLabel} | ||
| yAxisLabel={strings.rewardValue} | ||
| primaryBarLabel={strings.rewardsLabel} | ||
| data={rewardList} | ||
| hideYAxis={rewardsGraphData.hideYAxis} | ||
| /> | ||
| </Box> | ||
| )} | ||
| </Box> | ||
| ); | ||
| }; | ||
|
|
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.
maybe better to import these from another local components file, just to clean up a bit and make component smaller, nothing major
SorinC6
left a comment
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.
/check
| return; | ||
| } | ||
| return createWithdrawalTx(); | ||
| }; |
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.
Bug: Governance participation check treats unknown as true
isParticipatingToGovernance uses stores.delegation.governanceStatus?.drepDelegation !== null, which becomes true when governanceStatus is null/undefined (because undefined !== null). This can incorrectly skip GovernanceParticipateDialog and proceed to withdrawal flow before governance status is actually available.
packages/yoroi-extension/app/UI/features/staking/module/StakingContextProvider.tsx
Outdated
Show resolved
Hide resolved
| return; | ||
| } | ||
| return createWithdrawalTx(); | ||
| }; |
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.
Bug: Withdraw uses stale governance status
isParticipatingToGovernance is derived from stores.delegation.governanceStatus inside WithdrawButton, but the component isn’t observer-wrapped and doesn’t subscribe to that observable via state. If governance status changes after render, the click handler can use a stale value and incorrectly open GovernanceParticipateDialog even when the user is already participating.
| delegationRequests, | ||
| isWalletWithNoFunds, | ||
| currentlyDelegating, | ||
| }; |
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.
Bug: Reward history button does nothing
RewardsSummaryCard consumes onOpenRewardList from useStaking, but StakingContextProvider never sets it, so it stays the default no-op from defaultStakingState. As a result, the “Reward History” action won’t open anything when clicked.
Additional Locations (1)
| {truncateToken(tokenInfo?.assetName || '', 12)} | ||
| </> | ||
| ); | ||
| }; |
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.
Bug: Token label blank in summary
formatTokenEntry renders the token label using truncateToken(tokenInfo?.assetName || '', 12), but getTokenInfo returns a TokenRow where the display name/ticker isn’t exposed as assetName. This can render an empty token label (eg, missing “ADA”) in the rewards summary amounts.
Nebyt
left a comment
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.
/check
Tasks:
Note
Introduces the revamp Staking dashboard with rewards summary/graphs, new routing/context, query key factories, and supporting icons/utilities.
StakingPageandlayoutunderUI/pages/Staking/*,StakingRootwithRewardsSummaryCardand placeholderPoolList.StakingContextProviderwith graph/data helpers (common/helpers/graph.ts), types, and i18n strings.RewardHistoryGraph,RewardGraphClean) andWithdrawButtonflow.ROUTES.STAKING_REVAMP.ROOT(/staking-revamp) and wrap withStakingSubpages(StakingContextProvider).Routes.js.poolQueryKeysfor pool list/info.Icon.StakingActiveandIcon.TotalDelegatedand export inicons/index.tsx.GovernanceContextProviderby removing pricing/formatting props and helpers.HIDDEN_AMOUNTconstant.@yoroi/apitopackage.json.Written by Cursor Bugbot for commit 7e4e9af. This will update automatically on new commits. Configure here.