From dddf499a362ce277a7f5d0c18686c9d7d6ebe686 Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Wed, 25 Sep 2024 17:50:26 +0100
Subject: [PATCH] Additional tooltip metrics for waterfall and scatter charts
(#48088)
---
.../visualizations-charts/scatter.cy.spec.js | 54 ++++++-
.../waterfall.cy.spec.js | 147 ++++++++++++++----
.../echarts/cartesian/option/tooltip.tsx | 6 +
.../visualizations/echarts/tooltip/index.tsx | 21 ++-
.../visualizations/lib/settings/graph.js | 7 +-
.../shared/settings/cartesian-chart.ts | 35 +++--
.../CartesianChart/CartesianChart.tsx | 5 +-
.../CartesianChart/chart-definition.ts | 7 -
.../visualizations/CartesianChart/events.ts | 25 ++-
.../CartesianChart/use-models-and-option.ts | 10 +-
10 files changed, 243 insertions(+), 74 deletions(-)
diff --git a/e2e/test/scenarios/visualizations-charts/scatter.cy.spec.js b/e2e/test/scenarios/visualizations-charts/scatter.cy.spec.js
index 74612c2db25..f8e7c4493f7 100644
--- a/e2e/test/scenarios/visualizations-charts/scatter.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/scatter.cy.spec.js
@@ -2,7 +2,9 @@ import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
import {
assertEChartsTooltip,
+ assertEChartsTooltipNotContain,
cartesianChartCircle,
+ leftSidebar,
restore,
visitQuestionAdhoc,
} from "e2e/support/helpers";
@@ -43,11 +45,8 @@ describe("scenarios > visualizations > scatter", () => {
triggerPopoverForBubble();
assertEChartsTooltip({
+ header: "May 2023",
rows: [
- {
- name: "Created At",
- value: "May 2023",
- },
{
name: "Count",
value: "271",
@@ -80,11 +79,8 @@ describe("scenarios > visualizations > scatter", () => {
triggerPopoverForBubble();
assertEChartsTooltip({
+ header: "May 2023",
rows: [
- {
- name: "Created At",
- value: "May 2023",
- },
{
name: "Orders count",
value: "271",
@@ -145,6 +141,48 @@ select 10 as size, 2 as x, 5 as y`,
});
});
});
+
+ it("should allow adding non-series columns to the tooltip", () => {
+ visitQuestionAdhoc({
+ display: "scatter",
+ dataset_query: {
+ type: "query",
+ database: SAMPLE_DB_ID,
+ query: { "source-table": ORDERS_ID },
+ },
+ visualization_settings: {
+ "graph.metrics": ["TAX"],
+ "graph.dimensions": ["SUBTOTAL"],
+ },
+ });
+
+ cartesianChartCircle().first().realHover();
+ assertEChartsTooltip({
+ header: "15.69",
+ rows: [{ name: "Tax", value: "0.86" }],
+ });
+ assertEChartsTooltipNotContain(["Total", "Discount", "Quantity"]);
+
+ cy.findByTestId("viz-settings-button").click();
+
+ leftSidebar().within(() => {
+ cy.findByText("Display").click();
+ cy.findByPlaceholderText("Enter metric names").click();
+ });
+ cy.findByRole("option", { name: "Total" }).click();
+ cy.findByRole("option", { name: "Discount" }).click();
+
+ cartesianChartCircle().first().realHover();
+ assertEChartsTooltip({
+ header: "15.69",
+ rows: [
+ { name: "Tax", value: "0.86" },
+ { name: "Total", value: "16.55" },
+ { name: "Discount", value: "(empty)" },
+ ],
+ });
+ assertEChartsTooltipNotContain(["Quantity"]);
+ });
});
function triggerPopoverForBubble(index = 13) {
diff --git a/e2e/test/scenarios/visualizations-charts/waterfall.cy.spec.js b/e2e/test/scenarios/visualizations-charts/waterfall.cy.spec.js
index 340fa886fd0..aec96100d95 100644
--- a/e2e/test/scenarios/visualizations-charts/waterfall.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/waterfall.cy.spec.js
@@ -2,10 +2,14 @@ import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
import {
assertEChartsTooltip,
+ assertEChartsTooltipNotContain,
chartPathWithFillColor,
+ createQuestion,
echartsContainer,
+ leftSidebar,
openNativeEditor,
openOrdersTable,
+ popover,
restore,
summarize,
visitQuestionAdhoc,
@@ -138,32 +142,79 @@ describe("scenarios > visualizations > waterfall", () => {
echartsContainer().get("text").contains("Total").should("not.exist");
});
- it("should show error for multi-series questions (metabase#15152)", () => {
- visitQuestionAdhoc({
- dataset_query: {
- type: "query",
- query: {
- "source-table": ORDERS_ID,
- aggregation: [["count"], ["sum", ["field-id", ORDERS.TOTAL]]],
- breakout: [["field", ORDERS.CREATED_AT, { "temporal-unit": "year" }]],
- },
- database: SAMPLE_DB_ID,
+ describe("multi-series (metabase#15152)", () => {
+ const DATASET_QUERY = {
+ type: "query",
+ query: {
+ "source-table": ORDERS_ID,
+ aggregation: [["count"], ["sum", ["field-id", ORDERS.TOTAL]]],
+ breakout: [["field", ORDERS.CREATED_AT, { "temporal-unit": "year" }]],
},
- display: "line",
- });
+ database: SAMPLE_DB_ID,
+ };
- // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
- cy.findByText("Visualization").click();
- switchToWaterfallDisplay();
- // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
- cy.findByText("Waterfall chart does not support multiple series");
+ function testSwitchingToWaterfall() {
+ cy.findByTestId("viz-type-button").click();
+ switchToWaterfallDisplay();
- echartsContainer().should("not.exist");
- cy.findByTestId("remove-count").click();
- echartsContainer().should("exist"); // Chart renders after removing the second metric
+ echartsContainer().within(() => {
+ cy.findByText("Created At").should("exist"); // x-axis
+ cy.findByText("Count").should("exist"); // y-axis
+ cy.findByText("Sum of Total").should("not.exist");
+
+ // x-axis labels (some)
+ ["2022", "2023", "2026", "Total"].forEach(label => {
+ cy.findByText(label).should("exist");
+ });
+
+ // y-axis labels (some)
+ ["0", "3,000", "6,000", "18,000", "21,000"].forEach(label => {
+ cy.findByText(label).should("exist");
+ });
+ });
+
+ leftSidebar().within(() => {
+ cy.findByText("Count").should("exist");
+ cy.findByText("Sum of Total").should("not.exist");
+ cy.findByText(/Add another/).should("not.exist");
+
+ cy.findByText("Count").click();
+ });
+ popover().findByText("Sum of Total").click();
+ leftSidebar().within(() => {
+ cy.findByText("Sum of Total").should("exist");
+ cy.findByText("Count").should("not.exist");
+ });
+
+ echartsContainer().within(() => {
+ cy.findByText("Sum of Total").should("exist"); // x-axis
+ cy.findByText("Created At").should("exist"); // y-axis
+ cy.findByText("Count").should("not.exist");
+
+ // x-axis labels (some)
+ ["2022", "2023", "2026", "Total"].forEach(label => {
+ cy.findByText(label).should("exist");
+ });
+
+ // y-axis labels (some)
+ ["0", "300,000", "900,000", "1,800,000"].forEach(label => {
+ cy.findByText(label).should("exist");
+ });
+ });
+ }
- // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
- cy.findByText(/Add another/).should("not.exist");
+ it("should correctly switch into single-series mode for ad-hoc queries", () => {
+ visitQuestionAdhoc({ dataset_query: DATASET_QUERY, display: "line" });
+ testSwitchingToWaterfall();
+ });
+
+ it("should correctly switch into single-series mode for ad-hoc queries", () => {
+ createQuestion(
+ { name: "Q1", query: DATASET_QUERY.query, display: "line" },
+ { visitQuestion: true },
+ );
+ testSwitchingToWaterfall();
+ });
});
it("should not allow you to choose X-axis breakout", () => {
@@ -305,10 +356,6 @@ describe("scenarios > visualizations > waterfall", () => {
assertEChartsTooltip({
rows: [
- {
- name: "C1",
- value: "a",
- },
{
name: "C2",
value: "0.2",
@@ -317,6 +364,54 @@ describe("scenarios > visualizations > waterfall", () => {
});
});
+ it("should allow adding non-series columns to the tooltip", () => {
+ const INCREASE_COLOR = "#00FF00";
+
+ function getFirstWaterfallSegment() {
+ return echartsContainer().find(`path[fill='${INCREASE_COLOR}']`).first();
+ }
+
+ visitQuestionAdhoc({
+ display: "waterfall",
+ dataset_query: {
+ type: "query",
+ database: SAMPLE_DB_ID,
+ query: {
+ "source-table": ORDERS_ID,
+ aggregation: [["count"], ["sum", ["field-id", ORDERS.TOTAL]]],
+ breakout: [["field", ORDERS.CREATED_AT, { "temporal-unit": "year" }]],
+ },
+ },
+ visualization_settings: {
+ "waterfall.increase_color": INCREASE_COLOR,
+ },
+ });
+
+ getFirstWaterfallSegment().realHover();
+ assertEChartsTooltip({
+ header: "2022",
+ rows: [{ name: "Count", value: "744", color: INCREASE_COLOR }],
+ });
+ assertEChartsTooltipNotContain(["Sum of Total"]);
+
+ cy.findByTestId("viz-settings-button").click();
+
+ leftSidebar().within(() => {
+ cy.findByText("Display").click();
+ cy.findByPlaceholderText("Enter metric names").click();
+ });
+ cy.findByRole("option", { name: "Sum of Total" }).click();
+
+ getFirstWaterfallSegment().realHover();
+ assertEChartsTooltip({
+ header: "2022",
+ rows: [
+ { name: "Count", value: "744", color: INCREASE_COLOR },
+ { name: "Sum of Total", value: "42,156.87" },
+ ],
+ });
+ });
+
describe("scenarios > visualizations > waterfall settings", () => {
beforeEach(() => {
restore();
diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/option/tooltip.tsx b/frontend/src/metabase/visualizations/echarts/cartesian/option/tooltip.tsx
index 5cde775c28f..1aba5917a9a 100644
--- a/frontend/src/metabase/visualizations/echarts/cartesian/option/tooltip.tsx
+++ b/frontend/src/metabase/visualizations/echarts/cartesian/option/tooltip.tsx
@@ -4,6 +4,7 @@ import { renderToString } from "react-dom/server";
import { EChartsTooltip } from "metabase/visualizations/components/ChartTooltip/EChartsTooltip";
import type { ComputedVisualizationSettings } from "metabase/visualizations/types";
import { getTooltipModel } from "metabase/visualizations/visualizations/CartesianChart/events";
+import type { CardDisplayType } from "metabase-types/api";
import { getTooltipBaseOption } from "../../tooltip";
import {
@@ -14,6 +15,7 @@ import type { BaseCartesianChartModel, DataKey } from "../model/types";
interface ChartItemTooltip {
dataIndex: number;
+ display: CardDisplayType;
seriesId?: DataKey | null;
settings: ComputedVisualizationSettings;
chartModel: BaseCartesianChartModel;
@@ -23,6 +25,7 @@ const ChartItemTooltip = ({
chartModel,
settings,
dataIndex,
+ display,
seriesId,
}: ChartItemTooltip) => {
if (dataIndex == null || seriesId == null) {
@@ -33,6 +36,7 @@ const ChartItemTooltip = ({
chartModel,
settings,
dataIndex,
+ display,
seriesId,
);
@@ -46,6 +50,7 @@ const ChartItemTooltip = ({
export const getTooltipOption = (
chartModel: BaseCartesianChartModel,
settings: ComputedVisualizationSettings,
+ display: CardDisplayType,
containerRef: React.RefObject<HTMLDivElement>,
): TooltipOption => {
return {
@@ -70,6 +75,7 @@ export const getTooltipOption = (
settings={settings}
chartModel={chartModel}
dataIndex={dataIndex}
+ display={display}
seriesId={seriesId}
/>,
);
diff --git a/frontend/src/metabase/visualizations/echarts/tooltip/index.tsx b/frontend/src/metabase/visualizations/echarts/tooltip/index.tsx
index 1c0eabdc096..0e4d6d428d1 100644
--- a/frontend/src/metabase/visualizations/echarts/tooltip/index.tsx
+++ b/frontend/src/metabase/visualizations/echarts/tooltip/index.tsx
@@ -5,6 +5,7 @@ import _ from "underscore";
import { isNotNull } from "metabase/lib/types";
import TooltipStyles from "metabase/visualizations/components/ChartTooltip/EChartsTooltip/EChartsTooltip.module.css";
+import type { ComputedVisualizationSettings } from "metabase/visualizations/types";
import type { ClickObject } from "metabase-lib";
import type { BaseCartesianChartModel } from "../cartesian/model/types";
@@ -130,14 +131,20 @@ export const useClickedStateTooltipSync = (
export const useCartesianChartSeriesColorsClasses = (
chartModel: BaseCartesianChartModel,
+ settings: ComputedVisualizationSettings,
) => {
- const hexColors = useMemo(
- () =>
- chartModel.seriesModels
- .map(seriesModel => seriesModel.color)
- .filter(isNotNull),
- [chartModel],
- );
+ const hexColors = useMemo(() => {
+ const seriesColors = chartModel.seriesModels
+ .map(seriesModel => seriesModel.color)
+ .filter(isNotNull);
+
+ const settingColors = [
+ settings["waterfall.increase_color"],
+ settings["waterfall.decrease_color"],
+ ].filter(isNotNull);
+
+ return [...seriesColors, ...settingColors];
+ }, [chartModel, settings]);
return useInjectSeriesColorsClasses(hexColors);
};
diff --git a/frontend/src/metabase/visualizations/lib/settings/graph.js b/frontend/src/metabase/visualizations/lib/settings/graph.js
index 634a075c96b..d1541aa8bb6 100644
--- a/frontend/src/metabase/visualizations/lib/settings/graph.js
+++ b/frontend/src/metabase/visualizations/lib/settings/graph.js
@@ -279,12 +279,7 @@ export const LEGEND_SETTINGS = {
export const TOOLTIP_SETTINGS = {
"graph.tooltip_type": {
- getDefault: ([{ card }]) => {
- const shouldShowComparisonTooltip = !["waterfall", "scatter"].includes(
- card.display,
- );
- return shouldShowComparisonTooltip ? "series_comparison" : "default";
- },
+ getDefault: () => "series_comparison",
hidden: true,
},
"graph.tooltip_columns": {
diff --git a/frontend/src/metabase/visualizations/shared/settings/cartesian-chart.ts b/frontend/src/metabase/visualizations/shared/settings/cartesian-chart.ts
index 77bd77cd985..3df9e4d8c73 100644
--- a/frontend/src/metabase/visualizations/shared/settings/cartesian-chart.ts
+++ b/frontend/src/metabase/visualizations/shared/settings/cartesian-chart.ts
@@ -2,7 +2,10 @@ import { t } from "ttag";
import _ from "underscore";
import { isNotNull } from "metabase/lib/types";
-import { getMaxDimensionsSupported } from "metabase/visualizations";
+import {
+ getMaxDimensionsSupported,
+ getMaxMetricsSupported,
+} from "metabase/visualizations";
import { getCardsColumns } from "metabase/visualizations/echarts/cartesian/model";
import { dimensionIsNumeric } from "metabase/visualizations/lib/numeric";
import { dimensionIsTimeseries } from "metabase/visualizations/lib/timeseries";
@@ -38,19 +41,19 @@ export function getDefaultMetricFilter(display: string) {
}
export function getAreDimensionsAndMetricsValid(rawSeries: RawSeries) {
- return rawSeries.some(
- ({ card, data }) =>
- columnsAreValid(
- card.visualization_settings["graph.dimensions"],
- data,
- getDefaultDimensionFilter(card.display),
- ) &&
- columnsAreValid(
- card.visualization_settings["graph.metrics"],
- data,
- getDefaultMetricFilter(card.display),
- ),
- );
+ return rawSeries.some(({ card, data }) => {
+ const dimensions = card.visualization_settings["graph.dimensions"];
+ const metrics = card.visualization_settings["graph.metrics"];
+
+ const dimensionsFilter = getDefaultDimensionFilter(card.display);
+ const metricsFilter = getDefaultMetricFilter(card.display);
+
+ return (
+ columnsAreValid(dimensions, data, dimensionsFilter) &&
+ columnsAreValid(metrics, data, metricsFilter) &&
+ (metrics ?? []).length <= getMaxMetricsSupported(card.display)
+ );
+ });
}
export function getDefaultDimensions(
@@ -74,6 +77,7 @@ export function getDefaultMetrics(
rawSeries: RawSeries,
settings: ComputedVisualizationSettings,
) {
+ const [{ card }] = rawSeries;
const prevMetrics = settings["graph.metrics"] ?? [];
const defaultMetrics = getDefaultColumns(rawSeries).metrics;
if (
@@ -83,8 +87,7 @@ export function getDefaultMetrics(
) {
return prevMetrics;
}
-
- return defaultMetrics;
+ return defaultMetrics.slice(0, getMaxMetricsSupported(card.display));
}
export const STACKABLE_SERIES_DISPLAY_TYPES = new Set(["area", "bar"]);
diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/CartesianChart.tsx b/frontend/src/metabase/visualizations/visualizations/CartesianChart/CartesianChart.tsx
index 9fb5ab77cab..67c95abeca7 100644
--- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/CartesianChart.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/CartesianChart.tsx
@@ -121,7 +121,10 @@ function _CartesianChart(props: VisualizationProps) {
}, []);
const canSelectTitle = !!onChangeCardAndRun;
- const seriesColorsCss = useCartesianChartSeriesColorsClasses(chartModel);
+ const seriesColorsCss = useCartesianChartSeriesColorsClasses(
+ chartModel,
+ settings,
+ );
useCloseTooltipOnScroll(chartRef);
diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition.ts b/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition.ts
index b73ee179770..700f507cb40 100644
--- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition.ts
+++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition.ts
@@ -1,4 +1,3 @@
-import { t } from "ttag";
import _ from "underscore";
import { GRAPH_GOAL_SETTINGS } from "metabase/visualizations/lib/settings/goal";
@@ -50,12 +49,6 @@ export const getCartesianChartDefinition = (
},
checkRenderable(series, settings) {
- if (series.length > (this.maxMetricsSupported ?? Infinity)) {
- throw new Error(
- t`${this.uiName} chart does not support multiple series`,
- );
- }
-
validateDatasetRows(series);
validateChartDataSettings(settings);
validateStacking(settings);
diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts b/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts
index 7641dc0b917..35c3c05e83f 100644
--- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts
+++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts
@@ -63,6 +63,7 @@ import type Metadata from "metabase-lib/v1/metadata/Metadata";
import { isNative } from "metabase-lib/v1/queries/utils/card";
import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
import type {
+ CardDisplayType,
CardId,
RawSeries,
TimelineEvent,
@@ -371,6 +372,7 @@ export const getTooltipModel = (
chartModel: BaseCartesianChartModel,
settings: ComputedVisualizationSettings,
echartsDataIndex: number,
+ display: CardDisplayType,
seriesDataKey: DataKey,
): EChartsTooltipModel | null => {
const dataIndex = getDataIndex(
@@ -419,6 +421,7 @@ export const getTooltipModel = (
settings,
datum,
dataIndex,
+ display,
hoveredSeries,
);
};
@@ -473,6 +476,7 @@ export const getSeriesOnlyTooltipModel = (
settings: ComputedVisualizationSettings,
datum: Datum,
dataIndex: number,
+ display: CardDisplayType,
hoveredSeries: SeriesModel,
): EChartsTooltipModel | null => {
const header = String(
@@ -498,7 +502,9 @@ export const getSeriesOnlyTooltipModel = (
return {
isFocused,
name: seriesModel.name,
- markerColorClass: getMarkerColorClass(seriesModel.color),
+ markerColorClass: getMarkerColorClass(
+ getSeriesOnlyTooltipRowColor(seriesModel, datum, settings, display),
+ ),
values: [
formatValueForTooltip({
value: datum[seriesModel.dataKey],
@@ -530,6 +536,23 @@ export const getSeriesOnlyTooltipModel = (
};
};
+const getSeriesOnlyTooltipRowColor = (
+ seriesModel: SeriesModel,
+ datum: Datum,
+ settings: ComputedVisualizationSettings,
+ display: CardDisplayType,
+) => {
+ const value = datum[seriesModel.dataKey];
+ if (display === "waterfall" && typeof value === "number") {
+ const color =
+ value >= 0
+ ? settings["waterfall.increase_color"]
+ : settings["waterfall.decrease_color"];
+ return color ?? seriesModel.color;
+ }
+ return seriesModel.color;
+};
+
export const getStackedTooltipModel = (
chartModel: BaseCartesianChartModel,
settings: ComputedVisualizationSettings,
diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts
index e4c1808b854..9830c97045a 100644
--- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts
+++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts
@@ -18,6 +18,7 @@ import { getWaterfallChartModel } from "metabase/visualizations/echarts/cartesia
import { getWaterfallChartOption } from "metabase/visualizations/echarts/cartesian/waterfall/option";
import { useBrowserRenderingContext } from "metabase/visualizations/hooks/use-browser-rendering-context";
import type { VisualizationProps } from "metabase/visualizations/types";
+import type { CardDisplayType } from "metabase-types/api";
export function useModelsAndOption(
{
@@ -122,8 +123,13 @@ export function useModelsAndOption(
}, [selectedTimelineEventIds, hovered?.timelineEvents]);
const tooltipOption = useMemo(() => {
- return getTooltipOption(chartModel, settings, containerRef);
- }, [chartModel, settings, containerRef]);
+ return getTooltipOption(
+ chartModel,
+ settings,
+ card.display as CardDisplayType,
+ containerRef,
+ );
+ }, [chartModel, settings, card.display, containerRef]);
const option = useMemo(() => {
if (width === 0 || height === 0) {
--
GitLab