Skip to content
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

Intl.NumberFormat on Android with style: 'currency' and signDisplay: 'always' truncates currency sign for positive values #789

Open
1 task done
stefan-schweiger opened this issue Jul 29, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@stefan-schweiger
Copy link

stefan-schweiger commented Jul 29, 2022

Bug Description

When you use Intl.NumberFormat number with style: 'currency' and signDisplay: 'always' it works well enough for negative numbers, but as soon as you try it with a positive number it just removes the currency sign. See the code below.

This problem only occures on Android

This is not relevant as Intl doesn't work with JSC:

  • I have run gradle clean and confirmed this bug does not occur with JSC

Hermes version: [email protected]
React Native version: 0.69.3
Platform: arm64-v8a

Steps To Reproduce

const App = () => {
  return (
    <View>
      <Text>
        {new Intl.NumberFormat('en-US', {style: 'currency', currency: 'EUR', signDisplay: 'always'}).format(100.32)}
        {'\n'}
        {new Intl.NumberFormat('en-US', {style: 'currency', currency: 'EUR', signDisplay: 'always'}).format(-100.32)}
        {'\n'}
        {new Intl.NumberFormat('en-US', {style: 'currency', currency: 'EUR', signDisplay: 'auto'}).format(100.32)}
        {'\n'}
        {new Intl.NumberFormat('en-US', {style: 'currency', currency: 'EUR', signDisplay: 'auto'}).format(-100.32)}
        {'\n'}

        {!!global.HermesInternal ? 'Hermes' : 'JSC'}
      </Text>
    </View>
  );
};
export default App;

Produces the following output on Android:

+100.32
-€100.32
€100.32
-€100.32

For iOS and Web it produces the correct output:

+€100.32
-€100.32
€100.32
-€100.32

The Expected Behavior

Intl.NumberFormat should produce the same output on each platform.

@stefan-schweiger stefan-schweiger added the bug Something isn't working label Jul 29, 2022
@dwatring
Copy link

Similarly, for android this configuration is also not consistent:

const App = () => {
  return (
    <View>
      <Text>
        {new Intl.NumberFormat('en-US', {style: 'percent', signDisplay: 'always'}).format(.32)}
        {'\n'}
        {new Intl.NumberFormat('en-US', {style: 'percent', signDisplay: 'always'}).format(-.32)}
        {'\n'}
        {new Intl.NumberFormat('en-US', {style: 'percent', signDisplay: 'auto'}).format(.32)}
        {'\n'}
        {new Intl.NumberFormat('en-US', {style: 'percent', signDisplay: 'auto'}).format(-.32)}
        {'\n'}

        {!!global.HermesInternal ? 'Hermes' : 'JSC'}
      </Text>
    </View>
  );
};
export default App;

Produces the following output on Android:

+32+
-32%
32%
-32%

For iOS and Web it produces the correct output:

+32%
-32%
32%
-32%

@Bi0max
Copy link

Bi0max commented Aug 9, 2023

Having the same issue

@tiago138
Copy link

November 30, 2023 and I am running into the same problem.
:(

@shubhamguptadream11
Copy link
Contributor

I had raised a PR to fix this: #1483

facebook-github-bot pushed a commit that referenced this issue Sep 17, 2024
Summary:
Fixes following issue:
- #1466
- #1138
- #789

**Why this change is made?**

There are few cases where signDisplay is not being handled correctly.
Examples:
```
1.
new Intl.NumberFormat('de-DE', {
 style: 'currency',
 currency: 'EUR',
 signDisplay: 'exceptZero',
}).format(8537.71)

Output: +8,537,71+
Expected: +8.537,71 €

2.
new Intl.NumberFormat('ja-JP', {
 style: 'currency',
 currency: 'JPY',
 signDisplay: 'exceptZero'
}).format(123456.789)

Output: +123,457
Expected: +¥123,457
```

**ChangeLog**

This PR updates the implementation and testing of the signDisplay functionality in the DecimalFormat class within the Hermes engine, specifically for Android API level 31 and above.

**Key Changes:**

Implementation:
- Integrated the **setSignAlwaysShown** method of DecimalFormat for API level 31 and above to control the display of the sign (+ or -) based on the signDisplay option. [For more detail about setSignAlwaysShown check [here](https://developer.android.com/reference/android/icu/text/DecimalFormat#setSignAlwaysShown(boolean))]
- For API levels below 31, maintained the existing logic for handling sign display, ensuring backward compatibility.

Pull Request resolved: #1483

Test Plan:
- Added a comprehensive set of test cases in **HermesIntlAndroidTest.java** to validate the behaviour of the signDisplay functionality, specifically for API level 31 and above.
- Test cases cover scenarios for signDisplay: `NEVER | ALWAYS | EXCEPTZERO`

Reviewed By: avp

Differential Revision: D62153166

Pulled By: neildhar

fbshipit-source-id: d45c55ae7ffbfbc38ec4b331339859b8c96486c7
facebook-github-bot pushed a commit that referenced this issue Sep 17, 2024
…1483)

Summary:
Original Author: [email protected]
Original Git: a75e14a
Original Reviewed By: avp
Original Revision: D62153166

Fixes following issue:
- #1466
- #1138
- #789

**Why this change is made?**

There are few cases where signDisplay is not being handled correctly.
Examples:
```
1.
new Intl.NumberFormat('de-DE', {
 style: 'currency',
 currency: 'EUR',
 signDisplay: 'exceptZero',
}).format(8537.71)

Output: +8,537,71+
Expected: +8.537,71 €

2.
new Intl.NumberFormat('ja-JP', {
 style: 'currency',
 currency: 'JPY',
 signDisplay: 'exceptZero'
}).format(123456.789)

Output: +123,457
Expected: +¥123,457
```

**ChangeLog**

This PR updates the implementation and testing of the signDisplay functionality in the DecimalFormat class within the Hermes engine, specifically for Android API level 31 and above.

**Key Changes:**

Implementation:
- Integrated the **setSignAlwaysShown** method of DecimalFormat for API level 31 and above to control the display of the sign (+ or -) based on the signDisplay option. [For more detail about setSignAlwaysShown check [here](https://developer.android.com/reference/android/icu/text/DecimalFormat#setSignAlwaysShown(boolean))]
- For API levels below 31, maintained the existing logic for handling sign display, ensuring backward compatibility.

Pull Request resolved: #1483

Pulled By: neildhar

Reviewed By: neildhar

Differential Revision: D62883944

fbshipit-source-id: df41029b45400b9db038f32727293d5ccda27211
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants