fix(ui): locale-aware negative amount formatting#9064
Conversation
🦋 Changeset detectedLatest commit: 83f12d7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughIntroduces a ChangesNegative amount formatting
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/utils/billing.ts (1)
7-8: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNegative-zero edge case.
If
amount.amountis0,0 < 0is false, so the function negates it to-0and prefixesamountFormattedwith-, producing something like-$0.00for a zero value. Minor edge case, likely rare in practice (line items with zero amounts probably aren't rendered as discounts/credits), but worth a guard if zero amounts can reach this function.🤖 Prompt for 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. In `@packages/ui/src/utils/billing.ts` around lines 7 - 8, The zero-value edge case in the billing amount formatter can produce a negative zero display, so update the logic in the amount formatting function in billing.ts to treat zero as a non-negative value before applying the negation/prefix. Adjust the existing negative check around amount.amount so only values strictly less than zero are negated, and ensure zero returns without the minus sign while preserving the current behavior for real negatives in the same helper.packages/ui/src/utils/__tests__/billing.test.ts (1)
5-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest suite doesn't validate the "locale/currency-aware" claim.
Both tests use a USD-style
amountFormatted('1.00'/'-1.00') and only assert the blanket-prefix behavior. Given the PR's objective is specifically about correct sign placement for non-USD locales (e.g., CLP$-5.000), consider adding a case with a formatted string where the currency symbol trails the number, or where the expected placement differs, to actually exercise the locale-aware behavior this PR claims to introduce.🤖 Prompt for 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. In `@packages/ui/src/utils/__tests__/billing.test.ts` around lines 5 - 35, The current `toNegativeAmount` tests only cover USD-style formatting, so they do not verify the locale/currency-aware sign placement behavior this PR is meant to add. Update the `toNegativeAmount` test suite to include a non-USD locale case (for example, a format where the currency symbol trails the number or uses a different sign position) and assert the expected `amountFormatted` output after calling `toNegativeAmount`. Keep the existing positive/negative amount assertions, but make sure the new case exercises the locale-sensitive formatting logic in `toNegativeAmount` rather than only checking a simple `-` prefix.
🤖 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.
Nitpick comments:
In `@packages/ui/src/utils/__tests__/billing.test.ts`:
- Around line 5-35: The current `toNegativeAmount` tests only cover USD-style
formatting, so they do not verify the locale/currency-aware sign placement
behavior this PR is meant to add. Update the `toNegativeAmount` test suite to
include a non-USD locale case (for example, a format where the currency symbol
trails the number or uses a different sign position) and assert the expected
`amountFormatted` output after calling `toNegativeAmount`. Keep the existing
positive/negative amount assertions, but make sure the new case exercises the
locale-sensitive formatting logic in `toNegativeAmount` rather than only
checking a simple `-` prefix.
In `@packages/ui/src/utils/billing.ts`:
- Around line 7-8: The zero-value edge case in the billing amount formatter can
produce a negative zero display, so update the logic in the amount formatting
function in billing.ts to treat zero as a non-negative value before applying the
negation/prefix. Adjust the existing negative check around amount.amount so only
values strictly less than zero are negated, and ensure zero returns without the
minus sign while preserving the current behavior for real negatives in the same
helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ac07ce35-29a4-414b-b0c5-46b5a608a4d9
📒 Files selected for processing (6)
.changeset/sixty-hairs-sniff.mdpackages/ui/src/components/Checkout/CheckoutForm.tsxpackages/ui/src/components/Checkout/__tests__/Checkout.test.tsxpackages/ui/src/components/PaymentAttempts/PaymentAttemptPage.tsxpackages/ui/src/utils/__tests__/billing.test.tspackages/ui/src/utils/billing.ts
2 participants
Description
This PR introduces locale- and currency-aware formatting of negative monetary values. For example, formatting a negative value in Chilean peso would result in
$-5.000instead of the previous implementation which would have put the-symbol before the currency symbol.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
-$5.00instead of- $5.00.