[Formatters] Add new currency and short number formatter, replacing default number and percent #53972

wylieconlon · 2020-01-03T22:22:44Z

Release note

Adds new number formatters for currency and short numbers (thousands, millions), and allow # of decimals to be customized on most number formatters. The new formatters have a wider range of locales and currencies supported, and can target your browser's locale for display.

This change adds the following advanced settings:

format:defaultLocale
format:default_number:maxDecimals
format:currency:defaultCurrency
format:currency:maxDecimals

CSV export no longer formats numbers because node.js does not support localization in the version used by Kibana.

Deprecation note

Numeral.js patterns are no longer supported on the bytes and percent formatter. The following settings have been deprecated, and no longer have any affect:

format:bytes:defaultPattern
format:currency:defaultPattern
format:percent:defaultPattern

While these settings are deprecated, there remain custom formatters in TSVB, Canvas, and SIEM that map to the deprecated settings.

Summary

Closes #39211 and closes #25993

The user-facing change here is that there are new number formatters that are more flexible than our previous ones which required knowledge of the numeral.js syntax. The higher level of flexibility is a pre-requisite for our usage of formatters in Lens. The new formatters are using the standard Intl.NumberFormat API, which is supported by all browsers we support.

Summary of changes:

All number formats use a display locale. The new formatters support an option to detect the locale dynamically based on the viewer's browser, so for example a browser using de-DE for German in Germany will get the following languages: ['de-DE', 'de', 'en'] which will be used in order for formatting.

Numeral.js does not use the same technique because of its limited support, and it's one reason it's minimized in this change.

There is a new Currency formatter. It allows selection of the top 35 currencies, but also allows typing another currency. It has a preference for default currency.

There is a new Shorter Number formatter, which displays thousands/millions/billions:

There is a new set of options for the Default Number formatter:

The bytes formatter is mostly the same, but has no numeral.js pattern any more. Users who want to customize this will continue to use numeral.js

All numeral.js format patterns will migrate to the "Custom numeral.js" formatter type for backwards compatibility. Numeral.js currently supports these locales:

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

...egacy/ui/public/field_editor/components/field_format_editor/editors/default_number/number.js

bmcconaghy · 2020-01-07T14:35:09Z

@wylieconlon I love where this is going. I especially like the way the formatters explain where the defaults are coming from. I think we maybe need to think about this as part of the choose locale in browser work @Bamieh has some prototype work for. Would love to sync up on this some.

Bamieh · 2020-01-08T01:11:12Z

@wylieconlon I've tried pulling the code locally to play around with it but the bootstrap is failing:

Looking at the code in the PR the general approach looks good to me.

I am not sure what set of locales we want to provide for number formatting. For currencies, the top 35 currencies sound reasonable.

wylieconlon · 2020-01-08T15:16:21Z

@Bamieh Bootstrap fails due to some type checking issues, but the code still starts.

wylieconlon · 2020-01-10T16:21:49Z

src/legacy/ui/public/field_editor/components/field_format_editor/index.js

@@ -18,3 +18,4 @@
 */

 export { FieldFormatEditor } from './field_format_editor';
+export { editors } from './get_editors';


This export is one way to access the same editors in Lens and Management. This is no longer the desired UX for Lens, so I will probably remove this.

chrisdavies

LGTM w/ tweaks.

src/legacy/core_plugins/kibana/migrations/migrations.js

src/legacy/core_plugins/vis_type_timeseries/public/components/lib/__tests__/tick_formatter.js

src/plugins/data/common/field_formats/converters/intl_number_format.ts

chrisdavies · 2020-01-10T16:50:58Z

src/plugins/data/common/field_formats/converters/intl_number_format.ts

