From 15008b0a09e4bd1d3e4684678a8f1ac22d4b78b4 Mon Sep 17 00:00:00 2001 From: Ryan Laurie <30528226+iethree@users.noreply.github.com> Date: Wed, 5 Apr 2023 08:41:27 -0600 Subject: [PATCH] Consistently handle infinite and empty values in scalar visualizations (#29805) * update and test number display for gauge viz * update and test numeric display for progress viz * display null in scalar viz --- .../components/ScalarValue/ScalarValue.jsx | 3 ++- .../visualizations/{ => Gauge}/Gauge.jsx | 3 ++- .../{ => Gauge}/Gauge.styled.tsx | 0 .../visualizations/Gauge/index.js | 1 + .../visualizations/Gauge/utils.ts | 13 +++++++++ .../visualizations/Gauge/utils.unit.spec.ts | 27 +++++++++++++++++++ .../{ => Progress}/Progress.jsx | 5 +++- .../visualizations/Progress/index.js | 1 + .../visualizations/Progress/utils.ts | 13 +++++++++ .../Progress/utils.unit.spec.ts | 27 +++++++++++++++++++ 10 files changed, 90 insertions(+), 3 deletions(-) rename frontend/src/metabase/visualizations/visualizations/{ => Gauge}/Gauge.jsx (99%) rename frontend/src/metabase/visualizations/visualizations/{ => Gauge}/Gauge.styled.tsx (100%) create mode 100644 frontend/src/metabase/visualizations/visualizations/Gauge/index.js create mode 100644 frontend/src/metabase/visualizations/visualizations/Gauge/utils.ts create mode 100644 frontend/src/metabase/visualizations/visualizations/Gauge/utils.unit.spec.ts rename frontend/src/metabase/visualizations/visualizations/{ => Progress}/Progress.jsx (98%) create mode 100644 frontend/src/metabase/visualizations/visualizations/Progress/index.js create mode 100644 frontend/src/metabase/visualizations/visualizations/Progress/utils.ts create mode 100644 frontend/src/metabase/visualizations/visualizations/Progress/utils.unit.spec.ts diff --git a/frontend/src/metabase/visualizations/components/ScalarValue/ScalarValue.jsx b/frontend/src/metabase/visualizations/components/ScalarValue/ScalarValue.jsx index 245fd69d9cd..e3064b072d4 100644 --- a/frontend/src/metabase/visualizations/components/ScalarValue/ScalarValue.jsx +++ b/frontend/src/metabase/visualizations/components/ScalarValue/ScalarValue.jsx @@ -3,6 +3,7 @@ */ /* eslint-disable react/prop-types */ import React, { useMemo } from "react"; +import { t } from "ttag"; import Icon from "metabase/components/Icon"; import Tooltip from "metabase/core/components/Tooltip"; @@ -53,7 +54,7 @@ const ScalarValue = ({ fontSize={fontSize} data-testid="scalar-value" > - {value} + {value ?? t`null`} </ScalarValueWrapper> ); }; diff --git a/frontend/src/metabase/visualizations/visualizations/Gauge.jsx b/frontend/src/metabase/visualizations/visualizations/Gauge/Gauge.jsx similarity index 99% rename from frontend/src/metabase/visualizations/visualizations/Gauge.jsx rename to frontend/src/metabase/visualizations/visualizations/Gauge/Gauge.jsx index f2fc18c3c68..b1bab274932 100644 --- a/frontend/src/metabase/visualizations/visualizations/Gauge.jsx +++ b/frontend/src/metabase/visualizations/visualizations/Gauge/Gauge.jsx @@ -14,6 +14,7 @@ import { columnSettings } from "metabase/visualizations/lib/settings/column"; import ChartSettingGaugeSegments from "metabase/visualizations/components/settings/ChartSettingGaugeSegments"; import { isNumeric } from "metabase-lib/types/utils/isa"; import { GaugeArcPath } from "./Gauge.styled"; +import { getValue } from "./utils"; const MAX_WIDTH = 500; const PADDING_BOTTOM = 10; @@ -207,7 +208,7 @@ export default class Gauge extends Component { ]) .clamp(true); - const value = rows[0][0] || 0; + const value = getValue(rows); const column = cols[0]; const valuePosition = (value, distance) => { diff --git a/frontend/src/metabase/visualizations/visualizations/Gauge.styled.tsx b/frontend/src/metabase/visualizations/visualizations/Gauge/Gauge.styled.tsx similarity index 100% rename from frontend/src/metabase/visualizations/visualizations/Gauge.styled.tsx rename to frontend/src/metabase/visualizations/visualizations/Gauge/Gauge.styled.tsx diff --git a/frontend/src/metabase/visualizations/visualizations/Gauge/index.js b/frontend/src/metabase/visualizations/visualizations/Gauge/index.js new file mode 100644 index 00000000000..bbfe8eb69d5 --- /dev/null +++ b/frontend/src/metabase/visualizations/visualizations/Gauge/index.js @@ -0,0 +1 @@ +export { default } from "./Gauge"; diff --git a/frontend/src/metabase/visualizations/visualizations/Gauge/utils.ts b/frontend/src/metabase/visualizations/visualizations/Gauge/utils.ts new file mode 100644 index 00000000000..1317ee308e2 --- /dev/null +++ b/frontend/src/metabase/visualizations/visualizations/Gauge/utils.ts @@ -0,0 +1,13 @@ +export const getValue = (rows: unknown[][]) => { + const rawValue = rows[0] && rows[0][0]; + + if (rawValue === "Infinity") { + return Infinity; + } + + if (typeof rawValue !== "number") { + return 0; + } + + return rawValue; +}; diff --git a/frontend/src/metabase/visualizations/visualizations/Gauge/utils.unit.spec.ts b/frontend/src/metabase/visualizations/visualizations/Gauge/utils.unit.spec.ts new file mode 100644 index 00000000000..d5b653ad0fe --- /dev/null +++ b/frontend/src/metabase/visualizations/visualizations/Gauge/utils.unit.spec.ts @@ -0,0 +1,27 @@ +import { getValue } from "./utils"; + +describe("Visualizations > Gauge > utils", () => { + const valueTestCases = [ + [[[null]], 0], + [[[undefined]], 0], + [[["foo"]], 0], + [[[""]], 0], + [[[0]], 0], + [[[1]], 1], + [ + [ + [1, 2, 3], + [4, 5, 6], + ], + 1, + ], + [[3], 0], + [[["Infinity"]], Infinity], + ]; + + valueTestCases.forEach(([input, output]) => { + it(`should return ${output} for ${JSON.stringify(input)}`, () => { + expect(getValue(input as unknown[][])).toEqual(output); + }); + }); +}); diff --git a/frontend/src/metabase/visualizations/visualizations/Progress.jsx b/frontend/src/metabase/visualizations/visualizations/Progress/Progress.jsx similarity index 98% rename from frontend/src/metabase/visualizations/visualizations/Progress.jsx rename to frontend/src/metabase/visualizations/visualizations/Progress/Progress.jsx index 7d216502f2b..dda23bf8677 100644 --- a/frontend/src/metabase/visualizations/visualizations/Progress.jsx +++ b/frontend/src/metabase/visualizations/visualizations/Progress/Progress.jsx @@ -13,6 +13,8 @@ import { color } from "metabase/lib/colors"; import { columnSettings } from "metabase/visualizations/lib/settings/column"; import { isNumeric } from "metabase-lib/types/utils/isa"; +import { getValue } from "./utils"; + const BORDER_RADIUS = 5; const MAX_BAR_HEIGHT = 65; @@ -136,7 +138,8 @@ export default class Progress extends Component { onVisualizationClick, visualizationIsClickable, } = this.props; - const value = rows[0] && typeof rows[0][0] === "number" ? rows[0][0] : 0; + + const value = getValue(rows); const column = cols[0]; const goal = settings["progress.goal"] || 0; diff --git a/frontend/src/metabase/visualizations/visualizations/Progress/index.js b/frontend/src/metabase/visualizations/visualizations/Progress/index.js new file mode 100644 index 00000000000..c9ad22f2dcb --- /dev/null +++ b/frontend/src/metabase/visualizations/visualizations/Progress/index.js @@ -0,0 +1 @@ +export { default } from "./Progress"; diff --git a/frontend/src/metabase/visualizations/visualizations/Progress/utils.ts b/frontend/src/metabase/visualizations/visualizations/Progress/utils.ts new file mode 100644 index 00000000000..1317ee308e2 --- /dev/null +++ b/frontend/src/metabase/visualizations/visualizations/Progress/utils.ts @@ -0,0 +1,13 @@ +export const getValue = (rows: unknown[][]) => { + const rawValue = rows[0] && rows[0][0]; + + if (rawValue === "Infinity") { + return Infinity; + } + + if (typeof rawValue !== "number") { + return 0; + } + + return rawValue; +}; diff --git a/frontend/src/metabase/visualizations/visualizations/Progress/utils.unit.spec.ts b/frontend/src/metabase/visualizations/visualizations/Progress/utils.unit.spec.ts new file mode 100644 index 00000000000..0e5677f50d6 --- /dev/null +++ b/frontend/src/metabase/visualizations/visualizations/Progress/utils.unit.spec.ts @@ -0,0 +1,27 @@ +import { getValue } from "./utils"; + +describe("Visualizations > Progress > utils", () => { + const valueTestCases = [ + [[[null]], 0], + [[[undefined]], 0], + [[["foo"]], 0], + [[[""]], 0], + [[[0]], 0], + [[[1]], 1], + [ + [ + [1, 2, 3], + [4, 5, 6], + ], + 1, + ], + [[3], 0], + [[["Infinity"]], Infinity], + ]; + + valueTestCases.forEach(([input, output]) => { + it(`should return ${output} for ${JSON.stringify(input)}`, () => { + expect(getValue(input as unknown[][])).toEqual(output); + }); + }); +}); -- GitLab