Skip to content
Snippets Groups Projects
Unverified Commit ed4abaad authored by github-automation-metabase's avatar github-automation-metabase Committed by GitHub
Browse files

fix scatter chart tooltip (#50889) (#50969)


* fix scatter chart tooltip shows irrelevant info

* remove outdated spec as we always show the tooltip_column setting

Co-authored-by: default avatarAleksandr Lesnenko <alxnddr@users.noreply.github.com>
parent bcb8c01e
No related branches found
No related tags found
No related merge requests found
......@@ -20,6 +20,20 @@ const testQuery = {
type: "query",
};
const testQueryBreakout = {
database: SAMPLE_DB_ID,
query: {
"source-table": ORDERS_ID,
aggregation: [["count"]],
breakout: [
["field", ORDERS.CREATED_AT, { "temporal-unit": "year" }],
["field", PRODUCTS.CATEGORY, { "source-field": ORDERS.PRODUCT_ID }],
["field", PRODUCTS.RATING, null],
],
},
type: "query",
};
describe("scenarios > visualizations > scatter", () => {
beforeEach(() => {
H.restore();
......@@ -86,6 +100,31 @@ describe("scenarios > visualizations > scatter", () => {
});
});
it("should not show non-hovered breakout series in the tooltip (metabase#50630)", () => {
H.visitQuestionAdhoc({
dataset_query: testQueryBreakout,
display: "scatter",
visualization_settings: {
"graph.dimensions": ["CREATED_AT", "CATEGORY"],
"graph.metrics": ["count"],
},
});
// Use force=true because this chart has too many bubbles that overlap with each other
triggerPopoverForBubble(300, true);
H.assertEChartsTooltip({
header: "2025",
rows: [
{
name: "Widget",
value: "173",
},
],
});
H.assertEChartsTooltipNotContain(["Gizmo", "Gadget", "Doohickey"]);
});
it("should not display data points even when enabled in settings (metabase#13247)", () => {
H.visitQuestionAdhoc({
display: "scatter",
......@@ -136,6 +175,15 @@ select 10 as size, 2 as x, 5 as y`,
});
it("should allow adding non-series columns to the tooltip", () => {
const additionalColumns = [
"Total",
"Discount",
"Created At",
"ID",
"User ID",
"Product ID",
];
H.visitQuestionAdhoc({
display: "scatter",
dataset_query: {
......@@ -154,7 +202,8 @@ select 10 as size, 2 as x, 5 as y`,
header: "15.69",
rows: [{ name: "Tax", value: "0.86" }],
});
H.assertEChartsTooltipNotContain(["Total", "Discount", "Quantity"]);
H.assertEChartsTooltipNotContain(additionalColumns);
cy.findByTestId("viz-settings-button").click();
......@@ -162,8 +211,13 @@ select 10 as size, 2 as x, 5 as y`,
cy.findByText("Display").click();
cy.findByPlaceholderText("Enter metric names").click();
});
cy.findByRole("option", { name: "Total" }).click();
cy.findByRole("option", { name: "Discount" }).click();
additionalColumns.forEach(name =>
cy.findByRole("option", { name }).click(),
);
// Close the popover
cy.get("body").type("{esc}");
H.cartesianChartCircle().first().realHover();
H.assertEChartsTooltip({
......@@ -178,7 +232,7 @@ select 10 as size, 2 as x, 5 as y`,
});
});
function triggerPopoverForBubble(index = 13) {
function triggerPopoverForBubble(index = 13, force = false) {
// Hack that is needed because of the flakiness caused by adding throttle to the ExplicitSize component
// See: https://github.com/metabase/metabase/pull/15235
cy.findByTestId("view-footer").within(() => {
......@@ -188,5 +242,5 @@ function triggerPopoverForBubble(index = 13) {
H.cartesianChartCircle()
.eq(index) // Random bubble
.trigger("mousemove");
.trigger("mousemove", { force });
}
......@@ -304,39 +304,45 @@ export const LEGEND_SETTINGS = {
export const TOOLTIP_SETTINGS = {
"graph.tooltip_type": {
getDefault: () => "series_comparison",
getDefault: ([{ card }]) => {
const shouldShowComparisonTooltip = !["scatter", "waterfall"].includes(
card.display,
);
return shouldShowComparisonTooltip ? "series_comparison" : "default";
},
hidden: true,
},
"graph.tooltip_columns": {
section: t`Display`,
title: t`Additional tooltip metrics`,
placeholder: t`Enter metric names`,
widget: "multiselect",
useRawSeries: true,
getValue: getComputedAdditionalColumnsValue,
getHidden: (rawSeries, vizSettings) => {
// Default tooltip shows all columns
if (vizSettings["graph.tooltip_type"] === "default") {
return true;
}
return getAvailableAdditionalColumns(rawSeries, vizSettings).length === 0;
const isAggregatedChart = rawSeries[0].card.display !== "scatter";
return (
getAvailableAdditionalColumns(rawSeries, vizSettings, isAggregatedChart)
.length === 0
);
},
getProps: (rawSeries, vizSettings) => {
const options = getAvailableAdditionalColumns(rawSeries, vizSettings).map(
col => ({
label: col.display_name,
value: getColumnKey(col),
}),
);
const isAggregatedChart = rawSeries[0].card.display !== "scatter";
const options = getAvailableAdditionalColumns(
rawSeries,
vizSettings,
isAggregatedChart,
).map(col => ({
label: col.display_name,
value: getColumnKey(col),
}));
return {
options,
};
},
readDependencies: [
"graph.metrics",
"graph.dimensions",
"graph.tooltip_type",
],
readDependencies: ["graph.metrics", "graph.dimensions"],
},
};
......
......@@ -439,14 +439,6 @@ describe("graph.tooltip_columns", () => {
const tooltipColumnsSetting = TOOLTIP_SETTINGS["graph.tooltip_columns"];
describe("getHidden", () => {
it("should be hidden when tooltip type is default", () => {
const isHidden = tooltipColumnsSetting.getHidden([], {
"graph.tooltip_type": "default",
});
expect(isHidden).toBe(true);
});
it("should be hidden when there are no available additional columns", () => {
const mockSeries = [
createMockSingleSeries(
......@@ -463,7 +455,7 @@ describe("graph.tooltip_columns", () => {
];
const isHidden = tooltipColumnsSetting.getHidden(mockSeries, {
"graph.tooltip_type": "customized",
"graph.tooltip_type": "series_comparison",
"graph.dimensions": ["dim"],
"graph.metrics": ["metric"],
});
......@@ -488,7 +480,7 @@ describe("graph.tooltip_columns", () => {
];
const isHidden = tooltipColumnsSetting.getHidden(mockSeries, {
"graph.tooltip_type": "customized",
"graph.tooltip_type": "series_comparison",
"graph.dimensions": ["dim"],
"graph.metrics": ["metric1"],
});
......
......@@ -392,6 +392,7 @@ function getDefaultLineAreaBarColumns(series: RawSeries) {
export function getAvailableAdditionalColumns(
rawSeries: RawSeries,
settings: ComputedVisualizationSettings,
metricsOnly: boolean,
): DatasetColumn[] {
const alreadyIncludedColumns = new Set<DatasetColumn>();
......@@ -418,16 +419,22 @@ export function getAvailableAdditionalColumns(
.flatMap(singleSeries => {
return singleSeries.data.cols;
})
.filter(column => isMetric(column) && !alreadyIncludedColumns.has(column));
.filter(
column =>
(isMetric(column) || !metricsOnly) &&
!alreadyIncludedColumns.has(column),
);
}
export function getComputedAdditionalColumnsValue(
rawSeries: RawSeries,
settings: ComputedVisualizationSettings,
) {
const isAggregatedChart = rawSeries[0].card.display !== "scatter";
const availableAdditionalColumnKeys = new Set(
getAvailableAdditionalColumns(rawSeries, settings).map(column =>
getColumnKey(column),
getAvailableAdditionalColumns(rawSeries, settings, isAggregatedChart).map(
column => getColumnKey(column),
),
);
......
......@@ -14,7 +14,6 @@ import type {
EChartsTooltipModel,
EChartsTooltipRow,
} from "metabase/visualizations/components/ChartTooltip/EChartsTooltip";
import { getRowFromDataPoint } from "metabase/visualizations/components/ChartTooltip/KeyValuePairChartTooltip/KeyValuePairChartTooltip";
import {
getPercent,
getTotalValue,
......@@ -380,14 +379,14 @@ export const getTooltipModel = (
stackModel.seriesKeys.includes(hoveredSeries.dataKey),
);
const shouldShowAllColumnValuesTooltip =
settings["graph.tooltip_type"] === "default";
if (shouldShowAllColumnValuesTooltip) {
return getAllColumnsTooltipModel(
if (settings["graph.tooltip_type"] === "default") {
return getSingleSeriesTooltipModel(
chartModel,
datum,
settings,
dataIndex,
hoveredSeries,
display,
);
}
......@@ -403,39 +402,69 @@ export const getTooltipModel = (
hoveredSeries,
);
}
return getSeriesOnlyTooltipModel(
return getSeriesComparisonTooltipModel(
chartModel,
settings,
datum,
dataIndex,
display,
hoveredSeries,
);
};
const getAllColumnsTooltipModel = (
const getSingleSeriesTooltipModel = (
chartModel: BaseCartesianChartModel,
datum: Datum,
settings: ComputedVisualizationSettings,
dataIndex: number,
seriesModel: SeriesModel,
hoveredSeries: SeriesModel,
display: CardDisplayType,
): EChartsTooltipModel | null => {
const rows = getEventColumnsData(chartModel, seriesModel, dataIndex)
.map(getRowFromDataPoint)
.map(dataPoint => {
return {
name: dataPoint.key,
values: [
formatValueForTooltip({
isAlreadyScaled: true,
value: dataPoint.value,
column: dataPoint.col,
settings,
}),
],
};
});
const header = String(
formatValueForTooltip({
value: datum[X_AXIS_DATA_KEY],
column: chartModel.dimensionModel.column,
settings,
}),
);
const additionalColumnsRows = getAdditionalTooltipRowsData(
chartModel,
settings,
hoveredSeries,
dataIndex,
);
const seriesToShow = chartModel.seriesModels.filter(
series => series === hoveredSeries || !isBreakoutSeries(series),
);
const seriesTooltipRows = seriesToShow.map(series => {
const isFocused =
hoveredSeries.dataKey === series.dataKey && seriesToShow.length > 1;
return {
isFocused,
name: series.name,
markerColorClass: getMarkerColorClass(
getSeriesOnlyTooltipRowColor(series, datum, settings, display),
),
values: [
formatValueForTooltip({
value: datum[series.dataKey],
column: series.column,
settings,
isAlreadyScaled: true,
}),
],
};
});
const rows: EChartsTooltipRow[] = [
...seriesTooltipRows,
...additionalColumnsRows,
];
return {
header,
rows,
};
};
......@@ -458,12 +487,11 @@ export const mergeSeriesRowsAndAdditionalColumnsRows = (
return rows;
};
export const getSeriesOnlyTooltipModel = (
const getSeriesComparisonTooltipModel = (
chartModel: BaseCartesianChartModel,
settings: ComputedVisualizationSettings,
datum: Datum,
dataIndex: number,
display: CardDisplayType,
hoveredSeries: SeriesModel,
): EChartsTooltipModel | null => {
const header = String(
......@@ -493,9 +521,7 @@ export const getSeriesOnlyTooltipModel = (
return {
isFocused,
name: seriesModel.name,
markerColorClass: getMarkerColorClass(
getSeriesOnlyTooltipRowColor(seriesModel, datum, settings, display),
),
markerColorClass: getMarkerColorClass(seriesModel.color),
values: [
formatValueForTooltip({
value: value,
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment