-
Notifications
You must be signed in to change notification settings - Fork 5
style: hide arrow button when only one entry exists #2062
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe PositionItem component is enhanced with a totalPositions prop that controls conditional rendering and layout styling. The parent component calculates the total position count and passes it to PositionItem, which uses it to determine whether to display an ArrowButton and adjust padding accordingly. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/extension-polkagate/src/fullscreen/stake/stakingPositions/index.tsx (1)
32-69:totalPositionsdoes not reflect the actual number of rendered position cardsCurrently,
totalPositionscounts all tokens inpositionItems, but rendering can produce a different number ofPositionItemcomponents due to:
- Testnet gating (line 39–40): tokens from
TEST_NETSare skipped ifstate.isTestnetis false- Zero-balance filtering (lines 45, 58): empty
pooledBalanceorsoloTotalskip rendering- Staking type filtering (lines 45, 58): type constraints ('pool', 'solo', 'both') reduce rendered items
Since the arrow button (PositionItem.tsx line 142–143) is shown only when
totalPositions > 1, mismatches occur:
- One token with both pool and solo: renders two
PositionItems buttotalPositions === 1, so the arrow is hidden despite two visible entries.- One mainnet and one testnet token with testnets disabled: renders one
PositionItembuttotalPositions === 2, so the arrow is shown for a single entry.To match the arrow visibility to actual rendered cards, compute
totalPositionsafter filtering:const totalPositions = positionItems?.reduce((count, { pooledBalance, soloTotal, genesisHash }) => { if (TEST_NETS.includes(genesisHash) && !state.isTestnet) { return count; } if (pooledBalance && !pooledBalance.isZero() && ['both', 'pool'].includes(state.stakingType)) { count += 1; } if (soloTotal && !soloTotal.isZero() && ['both', 'solo'].includes(state.stakingType)) { count += 1; } return count; }, 0) ?? 0;
🧹 Nitpick comments (1)
packages/extension-polkagate/src/fullscreen/stake/stakingPositions/PositionItem.tsx (1)
125-144: Arrow visibility & padding logic are correct; consider simplifying ArrowButton click handlingThe conditional padding (
pvariant based ontotalPositions) pluspl='18px'cleanly gives extra right padding only when the arrow is hidden, which matches the PR goal.Using
totalPositions > 1to gate theArrowButtonalso aligns with “hide arrow when only one entry exists”. WithonClick={noop}, clicks on the arrow still bubble to the parentContainer’sonClick, so navigation continues to work without double-calling the handler.If you want to simplify, you could:
- Drop the
onClickprop fromArrowButtonentirely and let it be purely visual, or- Remove the
onClickon the ArrowButton’s innerGridand rely solely on the parent container click.Both are minor cleanups; current behavior is functionally fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension-polkagate/src/fullscreen/stake/stakingPositions/PositionItem.tsx(3 hunks)packages/extension-polkagate/src/fullscreen/stake/stakingPositions/index.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (1)
packages/extension-polkagate/src/fullscreen/stake/stakingPositions/PositionItem.tsx (1)
89-100: NewtotalPositionsprop wiring looks consistentThe added
totalPositionsprop and its use in thePositionItemsignature are consistent and typed appropriately. Assuming all call sites pass this new prop (as inPositionOptions), this API change looks good.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.