[Lens] Allow number formatting within Lens #56253

wylieconlon · 2020-01-28T23:05:39Z

Release note

Lens now allows you to format numbers as Percent or Bytes from within Lens, with a setting for the number of decimal places to show:

Detail

The original scope of this change was larger, but contains two breaking changes that aren't possible to mitigate: #53972

The reduced-scope change introduces an expression function called lens_format_column columnId="a" format="bytes" maxDecimals=3 which sets the formatHint property of the kibana_datatable. This function is responsible for generating numeral.js patterns, such as 0,0.[000]%, depending on the formatter and number of decimals selected.

Users do not get to set numeral.js patterns directly, because this lets us change the implementation to Intl.NumberFormat in 8.0 without causing another breaking change.

Checklist

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2020-01-28T23:05:42Z

Pinging @elastic/kibana-app (Team:KibanaApp)

wylieconlon · 2020-01-28T23:07:26Z

src/plugins/data/common/field_formats/types.ts

@@ -83,6 +83,7 @@ export type IFieldFormatId = FIELD_FORMAT_IDS | string;

 export type IFieldFormatType = (new (params?: any, getConfig?: GetConfigFn) => FieldFormat) & {
  id: IFieldFormatId;
+  title?: string;


Title is already present on formatters, but the type was not set

Is there a reason this needs to be optional? It seems from the FieldFormat class, that the title is the same as id and fieldType always present, and could just be a mandatory string.

wylieconlon · 2020-01-29T14:52:45Z

@elasticmachine merge upstream

lukeelmers

LGTM for app arch (just looking at the update to IFieldFormatType)

wylieconlon · 2020-01-29T20:16:50Z

Tagging for product/design review @AlonaNadler @cchaos

Related issue: #53535

This doesn't yet implement a tab-based UI, I expect that to be coming soon for use in Y-axis styling

cchaos · 2020-01-30T21:12:20Z

@wylieconlon Can you explain what "Default format (Number)" is? I would have suspected that we don't need to explicitly say "Default" but that we'd just automatically select "Number".

So then there'd be no distinction between "Default" and "Custom" numbers, but just a single "Number" option that always shows the decimals option.

When I select a price field, the formatter option still says "Default format (Number)" when in fact it's showing as a currency.

wylieconlon · 2020-01-30T21:47:18Z

So we don't have a currency formatter, the Number formatter is being used with a custom pattern. So your understanding is correct, the default shows: 1. If the field has a custom format in the index pattern, we display its name 2. If there is no custom format, then there is a space-wide setting which assigns formats to type. Most of the time it's Number, but it could be String or something else.

…

On Thu, Jan 30, 2020 at 4:12 PM Caroline Horn ***@***.***> wrote: @wylieconlon <https://github.com/wylieconlon> Can you explain what "Default format (Number)" is? I would have suspected that we don't need to explicitly say "Default" but that we'd just automatically select "Number". [image: Screen Shot 2020-01-30 at 15 59 48 PM] <https://user-images.githubusercontent.com/549577/73489983-951c8700-4379-11ea-8e16-2a802d81e761.png> So then there'd be no distinction between "Default" and "Custom" numbers, but just a single "Number" option that always shows the decimals option. ------------------------------ When I select a price field, the formatter option still says "Default format (Number)" when in fact it's showing as a currency. [image: Screen Shot 2020-01-30 at 15 50 32 PM] <https://user-images.githubusercontent.com/549577/73490099-ceed8d80-4379-11ea-8f54-0f2672ccdbd1.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#56253?email_source=notifications&email_token=AAFCW23QWRHQTD7XTV33XRLRAM7DLA5CNFSM4KM3X3N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKMSLUQ#issuecomment-580462034>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFCW277ESYLFU3XNF3CZJ3RAM7DLANCNFSM4KM3X3NQ> .

wylieconlon · 2020-01-30T21:54:43Z

That's also why there is a difference between the Default and the "Custom number" formatter, because "Custom number" as implemented here only lets you customize the number of decimal places, while in the Management -> Index pattern settings, you can use a numeral.js pattern.

AlonaNadler · 2020-01-30T23:02:05Z

Great work @wylieconlon
Few comments:

Custom number decimal didnt work as expected. I configured 3 decimal points but it only showed one. even if it is zeros it needs to show the numbers according to what is configured.
When changing the function the formating is lost I would expect it is persistent with the number
In TSVB it changes between KB and MB dynamically by the scale. It seems like in Lens it is always KB is that accurate ? can we use GB and MB similar to TSVB
Format should be applied in the suggestions as well, sometimes it does sometimes it don't

wylieconlon · 2020-01-31T15:46:21Z

Thanks for the feedback! Responses inline.

Custom number decimal didnt work as expected. I configured 3 decimal points but it only showed one. even if it is zeros it needs to show the numbers according to what is configured.
image

Talked with you about this, what I've implemented is the maximum number of decimal places, and what you are asking for is the minimum number of decimal places. We need to talk more about this before making a decision.

When changing the function the formating is lost I would expect it is persistent with the number format

Will fix this.

In TSVB it changes between KB and MB dynamically by the scale. It seems like in Lens it is always KB is that accurate ? can we use GB and MB similar to TSVB

We're using the same format, so this is already happening.

Format should be applied in the suggestions as well, sometimes it does sometimes it don't

I'll look into this, I think we can apply the format to some suggestions but not others. If we are using the same field then it should be doable.

cchaos

Based on our meeting, I've added some review notes.

x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/format_selector.tsx

cchaos · 2020-01-31T19:21:15Z

x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/format_selector.tsx

+    <>
+      <EuiFormRow
+        label={i18n.translate('xpack.lens.indexPattern.columnFormatLabel', {
+          defaultMessage: 'Formatter',


Suggested change

defaultMessage: 'Formatter',

defaultMessage: 'Display values as',

@AlonaNadler Trying to better describe the input via the label here without being to verbose. But I think "Formatter" is wrong, since it just changes the the way the values are displayed not actual formatting.

I'm trying "Display format" because I do think it's important to be consistent with the rest of Kibana. We call this "Format" everywhere.

Can you do Value format instead?

x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/format_selector.tsx

cchaos · 2020-02-11T21:32:03Z

Yes I think it's important to persist the decimal point no matter what they've chosen before.
I can consistently reproduce this it just all depends on your output values, I just added bytes field from the sample logs data and you can see the result in the y-axis after changing to Bytes format

3. Hmm ok

wylieconlon · 2020-02-11T21:47:25Z

Okay, both 2 and 3 are related to the same overall issue, which is that our formatters don't have any context. It's especially noticeable because the bytes formatter scales itself between orders of magnitude, but affects dates as well. For example: #7539 #51227

I don't expect to solve these issues in this PR, as it's something the bytes formatter has across Kibana. Feel free to open an issue to discuss the problem of bytes formatting on the Y axis.

cchaos

PR looks good from my end! 🎉

I will be doing a follow up PR closer to FF for any style tweaks.

wylieconlon · 2020-02-18T21:35:50Z

To do: Change from 3 to 2 decimal places by default

wylieconlon · 2020-02-24T16:15:43Z

@elasticmachine merge upstream

timroes

LGTM

wylieconlon · 2020-02-26T18:18:03Z

@elasticmachine merge upstream

wylieconlon · 2020-02-27T16:10:42Z

@elasticmachine merge upstream

wylieconlon · 2020-02-28T16:14:17Z

@elasticmachine merge upstream

kibanamachine · 2020-02-28T18:05:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 655700d

History

💔 Build #29451 failed 40f5a2f
💚 Build #28506 succeeded 17a6992
💔 Build #28195 failed e002e9d
💚 Build #28017 succeeded 575fa9d
💛 Build #26968 was flaky 6a2f438

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

* [Lens] Allow custom number formats on dimensions * Fix merge issues * Text and decimal changes from review * Persist number format across operations * Respond to review comments * Change label * Add persistence * Fix import * 2 decimals * Persist number formatting on drop too Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

…dex-server-side * 'master' of github.com:elastic/kibana: (34 commits) [Upgrade Assistant] Remove "boom" from reindex service (elastic#58715) [data] Clean up QueryStringInput unit tests (elastic#58704) [SIEM] Detection Fix typo in Adobe Hijack Persistence rule (elastic#58804) Restores [SIEM][CASE] Init Configure Case Page (elastic#58121) (elastic#58924) Skips additional failing Ingest Manager integration tests Skips failing Ingest Manager integration tests Move dev tools styles to NP (elastic#58855) change to have kibana --ssl cli option use more recent certs (elastic#57933) disable failing suite (elastic#58942) Don't start pollEsNodesVersion unless someone subscribes (elastic#56923) Do not write UUID file during optimize process (elastic#58899) [Endpoint] Task/add nav bar (elastic#58604) [Metric Alerts] Add backend support for multiple expressions per alert (elastic#58672) [Metrics Alerts] Fix alerting on a rate aggregation (elastic#58789) disable flaky suite (elastic#55953) Revert "[SIEM] apollo@3 (elastic#51926)" and "[SIEM][CASE] Init Confi… (elastic#58806) [resubmit] Prep agg types for new platform (elastic#58893) [Lens] Allow number formatting within Lens (elastic#56253) [Autocomplete] Use settings from config rather than UI settings (elastic#58784) Improve action and trigger types (elastic#58657) ... # Conflicts: # x-pack/plugins/upgrade_assistant/server/routes/reindex_indices/reindex_indices.ts

wylieconlon added 3 commits January 28, 2020 17:28

[Lens] Allow custom number formats on dimensions

7ba8962

Merge remote-tracking branch 'origin/master' into lens/format-selection

3483bae

Fix merge issues

d60ed38

wylieconlon added release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 Feature:Lens v7.7.0 labels Jan 28, 2020

wylieconlon requested review from timroes and a team January 28, 2020 23:05

wylieconlon requested a review from a team as a code owner January 28, 2020 23:05

wylieconlon added the Feature:FieldFormatters label Jan 28, 2020

wylieconlon commented Jan 28, 2020

View reviewed changes

Merge branch 'master' into lens/format-selection

dfba40b

lukeelmers approved these changes Jan 29, 2020

View reviewed changes

wylieconlon requested review from cchaos and AlonaNadler January 29, 2020 20:15

wylieconlon mentioned this pull request Jan 29, 2020

Use i18n titles for field formatters, human names for numeral locales #56348

Merged

Merge remote-tracking branch 'origin/master' into lens/format-selection

ab5898f

cchaos suggested changes Jan 31, 2020

View reviewed changes

wylieconlon mentioned this pull request Jan 31, 2020

Remove external dependency on Numeral.js #8583

Closed

cchaos mentioned this pull request Feb 11, 2020

[Lens] Bytes formatting needs to be consistent across the Y-axis values #57384

Closed

wylieconlon added 4 commits February 12, 2020 15:33

Merge remote-tracking branch 'origin/master' into lens/format-selection

59d7d66

Add persistence

6a394cb

Merge remote-tracking branch 'origin/master' into lens/format-selection

7b66f45

Fix import

6a2f438

cchaos approved these changes Feb 18, 2020

View reviewed changes

wylieconlon added 4 commits February 20, 2020 16:22

Merge remote-tracking branch 'origin/master' into lens/format-selection

0a4e8cb

2 decimals

575fa9d

Merge remote-tracking branch 'origin/master' into lens/format-selection

fe4c842

Persist number formatting on drop too

e002e9d

Merge branch 'master' into lens/format-selection

17a6992

mbondyra approved these changes Feb 25, 2020

View reviewed changes

timroes approved these changes Feb 26, 2020

View reviewed changes

Merge branch 'master' into lens/format-selection

5c1ea81

Merge branch 'master' into lens/format-selection

40f5a2f

Merge branch 'master' into lens/format-selection

655700d

wylieconlon merged commit 6c6bc1f into elastic:master Feb 28, 2020

wylieconlon deleted the lens/format-selection branch February 28, 2020 18:08

wylieconlon mentioned this pull request Feb 28, 2020

[7.x] [Lens] Allow number formatting within Lens (#56253) #58920

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Allow number formatting within Lens #56253

[Lens] Allow number formatting within Lens #56253

wylieconlon commented Jan 28, 2020

elasticmachine commented Jan 28, 2020

wylieconlon Jan 28, 2020

timroes Feb 7, 2020

wylieconlon commented Jan 29, 2020

lukeelmers left a comment

wylieconlon commented Jan 29, 2020 •

edited

Loading

cchaos commented Jan 30, 2020

wylieconlon commented Jan 30, 2020 via email

wylieconlon commented Jan 30, 2020

AlonaNadler commented Jan 30, 2020

wylieconlon commented Jan 31, 2020

cchaos left a comment

cchaos Jan 31, 2020

wylieconlon Jan 31, 2020

cchaos Feb 3, 2020

cchaos commented Feb 11, 2020

wylieconlon commented Feb 11, 2020

cchaos left a comment

wylieconlon commented Feb 18, 2020

wylieconlon commented Feb 24, 2020

timroes left a comment

wylieconlon commented Feb 26, 2020

wylieconlon commented Feb 27, 2020

wylieconlon commented Feb 28, 2020

kibanamachine commented Feb 28, 2020

	defaultMessage: 'Formatter',
	defaultMessage: 'Display values as',

[Lens] Allow number formatting within Lens #56253

[Lens] Allow number formatting within Lens #56253

Conversation

wylieconlon commented Jan 28, 2020

Release note

Detail

Checklist

elasticmachine commented Jan 28, 2020

wylieconlon Jan 28, 2020

Choose a reason for hiding this comment

timroes Feb 7, 2020

Choose a reason for hiding this comment

wylieconlon commented Jan 29, 2020

lukeelmers left a comment

Choose a reason for hiding this comment

wylieconlon commented Jan 29, 2020 • edited Loading

cchaos commented Jan 30, 2020

wylieconlon commented Jan 30, 2020 via email

wylieconlon commented Jan 30, 2020

AlonaNadler commented Jan 30, 2020

wylieconlon commented Jan 31, 2020

cchaos left a comment

Choose a reason for hiding this comment

cchaos Jan 31, 2020

Choose a reason for hiding this comment

wylieconlon Jan 31, 2020

Choose a reason for hiding this comment

cchaos Feb 3, 2020

Choose a reason for hiding this comment

cchaos commented Feb 11, 2020

wylieconlon commented Feb 11, 2020

cchaos left a comment

Choose a reason for hiding this comment

wylieconlon commented Feb 18, 2020

wylieconlon commented Feb 24, 2020

timroes left a comment

Choose a reason for hiding this comment

wylieconlon commented Feb 26, 2020

wylieconlon commented Feb 27, 2020

wylieconlon commented Feb 28, 2020

kibanamachine commented Feb 28, 2020

💚 Build Succeeded

History

wylieconlon commented Jan 29, 2020 •

edited

Loading