diff --git a/frontend/src/metabase/visualizations/lib/apply_tooltips.js b/frontend/src/metabase/visualizations/lib/apply_tooltips.js
index f4dfe673d44b0af10cfc4dcb44e6441d2f8db165..fb73c02c453db63fef33d9388737cbefcabb1158 100644
--- a/frontend/src/metabase/visualizations/lib/apply_tooltips.js
+++ b/frontend/src/metabase/visualizations/lib/apply_tooltips.js
@@ -112,10 +112,26 @@ function applyChartTooltips(
// edge cases handled by the code below
const seriesIndex = determineSeriesIndexFromElement(this, isStacked);
+ const seriesSettings = chart.settings.series(series[seriesIndex]);
+ const seriesTitle = seriesSettings && seriesSettings.title;
+
const card = series[seriesIndex].card;
- const isSingleSeriesBar =
- this.classList.contains("bar") && series.length === 1;
+
+ const isMultiseries = series.length > 1;
+ const isBreakoutMultiseries = isMultiseries && card._breakoutColumn;
const isArea = this.classList.contains("area");
+ const isBar = this.classList.contains("bar");
+ const isSingleSeriesBar = isBar && !isMultiseries;
+
+ // always format the second column as the series name?
+ function getColumnDisplayName(col) {
+ // don't replace with series title for breakout multiseries since the series title is shown in the breakout value
+ if (col === cols[1] && !isBreakoutMultiseries && seriesTitle) {
+ return seriesTitle;
+ } else {
+ return getFriendlyName(col);
+ }
+ }
let data = [];
if (Array.isArray(d.key)) {
@@ -123,11 +139,15 @@ function applyChartTooltips(
if (d.key._origin) {
data = d.key._origin.row.map((value, index) => {
const col = d.key._origin.cols[index];
- return { key: getFriendlyName(col), value: value, col };
+ return {
+ key: getColumnDisplayName(col),
+ value: value,
+ col,
+ };
});
} else {
data = d.key.map((value, index) => ({
- key: getFriendlyName(cols[index]),
+ key: getColumnDisplayName(cols[index]),
value: value,
col: cols[index],
}));
@@ -140,12 +160,12 @@ function applyChartTooltips(
data = [
{
- key: getFriendlyName(cols[0]),
+ key: getColumnDisplayName(cols[0]),
value: d.data.key,
col: cols[0],
},
{
- key: getFriendlyName(cols[1]),
+ key: getColumnDisplayName(cols[1]),
value: isNormalized
? formatValue(d.data.value, {
number_style: "percent",
@@ -156,6 +176,10 @@ function applyChartTooltips(
},
];
+ // NOTE: The below overcomplicated code is due to using index (i) of
+ // the element in the DOM, as returned by d3
+ // It would be much preferable to somehow get the row more directly
+
// now add entries to the tooltip for columns that aren't the X or Y axis. These aren't in
// the normal `cols` array, which is just the cols used in the graph axes; look in `_rawCols`
// for any other columns. If we find them, add them at the end of the `data` array.
@@ -204,7 +228,7 @@ function applyChartTooltips(
}
// otherwise just create a new object for any other columns.
return {
- key: getFriendlyName(col),
+ key: getColumnDisplayName(col),
value: rawRow[i],
col: col,
};
@@ -212,14 +236,14 @@ function applyChartTooltips(
}
}
- if (data && series.length > 1) {
- if (card._breakoutColumn) {
- data.unshift({
- key: getFriendlyName(card._breakoutColumn),
- value: card._breakoutValue,
- col: card._breakoutColumn,
- });
- }
+ if (isBreakoutMultiseries) {
+ data.unshift({
+ key: getFriendlyName(card._breakoutColumn),
+ // Use series title if it's set
+ value: seriesTitle ? seriesTitle : card._breakoutValue,
+ // Don't include the column if series title is set (it's already formatted)
+ col: seriesTitle ? null : card._breakoutColumn,
+ });
}
data = _.uniq(data, d => d.col);
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 0e00fd414906ec014cec10c8724bba9907988ec3..dae9c2e8aff172bd2b817eb9f8ce352ae4b46139 100644
--- a/frontend/test/metabase/visualizations/components/LineAreaBarRenderer-bar.unit.spec.js
+++ b/frontend/test/metabase/visualizations/components/LineAreaBarRenderer-bar.unit.spec.js
@@ -13,10 +13,50 @@ const DEFAULT_SETTINGS = {
"graph.x_axis.axis_enabled": true,
"graph.y_axis.axis_enabled": true,
"graph.colors": ["#00FF00", "#FF0000"],
- series: () => ({ display: "bar" }),
- column: () => ({ date_style: "MMMM D, YYYY" }),
+ series: () => DEFAULT_SERIES_SETTINGS,
+ column: () => DEFAULT_COLUMN_SETTINGS,
};
+const DEFAULT_SERIES_SETTINGS = {
+ display: "bar",
+};
+
+const DEFAULT_COLUMN_SETTINGS = {
+ date_style: "MMMM D, YYYY",
+};
+
+function MainSeries(chartType, settings = {}) {
+ return {
+ card: {
+ display: chartType,
+ visualization_settings: {
+ ...DEFAULT_SETTINGS,
+ ...settings,
+ },
+ },
+ data: {
+ cols: [
+ StringColumn({ display_name: "Category", source: "breakout" }),
+ NumberColumn({ display_name: "Sum", source: "aggregation" }),
+ ],
+ rows: [["A", 1]],
+ },
+ };
+}
+
+function ExtraSeries() {
+ return {
+ card: {},
+ data: {
+ cols: [
+ StringColumn({ display_name: "Category", source: "breakout" }),
+ NumberColumn({ display_name: "Count", source: "aggregation" }),
+ ],
+ rows: [["A", 2]],
+ },
+ };
+}
+
describe("LineAreaBarRenderer-bar", () => {
let element;
const qsa = selector => [...element.querySelectorAll(selector)];
@@ -33,86 +73,115 @@ describe("LineAreaBarRenderer-bar", () => {
document.body.removeChild(document.getElementById("fixture"));
});
- ["area", "bar"].forEach(chartType =>
- ["stacked", "normalized"].forEach(stack_type =>
- // FIXME: failing on CI
- xit(`should render a ${stack_type ||
- ""} ${chartType} chart with 2 series`, () => {
- const onHoverChange = jest.fn();
- renderLineAreaBar(
- element,
- [
- {
- card: {
- display: chartType,
- visualization_settings: {
- ...DEFAULT_SETTINGS,
- "stackable.stack_type": stack_type,
- },
- },
- data: {
- cols: [
- StringColumn({
- display_name: "Category",
- source: "breakout",
- }),
- NumberColumn({
- display_name: "Sum",
- source: "aggregation",
- }),
- ],
- rows: [["A", 1]],
- },
- },
- {
- card: {},
- data: {
- cols: [
- StringColumn({
- display_name: "Category",
- source: "breakout",
- }),
- NumberColumn({
- display_name: "Count",
- source: "aggregation",
- }),
- ],
- rows: [["A", 2]],
- },
- },
- ],
- {
- onHoverChange,
- },
- );
-
- // hover over each bar
- dispatchUIEvent(qsa(".bar, .dot")[0], "mousemove");
- dispatchUIEvent(qsa(".bar, .dot")[1], "mousemove");
-
- const { calls } = onHoverChange.mock;
- if (stack_type === "normalized") {
- expect(getDataKeyValues(calls[0][0])).toEqual([
- { key: "Category", value: "A" },
- { key: "% Sum", value: "33%" },
- ]);
- expect(getDataKeyValues(calls[1][0])).toEqual([
- { key: "Category", value: "A" },
- { key: "% Count", value: "67%" },
- ]);
- } else {
- expect(getDataKeyValues(calls[0][0])).toEqual([
- { key: "Category", value: "A" },
- { key: "Sum", value: 1 },
- ]);
- expect(getDataKeyValues(calls[1][0])).toEqual([
- { key: "Category", value: "A" },
- { key: "Count", value: 2 },
- ]);
- }
- }),
- ),
- );
+ it(`should render an bar chart with 1 series`, () => {
+ const onHoverChange = jest.fn();
+ renderLineAreaBar(element, [MainSeries("bar")], {
+ onHoverChange,
+ });
+
+ // hover over each bar
+ dispatchUIEvent(qsa(".bar, .dot")[0], "mousemove");
+
+ const { calls } = onHoverChange.mock;
+ expect(getDataKeyValues(calls[0][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "Sum", value: 1 },
+ ]);
+ });
+
+ it(`should render an bar chart with 2 series`, () => {
+ const onHoverChange = jest.fn();
+ renderLineAreaBar(element, [MainSeries("bar"), ExtraSeries()], {
+ onHoverChange,
+ });
+
+ // hover over each bar
+ dispatchUIEvent(qsa(".bar, .dot")[0], "mousemove");
+ dispatchUIEvent(qsa(".bar, .dot")[1], "mousemove");
+
+ const { calls } = onHoverChange.mock;
+ expect(getDataKeyValues(calls[0][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "Sum", value: 1 },
+ ]);
+ expect(getDataKeyValues(calls[1][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "Count", value: 2 },
+ ]);
+ });
+
+ it(`should render an bar stacked chart with 2 series`, () => {
+ const onHoverChange = jest.fn();
+ renderLineAreaBar(
+ element,
+ [MainSeries("bar", { "stackable.stack_type": "stacked" }), ExtraSeries()],
+ {
+ onHoverChange,
+ },
+ );
+
+ // hover over each bar
+ dispatchUIEvent(qsa(".bar, .dot")[0], "mousemove");
+ dispatchUIEvent(qsa(".bar, .dot")[1], "mousemove");
+
+ const { calls } = onHoverChange.mock;
+ expect(getDataKeyValues(calls[0][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "Sum", value: 1 },
+ ]);
+ expect(getDataKeyValues(calls[1][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "Count", value: 2 },
+ ]);
+ });
+
+ it(`should render an bar normalized chart with 2 series`, () => {
+ const onHoverChange = jest.fn();
+ renderLineAreaBar(
+ element,
+ [
+ MainSeries("bar", { "stackable.stack_type": "normalized" }),
+ ExtraSeries(),
+ ],
+ { onHoverChange },
+ );
+
+ // hover over each bar
+ dispatchUIEvent(qsa(".bar, .dot")[0], "mousemove");
+ dispatchUIEvent(qsa(".bar, .dot")[1], "mousemove");
+
+ const { calls } = onHoverChange.mock;
+ expect(getDataKeyValues(calls[0][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "% Sum", value: "33%" },
+ ]);
+ expect(getDataKeyValues(calls[1][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "% Count", value: "67%" },
+ ]);
+ });
+
+ it("should replace the aggregation name with the series name", () => {
+ const onHoverChange = jest.fn();
+ renderLineAreaBar(
+ element,
+ [
+ MainSeries("bar", {
+ series: () => ({ ...DEFAULT_SERIES_SETTINGS, title: "Foo" }),
+ }),
+ ],
+ { onHoverChange },
+ );
+
+ // hover over each bar
+ dispatchUIEvent(qsa(".bar, .dot")[0], "mousemove");
+
+ const { calls } = onHoverChange.mock;
+ expect(getDataKeyValues(calls[0][0])).toEqual([
+ { key: "Category", value: "A" },
+ { key: "Foo", value: 1 },
+ ]);
+ });
});
function getDataKeyValues(hover) {