feat: improve Amount formatting capabilities - #871
Conversation
- Add notation prop for compact formatting ($1.2M) in dashboards - Add narrowSymbol option to currencyDisplay - Add signDisplay prop for gains/losses (+/- sign control) - Add tabularNums prop (default true) to toggle fixed-width figures - Memoize Intl.NumberFormat instances module-wide; resolve currency validity and decimals in one cached lookup
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe Sequence Diagram(s)sequenceDiagram
participant Amount
participant getCurrencyInfo
participant getFormatter
participant IntlNumberFormat
Amount->>getCurrencyInfo: Validate currency and read decimal count
getCurrencyInfo->>getFormatter: Request cached currency formatter
getFormatter->>IntlNumberFormat: Create or reuse formatter
Amount->>getFormatter: Format with notation and signDisplay
getFormatter->>IntlNumberFormat: Create or reuse formatter
Amount->>Amount: Render value with optional tabular styling
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/raystack/components/amount/amount.tsx`:
- Around line 142-163: Bound currencyInfoCache before inserting entries in
getCurrencyInfo, using the same 64-entry limit as formatterCache; alternatively,
skip caching when the currency code is invalid. Preserve cached lookups for
valid currencies while ensuring unlimited distinct invalid inputs cannot cause
persistent module-level growth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d73b795e-3e56-4627-9a51-35c6ad4032e4
📒 Files selected for processing (6)
apps/www/src/components/playground/amount-examples.tsxapps/www/src/content/docs/components/amount/demo.tsapps/www/src/content/docs/components/amount/index.mdxpackages/raystack/components/amount/__tests__/amount.test.tsxpackages/raystack/components/amount/amount.module.csspackages/raystack/components/amount/amount.tsx
…tting-improvements # Conflicts: # apps/www/src/components/playground/amount-examples.tsx # apps/www/src/content/docs/components/amount/index.mdx # packages/raystack/components/amount/amount.tsx
- Fall back to USD when currency is not a string. The lookup called toUpperCase outside its try, so null rendered the raw value. - Set proportional-nums when tabularNums is false, so a tabular parent such as Table or DataTable does not override it. - Read currency decimals with a plain Intl.NumberFormat, so the lookup does not use a slot in the formatter cache. - Pin the short string case from #595 and use the CSS module in the tabularNums tests. - Document narrowSymbol, and bring the new JSDoc and docs in line with the writing style rules.
- hideDecimals turned values between -1 and 0 into -0, which printed "-$0". Both the number and the string path now print "$0". - Document that hideDecimals truncates, and that compact notation rounds the abbreviated value instead. Pin both with tests.
commit: |
Summary
Implements the remaining items from #595, plus a
tabularNumsprop.New props
notation('standard' | 'compact', default'standard') — compact formatting like$1.2M/$13Kfor dashboards and summary views. Note: compact rounds aggressively by design (Intl behavior), so small values like$12.99render as$13.currencyDisplay: 'narrowSymbol'— always shows the narrow symbol ($instead ofUS$in non-US locales).signDisplay('auto' | 'always' | 'exceptZero' | 'never', default'auto') — controls the+/-sign, useful for gains/losses.tabularNums(defaulttrue) — fixed-width figures so digits align across table rows. Previously this was always on via CSS; it is now a prop so running text can opt out for proportional figures. Default behavior is unchanged.Performance
Intl.NumberFormatinstances are now memoized in a module-level cache (keyed by locale + options, capped at 64 entries). Previously every render created 3 formatter instances, which adds up fast when Amount renders in table rows.isValidCurrency()andgetCurrencyDecimals()are merged into a single cachedgetCurrencyInfo()lookup. Decimals now come fromresolvedOptions().maximumFractionDigitsinstead of regex-matching formatted output.Not included
padStartconversion handles short strings correctly ("5"→$0.05).Docs
notation,signDisplay, andtabularNumsexample sections.Test plan
Closes #595