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

"number-format" expression #7626

Merged
merged 1 commit into from
Dec 20, 2018
Merged

"number-format" expression #7626

merged 1 commit into from
Dec 20, 2018

Conversation

ChrisLoer
Copy link
Contributor

@ChrisLoer ChrisLoer commented Nov 27, 2018

Add "number-format" expression, to fix #4119. Example:

["number-format", 1234.5678, { "currency": "EUR" } ] -> "€ 1,234.56"

Format options are:

  • "locale": BCP 47 language tag, defaults to current locale. May or may not succeed in loading the locale, currently the only way to inspect this is to use the resolved-locale expression on a collator.
  • "currency": If set, ISO 4217 currency code, used for currency formatting. Returned string includes a symbol representing the currency (e.g. "JP¥" or "€").
  • "min-fraction-digits": Formatter will include at least this many digits after the decimal point. Defaults to 0.
  • "max-fraction-digits": Formatter will include at most this many digits after the decimal point. Defaults to 3.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/studio and/or @mapbox/maps-design if this PR includes style spec changes

@ChrisLoer
Copy link
Contributor Author

This would fix #4119

@ChrisLoer
Copy link
Contributor Author

cc @chloekraw and @mapbox/maps-design -- I know this isn't near the top of our prioritized list, but it's potential low-hanging fruit and could be pretty useful for displaying contextual information, especially with POIs.

Caveat on "low-hanging fruit" claim: just like the collator expressions, this requires us to come up with uniform behavior across multiple differing platform implementations, so there are pitfalls. The good news is that unlike the collator work, no further changes to the expression language itself are necessary.

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Thanks for making this proposal! This expression operator would be a boon for data visualization use cases especially, but also more subtly when dealing with elevations or clustering. It would also represent a significant usability win even for small numbers in languages that use non-Western numeral systems, such as Arabic and Hindi.

number: Expression;
style: Expression;
locale: Expression | null; // BCP 47 language tag
currency: Expression | null; // ISO 4217 currency code, required if style=currency
Copy link
Contributor

Choose a reason for hiding this comment

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

Intl.NumberFormat requires this in "currency" mode, it doesn't just guess based on the locale

Yeah, that’s the right approach. Operating systems allow you to choose the preferred currency independently of the system region. By default, the system region typically affects mechanics like decimal separators, but not semantics or content.

number: Expression;
style: Expression;
locale: Expression | null; // BCP 47 language tag
currency: Expression | null; // ISO 4217 currency code, required if style=currency
Copy link
Contributor

@1ec5 1ec5 Nov 30, 2018

Choose a reason for hiding this comment

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

Here’s a correspondence to other implementation languages:

Style specification Objective-C
NumberFormat.style NSNumberFormatter.numberStyle
NumberFormat.locale NSNumberFormatter.locale
NumberFormat.currency NSNumberFormatter.currencyCode
Minimum/maximum fraction digits NSNumberFormatter.minimumFractionDigits, maximumFractionDigits

In Java, use NumberFormat.get{Currency,Number,Percent}Instance() and NumberFormat.setCurrency().

type: Type;
number: Expression;
style: Expression;
locale: Expression | null; // BCP 47 language tag
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we support any extensions, such as zh-Hans-CN-u-nu-hanidec to specify Chinese decimal numbering? Perhaps it would be an “if it works, it works” situation, with no availability guarantees from one platform to another.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps it would be an “if it works, it works” situation, with no availability guarantees from one platform to another.

That sounds right to me.

}

type NumberFormatOptions = {
style?: 'decimal' | 'currency' | 'percent';
Copy link
Contributor

Choose a reason for hiding this comment

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

NSNumberFormatterStyle actually supports a number of styles, including NSNumberFormatterSpellOutStyle and NSNumberFormatterOrdinalStyle. However, I recognize the need to stick to JavaScript’s Intl as a common denominator, without implementing our own custom types that could conflict with future JavaScript types.

This does mean that the style author will need to hard-code some logic to implement short formats, such as the “1k” for 1,000 that Supercluster implements (only for English) in the point_count_abbreviated property (mapbox/supercluster#110).

/ref tc39/ecma402#37 tc39/ecma402#215.

@ChrisLoer ChrisLoer force-pushed the number-formatter branch 2 times, most recently from b28eddb to 956099a Compare December 17, 2018 22:29
@ChrisLoer ChrisLoer changed the title [WIP] Possible "number-format" expression implementation. Possible "number-format" expression implementation. Dec 17, 2018
@ChrisLoer
Copy link
Contributor Author

I think this is ready. The tests may break if run on a machine that doesn't have a US locale installed, which makes me feel squirmy, although I think so far that has worked out OK with the collator tests. If it becomes a problem we can make the tests more verbose (check for "resolved-locale" and just pass the test the locale isn't available).

@asheemmamoowala would you have a chance to review?

Also tagging @mapbox/studio b/c I don't think they got the original notification.

@ChrisLoer ChrisLoer changed the title Possible "number-format" expression implementation. "number-format" expression Dec 17, 2018
Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

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

This proposal is 👌 ! If the need arises, a number-style parameter can be added to the supported options to include other formatting styles.

Would there be a need to use data-driven styling for the currency field?

Copy link

@dasulit dasulit left a comment

Choose a reason for hiding this comment

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

Thanks for tagging Studio in!

@ChrisLoer
Copy link
Contributor Author

Would there be a need to use data-driven styling for the currency field?

I could imagine DDS being used for currency in something like a map showing hotel costs in local currencies, where the currency to use would depend on data in the map.

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

Successfully merging this pull request may close these issues.

Add expression operator for formatting numbers
4 participants