From 09de384805564a5427e23c19eb11153d14eba176 Mon Sep 17 00:00:00 2001
From: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Date: Wed, 28 Aug 2019 11:49:01 -0400
Subject: [PATCH] Convert nulls to "(empty)" in row charts and line area bar
charts (#10609)
---
.../visualizations/lib/RowRenderer.js | 15 ++++---
.../metabase/visualizations/lib/constants.js | 3 ++
.../visualizations/lib/renderer_utils.js | 7 ++-
.../LineAreaBarRenderer-bar.unit.spec.js | 28 ++++++++++--
.../components/RowRenderer.unit.spec.js | 43 +++++++++++++++++++
5 files changed, 87 insertions(+), 9 deletions(-)
create mode 100644 frontend/src/metabase/visualizations/lib/constants.js
create mode 100644 frontend/test/metabase/visualizations/components/RowRenderer.unit.spec.js
diff --git a/frontend/src/metabase/visualizations/lib/RowRenderer.js b/frontend/src/metabase/visualizations/lib/RowRenderer.js
index 62300e5327c..e9d9e954213 100644
--- a/frontend/src/metabase/visualizations/lib/RowRenderer.js
+++ b/frontend/src/metabase/visualizations/lib/RowRenderer.js
@@ -9,6 +9,7 @@ import { formatValue } from "metabase/lib/formatting";
import { initChart, forceSortedGroup, makeIndexMap } from "./renderer_utils";
import { getFriendlyName } from "./utils";
import { checkXAxisLabelOverlap } from "./LineAreaBarPostRender";
+import { NULL_DISPLAY_VALUE } from "./constants";
export default function rowRenderer(
element,
@@ -25,16 +26,20 @@ export default function rowRenderer(
// disable clicks
chart.onClick = () => {};
- const formatDimension = row =>
- formatValue(row[0], { column: cols[0], type: "axis" });
-
// dc.js doesn't give us a way to format the row labels from unformatted data, so we have to
// do it here then construct a mapping to get the original dimension for tooltipsd/clicks
- const rows = series[0].data.rows.map(row => [formatDimension(row), row[1]]);
+ const rowsWithFormattedNull = series[0].data.rows.map(([first, ...rest]) => [
+ first === null ? NULL_DISPLAY_VALUE : first,
+ ...rest,
+ ]);
+ const rows = rowsWithFormattedNull.map(([a, b]) => [
+ formatValue(a, { column: cols[0], type: "axis" }),
+ b,
+ ]);
const formattedDimensionMap = new Map(
rows.map(([formattedDimension], index) => [
formattedDimension,
- series[0].data.rows[index][0],
+ rowsWithFormattedNull[index][0],
]),
);
diff --git a/frontend/src/metabase/visualizations/lib/constants.js b/frontend/src/metabase/visualizations/lib/constants.js
new file mode 100644
index 00000000000..1a96249d380
--- /dev/null
+++ b/frontend/src/metabase/visualizations/lib/constants.js
@@ -0,0 +1,3 @@
+import { t } from "ttag";
+
+export const NULL_DISPLAY_VALUE = t`(empty)`;
diff --git a/frontend/src/metabase/visualizations/lib/renderer_utils.js b/frontend/src/metabase/visualizations/lib/renderer_utils.js
index 54d11e8c065..739e2a7792f 100644
--- a/frontend/src/metabase/visualizations/lib/renderer_utils.js
+++ b/frontend/src/metabase/visualizations/lib/renderer_utils.js
@@ -10,6 +10,7 @@ import { dimensionIsNumeric } from "./numeric";
import { dimensionIsTimeseries } from "./timeseries";
import { getAvailableCanvasWidth, getAvailableCanvasHeight } from "./utils";
import { invalidDateWarning, nullDimensionWarning } from "./warnings";
+import { NULL_DISPLAY_VALUE } from "./constants";
export function initChart(chart, element) {
// set the bounds
@@ -109,7 +110,11 @@ const memoizedParseXValue = _.memoize(
if (isTimeseries && !isQuantitative) {
return parseTimestampAndWarn(xValue, unit);
}
- const parsedValue = isNumeric ? xValue : String(xValue);
+ const parsedValue = isNumeric
+ ? xValue
+ : xValue === null
+ ? NULL_DISPLAY_VALUE
+ : String(xValue);
return { parsedValue };
},
// create cache key from args
diff --git a/frontend/test/metabase/visualizations/components/LineAreaBarRenderer-bar.unit.spec.js b/frontend/test/metabase/visualizations/components/LineAreaBarRenderer-bar.unit.spec.js
index e4659a96330..dfa6d4c345b 100644
--- a/frontend/test/metabase/visualizations/components/LineAreaBarRenderer-bar.unit.spec.js
+++ b/frontend/test/metabase/visualizations/components/LineAreaBarRenderer-bar.unit.spec.js
@@ -27,7 +27,7 @@ const DEFAULT_COLUMN_SETTINGS = {
date_style: "MMMM D, YYYY",
};
-function MainSeries(chartType, settings = {}, value = 1) {
+function MainSeries(chartType, settings = {}, { key = "A", value = 1 } = {}) {
return {
card: {
display: chartType,
@@ -41,7 +41,7 @@ function MainSeries(chartType, settings = {}, value = 1) {
StringColumn({ display_name: "Category", source: "breakout" }),
NumberColumn({ display_name: "Sum", source: "aggregation" }),
],
- rows: [["A", value]],
+ rows: [[key, value]],
},
};
}
@@ -184,7 +184,13 @@ describe("LineAreaBarRenderer-bar", () => {
const onHoverChange = jest.fn();
renderLineAreaBar(
element,
- [MainSeries("bar", { "stackable.stack_type": "normalized" }, 3)],
+ [
+ MainSeries(
+ "bar",
+ { "stackable.stack_type": "normalized" },
+ { value: 3 },
+ ),
+ ],
{ onHoverChange },
);
@@ -218,6 +224,22 @@ describe("LineAreaBarRenderer-bar", () => {
{ key: "Foo", value: 1 },
]);
});
+
+ it('should render "(empty)" for nulls', () => {
+ const onHoverChange = jest.fn();
+ renderLineAreaBar(element, [MainSeries("bar", {}, { key: null })], {
+ onHoverChange,
+ });
+
+ dispatchUIEvent(qsa(".bar, .dot")[0], "mousemove");
+
+ const { calls } = onHoverChange.mock;
+ const [{ value }] = getDataKeyValues(calls[0][0]);
+ expect(value).toEqual("(empty)");
+
+ const tick = element.querySelector(".axis.x .tick text");
+ expect(tick.textContent).toEqual("(empty)");
+ });
});
function getDataKeyValues(hover) {
diff --git a/frontend/test/metabase/visualizations/components/RowRenderer.unit.spec.js b/frontend/test/metabase/visualizations/components/RowRenderer.unit.spec.js
new file mode 100644
index 00000000000..d82d1ec925e
--- /dev/null
+++ b/frontend/test/metabase/visualizations/components/RowRenderer.unit.spec.js
@@ -0,0 +1,43 @@
+import "__support__/mocks";
+
+import {
+ createFixture,
+ cleanupFixture,
+ renderChart,
+ NumberColumn,
+ StringColumn,
+} from "../__support__/visualizations";
+
+import rowRenderer from "metabase/visualizations/lib/RowRenderer";
+
+describe("RowChart", () => {
+ let element;
+
+ beforeEach(function() {
+ element = createFixture();
+ });
+
+ afterEach(function() {
+ cleanupFixture(element);
+ });
+
+ it("should render", () => {
+ renderChart(rowRenderer, element, [
+ {
+ card: { display: "row", visualization_settings: {} },
+ data: { cols: [StringColumn(), NumberColumn()], rows: [["a", 1]] },
+ },
+ ]);
+ });
+
+ it('should render null as "(empty)"', () => {
+ renderChart(rowRenderer, element, [
+ {
+ card: { display: "row", visualization_settings: {} },
+ data: { cols: [StringColumn(), NumberColumn()], rows: [[null, 1]] },
+ },
+ ]);
+
+ expect(element.querySelector("text.row").innerHTML).toBe("(empty)");
+ });
+});
--
GitLab