diff --git a/e2e/test/scenarios/visualizations/bar_chart.cy.spec.js b/e2e/test/scenarios/visualizations/bar_chart.cy.spec.js index 67bf128b82b8b874268cdd2b5834abbab3d65845..226dacfe82e2438f303146b36a36abbfad69b204 100644 --- a/e2e/test/scenarios/visualizations/bar_chart.cy.spec.js +++ b/e2e/test/scenarios/visualizations/bar_chart.cy.spec.js @@ -225,6 +225,59 @@ describe("scenarios > visualizations > bar chart", () => { }); }); + // Note (EmmadUsmani): see `line_chart.cy.spec.js` for more test cases of this + describe("with split y-axis (metabase#12939)", () => { + it("should split the y-axis when column settings differ", () => { + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": ORDERS_ID, + aggregation: [ + ["avg", ["field", ORDERS.TOTAL, null]], + ["min", ["field", ORDERS.TOTAL, null]], + ], + breakout: [ + ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }], + ], + }, + database: SAMPLE_DB_ID, + }, + display: "bar", + visualization_settings: { + column_settings: { + '["name","avg"]': { number_style: "decimal" }, + '["name","min"]': { number_style: "percent" }, + }, + }, + }); + + cy.get("g.axis.yr").should("be.visible"); + }); + + it("should not split the y-axis when semantic_type, column settings are same and values are not far", () => { + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": ORDERS_ID, + aggregation: [ + ["avg", ["field", ORDERS.TOTAL, null]], + ["min", ["field", ORDERS.TOTAL, null]], + ], + breakout: [ + ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }], + ], + }, + database: SAMPLE_DB_ID, + }, + display: "bar", + }); + + cy.get("g.axis.yr").should("not.exist"); + }); + }); + it("supports up to 100 series (metabase#28796)", () => { visitQuestionAdhoc({ display: "bar", diff --git a/e2e/test/scenarios/visualizations/line_chart.cy.spec.js b/e2e/test/scenarios/visualizations/line_chart.cy.spec.js index 8d0eaf8868df02193ae9eca017bb8a6048836c0f..7c30edadf9a5ebbaec475c5ffdbfc8e9e27e44b6 100644 --- a/e2e/test/scenarios/visualizations/line_chart.cy.spec.js +++ b/e2e/test/scenarios/visualizations/line_chart.cy.spec.js @@ -10,7 +10,8 @@ import { import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; -const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE; +const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID, PEOPLE, PEOPLE_ID } = + SAMPLE_DATABASE; const Y_AXIS_RIGHT_SELECTOR = ".axis.yr"; @@ -201,6 +202,99 @@ describe("scenarios > visualizations > line chart", () => { cy.get(`.sub._0`).find("circle").should("have.length", 2); }); + describe("y-axis splitting (metabase#12939)", () => { + it("should not split the y-axis when columns are of the same semantic_type and have close values", () => { + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": ORDERS_ID, + aggregation: [ + ["avg", ["field", ORDERS.TOTAL, null]], + ["min", ["field", ORDERS.TOTAL, null]], + ], + breakout: [ + ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }], + ], + }, + database: SAMPLE_DB_ID, + }, + display: "line", + }); + + cy.get("g.axis.yr").should("not.exist"); + }); + + it("should split the y-axis when columns are of different semantic_type", () => { + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": PEOPLE_ID, + aggregation: [ + ["avg", ["field", PEOPLE.LATITUDE, null]], + ["avg", ["field", PEOPLE.LONGITUDE, null]], + ], + breakout: [ + ["field", PEOPLE.CREATED_AT, { "temporal-unit": "month" }], + ], + }, + database: SAMPLE_DB_ID, + }, + display: "line", + }); + + cy.get("g.axis.yr").should("be.visible"); + }); + + it("should split the y-six when columns are of the same semantic_type but have far values", () => { + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": ORDERS_ID, + aggregation: [ + ["sum", ["field", ORDERS.TOTAL, null]], + ["min", ["field", ORDERS.TOTAL, null]], + ], + breakout: [ + ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }], + ], + }, + database: SAMPLE_DB_ID, + }, + display: "line", + }); + + cy.get("g.axis.yr").should("be.visible"); + }); + + it("should not split the y-axis when the setting is disabled", () => { + visitQuestionAdhoc({ + dataset_query: { + type: "query", + query: { + "source-table": ORDERS_ID, + aggregation: [ + ["sum", ["field", ORDERS.TOTAL, null]], + ["min", ["field", ORDERS.TOTAL, null]], + ], + breakout: [ + ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }], + ], + }, + database: SAMPLE_DB_ID, + }, + display: "line", + visualization_settings: { + "graph.y_axis.auto_split": false, + }, + }); + + cy.get("g.axis.yr").should("not.exist"); + }); + }); + describe("tooltip of combined dashboard cards (multi-series) should show the correct column title (metabase#16249", () => { const RENAMED_FIRST_SERIES = "Foo"; const RENAMED_SECOND_SERIES = "Bar"; diff --git a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js index c3a9f327ccb1220833b941552873e0e151a36f31..4ccb083814440f414df7ab26168ccf2219a30ca6 100644 --- a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js +++ b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js @@ -59,6 +59,7 @@ import { isMultiCardSeries, hasClickBehavior, replaceNullValuesForOrdinal, + shouldSplitYAxis, } from "./renderer_utils"; import lineAndBarOnRender from "./LineAreaBarPostRender"; @@ -290,15 +291,12 @@ function getYAxisSplit( } } - // don't auto-split if the metric columns are all identical, i.e. it's a breakout multiseries - const hasDifferentYAxisColumns = - _.uniq(series.map(s => JSON.stringify(s.data.cols[1]))).length > 1; if ( - !isScalarSeries && - chartType !== "scatter" && - !isStacked(settings, datas) && - hasDifferentYAxisColumns && - settings["graph.y_axis.auto_split"] !== false + shouldSplitYAxis( + { settings, chartType, isScalarSeries, series }, + datas, + yExtents, + ) ) { // NOTE: this version computes the split after assigning fixed left/right // which causes other series to move around when changing the setting diff --git a/frontend/src/metabase/visualizations/lib/renderer_utils.js b/frontend/src/metabase/visualizations/lib/renderer_utils.js index a562c862d7458f43aeb9a36162cba9e59a90a0a2..09993917da0b2f4bd190d828ba7529e7dadab9cb 100644 --- a/frontend/src/metabase/visualizations/lib/renderer_utils.js +++ b/frontend/src/metabase/visualizations/lib/renderer_utils.js @@ -22,6 +22,7 @@ import { computeNumericDataInverval, dimensionIsNumeric } from "./numeric"; import { getAvailableCanvasWidth, getAvailableCanvasHeight } from "./utils"; import { invalidDateWarning, nullDimensionWarning } from "./warnings"; +import { getLineAreaBarComparisonSettings } from "./settings"; export function initChart(chart, element) { // set the bounds @@ -352,6 +353,46 @@ export function xValueForWaterfallTotal({ settings, series }) { return TOTAL_ORDINAL_VALUE; } +export function shouldSplitYAxis( + { settings, chartType, isScalarSeries, series }, + datas, + yExtents, +) { + if ( + isScalarSeries || + chartType === "scatter" || + settings["graph.y_axis.auto_split"] === false || + isStacked(settings, datas) + ) { + return false; + } + + const hasDifferentYAxisColTypes = + _.uniq(series.map(s => s.data.cols[1].semantic_type)).length > 1; + if (hasDifferentYAxisColTypes) { + return true; + } + + const columnSettings = series.map(s => + getLineAreaBarComparisonSettings(settings.column(s.data.cols[1])), + ); + const hasDifferentColumnSettings = columnSettings.some(s1 => + columnSettings.some(s2 => !_.isEqual(s1, s2)), + ); + if (hasDifferentColumnSettings) { + return true; + } + + const minRange = Math.min(...yExtents.map(extent => extent[1] - extent[0])); + const maxExtent = Math.max(...yExtents.map(extent => extent[1])); + const minExtent = Math.min(...yExtents.map(extent => extent[0])); + const chartRange = maxExtent - minExtent; + + // Note (EmmadUsmani): When the series with the smallest range is less than 5% + // of the chart's total range, we split the y-axis so it doesn't look too small. + return minRange / chartRange <= 0.05; +} + /************************************************************ PROPERTIES ************************************************************/ export const isTimeseries = settings => diff --git a/frontend/src/metabase/visualizations/lib/settings.js b/frontend/src/metabase/visualizations/lib/settings.js index b8fbbc0e6f0b93e429f5a9ded8ea50eccff2c6c9..e52e625fe761a50fcf6acf5eeb5b8e279746ffbe 100644 --- a/frontend/src/metabase/visualizations/lib/settings.js +++ b/frontend/src/metabase/visualizations/lib/settings.js @@ -1,6 +1,7 @@ /* eslint-disable react/prop-types */ import { getIn } from "icepick"; +import _ from "underscore"; import ChartSettingInput from "metabase/visualizations/components/settings/ChartSettingInput"; import ChartSettingInputGroup from "metabase/visualizations/components/settings/ChartSettingInputGroup"; import ChartSettingInputNumeric from "metabase/visualizations/components/settings/ChartSettingInputNumeric"; @@ -276,3 +277,26 @@ function getColumnClickBehavior(columnSettings) { }; }, null); } + +const KEYS_TO_COMPARE = new Set([ + "number_style", + "currency", + "currency_style", + "number_separators", + "decimals", + "scale", + "prefix", + "suffix", +]); + +export function getLineAreaBarComparisonSettings(columnSettings) { + return _.pick(columnSettings, (value, key) => { + if (!KEYS_TO_COMPARE.has(key)) { + return false; + } + if ((key === "prefix" || key === "suffix") && value === "") { + return false; + } + return true; + }); +}