From f1943a7e69990704289bfe70369824523813e329 Mon Sep 17 00:00:00 2001 From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Date: Thu, 24 Nov 2022 18:53:11 +0100 Subject: [PATCH] Revert static viz fixes (#26739) * Revert "Fix incorrect static viz formatting (#26737)" This reverts commit c155666ca52f8a91146fe78bb861ebe9c1c59c2d. * Revert "Fix weird static viz data point values rounding (#26704)" This reverts commit 25329609d2f44117c77ed88b128e0456649df7fa. --- .../src/metabase/lib/formatting/numbers.tsx | 9 +-- .../src/metabase/static-viz/lib/numbers.ts | 70 ++++++++++++++++++- .../static-viz/lib/numbers.unit.spec.js | 2 +- .../test/metabase/lib/formatting.unit.spec.js | 6 -- 4 files changed, 70 insertions(+), 17 deletions(-) diff --git a/frontend/src/metabase/lib/formatting/numbers.tsx b/frontend/src/metabase/lib/formatting/numbers.tsx index c9279fc4271..4b6ab5ce99a 100644 --- a/frontend/src/metabase/lib/formatting/numbers.tsx +++ b/frontend/src/metabase/lib/formatting/numbers.tsx @@ -24,7 +24,7 @@ interface FormatNumberOptionsType { negativeInParentheses?: boolean; number_separators?: string; number_style?: string; - scale?: number; + scale?: string; type?: string; } @@ -83,12 +83,7 @@ export function formatNumber( } else { try { let nf; - if ( - number < 1 && - number > -1 && - options.decimals == null && - options.number_style !== "percent" - ) { + if (number < 1 && number > -1 && options.decimals == null) { // NOTE: special case to match existing behavior for small numbers, use // max significant digits instead of max fraction digits nf = numberFormatterForOptions({ diff --git a/frontend/src/metabase/static-viz/lib/numbers.ts b/frontend/src/metabase/static-viz/lib/numbers.ts index ded9337bf66..4f20c87d58b 100644 --- a/frontend/src/metabase/static-viz/lib/numbers.ts +++ b/frontend/src/metabase/static-viz/lib/numbers.ts @@ -1,5 +1,4 @@ import { merge } from "icepick"; -import { formatNumber as appFormatNumber } from "metabase/lib/formatting/numbers"; export type NumberFormatOptions = { number_style?: "currency" | "decimal" | "scientific" | "percentage"; @@ -25,9 +24,74 @@ const DEFAULT_OPTIONS = { }; export const formatNumber = (number: number, options?: NumberFormatOptions) => { - const { prefix, suffix } = { ...DEFAULT_OPTIONS, ...options }; + const { + number_style, + currency, + currency_style, + number_separators: [decimal_separator, grouping_separator], + decimals, + scale, + prefix, + suffix, + compact, + } = handleSmallNumberFormat(number, { ...DEFAULT_OPTIONS, ...options }); - return `${prefix}${appFormatNumber(number, options)}${suffix}`; + function createFormat(compact?: boolean) { + if (compact) { + return new Intl.NumberFormat("en", { + style: number_style !== "scientific" ? number_style : "decimal", + notation: "compact", + compactDisplay: "short", + currency: currency, + currencyDisplay: currency_style, + useGrouping: true, + maximumFractionDigits: decimals != null ? decimals : 2, + }); + } + + return new Intl.NumberFormat("en", { + style: number_style !== "scientific" ? number_style : "decimal", + notation: number_style !== "scientific" ? "standard" : "scientific", + currency: currency, + currencyDisplay: currency_style, + useGrouping: true, + minimumFractionDigits: decimals, + maximumFractionDigits: decimals != null ? decimals : 2, + }); + } + + const format = createFormat(compact); + + const separatorMap = { + ",": grouping_separator || "", + ".": decimal_separator, + }; + const formattedNumber = format + .format(number * scale) + .replace(/,|\./g, separator => separatorMap[separator as "." | ","]); + + return `${prefix}${formattedNumber}${suffix}`; +}; + +// Simple hack to handle small decimal numbers (0-1) +function handleSmallNumberFormat<T>(value: number, options: T): T { + const hasAtLeastThreeDecimalPoints = Math.abs(value) < 0.01; + if (hasAtLeastThreeDecimalPoints && Math.abs(value) > 0) { + options = maybeMerge(options, { + compact: true, + decimals: 4, + }); + } + + return options; +} + +const maybeMerge = <T, S1>(collection: T, object: S1) => { + if (collection == null) { + return collection; + } + + return merge(collection, object); }; export const formatPercent = (percent: number) => diff --git a/frontend/src/metabase/static-viz/lib/numbers.unit.spec.js b/frontend/src/metabase/static-viz/lib/numbers.unit.spec.js index 889d0dacd64..acf4be395e7 100644 --- a/frontend/src/metabase/static-viz/lib/numbers.unit.spec.js +++ b/frontend/src/metabase/static-viz/lib/numbers.unit.spec.js @@ -46,7 +46,7 @@ describe("formatNumber", () => { number_style: "scientific", }); - expect(text).toEqual("1.2e+3"); + expect(text).toEqual("1.2E3"); }); it("should format a number with custom number separators", () => { diff --git a/frontend/test/metabase/lib/formatting.unit.spec.js b/frontend/test/metabase/lib/formatting.unit.spec.js index f2a53d0c36c..a39b9cfb2cd 100644 --- a/frontend/test/metabase/lib/formatting.unit.spec.js +++ b/frontend/test/metabase/lib/formatting.unit.spec.js @@ -103,12 +103,6 @@ describe("formatting", () => { }); it("should format percentages", () => { const options = { compact: true, number_style: "percent" }; - expect(formatNumber(0.867, { number_style: "percent" })).toEqual( - "86.7%", - ); - expect(formatNumber(1.2345, { number_style: "percent" })).toEqual( - "123.45%", - ); expect(formatNumber(0, options)).toEqual("0%"); expect(formatNumber(0.001, options)).toEqual("0.1%"); expect(formatNumber(0.0001, options)).toEqual("0.01%"); -- GitLab