Shopify / quilt

[⚠️ Deprecated] A loosely related set of packages for JavaScript/TypeScript projects at Shopify
MIT License
1.7k stars 220 forks source link

[react-i18n] Replace dash with minus symbol in negative values when formatting currency #2317

Open trishrempel opened 2 years ago

trishrempel commented 2 years ago

Overview

Shopify designers identified in this issue that formatting numbers and currencies with the "minus" symbol (U+2212 or −) would provide better visual alignment than the "hyphen-minus" symbol (U+002D or -), and that it is the more correct mathematical symbol.

image

A similar change was recently made in the Shopify/condense-number library.

However, we need to be careful when making this change to take all locales into account and different implementations of the "minus" symbol.

Is it truly the correct decision to always use the U+2212 symbol for all locales, or should we try to respect the symbol returned by INTL.NumberFormat as much as possible?

Checklist

spy-v2[bot] commented 2 years ago

Trevor Harmon [2022-06-17 08:47PM UTC]
:wave-hand: I have a question about number formatting with react-i18n :thread:

:thread: Slack Thread
[Slack Thread](https://shopify.slack.com/archives/C7TJQLVC7/p1655498859886529?thread_ts=1655498859.886529&cid=C7TJQLVC7)
**[Trevor Harmon](https://github.com/thetrevorharmon)** [2022-06-17 08:47PM UTC]
:wave-hand: I have a question about number formatting with `react-i18n` :thread:
**[Trevor Harmon](https://github.com/thetrevorharmon)** [2022-06-17 08:53PM UTC]
I have been investigating [this issue](https://github.com/Shopify/core-issues/issues/39992) around displaying the actual [−](https://www.toptal.com/designers/htmlarrows/math/minus-sign/) symbol instead of just a [hyphen](https://www.toptal.com/designers/htmlarrows/punctuation/hyphen/) on charts. I discovered that `react-i18n` returns strings out of the number formatters with a hyphen instead of the minus symbol. I updated [condense-number](https://github.com/Shopify/condense-number/pull/88/files), and in that library, there is a specific `minusSign` variable that is used to construct a number during formatting; changing that changed it everywhere. Is this something that y'all would _want_ to change? Adjust `.formatCurrency` and `.formatNumber` to return the correct version of the symbol? Or is this something that we should do on our app because there is a desire to keep things the way they are for various reasons?
**[Trevor Harmon](https://github.com/thetrevorharmon)** [2022-06-17 08:53PM UTC]
I went poking around the `react-i18n` repo but didn't see a clear symbol that I could just swap out and put up a quick PR, so I figured I would test the waters before getting too deep into it.
**[Trish Rempel (she/they)](https://github.com/trishrempel)** [2022-06-17 09:09PM UTC]
Hi Trevor, thanks for looking into this! I'm not aware that there's any issue created yet for this in the quilt repo, so please feel free to, or I can create an issue later. I can see that there's some explicit handling of the negative sign in [normalizedNumber](https://github.com/Shopify/quilt/blob/main/packages/react-i18n/src/i18n.ts#L643), but that is only used for `unformatNumber` and `unformatCurrency`. These values are meant to be cast back to `Number` on the calling client, so it's probably best to leave these as dashes. For `formatNumber` and `formatCurrency`, it looks like we might have to do the swapping wherever `memoizedNumberFormatter.format` is called ([formatCurrencyNone](https://github.com/Shopify/quilt/blob/main/packages/react-i18n/src/i18n.ts#L447) and [formatNumber](https://github.com/Shopify/quilt/blob/main/packages/react-i18n/src/i18n.ts#L225)).
**[Christian Jaekl](https://github.com/cejaekl)** [2022-06-17 09:11PM UTC]
> didn't see a clear symbol that I could just swap out and put up a quick PR I believe that the relevant line of code is here: [https://github.com/Shopify/quilt/blob/0c4d9d21762392915bb24cd2f0b3ec1329f54388/packages/react-i18n/src/i18n.ts#L443](https://github.com/Shopify/quilt/blob/0c4d9d21762392915bb24cd2f0b3ec1329f54388/packages/react-i18n/src/i18n.ts#L443) Changing the "minus sign" is ... complicated. On the upside, as far as I'm aware, every culture world-wide uses something that looks like `-` to indicate a negative number. On the downside, Unicode has got a lot of contenders for the "minus sign" role, and there's no universal consensus. In Rails-land, Shopify-i18n asks Unicode CLDR what the appropriate character is for the active locale. In React-land it seems that we just assume the ASCII hyphen-minus. As Trish has pointed out, if we move away from that, there are a number of spots that'll need to adjust.
**[Christian Jaekl](https://github.com/cejaekl)** [2022-06-17 09:15PM UTC]
(Variants that some may want to use include U+002D, U+02D7, U+2012, U+2013, U+2014, U+2212, U+2796, U+FE63, U+FF0D)