From 0b94ad792e04d262ff9d3d6bc9bbe4f4c296b9a2 Mon Sep 17 00:00:00 2001
From: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Date: Fri, 14 Feb 2020 18:56:10 -0600
Subject: [PATCH] Add setting for chart value formatting (#11893)
* add setting for chart value formatting
(cherry picked from commit 19b35a9960c8c3b4054d120b45ead57005e45290)
* compact formatting updates
* small tweaks
* fix tests
* implement tom's idea to prevent auto compact from depending on currency settings
* more `_numberFormatter: undefined` to where compact setting is tested
---
frontend/src/metabase/lib/formatting.js | 32 +++++++++-------
.../lib/LineAreaBarPostRender.js | 28 +++++++++++++-
.../visualizations/lib/settings/graph.js | 18 +++++++++
.../test/metabase/lib/formatting.unit.spec.js | 38 ++++++++++++-------
4 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/frontend/src/metabase/lib/formatting.js b/frontend/src/metabase/lib/formatting.js
index ab65223128e..3e6614b6735 100644
--- a/frontend/src/metabase/lib/formatting.js
+++ b/frontend/src/metabase/lib/formatting.js
@@ -109,7 +109,7 @@ function getDefaultNumberOptions(options) {
return defaults;
}
-const PRECISION_NUMBER_FORMATTER = d3.format(".2r");
+const PRECISION_NUMBER_FORMATTER = d3.format(".2f");
const FIXED_NUMBER_FORMATTER = d3.format(",.f");
const DECIMAL_DEGREES_FORMATTER = d3.format(".08f");
const DECIMAL_DEGREES_FORMATTER_COMPACT = d3.format(".02f");
@@ -250,23 +250,32 @@ function formatNumberScientific(
}
}
+const DISPLAY_COMPACT_DECIMALS_CUTOFF = 1000;
+export const COMPACT_CURRENCY_OPTIONS = {
+ // Currencies vary in how many decimals they display, so this is probably
+ // wrong in some cases. Intl.NumberFormat has some of that data built-in, but
+ // I couldn't figure out how to use it here.
+ digits: 2,
+ currency_style: "symbol",
+};
+
function formatNumberCompact(value: number, options: FormattingOptions) {
if (options.number_style === "percent") {
return formatNumberCompactWithoutOptions(value * 100) + "%";
}
if (options.number_style === "currency") {
try {
- const { value: currency } = numberFormatterForOptions({
+ const nf = numberFormatterForOptions({
...options,
- currency_style: "symbol",
- })
- .formatToParts(value)
- .find(p => p.type === "currency");
+ ...COMPACT_CURRENCY_OPTIONS,
+ });
- // this special case ensures the "~" comes before the currency
- if (value !== 0 && value >= -0.01 && value <= 0.01) {
- return `~${currency}0`;
+ if (Math.abs(value) < DISPLAY_COMPACT_DECIMALS_CUTOFF) {
+ return nf.format(value);
}
+ const { value: currency } = nf
+ .formatToParts(value)
+ .find(p => p.type === "currency");
return currency + formatNumberCompactWithoutOptions(value);
} catch (e) {
// Intl.NumberFormat failed, so we fall back to a non-currency number
@@ -288,10 +297,7 @@ function formatNumberCompactWithoutOptions(value: number) {
if (value === 0) {
// 0 => 0
return "0";
- } else if (value >= -0.01 && value <= 0.01) {
- // 0.01 => ~0
- return "~ 0";
- } else if (value > -1 && value < 1) {
+ } else if (Math.abs(value) < DISPLAY_COMPACT_DECIMALS_CUTOFF) {
// 0.1 => 0.1
return PRECISION_NUMBER_FORMATTER(value).replace(/\.?0+$/, "");
} else {
diff --git a/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js b/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js
index 8638e8c824e..910a357ae12 100644
--- a/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js
+++ b/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js
@@ -5,6 +5,7 @@ import _ from "underscore";
import { color } from "metabase/lib/colors";
import { clipPathReference } from "metabase/lib/dom";
+import { COMPACT_CURRENCY_OPTIONS } from "metabase/lib/formatting";
import { adjustYAxisTicksIfNeeded } from "./apply_axis";
import { isHistogramBar } from "./renderer_utils";
@@ -271,6 +272,31 @@ function onRenderValueLabels(chart, formatYValue, [data]) {
return { x, y, showLabelBelow };
});
+ const formattingSetting = chart.settings["graph.label_value_formatting"];
+ let compact;
+ if (formattingSetting === "compact") {
+ compact = true;
+ } else if (formattingSetting === "full") {
+ compact = false;
+ } else {
+ // for "auto" we use compact if it shortens avg label length by >3 chars
+ const getAvgLength = compact => {
+ const options = {
+ compact,
+ // We include compact currency options here for both compact and
+ // non-compact formatting. This prevents auto's logic from depending on
+ // those settings.
+ ...COMPACT_CURRENCY_OPTIONS,
+ // We need this to ensure the settings are used. Otherwise, a cached
+ // _numberFormatter would take precedence.
+ _numberFormatter: undefined,
+ };
+ const lengths = data.map(d => formatYValue(d.y, options).length);
+ return lengths.reduce((sum, l) => sum + l, 0) / lengths.length;
+ };
+ compact = getAvgLength(true) < getAvgLength(false) - 3;
+ }
+
// use the chart body so things line up properly
const parent = chart.svg().select(".chart-body");
@@ -322,7 +348,7 @@ function onRenderValueLabels(chart, formatYValue, [data]) {
.append("text")
.attr("class", klass)
.attr("text-anchor", "middle")
- .text(({ y }) => formatYValue(y, { compact: true })),
+ .text(({ y }) => formatYValue(y, { compact })),
);
};
diff --git a/frontend/src/metabase/visualizations/lib/settings/graph.js b/frontend/src/metabase/visualizations/lib/settings/graph.js
index da86c326740..d2f8158098a 100644
--- a/frontend/src/metabase/visualizations/lib/settings/graph.js
+++ b/frontend/src/metabase/visualizations/lib/settings/graph.js
@@ -346,6 +346,24 @@ export const GRAPH_DISPLAY_VALUES_SETTINGS = {
default: "fit",
readDependencies: ["graph.show_values"],
},
+ "graph.label_value_formatting": {
+ section: t`Display`,
+ title: t`Value formatting`,
+ widget: "radio",
+ getHidden: (series, vizSettings) =>
+ series.length > 1 ||
+ vizSettings["graph.show_values"] !== true ||
+ vizSettings["stackable.stack_type"] === "normalized",
+ props: {
+ options: [
+ { name: t`Auto`, value: "auto" },
+ { name: t`Compact`, value: "compact" },
+ { name: t`Full`, value: "full" },
+ ],
+ },
+ default: "auto",
+ readDependencies: ["graph.show_values"],
+ },
};
export const GRAPH_COLORS_SETTINGS = {
diff --git a/frontend/test/metabase/lib/formatting.unit.spec.js b/frontend/test/metabase/lib/formatting.unit.spec.js
index 8d699440e4d..b0e38eee38a 100644
--- a/frontend/test/metabase/lib/formatting.unit.spec.js
+++ b/frontend/test/metabase/lib/formatting.unit.spec.js
@@ -37,14 +37,14 @@ describe("formatting", () => {
it("shouldn't display small numbers as 0", () => {
expect(formatNumber(0.1, { compact: true })).toEqual("0.1");
expect(formatNumber(-0.1, { compact: true })).toEqual("-0.1");
- expect(formatNumber(0.01, { compact: true })).toEqual("~ 0");
- expect(formatNumber(-0.01, { compact: true })).toEqual("~ 0");
+ expect(formatNumber(0.01, { compact: true })).toEqual("0.01");
+ expect(formatNumber(-0.01, { compact: true })).toEqual("-0.01");
});
it("should round up and down", () => {
- expect(formatNumber(1.01, { compact: true })).toEqual("1");
- expect(formatNumber(-1.01, { compact: true })).toEqual("-1");
- expect(formatNumber(1.9, { compact: true })).toEqual("2");
- expect(formatNumber(-1.9, { compact: true })).toEqual("-2");
+ expect(formatNumber(1.01, { compact: true })).toEqual("1.01");
+ expect(formatNumber(-1.01, { compact: true })).toEqual("-1.01");
+ expect(formatNumber(1.9, { compact: true })).toEqual("1.9");
+ expect(formatNumber(-1.9, { compact: true })).toEqual("-1.9");
});
it("should format large numbers with metric units", () => {
expect(formatNumber(1, { compact: true })).toEqual("1");
@@ -55,12 +55,12 @@ describe("formatting", () => {
const options = { compact: true, number_style: "percent" };
expect(formatNumber(0, options)).toEqual("0%");
expect(formatNumber(0.001, options)).toEqual("0.1%");
- expect(formatNumber(0.0001, options)).toEqual("~ 0%");
+ expect(formatNumber(0.0001, options)).toEqual("0.01%");
expect(formatNumber(0.001234, options)).toEqual("0.12%");
expect(formatNumber(0.1, options)).toEqual("10%");
- expect(formatNumber(0.1234, options)).toEqual("12%");
- expect(formatNumber(0.019, options)).toEqual("2%");
- expect(formatNumber(0.021, options)).toEqual("2%");
+ expect(formatNumber(0.1234, options)).toEqual("12.34%");
+ expect(formatNumber(0.019, options)).toEqual("1.9%");
+ expect(formatNumber(0.021, options)).toEqual("2.1%");
expect(formatNumber(11.11, options)).toEqual("1.1k%");
expect(formatNumber(-0.22, options)).toEqual("-22%");
});
@@ -79,9 +79,11 @@ describe("formatting", () => {
number_style: "currency",
currency: "USD",
};
- expect(formatNumber(0, options)).toEqual("$0");
- expect(formatNumber(0.001, options)).toEqual("~$0");
- expect(formatNumber(7.24, options)).toEqual("$7");
+ expect(formatNumber(0, options)).toEqual("$0.00");
+ expect(formatNumber(0.001, options)).toEqual("$0.00");
+ expect(formatNumber(7.24, options)).toEqual("$7.24");
+ expect(formatNumber(7.249, options)).toEqual("$7.25");
+ expect(formatNumber(724.9, options)).toEqual("$724.90");
expect(formatNumber(1234.56, options)).toEqual("$1.2k");
expect(formatNumber(1234567.89, options)).toEqual("$1.2M");
expect(formatNumber(-1234567.89, options)).toEqual("$-1.2M");
@@ -297,6 +299,16 @@ describe("formatting", () => {
}),
).toEqual("data:text/plain;charset=utf-8,hello%20world");
});
+ it("should return link component for type/URL and view_as = link", () => {
+ const formatted = formatUrl("http://whatever", {
+ jsx: true,
+ rich: true,
+ column: { special_type: TYPE.URL },
+ view_as: "link",
+ });
+ expect(isElementOfType(formatted, ExternalLink)).toEqual(true);
+ });
+
it("should not crash if column is null", () => {
expect(
formatUrl("foobar", {
--
GitLab