Skip to content

fix: handle cases where decimalSeparator is empty #385

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

Merged
merged 7 commits into from
Feb 22, 2025

Conversation

pmmmwh
Copy link
Contributor

@pmmmwh pmmmwh commented Feb 3, 2025

Previously, for zero-decimal currencies (such as JPY), input will be lost on blur if decimalScale is used (no matter what value was passed in). This was down to the fact that decimalSeparator from getLocaleConfig will be undefined (since it doesn't exist in the output of formatToParts), and the fallback value of '' was used.

This PR adds handling in the pad/trim logic to skip all processing when we see '' as the separator.

Re-opened since #379 was closed due to staleness.

Fixes #310

@juancamilo100
Copy link

Hi @cchanxzy and maintainers! 👋

I'm currently encountering this exact issue with JPY in my production application. When entering values in a JPY input (e.g., "700"), after blur the value gets converted incorrectly to "7". This is causing significant problems for our Japanese users.

Would it be possible to get a review of this PR? Let me know if there's anything I can do to help move this forward, such as providing additional test cases or helping with review.

Thank you for maintaining this useful package!

Copy link

@Betree Betree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, everything works fine. The test coverage also seems quite complete.

We want to adopt this library for https://opencollective.com, and this is the last bit blocking adoption. Happy to help with whatever's needed to bring this PR to main!

image

Previously, for zero-decimal currencies (such as JPY), input will be lost on blur if `decimalScale` is used (no matter what value was passed in). This was down to the fact that `decimalSeparator` from `getLocaleConfig` will be `undefined` (since it doesn't exist in the output of `formatToParts`), and the fallback value of `''` was used.

This PR adds handling in the pad/trim logic to skip all processing when we see `''` as the separator.
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (396d57b) to head (bca2229).
Report is 6 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #385   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          414       427   +13     
  Branches       156       168   +12     
=========================================
+ Hits           414       427   +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cchanxzy cchanxzy merged commit 656e5c2 into cchanxzy:main Feb 22, 2025
2 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 22, 2025
## [3.9.2](v3.9.1...v3.9.2) (2025-02-22)

### Bug Fixes

* handle cases where decimalSeparator is empty ([#385](#385)) ([656e5c2](656e5c2))
Copy link

🎉 This PR is included in version 3.9.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@cchanxzy
Copy link
Owner

cchanxzy commented Feb 22, 2025

@pmmmwh looks good, thank you for the contribution!

If you're interested in adding more new features, feel free to check out v4.0.0-alpha

github-actions bot pushed a commit that referenced this pull request Feb 22, 2025
# [4.0.0-alpha.3](v4.0.0-alpha.2...v4.0.0-alpha.3) (2025-02-22)

### Bug Fixes

* add react 19 as peer dependency ([396d57b](396d57b)), closes [#380](#380)
* handle cases where decimalSeparator is empty ([#385](#385)) ([656e5c2](656e5c2))

### Features

* intlConfig support all NumberFormatOptions ([#386](#386)) ([0b84349](0b84349))
* merge changes in main into alpha branch ([6985156](6985156))
Copy link

🎉 This PR is included in version 4.0.0-alpha.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bluring component causes certain currencies to lose most of input
5 participants