Conversation
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3182/portfolio-tile-refactor)
WalkthroughThis update removes the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PrimeTokensBalance
participant BalancePnlGraph
User->>PrimeTokensBalance: View balance
PrimeTokensBalance->>PrimeTokensBalance: Calculate balance (raw sum)
PrimeTokensBalance->>PrimeTokensBalance: Format with toFixed(2)
PrimeTokensBalance-->>User: Display formatted balance
User->>BalancePnlGraph: View portfolio graph
BalancePnlGraph->>BalancePnlGraph: Get hover/latest values
BalancePnlGraph->>BalancePnlGraph: Format with toFixed(2)
BalancePnlGraph-->>User: Display formatted values
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
src/apps/pillarx-app/components/PrimeTokensBalance/PrimeTokensBalance.tsx(2 hunks)src/apps/pillarx-app/components/PrimeTokensBalance/test/PrimeTokensBalance.test.tsx(0 hunks)src/apps/pillarx-app/components/WalletPortfolioGraph/BalancePnlGraph.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/apps/pillarx-app/components/PrimeTokensBalance/test/PrimeTokensBalance.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (8)
src/apps/pillarx-app/components/PrimeTokensBalance/PrimeTokensBalance.tsx (6)
46-46: LGTM: Simplified balance calculation.Returning the raw
totalBalancedirectly instead of applying thelimitDigitsNumberutility simplifies the code and moves the formatting responsibility to the display layer where it belongs.
67-67: LGTM: Consistent currency formatting with proper safeguards.Using
toFixed(2)is the appropriate choice for currency display. The conditional check ensures the method is only called on valid positive numbers, with a fallback to '0.00' for edge cases.
46-46: LGTM: Proper separation of concerns.Moving the number formatting from data processing to display time is a good architectural decision. The raw
totalBalancevalue is now preserved for calculations, with formatting applied only when rendering.
67-67: LGTM: Consistent 2-decimal formatting implemented.The use of
toFixed(2)correctly implements the PR objective of displaying all balances with exactly 2 decimal places. The conditional logic properly guards against callingtoFixed(2)on undefined values.
46-46: LGTM! Clean separation of calculation and formatting logic.Returning the raw
totalBalancevalue allows the calculation logic to remain pure while delegating formatting responsibility to the display layer.
67-67: LGTM! Proper number formatting with safe conditional checks.The use of
toFixed(2)ensures consistent 2-decimal formatting for currency display. The conditional logic properly handles undefined/null values before callingtoFixed(2).src/apps/pillarx-app/components/WalletPortfolioGraph/BalancePnlGraph.tsx (2)
483-484: LGTM: Consistent formatting for hover and latest values.The implementation correctly uses
toFixed(2)for bothhoverValueandlatestValuewith proper conditional checks. The fallback logic ensures a consistent currency format ('$0.00') when values are undefined, maintaining a good user experience.
483-484: LGTM! Consistent currency formatting with proper null safety.Both
hoverValue.toFixed(2)andlatestValue.toFixed(2)are safely guarded by conditional checks. The formatting ensures consistent 2-decimal display for all currency values, maintaining uniformity with the fallback '$0.00'.
| ? `$${hoverValue.toFixed(2)}` | ||
| : `${latestValue ? `$${latestValue.toFixed(2)}` : '$0.00'}`} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM: Consistent formatting applied to all balance displays.
The toFixed(2) formatting is properly applied to both hover and latest values, ensuring consistent 2-decimal display across all balance presentations. The conditional logic appropriately protects against undefined values.
Please verify that limitDigitsNumber has been completely removed from the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Verify that limitDigitsNumber utility is no longer used anywhere in the codebase
# Expected: No remaining references to limitDigitsNumber
echo "Searching for any remaining references to limitDigitsNumber..."
rg -i "limitdigitsnumber" --type ts --type tsx --type js --type jsx
echo -e "\nSearching for the utility file itself..."
fd -e ts -e tsx -e js -e jsx | xargs grep -l "limitDigitsNumber.*function\|function.*limitDigitsNumber\|const.*limitDigitsNumber\|export.*limitDigitsNumber" 2>/dev/null || echo "No utility file found"Length of output: 699
Please remove all remaining limitDigitsNumber references
We still have references to the old utility in the codebase. Update these locations to use .toFixed(2) (or your new formatting helper) and delete the unused utility:
src/utils/number.tsx(defineslimitDigitsNumber)src/apps/pillarx-app/components/TokenMarketDataRow/tests/RightColumnTokenMarketDataRow.test.tsxsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx
Once you’ve refactored those tests and removed the file, re-run the test suite to confirm everything still passes.
🤖 Prompt for AI Agents
In src/apps/pillarx-app/components/WalletPortfolioGraph/BalancePnlGraph.tsx
around lines 483 to 484, the code correctly uses toFixed(2) for formatting
numbers, but there are still remaining references to the old limitDigitsNumber
utility elsewhere in the codebase. Locate and replace all instances of
limitDigitsNumber in src/utils/number.tsx,
src/apps/pillarx-app/components/TokenMarketDataRow/tests/RightColumnTokenMarketDataRow.test.tsx,
and
src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx
with toFixed(2) or the new formatting helper. After refactoring these files,
delete the limitDigitsNumber utility file and run the test suite to ensure no
regressions.
Deploying x with
|
| Latest commit: |
e2d10be
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a2e0fc8d.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3182-portfolio-tile.x-e62.pages.dev |
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit