-
Notifications
You must be signed in to change notification settings - Fork 2
🌐 app: normalize usd display #658
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
🦋 Changeset detectedLatest commit: 78b718f The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive update to how USD currency amounts are displayed throughout the mobile application. The primary goal is to ensure a uniform and predictable presentation of monetary values, enhancing user experience by providing clear and consistent financial information. This change moves away from locale-dependent currency formatting to a more controlled decimal representation with an explicit dollar sign prefix. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
@cursor review |
PR SummaryUnifies USD display and tightens typography for consistent UI.
Written by Cursor Bugbot for commit bd3a684. This will update automatically on new commits. Configure here. |
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 2
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| })} | ||
| {`$${(markets ? Number(borrowLimit(markets, marketUSDCAddress)) / 1e6 : 0).toLocaleString(language, { | ||
| style: "decimal", | ||
| })}`} |
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.
Missing fraction digits in CardContents USD formatting
Medium Severity
The toLocaleString calls in CardContents.tsx for the "Available balance" display only specify style: "decimal" without minimumFractionDigits or maximumFractionDigits. All other USD amount displays in this PR consistently use minimumFractionDigits: 2, maximumFractionDigits: 2. This omission causes inconsistent formatting—a balance of 1234.5 might display as $1,234.5 here but $1,234.50 everywhere else in the app.
Additional Locations (1)
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.
@coderabbitai is this fixed in the latest force-pushed code?
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.
@dieguezguille Yes, this has been fixed! Looking at the current file content, both locations now include the proper fraction digit specifications:
Credit branch (lines 79-82):
{`$${(markets ? Number(borrowLimit(markets, marketUSDCAddress)) / 1e6 : 0).toLocaleString(language, {
style: "decimal",
minimumFractionDigits: 2,
maximumFractionDigits: 2,
})}`}Debit branch (lines 97-103):
{`$${(markets ? Number(withdrawLimit(markets, marketUSDCAddress)) / 1e6 : 0).toLocaleString(
language,
{
style: "decimal",
minimumFractionDigits: 2,
maximumFractionDigits: 2,
},
)}`}Both instances now consistently format USD amounts with exactly 2 decimal places, matching the rest of the PR changes. ✅
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.
Code Review
This pull request consistently normalizes the display of USD values across the application by explicitly prepending a dollar sign ($) and setting the style option to decimal in toLocaleString calls. This ensures a uniform presentation of currency, which is a good practice for user interface consistency. Additionally, lineHeight properties have been removed from several Text components, aligning with a broader style update in src/components/shared/Text.tsx to streamline text styling. The changes are well-applied and improve the maintainability and consistency of the codebase.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR normalizes USD display formatting across 40+ mobile UI components by replacing locale-based currency formatting (with USD currency code and narrow symbol) with consistent decimal formatting (2 fractional digits) prefixed by a literal "$" symbol. Additionally, several components have lineHeight properties removed from Text styling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #658 +/- ##
==========================================
+ Coverage 78.91% 78.96% +0.05%
==========================================
Files 43 43
Lines 1987 1987
Branches 435 435
==========================================
+ Hits 1568 1569 +1
+ Misses 269 267 -2
- Partials 150 151 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.