+  abstract getArguments: () => Record<string, unknown>;
+
+  protected getConvertedValue(val: any): string {
+    if (val === -Infinity) return '-∞';


Should these be internationalized?

They aren't currently for the regular number formatter, which this is based on.

chrisdavies · 2020-01-10T16:53:41Z

src/plugins/data/common/field_formats/converters/intl_number_format.ts

+    if (isNaN(val)) return '';
+
+    const defaultLocale = this.getConfig && this.getConfig('format:defaultLocale');
+    let locales = [defaultLocale, 'en'];


Nit: This and the if block would be tidier like this:

const locales = defaultLocale === 'detect' ? navigator.languages.concat(['en']) : [navigator.language].concat([defaultLocale, 'en']);

I had to stare at this a bit longer than necessary as it is.

Even more nit:

const locales = defaultLocale === 'detect' ? [...navigator.languages, 'en'] : [navigator.language, defaultLocale, 'en'];

chrisdavies · 2020-01-10T16:55:39Z

src/plugins/data/common/field_formats/converters/percent.ts

-    pattern: this.getConfig!('format:percent:defaultPattern'),
-    fractional: true,
+    minDecimals: 0,
+    maxDecimals: 3,


I wonder if 3 is the right default for percentage? I almost think maybe 0 would be the right default, but maybe not.

I think I will keep it at 3 because it's the minimum possible change- 3 was the previous default.

timroes

Looks good so far (want to wait for final tests/migration before approving). Left a couple of comments. Since https://github.com/elastic/kibana/pull/54240/files has been merged, I think we need to incorporate that flag also in the new field formatters in this PR.

src/legacy/core_plugins/kibana/ui_setting_defaults.js

timroes · 2020-01-10T16:56:54Z

src/legacy/ui/public/field_editor/components/field_format_editor/get_editors.js

+import { UrlFormatEditor } from './editors/url/url';
+
+export const editors = [
+  BytesFormatEditor,


Unfortunately the order of this array is currently also used for ordering the select in the UI. Since it was ordered alphabetically so far, I think we should keep this array alphabetically (by their English names) sorted (until we might in a later step properly sort them in some other place via their names).

src/plugins/data/common/field_formats/converters/numeral.ts

src/legacy/ui/public/field_editor/components/field_format_editor/editors/currency/currency.js

packages/kbn-i18n/src/core/known_locales.ts

kibanamachine · 2020-01-22T19:35:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: ff46904

History

💔 Build #21906 failed ecd2198
💔 Build #21788 failed f7bb12e
💔 Build #21773 failed 40f6c5c
💔 Build #21748 failed fbbfaf5
💔 Build #21716 failed b96444a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

wylieconlon · 2020-01-22T19:42:28Z

Updates:

This is a breaking change as currently written. The only way to fix this is to write a migration that reads the config saved object when migrating index-patterns. This depends on Saved object migrations to allow pre-migration state collection #34996
There is another breaking change here which is that CSV export no longer applied number formatting.

tsullivan · 2020-01-22T20:36:19Z

There is another breaking change here which is that CSV export no longer applied number formatting.

Reporting team has another top focus right now, which is our new platform migration. Our ongoing changes will likely cause conflicts in this PR.

We would like to better understand what kind of formatting would be left if number formatting goes away for CSV export, and if there is any, would the remainder be useful to keep? We have a discuss issue relevant to this: #55484 If the proposal is accepted, then your point about this wouldn't be a blocker. But usually we are very careful not to ship breaking changes in a minor release. If not done carefully it just makes it look like we have shipped a bug.

rudolf · 2020-01-24T12:06:37Z

This is a breaking change as currently written. The only way to fix this is to write a migration that reads the config saved object when migrating index-patterns. This depends on #34996

@wylieconlon Can you explain what the breaking change is and why #34996 would solve it?

Would it be possible to make these changes without a migration by "migrating" index-patterns inside the data plugin before exposing them to other plugins? I'm assuming plugins don't directly call the SavedObjectsClient to retrieve index patterns so if we have a share API for reading them that all plugins use, we could apply the migration there and at the same time have access to the config object.

wylieconlon · 2020-01-24T18:32:30Z

@rudolf Even if your assumption that there is only one way to read index patterns were accurate (it isn't, as far as I know), it would not solve the problem. The reason the migration needs the configuration is that the logic needs to be as follows:

The previous version had set a custom pattern for the percent formatter, such as 0,0%
The percent formatter is being used for something like system.cpu.total.pct
Because there was a custom format being used, we have to migrate system.cpu.total.pct to the number format with the 0,0% custom numeraljs format string.

joshdover · 2020-04-23T21:43:28Z

@wylieconlon Now that Github allows it, should we set this PR back to draft? Just been hanging out in review queues for a long time now.

wylieconlon · 2020-09-16T18:32:38Z

Despite support for the new formatting options in Node 14, I am going to close this PR as I don't expect to make the changes here.

[Formatters] Add new field formatters

6a1a77e

wylieconlon added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:FieldFormatters v7.6.0 labels Jan 3, 2020

wylieconlon requested review from chrisdavies, spalger and gospodarsky January 3, 2020 22:22

This comment has been minimized.

Sign in to view

wylieconlon commented Jan 6, 2020

View reviewed changes

...egacy/ui/public/field_editor/components/field_format_editor/editors/default_number/number.js Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into add-formatters

3785cc3

wylieconlon mentioned this pull request Jan 8, 2020

fix: Table visualization shows total for duration, percentage and bytes field formatters #54240

Merged

1 task

wylieconlon and others added 3 commits January 9, 2020 17:33

Remove locale override. Support browser locale.

5682ade

Merge remote-tracking branch 'origin/master' into add-formatters

44f02c0

i18n currency names in field format editor

c600fee

wylieconlon commented Jan 10, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into add-formatters

c834a3e

chrisdavies approved these changes Jan 10, 2020

View reviewed changes

timroes reviewed Jan 10, 2020

View reviewed changes

timroes mentioned this pull request Jan 10, 2020

Add Norwegian locale for number formatters #54351

Closed

wylieconlon added 3 commits January 10, 2020 14:04

Add migration and update tests

aba8e7c

Merge remote-tracking branch 'origin/master' into add-formatters

f9d89df

Remove unused

2a6eced

wylieconlon requested review from timroes and chrisdavies and removed request for gospodarsky January 10, 2020 19:08

wylieconlon marked this pull request as ready for review January 10, 2020 19:13

wylieconlon requested a review from a team as a code owner January 10, 2020 19:13

wylieconlon added 5 commits January 21, 2020 21:32

Hopefully final tweaks

aeefd35

Merge remote-tracking branch 'origin/master' into add-formatters

40f6c5c

More fixes

f7bb12e

Fix tests

ecd2198

Fix tests

ff46904

wylieconlon mentioned this pull request Jan 22, 2020

Align numbers to the right in a tables across Kibana #53544

Closed

This was referenced Jan 28, 2020

Saved object migrations to allow pre-migration state collection #34996

Open

[Lens] Allow number formatting within Lens #56253

Merged

Remove external dependency on Numeral.js #8583

Closed

wylieconlon added blocked and removed v7.7.0 v8.0.0 labels Feb 3, 2020

tsullivan removed their request for review April 13, 2020 18:43

sorenlouv mentioned this pull request Apr 27, 2020

[APM] Respect number format defined in Kibana Advanced Settings #63458

Closed

wylieconlon marked this pull request as draft April 27, 2020 17:23

spalger removed their request for review May 28, 2020 16:06

mattkime added the WIP Work in progress label Jul 7, 2020

mattkime mentioned this pull request Jul 14, 2020

[Meta] Field formatters roadmap #71450

Closed

28 tasks

joshdover removed request for chrisdavies and a team August 31, 2020 19:04

wylieconlon closed this Sep 16, 2020

wylieconlon deleted the add-formatters branch September 16, 2020 18:32

Dosant mentioned this pull request Jun 15, 2021

[meta] Dealing with Numeral.js #102239

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Formatters] Add new currency and short number formatter, replacing default number and percent #53972

[Formatters] Add new currency and short number formatter, replacing default number and percent #53972

wylieconlon commented Jan 3, 2020 •

edited

Loading

This comment has been minimized.

bmcconaghy commented Jan 7, 2020

Bamieh commented Jan 8, 2020

wylieconlon commented Jan 8, 2020

wylieconlon Jan 10, 2020

chrisdavies left a comment

chrisdavies Jan 10, 2020

wylieconlon Jan 13, 2020

chrisdavies Jan 10, 2020

timroes Jan 10, 2020

chrisdavies Jan 10, 2020

wylieconlon Jan 13, 2020

timroes left a comment

timroes Jan 10, 2020

kibanamachine commented Jan 22, 2020

wylieconlon commented Jan 22, 2020

tsullivan commented Jan 22, 2020

rudolf commented Jan 24, 2020

wylieconlon commented Jan 24, 2020

joshdover commented Apr 23, 2020

wylieconlon commented Sep 16, 2020

[Formatters] Add new currency and short number formatter, replacing default number and percent #53972

[Formatters] Add new currency and short number formatter, replacing default number and percent #53972

Conversation

wylieconlon commented Jan 3, 2020 • edited Loading

Release note

Deprecation note

Summary

Checklist

For maintainers

This comment has been minimized.

bmcconaghy commented Jan 7, 2020

Bamieh commented Jan 8, 2020

wylieconlon commented Jan 8, 2020

Choose a reason for hiding this comment

chrisdavies left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timroes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Jan 22, 2020

💚 Build Succeeded

History

wylieconlon commented Jan 22, 2020

tsullivan commented Jan 22, 2020

rudolf commented Jan 24, 2020

wylieconlon commented Jan 24, 2020

joshdover commented Apr 23, 2020

wylieconlon commented Sep 16, 2020

wylieconlon commented Jan 3, 2020 •

edited

Loading