From 072a1aed5761394ddd5fdf38665c0c25b8c492a4 Mon Sep 17 00:00:00 2001 From: Rajat Agrawal Date: Wed, 13 Sep 2023 15:01:55 +0530 Subject: [PATCH 1/2] Fix/pie chart layout (#27023) Fixes #27035 --- .../EChartsConfigurationBuilder.test.ts | 12 ++++----- .../component/EChartsConfigurationBuilder.ts | 26 +++++++++++-------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts index 2f850f0801..02354e0d72 100644 --- a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts +++ b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts @@ -245,14 +245,14 @@ describe("EChartsConfigurationBuilder", () => { }, }, { - top: 265, - left: "33.333333333333336%", + top: 80, + left: 495, textAlign: "center", text: "series1", }, { - top: 265, - left: "66.66666666666667%", + top: 80, + left: 495, textAlign: "center", text: "series2", }, @@ -484,8 +484,8 @@ describe("EChartsConfigurationBuilder", () => { expectedConfig.series = [ { type: "pie", - radius: "60%", - center: ["50%", "60%"], + top: 100, + bottom: 30, name: "series1", encode: { itemName: "Category", diff --git a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts index 933d15d0ad..26600e47b1 100644 --- a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts +++ b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts @@ -17,6 +17,7 @@ export class EChartsConfigurationBuilder { seriesID: string, seriesData: ChartData, showDataPointLabel: boolean, + layoutConfig: Record>, ) { let seriesName = messages.Undefined; if (seriesData.seriesName && seriesData.seriesName.length > 0) { @@ -25,8 +26,8 @@ export class EChartsConfigurationBuilder { const config = { type: "pie", - radius: "60%", - center: ["50%", "60%"], + top: layoutConfig.grid.top, + bottom: layoutConfig.grid.bottom, name: seriesName, label: { show: showDataPointLabel, @@ -46,6 +47,7 @@ export class EChartsConfigurationBuilder { #seriesConfigForChart( props: ChartComponentProps, allSeriesData: AllChartData, + layoutConfig: Record>, ) { /** * { @@ -98,6 +100,7 @@ export class EChartsConfigurationBuilder { seriesID, seriesData, props.showDataPointLabel, + layoutConfig, ); break; } @@ -118,16 +121,13 @@ export class EChartsConfigurationBuilder { #titleConfigForPiechart( props: ChartComponentProps, allSeriesData: AllChartData, + layoutConfig: Record>, ) { const config: Record[] = []; - const numSeries = Object.keys(allSeriesData).length; - const interval = 100 / (numSeries + 1); - - Object.values(allSeriesData).forEach((seriesData, index) => { - const offset = `${(index + 1) * interval}%`; + Object.values(allSeriesData).forEach((seriesData) => { config.push({ - top: this.seriesTitleOffsetForPieChart(props), - left: offset, + top: (layoutConfig.grid.top as number) - 20, + left: props.dimensions.componentWidth / 2 - 5, textAlign: "center", text: seriesData.seriesName ?? "", }); @@ -172,7 +172,7 @@ export class EChartsConfigurationBuilder { if (props.chartType == "PIE_CHART") { return [ defaultTitleConfig, - ...this.#titleConfigForPiechart(props, allSeriesData), + ...this.#titleConfigForPiechart(props, allSeriesData, layoutConfig), ]; } else { return defaultTitleConfig; @@ -350,7 +350,11 @@ export class EChartsConfigurationBuilder { chartConfig.yAxis = this.#yAxisConfig(props, layoutConfig); chartConfig.dataZoom = this.#scrollConfig(props, layoutConfig); - chartConfig.series = this.#seriesConfigForChart(props, allSeriesData); + chartConfig.series = this.#seriesConfigForChart( + props, + allSeriesData, + layoutConfig, + ); return chartConfig; } } From d06c242cf9f9aa848e3a908edf6b52e3809c03ea Mon Sep 17 00:00:00 2001 From: Rajat Agrawal Date: Sat, 16 Sep 2023 11:07:48 +0530 Subject: [PATCH 2/2] Fix/fix chart layout (#27222) Fixes #26867 --- .../EChartsConfigurationBuilder.test.ts | 237 +++++++--- .../component/EChartsConfigurationBuilder.ts | 43 +- .../component/EChartsDatasetBuilder.test.ts | 99 ++--- .../component/EChartsDatasetBuilder.ts | 61 ++- ...EChartsElementVisibilityCalculator.test.ts | 158 ++++--- .../EChartsElementVisibilityCalculator.ts | 90 ++-- .../EChartsLayoutBuilder.test.ts | 180 ++++++-- .../LayoutBuilders/EChartsLayoutBuilder.ts | 108 +++-- .../EChartsXAxisLayoutBuilder.test.ts | 407 +++++++++++++++--- .../EChartsXAxisLayoutBuilder.ts | 101 ++++- .../EChartsYAxisLayoutBuilder.test.ts | 97 ++++- .../EChartsYAxisLayoutBuilder.ts | 58 ++- .../widgets/ChartWidget/component/helpers.ts | 26 ++ .../widgets/ChartWidget/component/index.tsx | 9 +- .../src/widgets/ChartWidget/constants.ts | 11 +- .../src/widgets/ChartWidget/widget/index.tsx | 7 +- 16 files changed, 1307 insertions(+), 385 deletions(-) diff --git a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts index 02354e0d72..273457f63a 100644 --- a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts +++ b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.test.ts @@ -6,16 +6,25 @@ import { EChartsConfigurationBuilder } from "./EChartsConfigurationBuilder"; describe("EChartsConfigurationBuilder", () => { const builder = new EChartsConfigurationBuilder(); + const longestLabels = { x: "x1", y: "y1" }; const dataZoomConfig = [ { filterMode: "filter", bottom: 30, - start: "20", + start: "0", + end: "50", type: "slider", show: true, height: 30, }, + { + filterMode: "filter", + start: "0", + end: "50", + type: "inside", + show: true, + }, ]; const chartData1: ChartData = { @@ -43,7 +52,7 @@ describe("EChartsConfigurationBuilder", () => { isVisible: true, isLoading: false, setAdaptiveYMin: false, - labelOrientation: LabelOrientation.AUTO, + labelOrientation: LabelOrientation.ROTATE, onDataPointClick: (point) => { point.x; }, @@ -74,7 +83,7 @@ describe("EChartsConfigurationBuilder", () => { type: "scroll", show: true, }, - grid: { top: 100, bottom: 80, left: 100, show: false }, + grid: { top: 100, bottom: 52, left: 52, show: false }, title: { show: true, text: defaultProps.chartName, @@ -94,17 +103,17 @@ describe("EChartsConfigurationBuilder", () => { xAxis: { type: "category", axisLabel: { - rotate: 0, + rotate: 90, fontFamily: "fontfamily", color: Colors.DOVE_GRAY2, show: true, - width: 60, - overflow: "break", + width: 2, + overflow: "truncate", }, show: true, name: "xaxisname", nameLocation: "middle", - nameGap: 40, + nameGap: 12, nameTextStyle: { fontSize: 14, fontFamily: "fontfamily", @@ -115,14 +124,14 @@ describe("EChartsConfigurationBuilder", () => { axisLabel: { fontFamily: "fontfamily", color: Colors.DOVE_GRAY2, - overflow: "break", + overflow: "truncate", show: true, - width: 60, + width: 12, }, show: true, name: "yaxisname", nameLocation: "middle", - nameGap: 70, + nameGap: 22, nameTextStyle: { fontSize: 14, fontFamily: "fontfamily", @@ -152,7 +161,11 @@ describe("EChartsConfigurationBuilder", () => { }; it("1. builds a right chart configuration", () => { - const output = builder.prepareEChartConfig(defaultProps, chartData); + const output = builder.prepareEChartConfig( + defaultProps, + chartData, + longestLabels, + ); expect(output).toEqual(defaultExpectedConfig); }); @@ -166,7 +179,11 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.dataZoom = []; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output).toStrictEqual(expectedConfig); }); @@ -181,7 +198,11 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.dataZoom = dataZoomConfig; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.dataZoom).toStrictEqual(expectedConfig.dataZoom); }); @@ -196,7 +217,11 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.dataZoom = []; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.dataZoom).toStrictEqual(expectedConfig.dataZoom); }); }); @@ -221,7 +246,11 @@ describe("EChartsConfigurationBuilder", () => { }, }; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.title).toStrictEqual(expectedConfig.title); }); @@ -258,7 +287,11 @@ describe("EChartsConfigurationBuilder", () => { }, ]; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.title).toStrictEqual(expectedConfig.title); }); }); @@ -268,11 +301,17 @@ describe("EChartsConfigurationBuilder", () => { const props = JSON.parse(JSON.stringify(defaultProps)); props.chartType = "BAR_CHART"; - const expectedConfig: any = { ...defaultExpectedConfig }; - expectedConfig.xAxis = { ...expectedConfig.xAxis }; + const expectedConfig: any = JSON.parse( + JSON.stringify(defaultExpectedConfig), + ); expectedConfig.xAxis.type = "value"; + expectedConfig.xAxis.axisLabel.rotate = 0; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.xAxis).toStrictEqual(expectedConfig.xAxis); }); @@ -280,13 +319,18 @@ describe("EChartsConfigurationBuilder", () => { const props = JSON.parse(JSON.stringify(defaultProps)); props.labelOrientation = LabelOrientation.SLANT; - const expectedConfig: any = { ...defaultExpectedConfig }; - expectedConfig.xAxis = JSON.parse(JSON.stringify(expectedConfig.xAxis)); + const expectedConfig: any = JSON.parse( + JSON.stringify(defaultExpectedConfig), + ); expectedConfig.xAxis.axisLabel.rotate = 45; // slant configuration needs rotate = 45; - expectedConfig.xAxis.axisLabel.width = 50; - expectedConfig.xAxis.nameGap = 60; + expectedConfig.xAxis.axisLabel.width = 2; + expectedConfig.xAxis.nameGap = 12; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.xAxis).toStrictEqual(expectedConfig.xAxis); }); @@ -299,12 +343,13 @@ describe("EChartsConfigurationBuilder", () => { JSON.stringify(defaultExpectedConfig), ); labelRotatedConfig.xAxis.axisLabel.rotate = 90; - labelRotatedConfig.xAxis.nameGap = 70; - labelRotatedConfig.grid.bottom = 110; + labelRotatedConfig.xAxis.nameGap = 12; + labelRotatedConfig.grid.bottom = 52; const output = builder.prepareEChartConfig( labelRotatedProps, chartData, + longestLabels, ); expect(output).toStrictEqual(labelRotatedConfig); }); @@ -314,12 +359,22 @@ describe("EChartsConfigurationBuilder", () => { const props = JSON.parse(JSON.stringify(defaultProps)); props.labelOrientation = LabelOrientation.AUTO; - const expectedConfig: any = { ...defaultExpectedConfig }; - expectedConfig.xAxis = { ...expectedConfig.xAxis }; - expectedConfig.xAxis.axisLabel = { ...expectedConfig.xAxis.axisLabel }; - expectedConfig.xAxis.axisLabel.rotate = 0; + const expectedConfig: any = JSON.parse( + JSON.stringify(defaultExpectedConfig), + ); + expectedConfig.xAxis.axisLabel = { + color: Colors.DOVE_GRAY2, + fontFamily: "fontfamily", + rotate: 0, + show: true, + }; + expectedConfig.xAxis.nameGap = 40; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.xAxis).toStrictEqual(expectedConfig.xAxis); }); @@ -332,12 +387,16 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.xAxis = { type: "category", - axisLabel: { ...defaultExpectedConfig.xAxis.axisLabel }, + axisLabel: { ...defaultExpectedConfig.xAxis.axisLabel, width: -50 }, show: false, }; expectedConfig.xAxis.axisLabel.show = false; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.xAxis).toStrictEqual(expectedConfig.xAxis); }); }); @@ -352,7 +411,11 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.yAxis.type = "category"; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.yAxis).toStrictEqual(expectedConfig.yAxis); }); @@ -365,7 +428,11 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.yAxis.min = "dataMin"; // "datamin" means that the y axis is adaptive in echarts - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output).toStrictEqual(expectedConfig); }); @@ -378,7 +445,11 @@ describe("EChartsConfigurationBuilder", () => { show: true, }; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.yAxis).toStrictEqual(config); }); }); @@ -394,7 +465,11 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.series[0].itemStyle.color = "primarycolor"; - const output = builder.prepareEChartConfig(props, modifiedChartData); + const output = builder.prepareEChartConfig( + props, + modifiedChartData, + longestLabels, + ); expect(output).toStrictEqual(expectedConfig); }); @@ -408,7 +483,11 @@ describe("EChartsConfigurationBuilder", () => { ); expectedConfig.series[1].itemStyle.color = ""; - const output = builder.prepareEChartConfig(props, modifiedChartData); + const output = builder.prepareEChartConfig( + props, + modifiedChartData, + longestLabels, + ); expect(output).toStrictEqual(expectedConfig); }); @@ -426,7 +505,11 @@ describe("EChartsConfigurationBuilder", () => { expectedConfig.series[1].type = "bar"; expectedConfig.series[1].label.position = "right"; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.series).toStrictEqual(expectedConfig.series); }); @@ -440,7 +523,11 @@ describe("EChartsConfigurationBuilder", () => { expectedConfig.series[0].type = "line"; expectedConfig.series[1].type = "line"; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.series).toStrictEqual(expectedConfig.series); }); @@ -454,7 +541,11 @@ describe("EChartsConfigurationBuilder", () => { expectedConfig.series[0].type = "bar"; expectedConfig.series[1].type = "bar"; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.series).toStrictEqual(expectedConfig.series); }); @@ -470,7 +561,11 @@ describe("EChartsConfigurationBuilder", () => { expectedConfig.series[0].areaStyle = {}; expectedConfig.series[1].areaStyle = {}; - const output = builder.prepareEChartConfig(props, chartData); + const output = builder.prepareEChartConfig( + props, + chartData, + longestLabels, + ); expect(output.series).toStrictEqual(expectedConfig.series); }); @@ -501,9 +596,13 @@ describe("EChartsConfigurationBuilder", () => { }, ]; - const output = builder.prepareEChartConfig(props, { - seriesID1: chartData1, - }); + const output = builder.prepareEChartConfig( + props, + { + seriesID1: chartData1, + }, + longestLabels, + ); expect(output.series).toStrictEqual(expectedConfig.series); }); @@ -512,14 +611,22 @@ describe("EChartsConfigurationBuilder", () => { const chartDataParams = JSON.parse(JSON.stringify(chartData)); chartDataParams.seriesID1.seriesName = ""; - let output = builder.prepareEChartConfig(props, chartDataParams); + let output = builder.prepareEChartConfig( + props, + chartDataParams, + longestLabels, + ); let firstSeriesName = (output.series as any[])[0].name; - expect(firstSeriesName).toEqual("Undefined"); + expect(firstSeriesName).toEqual("Series"); chartDataParams.seriesID1.seriesName = undefined; - output = builder.prepareEChartConfig(props, chartDataParams); + output = builder.prepareEChartConfig( + props, + chartDataParams, + longestLabels, + ); firstSeriesName = (output.series as any[])[0].name; - expect(firstSeriesName).toEqual("Undefined"); + expect(firstSeriesName).toEqual("Series"); }); it("6.9 PIE-CHART chooses a default series name for the legend if series name prop is empty", () => { @@ -529,18 +636,26 @@ describe("EChartsConfigurationBuilder", () => { const chartDataParams = JSON.parse(JSON.stringify(chartData1)); chartDataParams.seriesName = ""; - let output = builder.prepareEChartConfig(props, { - seriesID1: chartDataParams, - }); + let output = builder.prepareEChartConfig( + props, + { + seriesID1: chartDataParams, + }, + longestLabels, + ); let firstSeriesName = (output.series as any[])[0].name; - expect(firstSeriesName).toEqual("Undefined"); + expect(firstSeriesName).toEqual("Series"); chartDataParams.seriesName = undefined; - output = builder.prepareEChartConfig(props, { - seriesID1: chartDataParams, - }); + output = builder.prepareEChartConfig( + props, + { + seriesID1: chartDataParams, + }, + longestLabels, + ); firstSeriesName = (output.series as any[])[0].name; - expect(firstSeriesName).toEqual("Undefined"); + expect(firstSeriesName).toEqual("Series"); }); it("6.10 shows labels on series data if Show Labels if true otherwise false", () => { @@ -554,14 +669,14 @@ describe("EChartsConfigurationBuilder", () => { expectedConfig.series[0].label.show = true; expectedConfig.series[1].label.show = true; - let output = builder.prepareEChartConfig(props, chartData); + let output = builder.prepareEChartConfig(props, chartData, longestLabels); expect(output.series).toStrictEqual(expectedConfig.series); props.showDataPointLabel = false; expectedConfig.series[0].label.show = false; expectedConfig.series[1].label.show = false; - output = builder.prepareEChartConfig(props, chartData); + output = builder.prepareEChartConfig(props, chartData, longestLabels); expect(output.series).toStrictEqual(expectedConfig.series); }); @@ -570,7 +685,7 @@ describe("EChartsConfigurationBuilder", () => { props.chartType = "PIE_CHART"; props.showDataPointLabel = true; - let output = builder.prepareEChartConfig(props, chartData); + let output = builder.prepareEChartConfig(props, chartData, longestLabels); let seriesConfig = output.series as Record< string, Record @@ -580,7 +695,7 @@ describe("EChartsConfigurationBuilder", () => { props.showDataPointLabel = false; - output = builder.prepareEChartConfig(props, chartData); + output = builder.prepareEChartConfig(props, chartData, longestLabels); seriesConfig = output.series as Record>[]; expect(seriesConfig[0].label.show).toEqual(false); diff --git a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts index 26600e47b1..cb5761dd5c 100644 --- a/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts +++ b/app/client/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts @@ -1,5 +1,5 @@ import type { ChartComponentProps } from "."; -import type { AllChartData } from "../constants"; +import type { AllChartData, LongestLabelParams } from "../constants"; import { LabelOrientation, type ChartData, @@ -12,6 +12,7 @@ import { EChartsLayoutBuilder } from "./LayoutBuilders/EChartsLayoutBuilder"; export class EChartsConfigurationBuilder { fontFamily: string | undefined; + fontSize = 14; #seriesConfigurationForPieChart( seriesID: string, @@ -180,6 +181,10 @@ export class EChartsConfigurationBuilder { } #configForLabelOrientation(props: ChartComponentProps) { + if (props.chartType == "BAR_CHART") { + return 0; + } + switch (props.labelOrientation) { case LabelOrientation.SLANT: return 45; @@ -237,7 +242,7 @@ export class EChartsConfigurationBuilder { nameLocation: "middle", nameGap: layoutConfig.yAxis.nameGap, nameTextStyle: { - fontSize: 14, + fontSize: this.fontSize, fontFamily: this.fontFamily, color: Colors.DOVE_GRAY2, }, @@ -253,8 +258,7 @@ export class EChartsConfigurationBuilder { fontFamily: this.fontFamily, color: Colors.DOVE_GRAY2, show: layoutConfig.yAxis.show, - width: (layoutConfig.yAxis.axisLabel as Record).width, - overflow: "break", + ...(layoutConfig.yAxis.axisLabel as Record), }; return config; }; @@ -279,8 +283,7 @@ export class EChartsConfigurationBuilder { fontFamily: this.fontFamily, color: Colors.DOVE_GRAY2, rotate: this.#configForLabelOrientation(props), - width: (layoutConfig.xAxis.axisLabel as Record).width, - overflow: "break", + ...(layoutConfig.xAxis.axisLabel as Record), }; if (props.chartType == "BAR_CHART" && props.setAdaptiveYMin) { @@ -292,7 +295,7 @@ export class EChartsConfigurationBuilder { config.nameLocation = "middle"; config.nameGap = layoutConfig.xAxis.nameGap; config.nameTextStyle = { - fontSize: 14, + fontSize: this.fontSize, fontFamily: this.fontFamily, color: Colors.DOVE_GRAY2, }; @@ -313,32 +316,46 @@ export class EChartsConfigurationBuilder { show: layoutConfig.scrollBar.show, type: "slider", filterMode: "filter", - start: "20", + start: "0", + end: "50", bottom: layoutConfig.scrollBar.bottom, height: layoutConfig.scrollBar.height, }, + { + show: layoutConfig.scrollBar.show, + type: "inside", + filterMode: "filter", + start: "0", + end: "50", + }, ]; } } return []; }; - prepareEChartConfig(props: ChartComponentProps, allSeriesData: AllChartData) { + prepareEChartConfig( + props: ChartComponentProps, + allSeriesData: AllChartData, + longestLabels: LongestLabelParams, + ) { + this.fontFamily = this.#evaluateFontFamily(props.fontFamily); const layoutBuilder = new EChartsLayoutBuilder({ allowScroll: props.allowScroll, - height: props.dimensions.componentHeight, - width: props.dimensions.componentWidth, + widgetHeight: props.dimensions.componentHeight, + widgetWidth: props.dimensions.componentWidth, labelOrientation: props.labelOrientation ?? LabelOrientation.AUTO, chartType: props.chartType, chartTitle: props.chartName, + seriesConfigs: props.chartData, + font: `${this.fontSize}px ${this.fontFamily}`, + longestLabels: longestLabels, }); const layoutConfig: Record< string, Record > = layoutBuilder.layoutConfig; - this.fontFamily = this.#evaluateFontFamily(props.fontFamily); - const chartConfig: Record = this.#defaultEChartConfig(layoutConfig); chartConfig.title = this.#titleConfigForChart( diff --git a/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.test.ts b/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.test.ts index 1f4797aa9d..9e9ef4afe1 100644 --- a/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.test.ts +++ b/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.test.ts @@ -1,10 +1,8 @@ import { EChartsDatasetBuilder } from "./EChartsDatasetBuilder"; -import type { ChartData } from "../constants"; -import { LabelOrientation } from "../constants"; -import type { ChartComponentProps } from "."; +import type { ChartData, ChartType } from "../constants"; describe("EChartsConfigurationBuilder", () => { - describe("get chart data", () => { + describe("filteredChartData", () => { const chartData1: ChartData = { seriesName: "series1", data: [{ x: "x1", y: "y1" }], @@ -17,57 +15,21 @@ describe("EChartsConfigurationBuilder", () => { }; const chartData = { seriesID1: chartData1, seriesID2: chartData2 }; - const defaultProps: ChartComponentProps = { - allowScroll: true, - showDataPointLabel: true, - chartData: chartData, - chartName: "chart name", - chartType: "AREA_CHART", - customEChartConfig: {}, - customFusionChartConfig: { type: "type", dataSource: undefined }, - hasOnDataPointClick: true, - isVisible: true, - isLoading: false, - setAdaptiveYMin: false, - labelOrientation: LabelOrientation.AUTO, - onDataPointClick: (point) => { - return point; - }, - widgetId: "widgetID", - xAxisName: "xaxisname", - yAxisName: "yaxisname", - borderRadius: "1", - boxShadow: "1", - primaryColor: "primarycolor", - fontFamily: "fontfamily", - dimensions: { componentWidth: 11, componentHeight: 11 }, - parentColumnSpace: 1, - parentRowSpace: 1, - topRow: 0, - bottomRow: 0, - leftColumn: 0, - rightColumn: 0, - }; it("1. returns all series data if chart type is not pie", () => { - const output = EChartsDatasetBuilder.chartData( - defaultProps.chartType, - defaultProps.chartData, - ); - expect(output).toEqual(chartData); + const chartType: ChartType = "AREA_CHART"; + const builder = new EChartsDatasetBuilder(chartType, chartData); + expect(builder.filteredChartData).toEqual(chartData); }); it("2. returns only the first series data if chart type is pie", () => { - const props = JSON.parse(JSON.stringify(defaultProps)); - props.chartType = "PIE_CHART"; - const output = EChartsDatasetBuilder.chartData( - props.chartType, - props.chartData, - ); + const chartType: ChartType = "PIE_CHART"; + + const builder = new EChartsDatasetBuilder(chartType, chartData); const expectedOutput = { seriesID1: chartData1, }; - expect(output).toEqual(expectedOutput); + expect(builder.filteredChartData).toEqual(expectedOutput); }); }); @@ -95,7 +57,8 @@ describe("EChartsConfigurationBuilder", () => { seriesID2: chartData2, seriesID3: chartData3, }; - const chartDataSource = EChartsDatasetBuilder.datasetFromData(chartData); + const chartType: ChartType = "LINE_CHART"; + const builder = new EChartsDatasetBuilder(chartType, chartData); const expectedChartDataSource = { source: [ @@ -104,7 +67,45 @@ describe("EChartsConfigurationBuilder", () => { ["Product1", "", "y2", ""], ], }; - expect(chartDataSource).toEqual(expectedChartDataSource); + expect(builder.datasetFromData()).toEqual(expectedChartDataSource); + }); + }); + + describe("longestDataLabels", () => { + it("returns the x and y values with longest number of characters in chart data", () => { + const longestXLabel = "longestXLabel"; + const longestYValue = "1234.56"; + + const chartData1: ChartData = { + seriesName: "series1", + data: [ + { x: "smallLabel", y: "123" }, + { x: longestXLabel, y: "1234" }, + ], + color: "series1color", + }; + + const chartData2: ChartData = { + seriesName: "series2", + data: [ + { x: "largeLabel", y: longestYValue }, + { x: "largerLabel", y: "12" }, + ], + color: "series2color", + }; + + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsDatasetBuilder(chartType, { + series1ID: chartData1, + series2ID: chartData2, + }); + builder.datasetFromData(); + + expect(builder.longestDataLabels()).toEqual({ + x: longestXLabel, + y: longestYValue, + }); }); }); }); diff --git a/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.ts b/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.ts index b64aaad9a2..6b614eb523 100644 --- a/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.ts +++ b/app/client/src/widgets/ChartWidget/component/EChartsDatasetBuilder.ts @@ -1,37 +1,74 @@ -import type { AllChartData, ChartType } from "../constants"; +import type { AllChartData, ChartType, LongestLabelParams } from "../constants"; import { XAxisCategory } from "../constants"; export class EChartsDatasetBuilder { - static chartData( - chartType: ChartType, - chartData: AllChartData, - ): AllChartData { - if (chartType == "PIE_CHART") { + chartDataProp: AllChartData; + chartType: ChartType; + filteredChartData: AllChartData; + + maxXLabelString = ""; + maxXLabelLength = 0; + + maxYLabelString = ""; + maxYLabelLength = 0; + + constructor(chartType: ChartType, chartDataProp: AllChartData) { + this.chartDataProp = chartDataProp; + this.chartType = chartType; + this.filteredChartData = this.filterChartData(); + } + + longestDataLabels(): LongestLabelParams { + return { + x: this.maxXLabelString, + y: this.maxYLabelString, + }; + } + + filterChartData(): AllChartData { + if (this.chartType == "PIE_CHART") { // return only first series data - const firstSeriesKey = Object.keys(chartData)[0]; - return { [firstSeriesKey]: chartData[firstSeriesKey] }; + const firstSeriesKey = Object.keys(this.chartDataProp)[0]; + return { [firstSeriesKey]: this.chartDataProp[firstSeriesKey] }; } else { - return chartData; + return this.chartDataProp; } } - static datasetFromData(allSeriesData: AllChartData) { + checkForLongestLabel(x: number | string, y: number | string) { + const xString = x.toString(); + const yString = y.toString(); + + if (xString.length > this.maxXLabelLength) { + this.maxXLabelLength = xString.length; + this.maxXLabelString = xString; + } + + if (yString.length > this.maxYLabelLength) { + this.maxYLabelLength = yString.length; + this.maxYLabelString = yString; + } + } + + datasetFromData() { // ["Category", "seriesID1", "seriesID2"] const dimensions: string[] = [XAxisCategory]; // { Product1 : { "series1" : yValue1 }, "series2" : yValue2 } const categories: Record> = {}; - Object.keys(allSeriesData).forEach((seriesID) => { + Object.keys(this.filteredChartData).forEach((seriesID) => { dimensions.push(seriesID); - const seriesData = allSeriesData[seriesID]; + const seriesData = this.filteredChartData[seriesID]; const datapoints = seriesData.data; for (const datapoint of datapoints) { const categoryName = datapoint.x; const value = datapoint.y; + this.checkForLongestLabel(categoryName, value); + if (!categories[categoryName]) { categories[categoryName] = {}; } diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.test.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.test.ts index ae1c20fc69..d526ac529b 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.test.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.test.ts @@ -3,119 +3,119 @@ import type { EChartElementLayoutParams } from "./EChartsElementVisibilityCalcul describe("EChartsElementVisibilityCalculator", () => { describe("visibility calculator", () => { - it("returns no elements if current height is equal to minimumHeight", () => { + it("returns no elements if element minHeight is equal to gridMinimumHeight", () => { const elements: EChartElementLayoutParams[] = [ { elementName: "element1", - height: 10, + minHeight: 10, + maxHeight: 10, position: "top", }, { elementName: "element2", - height: 10, + minHeight: 10, + maxHeight: 10, position: "bottom", }, ]; - const minimumHeight = 80; + const gridMinimumHeight = 80; const heightAvailable = 80; const builder = new EChartElementVisibilityCalculator({ height: heightAvailable, padding: 0, - minimumHeight: minimumHeight, + gridMinimumHeight: gridMinimumHeight, layoutConfigs: elements, }); - expect(builder.visibleElements.top.length).toEqual(0); - expect(builder.visibleElements.bottom.length).toEqual(0); + expect(builder.visibleElements.length).toEqual(0); }); it("fits as many elements possible within the height available", () => { const elements: EChartElementLayoutParams[] = [ { elementName: "element1", - height: 10, + minHeight: 10, + maxHeight: 10, position: "top", }, { elementName: "element2", - height: 30, + minHeight: 30, + maxHeight: 30, position: "bottom", }, ]; - const minimumHeight = 80; + const gridMinimumHeight = 80; const heightAvailable = 120; const builder = new EChartElementVisibilityCalculator({ height: heightAvailable, padding: 0, - minimumHeight: minimumHeight, + gridMinimumHeight: gridMinimumHeight, layoutConfigs: elements, }); - expect(builder.visibleElements.top.length).toEqual(1); - expect(builder.visibleElements.bottom.length).toEqual(1); - - expect(builder.visibleElements.top[0]).toEqual(elements[0]); - expect(builder.visibleElements.bottom[0]).toEqual(elements[1]); + expect(builder.visibleElements.length).toEqual(2); }); it("excludes elements that can't fit within the height available", () => { const elements: EChartElementLayoutParams[] = [ { elementName: "element1", - height: 10, + minHeight: 10, + maxHeight: 10, position: "top", }, { elementName: "element2", - height: 30, + minHeight: 30, + maxHeight: 30, position: "bottom", }, ]; - const minimumHeight = 80; + const gridMinimumHeight = 80; const heightAvailable = 100; const builder = new EChartElementVisibilityCalculator({ height: heightAvailable, padding: 0, - minimumHeight: minimumHeight, + gridMinimumHeight: gridMinimumHeight, layoutConfigs: elements, }); - expect(builder.visibleElements.top.length).toEqual(1); - expect(builder.visibleElements.bottom.length).toEqual(0); - - expect(builder.visibleElements.top[0]).toEqual(elements[0]); + expect(builder.visibleElements.length).toEqual(1); + expect(builder.visibleElements[0].elementName).toEqual("element1"); }); it("excludes lower priority fitting elements if higher priority elements can't fit", () => { const elements: EChartElementLayoutParams[] = [ { elementName: "element1", - height: 30, + minHeight: 30, + maxHeight: 30, position: "top", }, { elementName: "element2", - height: 10, + minHeight: 10, + maxHeight: 10, position: "bottom", }, ]; - const minimumHeight = 80; + const gridMinimumHeight = 80; const heightAvailable = 90; const builder = new EChartElementVisibilityCalculator({ height: heightAvailable, padding: 0, - minimumHeight: minimumHeight, + gridMinimumHeight: gridMinimumHeight, layoutConfigs: elements, }); - expect(builder.visibleElements.top.length).toEqual(0); - expect(builder.visibleElements.bottom.length).toEqual(0); + expect(builder.visibleElements.length).toEqual(0); }); }); @@ -127,30 +127,28 @@ describe("EChartsElementVisibilityCalculator", () => { const elements: EChartElementLayoutParams[] = [ { elementName: "element1", - height: topElementHeight, + minHeight: topElementHeight, + maxHeight: topElementHeight, position: "top", }, { elementName: "element2", - height: bottomElementHeight, + minHeight: bottomElementHeight, + maxHeight: bottomElementHeight, position: "bottom", }, ]; - const minimumHeight = 80; + const gridMinimumHeight = 80; const heightAvailable = 120; const builder = new EChartElementVisibilityCalculator({ height: heightAvailable, padding: 0, - minimumHeight: minimumHeight, + gridMinimumHeight: gridMinimumHeight, layoutConfigs: elements, }); - expect(builder.visibleElements.top.length).toEqual(1); - expect(builder.visibleElements.bottom.length).toEqual(1); - - expect(builder.visibleElements.top[0]).toEqual(elements[0]); - expect(builder.visibleElements.bottom[0]).toEqual(elements[1]); + expect(builder.visibleElements.length).toEqual(2); const offsets = builder.calculateOffsets(); expect(offsets.top).toEqual(topElementHeight); @@ -162,23 +160,23 @@ describe("EChartsElementVisibilityCalculator", () => { const elements: EChartElementLayoutParams[] = [ { elementName: "element1", - height: elementHeight, + minHeight: elementHeight, + maxHeight: elementHeight, position: "bottom", }, ]; - const minimumHeight = 80; + const gridMinimumHeight = 80; const heightAvailable = 90; const customPadding = 5; const builder = new EChartElementVisibilityCalculator({ height: heightAvailable, padding: customPadding, - minimumHeight: minimumHeight, + gridMinimumHeight: gridMinimumHeight, layoutConfigs: elements, }); - expect(builder.visibleElements.top.length).toEqual(0); - expect(builder.visibleElements.bottom.length).toEqual(1); + expect(builder.visibleElements.length).toEqual(1); const offsets = builder.calculateOffsets(); @@ -191,28 +189,90 @@ describe("EChartsElementVisibilityCalculator", () => { const elements: EChartElementLayoutParams[] = [ { elementName: "element1", - height: elementHeight, + minHeight: elementHeight, + maxHeight: elementHeight, position: "top", }, ]; - const minimumHeight = 80; + const gridMinimumHeight = 80; const heightAvailable = 90; const customPadding = 5; const builder = new EChartElementVisibilityCalculator({ height: heightAvailable, padding: customPadding, - minimumHeight: minimumHeight, + gridMinimumHeight: gridMinimumHeight, layoutConfigs: elements, }); - expect(builder.visibleElements.top.length).toEqual(1); - expect(builder.visibleElements.bottom.length).toEqual(0); + expect(builder.visibleElements.length).toEqual(1); const offsets = builder.calculateOffsets(); expect(offsets.top).toEqual(elementHeight); expect(offsets.bottom).toEqual(customPadding); }); + + it("allocates max height to element if there is remaining space inside the widget", () => { + const minElementHeight = 10; + const maxElementHeight = 40; + const elements: EChartElementLayoutParams[] = [ + { + elementName: "element1", + minHeight: minElementHeight, + maxHeight: maxElementHeight, + position: "bottom", + }, + ]; + const gridMinimumHeight = 80; + const heightAvailable = 120; + + const customPadding = 5; + const builder = new EChartElementVisibilityCalculator({ + height: heightAvailable, + padding: customPadding, + gridMinimumHeight: gridMinimumHeight, + layoutConfigs: elements, + }); + + expect(builder.visibleElements.length).toEqual(1); + expect(builder.visibleElements[0].height).toEqual(maxElementHeight); + + const offsets = builder.calculateOffsets(); + + expect(offsets.top).toEqual(customPadding); + expect(offsets.bottom).toEqual(maxElementHeight); + }); + + it("allocates max height possible to element if there is remaining space inside the widget", () => { + const minElementHeight = 10; + const maxElementHeight = 40; + const elements: EChartElementLayoutParams[] = [ + { + elementName: "element1", + minHeight: minElementHeight, + maxHeight: maxElementHeight, + position: "bottom", + }, + ]; + const gridMinimumHeight = 80; + const heightAvailable = 110; + + const customPadding = 5; + const builder = new EChartElementVisibilityCalculator({ + height: heightAvailable, + padding: customPadding, + gridMinimumHeight: gridMinimumHeight, + layoutConfigs: elements, + }); + + expect(builder.visibleElements.length).toEqual(1); + expect(builder.visibleElements[0].height).toEqual(30); + + const offsets = builder.calculateOffsets(); + + expect(offsets.top).toEqual(customPadding); + expect(offsets.bottom).toEqual(30); + }); }); }); diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.ts index dedee3a8f3..a33a19b04f 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsElementVisibilityCalculator.ts @@ -1,46 +1,52 @@ -export type EChartElementLayoutParams = { +export type EChartElementLayoutParams = Omit< + EChartVisibleElementConfig, + "height" +>; + +export type EChartVisibleElementConfig = { elementName: string; height: number; + minHeight: number; + maxHeight: number; position: "top" | "bottom"; }; export type EChartElementVisibilityProps = { height: number; padding: number; - minimumHeight: number; + gridMinimumHeight: number; layoutConfigs: EChartElementLayoutParams[]; }; + export class EChartElementVisibilityCalculator { props: EChartElementVisibilityProps; - visibleElements: { - top: EChartElementLayoutParams[]; - bottom: EChartElementLayoutParams[]; - }; + visibleElements: EChartVisibleElementConfig[]; constructor(props: EChartElementVisibilityProps) { this.props = props; - this.visibleElements = this.elementsToInclude(); + this.visibleElements = this.selectElementsForVisibility(); } needsCustomTopPadding() { - return this.visibleElements.top.length == 0; + return this.visibleElements.every((config) => config.position != "top"); } needsCustomBottomPadding() { - return this.visibleElements.bottom.length == 0; + return this.visibleElements.every((config) => config.position != "bottom"); } calculateOffsets() { let top = this.needsCustomTopPadding() ? this.props.padding : 0; let bottom = this.needsCustomBottomPadding() ? this.props.padding : 0; - for (const config of this.visibleElements.top) { - top += config.height; + for (const config of this.visibleElements) { + if (config.position == "top") { + top += config.height; + } else if (config.position == "bottom") { + bottom += config.height; + } } - for (const config of this.visibleElements.bottom) { - bottom += config.height; - } return { top: top, bottom: bottom, @@ -48,33 +54,59 @@ export class EChartElementVisibilityCalculator { } availableHeight() { - return this.props.height - this.props.minimumHeight; + return this.props.height - this.props.gridMinimumHeight; } - elementsToInclude() { + selectElementsForVisibility(): EChartVisibleElementConfig[] { let remainingHeight = this.availableHeight(); let index = 0; const count = this.props.layoutConfigs.length; - const output = { - top: [] as EChartElementLayoutParams[], - bottom: [] as EChartElementLayoutParams[], - }; + const selectedElements: EChartVisibleElementConfig[] = []; while (index < count && remainingHeight > 0) { - const config = this.props.layoutConfigs[index]; - if (config.height <= remainingHeight) { - remainingHeight -= config.height; + const elementConfig = this.props.layoutConfigs[index]; + if (elementConfig.minHeight <= remainingHeight) { + remainingHeight -= elementConfig.minHeight; + + selectedElements.push({ + height: elementConfig.minHeight, + minHeight: elementConfig.minHeight, + maxHeight: elementConfig.maxHeight, + position: elementConfig.position, + elementName: elementConfig.elementName, + }); + index = index + 1; - if (config.position == "top") { - output.top.push(config); - } else { - output.bottom.push(config); - } } else { break; } } - return output; + + for (const elementConfig of selectedElements) { + if (remainingHeight > 0) { + const height = this.assignExtraHeightToElementConfig( + remainingHeight, + elementConfig, + ); + remainingHeight -= height; + elementConfig.height += height; + } else { + break; + } + } + return selectedElements; + } + + assignExtraHeightToElementConfig( + remainingHeight: number, + elementConfig: EChartElementLayoutParams, + ) { + const difference = elementConfig.maxHeight - elementConfig.minHeight; + if (remainingHeight > difference) { + return difference; + } else { + return remainingHeight; + } } } diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.test.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.test.ts index 4a78f876a4..a691ccbe07 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.test.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.test.ts @@ -1,20 +1,26 @@ import { LabelOrientation } from "widgets/ChartWidget/constants"; import { EChartsLayoutBuilder } from "./EChartsLayoutBuilder"; +const font = "14px Nunito Sans"; + describe("priority order of layout", () => { it("returns the correct priority order", () => { const builder = new EChartsLayoutBuilder({ allowScroll: false, - height: 0, - width: 0, + widgetHeight: 0, + widgetWidth: 0, labelOrientation: LabelOrientation.AUTO, chartType: "LINE_CHART", chartTitle: "chartTitle", + seriesConfigs: {}, + font: font, + longestLabels: { x: "123", y: "123" }, }); + expect(builder.priorityOrderOfInclusion).toEqual([ - "xAxis", "legend", "title", + "xAxis", "scrollBar", ]); }); @@ -25,28 +31,44 @@ describe("layout configs to include", () => { const allowScroll = true; const builder = new EChartsLayoutBuilder({ allowScroll: allowScroll, - height: 0, - width: 0, + widgetHeight: 0, + widgetWidth: 0, labelOrientation: LabelOrientation.AUTO, + seriesConfigs: { + series1ID: { + data: [], + seriesName: "series name", + }, + }, chartType: "LINE_CHART", chartTitle: "chartTitle", + font: font, + longestLabels: { x: "123", y: "123" }, }); const output = builder.configsToInclude(); - expect(output).toEqual(["xAxis", "legend", "title", "scrollBar"]); + expect(output).toEqual(["legend", "title", "xAxis", "scrollBar"]); }); it("excludes scroll bar if allow scroll is false", () => { const allowScroll = false; const builder = new EChartsLayoutBuilder({ allowScroll: allowScroll, - height: 0, - width: 0, + widgetHeight: 0, + widgetWidth: 0, labelOrientation: LabelOrientation.AUTO, + seriesConfigs: { + series1ID: { + data: [], + seriesName: "series name", + }, + }, chartType: "LINE_CHART", chartTitle: "chartTitle", + font: font, + longestLabels: { x: "123", y: "123" }, }); const output = builder.configsToInclude(); - expect(output).toEqual(["xAxis", "legend", "title"]); + expect(output).toEqual(["legend", "title", "xAxis"]); }); it("doesn't include title if chart title length is 0", () => { @@ -55,14 +77,106 @@ describe("layout configs to include", () => { const allowScroll = false; const builder = new EChartsLayoutBuilder({ allowScroll: allowScroll, - height: 0, - width: 0, + widgetHeight: 0, + widgetWidth: 0, labelOrientation: LabelOrientation.AUTO, chartType: "LINE_CHART", + font: font, + seriesConfigs: { + series1ID: { + data: [], + seriesName: "series name", + }, + }, + longestLabels: { x: "123", y: "123" }, chartTitle: emptyChartTitle, }); const output = builder.configsToInclude(); - expect(output).toEqual(["xAxis", "legend"]); + expect(output).toEqual(["legend", "xAxis"]); + }); + + it("includes legend if number of series data is more than 1", () => { + const seriesConfigs = { + series1ID: { + data: [], + }, + series2ID: { + data: [], + }, + }; + const builder = new EChartsLayoutBuilder({ + allowScroll: true, + widgetHeight: 0, + widgetWidth: 0, + labelOrientation: LabelOrientation.AUTO, + seriesConfigs: seriesConfigs, + chartType: "LINE_CHART", + chartTitle: "chartTitle", + font: font, + longestLabels: { x: "123", y: "123" }, + }); + const output = builder.configsToInclude(); + expect(output).toEqual(["legend", "title", "xAxis", "scrollBar"]); + }); + + it("doesn't include legend if number of series data is 0", () => { + const seriesConfigs = {}; + const builder = new EChartsLayoutBuilder({ + allowScroll: true, + widgetHeight: 0, + widgetWidth: 0, + labelOrientation: LabelOrientation.AUTO, + seriesConfigs: seriesConfigs, + chartType: "LINE_CHART", + chartTitle: "chartTitle", + font: font, + longestLabels: { x: "123", y: "123" }, + }); + const output = builder.configsToInclude(); + expect(output).toEqual(["title", "xAxis", "scrollBar"]); + }); + + it("if number of series configs is 1, doesn't include legend if series name is missing", () => { + const seriesConfigs = { + series1ID: { + data: [], + }, + }; + const builder = new EChartsLayoutBuilder({ + allowScroll: true, + widgetHeight: 0, + widgetWidth: 0, + labelOrientation: LabelOrientation.AUTO, + seriesConfigs: seriesConfigs, + chartType: "LINE_CHART", + chartTitle: "chartTitle", + font: font, + longestLabels: { x: "123", y: "123" }, + }); + const output = builder.configsToInclude(); + expect(output).toEqual(["title", "xAxis", "scrollBar"]); + }); + + it("if number of series configs is 1, includes legend if series name is present", () => { + const seriesConfigs = { + series1ID: { + seriesName: "seriesNamePresent", + data: [], + }, + }; + const builder = new EChartsLayoutBuilder({ + allowScroll: true, + widgetHeight: 0, + widgetWidth: 0, + labelOrientation: LabelOrientation.AUTO, + seriesConfigs: seriesConfigs, + chartType: "LINE_CHART", + chartTitle: "chartTitle", + font: font, + longestLabels: { x: "123", y: "123" }, + }); + const output = builder.configsToInclude(); + expect(output).toEqual(["legend", "title", "xAxis", "scrollBar"]); }); }); @@ -71,11 +185,14 @@ describe("legend top offset", () => { const chartTitle = "chartTitle"; const builder = new EChartsLayoutBuilder({ allowScroll: true, - height: 400, - width: 300, + widgetHeight: 400, + widgetWidth: 300, labelOrientation: LabelOrientation.AUTO, chartType: "LINE_CHART", + font: font, + seriesConfigs: {}, chartTitle: chartTitle, + longestLabels: { x: "123", y: "123" }, }); const output = builder.layoutConfigForElements(); expect(output.title.show).toEqual(true); @@ -86,10 +203,13 @@ describe("legend top offset", () => { const emptyChartTitle = ""; const builder = new EChartsLayoutBuilder({ allowScroll: true, - height: 400, - width: 300, + widgetHeight: 400, + widgetWidth: 300, labelOrientation: LabelOrientation.AUTO, chartType: "LINE_CHART", + font: font, + seriesConfigs: {}, + longestLabels: { x: "123", y: "123" }, chartTitle: emptyChartTitle, }); const output = builder.layoutConfigForElements(); @@ -102,19 +222,28 @@ describe("layout configs", () => { it("generates correct chart layout config", () => { const builder = new EChartsLayoutBuilder({ allowScroll: true, - height: 400, - width: 300, - labelOrientation: LabelOrientation.AUTO, + widgetHeight: 400, + widgetWidth: 300, + labelOrientation: LabelOrientation.ROTATE, chartType: "LINE_CHART", + font: font, + longestLabels: { x: "123", y: "123" }, + seriesConfigs: { + seriesID1: { + data: [], + seriesName: "seriesName", + }, + }, chartTitle: "chartTitle", }); const output = builder.layoutConfigForElements(); expect(output).toEqual({ xAxis: { show: true, - nameGap: 40, + nameGap: 13, axisLabel: { - width: 60, + width: 3, + overflow: "truncate", }, }, legend: { @@ -131,14 +260,15 @@ describe("layout configs", () => { }, grid: { top: 100, - bottom: 140, - left: 100, + bottom: 113, + left: 53, }, yAxis: { show: true, - nameGap: 70, + nameGap: 23, axisLabel: { - width: 60, + width: 13, + overflow: "truncate", }, }, }); diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.ts index 38b5123081..f9e7fb0e8f 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsLayoutBuilder.ts @@ -1,22 +1,28 @@ import type { + AllChartData, ChartType, LabelOrientation, + LongestLabelParams, } from "widgets/ChartWidget/constants"; +import type { EChartElementLayoutParams } from "./EChartsElementVisibilityCalculator"; import { EChartElementVisibilityCalculator } from "./EChartsElementVisibilityCalculator"; import { EChartsXAxisLayoutBuilder } from "./EChartsXAxisLayoutBuilder"; import { EChartsYAxisLayoutBuilder } from "./EChartsYAxisLayoutBuilder"; type LayoutProps = { allowScroll: boolean; - height: number; - width: number; + widgetHeight: number; + widgetWidth: number; labelOrientation: LabelOrientation; chartType: ChartType; chartTitle: string; + seriesConfigs: AllChartData; + font: string; + longestLabels: LongestLabelParams; }; export class EChartsLayoutBuilder { - minimumHeight = 80; + gridMinimumHeight = 80; gridPadding = 30; heightForAllowScollBar = 30; @@ -25,7 +31,7 @@ export class EChartsLayoutBuilder { heightForLegend = 50; heightForTitle = 50; - priorityOrderOfInclusion = ["xAxis", "legend", "title", "scrollBar"]; + priorityOrderOfInclusion = ["legend", "title", "xAxis", "scrollBar"]; positionLayoutForElement: Record = { xAxis: "bottom", @@ -44,16 +50,24 @@ export class EChartsLayoutBuilder { constructor(props: LayoutProps) { this.props = props; - this.xAxisLayoutBuilder = new EChartsXAxisLayoutBuilder( - this.props.labelOrientation, - this.props.chartType, - ); - this.yAxisLayoutBuilder = new EChartsYAxisLayoutBuilder(this.props.width); + this.xAxisLayoutBuilder = new EChartsXAxisLayoutBuilder({ + labelOrientation: this.props.labelOrientation, + chartType: this.props.chartType, + font: this.props.font, + longestLabel: this.props.longestLabels, + }); + + this.yAxisLayoutBuilder = new EChartsYAxisLayoutBuilder({ + widgetWidth: this.props.widgetWidth, + chartType: this.props.chartType, + font: this.props.font, + longestLabel: this.props.longestLabels, + }); this.elementVisibilityLayoutBuilder = new EChartElementVisibilityCalculator( { - height: props.height, - minimumHeight: this.minimumHeight, + height: props.widgetHeight, + gridMinimumHeight: this.gridMinimumHeight, layoutConfigs: this.configParamsForVisibilityCalculation(), padding: this.gridPadding, }, @@ -62,18 +76,32 @@ export class EChartsLayoutBuilder { this.layoutConfig = this.layoutConfigForElements(); } - heightForElement = (elementName: string): number => { + heightConfigForElement = ( + elementName: string, + ): { minHeight: number; maxHeight: number } => { switch (elementName) { case "xAxis": - return this.xAxisLayoutBuilder.heightForXAxis(); + return this.xAxisLayoutBuilder.heightConfigForXAxis(); case "legend": - return this.heightForLegend; + return { + minHeight: this.heightForLegend, + maxHeight: this.heightForLegend, + }; case "title": - return this.heightForTitle; + return { + minHeight: this.heightForTitle, + maxHeight: this.heightForTitle, + }; case "scrollBar": - return this.layoutHeightForScrollBar(); + return { + minHeight: this.layoutHeightForScrollBar(), + maxHeight: this.layoutHeightForScrollBar(), + }; default: - return 0; + return { + minHeight: 0, + maxHeight: 0, + }; } }; @@ -90,14 +118,8 @@ export class EChartsLayoutBuilder { }; }); config.grid = this.defaultConfigForGrid(); - config.yAxis = this.yAxisLayoutBuilder.config(); - config.xAxis = { - ...config.xAxis, - ...this.xAxisLayoutBuilder.configForXAxis(), - }; - config.scrollBar = { ...config.scrollBar, bottom: this.scrollBarBottomOffset, @@ -107,9 +129,21 @@ export class EChartsLayoutBuilder { return config; }; + layoutConfigForXAxis = () => { + const visibilityConfig = + this.elementVisibilityLayoutBuilder.visibleElements; + const xAxisConfig = visibilityConfig.find((config) => { + return config.elementName == "xAxis"; + }); + + const xAxisConfigHeight = xAxisConfig?.height ?? 0; + + return this.xAxisLayoutBuilder.configForXAxis(xAxisConfigHeight); + }; + layoutConfigForElements = () => { - const { bottom, top } = this.elementVisibilityLayoutBuilder.visibleElements; - const visibilityConfig = [...top, ...bottom]; + const visibilityConfig = + this.elementVisibilityLayoutBuilder.visibleElements; const output = this.defaultConfigForElements(); @@ -122,16 +156,20 @@ export class EChartsLayoutBuilder { output.grid.top = gridOffsets.top; output.grid.bottom = gridOffsets.bottom; output.legend.top = output.title.show ? 50 : 0; + output.xAxis = { + ...output.xAxis, + ...this.layoutConfigForXAxis(), + }; return output; }; - configParamsForVisibilityCalculation = () => { + configParamsForVisibilityCalculation = (): EChartElementLayoutParams[] => { return this.configsToInclude().map((element) => { return { elementName: element, position: this.positionLayoutForElement[element], - height: this.heightForElement(element), + ...this.heightConfigForElement(element), }; }); }; @@ -149,6 +187,9 @@ export class EChartsLayoutBuilder { if (configName == "scrollBar") { return this.props.allowScroll; } + if (configName == "legend") { + return this.needsLegend(this.props.seriesConfigs); + } if (configName == "xAxis") { return this.props.chartType != "PIE_CHART"; } @@ -158,4 +199,17 @@ export class EChartsLayoutBuilder { return true; }); } + + needsLegend(seriesConfigs: AllChartData) { + const seriesKeys = Object.keys(seriesConfigs); + const numSeries = seriesKeys.length; + if (numSeries == 0) { + return false; + } else if (numSeries == 1) { + const seriesTitle = seriesConfigs[seriesKeys[0]].seriesName ?? ""; + return seriesTitle.length > 0; + } else { + return true; + } + } } diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.test.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.test.ts index 4d4471f780..7d08701cfa 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.test.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.test.ts @@ -2,93 +2,370 @@ import type { ChartType } from "../../constants"; import { LabelOrientation } from "../../constants"; import { EChartsXAxisLayoutBuilder } from "./EChartsXAxisLayoutBuilder"; +const font = "14px Nunito Sans"; + describe("EChartsXAxisLayoutBuilder", () => { describe("width for xaxis labels", () => { - it("width is 50 when label orientation is 50", () => { - const labelOrientation = LabelOrientation.SLANT; - const builder = new EChartsXAxisLayoutBuilder( - labelOrientation, - "LINE_CHART", - ); - expect(builder.widthForXAxisLabels()).toEqual(50); + it("returns a default width when label orientation is auto", () => { + const labelOrientation = LabelOrientation.AUTO; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + + expect(builder.widthForXAxisLabels()).toEqual(0); }); - it("width is 60 when label orientation is not slant", () => { - const labelOrientation = LabelOrientation.AUTO; - const builder = new EChartsXAxisLayoutBuilder( - labelOrientation, - "LINE_CHART", - ); - expect(builder.widthForXAxisLabels()).toEqual(60); + describe("when label orientation is not auto", () => { + it("when chart type is not BAR_CHART, returns the width of the longest x label", () => { + const labelOrientation = LabelOrientation.ROTATE; + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + expect(builder.widthForXAxisLabels()).toEqual(6); + }); + + it("when chart type is BAR_CHART, returns the width of the longest y label", () => { + const labelOrientation = LabelOrientation.ROTATE; + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: "BAR_CHART", + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + expect(builder.widthForXAxisLabels()).toEqual(3); + }); }); }); - describe("height of x axis labels", () => { - it("when label orientation isn't auto, it is equal to sum of width of x axis labels and a fixed offset", () => { - const labelOrientation = LabelOrientation.SLANT; - const builder = new EChartsXAxisLayoutBuilder( - labelOrientation, - "LINE_CHART", - ); - expect(builder.gapBetweenLabelAndName).toEqual(10); - expect(builder.widthForXAxisLabels()).toEqual(50); - expect(builder.heightForXAxisLabels()).toEqual(60); - }); - - it("is equal to a default height when label orientation is auto", () => { + describe("maxHeightForLabels", () => { + it("returns a default height when label orientation is auto", () => { const labelOrientation = LabelOrientation.AUTO; - const builder = new EChartsXAxisLayoutBuilder( - labelOrientation, - "LINE_CHART", - ); - expect(builder.gapBetweenLabelAndName).toEqual(10); + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + expect(builder.defaultHeightForXAxisLabels).toEqual(30); - expect(builder.heightForXAxisLabels()).toEqual(40); + expect(builder.gapBetweenLabelAndName).toEqual(10); + + expect(builder.maxHeightForXAxisLabels()).toEqual(40); + }); + + it("returns sum of label width and an offset when label orientation is not auto", () => { + const labelOrientation = LabelOrientation.ROTATE; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + + expect(builder.gapBetweenLabelAndName).toEqual(10); + expect(builder.widthForXAxisLabels()).toEqual(6); + + expect(builder.maxHeightForXAxisLabels()).toEqual(16); }); }); - describe("height of x axis", () => { - it("is equal to sum of height of x axis labels and an offset", () => { - const labelOrientation = LabelOrientation.SLANT; - const builder = new EChartsXAxisLayoutBuilder( - labelOrientation, - "LINE_CHART", - ); - - expect(builder.defaultHeightForXAxisName).toEqual(40); - expect(builder.heightForXAxisLabels()).toEqual(60); - expect(builder.heightForXAxis()).toEqual(100); - }); - - it("is equal to 0 when chart type is pie chart", () => { - const labelOrientation = LabelOrientation.SLANT; + describe("minHeightForLabels", () => { + it("returns 0 height for pie chart", () => { + const labelOrientation = LabelOrientation.AUTO; const chartType: ChartType = "PIE_CHART"; - const builder = new EChartsXAxisLayoutBuilder( - labelOrientation, - chartType, - ); - expect(builder.heightForXAxis()).toEqual(0); + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + + expect(builder.minHeightForLabels()).toEqual(0); + }); + + describe("chart type is not PIE_CHART", () => { + it("returns a fixed height when label orientation is auto", () => { + const labelOrientation = LabelOrientation.AUTO; + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + expect(builder.defaultHeightForXAxisLabels).toEqual(30); + expect(builder.gapBetweenLabelAndName).toEqual(10); + expect(builder.defaultHeightForXAxisName).toEqual(40); + + expect(builder.minHeightForLabels()).toEqual(80); + }); + + it("returns a fixed height when label orientation is not auto", () => { + const labelOrientation = LabelOrientation.ROTATE; + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + expect(builder.defaultHeightForRotatedLabels).toEqual(50); + expect(builder.gapBetweenLabelAndName).toEqual(10); + expect(builder.defaultHeightForXAxisName).toEqual(40); + + expect(builder.minHeightForLabels()).toEqual(100); + }); }); }); - describe("config for x axis", () => { - it("returns x axis config for chart layout", () => { - const labelOrientation = LabelOrientation.SLANT; - const chartType: ChartType = "LINE_CHART"; - const builder = new EChartsXAxisLayoutBuilder( - labelOrientation, - chartType, - ); + describe("maxHeightForXAxis", () => { + it("when chart type is PIE_CHART, it returns zero height", () => { + const labelOrientation = LabelOrientation.ROTATE; - const expectedOutput = { - nameGap: 60, + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: "PIE_CHART", + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + + expect(builder.maxHeightForXAxis()).toEqual(0); + }); + + it("when chart type is not PIE_CHART, it returns sum of max height of labels and an offset", () => { + const labelOrientation = LabelOrientation.ROTATE; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "123456", y: "123" }, + }); + + expect(builder.maxHeightForXAxisLabels()).toEqual(16); + expect(builder.defaultHeightForXAxisName).toEqual(40); + expect(builder.maxHeightForXAxis()).toEqual(56); + }); + }); + + describe("heightConfigForLabels", () => { + it("returns the minHeight and maxHeight required for labels", () => { + const labelOrientation = LabelOrientation.ROTATE; + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { + x: "this is a long text with width more than min width for xaxis labels", + y: "123", + }, + }); + + expect(builder.minHeightForLabels()).toEqual(100); + expect(builder.maxHeightForXAxis()).toEqual(117); + + expect(builder.heightConfigForXAxis()).toEqual({ + minHeight: 100, + maxHeight: 117, + }); + }); + + it("returns minHeight the same as maxHeight if minHeight >= maxHeight", () => { + const labelOrientation = LabelOrientation.ROTATE; + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { + x: "123456", + y: "123", + }, + }); + + expect(builder.minHeightForLabels()).toEqual(100); + expect(builder.maxHeightForXAxis()).toEqual(56); + + expect(builder.heightConfigForXAxis()).toEqual({ + minHeight: 56, + maxHeight: 56, + }); + }); + }); + + describe("axisLabelConfig", () => { + it("when label orientation is AUTO, it returns empty axisLabelConfig", () => { + const labelOrientation = LabelOrientation.AUTO; + + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { + x: "123456", + y: "123", + }, + }); + expect(builder.axisLabelConfig(100)).toEqual({}); + }); + + it("when label orientation is not AUTO, it returns width for labels excluding padding and x axis name offsets", () => { + const labelOrientation = LabelOrientation.ROTATE; + + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { + x: "123456", + y: "123", + }, + }); + + const allocatedXAxisHeight = 100; + const labelWidth = + allocatedXAxisHeight - + builder.defaultHeightForXAxisName - + builder.gapBetweenLabelAndName; + + const expectedConfig = { + width: labelWidth, + overflow: "truncate", + }; + + expect(builder.axisLabelConfig(allocatedXAxisHeight)).toEqual( + expectedConfig, + ); + }); + }); + + describe("xAxis config", () => { + it("returns xAxis config with gap between labels and xaxis name equal to allocated height minus default xaxis name height", () => { + const labelOrientation = LabelOrientation.ROTATE; + + const chartType: ChartType = "LINE_CHART"; + + const builder = new EChartsXAxisLayoutBuilder({ + labelOrientation: labelOrientation, + chartType: chartType, + font: font, + longestLabel: { + x: "123456", + y: "123", + }, + }); + expect(builder.defaultHeightForXAxisName).toEqual(40); + + const allocatedXAxisHeight = 100; + const labelWidth = + allocatedXAxisHeight - + builder.defaultHeightForXAxisName - + builder.gapBetweenLabelAndName; + + const nameGap = allocatedXAxisHeight - builder.defaultHeightForXAxisName; + expect(builder.defaultHeightForXAxisName).toEqual(40); + expect(nameGap).toEqual(60); + + const expectedConfig = { + nameGap: nameGap, axisLabel: { - width: 50, + width: labelWidth, + overflow: "truncate", }, }; - expect(builder.configForXAxis()).toEqual(expectedOutput); + + expect(builder.configForXAxis(allocatedXAxisHeight)).toEqual( + expectedConfig, + ); }); }); + + // describe("height of x axis labels", () => { + // it("when label orientation isn't auto, it is equal to sum of width of x axis labels and a fixed offset", () => { + // const labelOrientation = LabelOrientation.SLANT; + // const builder = new EChartsXAxisLayoutBuilder( + // labelOrientation, + // "LINE_CHART", + // ); + + // expect(builder.gapBetweenLabelAndName).toEqual(10); + // expect(builder.widthForXAxisLabels()).toEqual(50); + // expect(builder.heightForXAxisLabels()).toEqual(60); + // }); + + // it("is equal to a default height when label orientation is auto", () => { + // const labelOrientation = LabelOrientation.AUTO; + // const builder = new EChartsXAxisLayoutBuilder( + // labelOrientation, + // "LINE_CHART", + // ); + + // expect(builder.gapBetweenLabelAndName).toEqual(10); + // expect(builder.defaultHeightForXAxisLabels).toEqual(30); + // expect(builder.heightForXAxisLabels()).toEqual(40); + // }); + // }); + + // describe("height of x axis", () => { + // it("is equal to sum of height of x axis labels and an offset", () => { + // const labelOrientation = LabelOrientation.SLANT; + // const builder = new EChartsXAxisLayoutBuilder( + // labelOrientation, + // "LINE_CHART", + // ); + + // expect(builder.defaultHeightForXAxisName).toEqual(40); + // expect(builder.heightForXAxisLabels()).toEqual(60); + // expect(builder.heightForXAxis()).toEqual(100); + // }); + + // it("is equal to 0 when chart type is pie chart", () => { + // const labelOrientation = LabelOrientation.SLANT; + // const chartType: ChartType = "PIE_CHART"; + // const builder = new EChartsXAxisLayoutBuilder( + // labelOrientation, + // chartType, + // ); + + // expect(builder.heightForXAxis()).toEqual(0); + // }); + // }); + + // describe("config for x axis", () => { + // it("returns x axis config for chart layout", () => { + // const labelOrientation = LabelOrientation.SLANT; + // const chartType: ChartType = "LINE_CHART"; + // const builder = new EChartsXAxisLayoutBuilder( + // labelOrientation, + // chartType, + // ); + + // const expectedOutput = { + // nameGap: 60, + // axisLabel: { + // width: 50, + // }, + // }; + // expect(builder.configForXAxis()).toEqual(expectedOutput); + // }); + // }); }); diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.ts index cfdcfbd08b..57c27d5a51 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsXAxisLayoutBuilder.ts @@ -1,50 +1,109 @@ import { LabelOrientation } from "widgets/ChartWidget/constants"; -import type { ChartType } from "widgets/ChartWidget/constants"; +import type { + ChartType, + LongestLabelParams, +} from "widgets/ChartWidget/constants"; +import { getTextWidth, labelKeyForChart } from "../helpers"; -export class EChartsXAxisLayoutBuilder { +interface XAxisLayoutBuilderParams { labelOrientation: LabelOrientation; chartType: ChartType; + font: string; + longestLabel: LongestLabelParams; +} +export class EChartsXAxisLayoutBuilder { + props: XAxisLayoutBuilderParams; gapBetweenLabelAndName = 10; defaultHeightForXAxisLabels = 30; + defaultHeightForRotatedLabels = 50; defaultHeightForXAxisName = 40; - constructor(labelOrientation: LabelOrientation, chartType: ChartType) { - this.labelOrientation = labelOrientation; - this.chartType = chartType; + constructor(props: XAxisLayoutBuilderParams) { + this.props = props; } - configForXAxis() { + configForXAxis(width: number) { return { - nameGap: this.heightForXAxisLabels(), - axisLabel: { - width: this.widthForXAxisLabels(), - }, + nameGap: width - this.defaultHeightForXAxisName, + axisLabel: this.axisLabelConfig(width), }; } - heightForXAxis = () => { - if (this.chartType == "PIE_CHART") { - return 0; + axisLabelConfig = (allocatedXAxisHeight: number) => { + if (this.props.labelOrientation == LabelOrientation.AUTO) { + return {}; + } else { + return { + width: + allocatedXAxisHeight - + this.defaultHeightForXAxisName - + this.gapBetweenLabelAndName, + overflow: "truncate", + }; } - return this.heightForXAxisLabels() + this.defaultHeightForXAxisName; }; - heightForXAxisLabels = () => { - let labelsHeight: number = this.defaultHeightForXAxisLabels; - if (this.labelOrientation != LabelOrientation.AUTO) { + heightConfigForXAxis = () => { + let minHeight = this.minHeightForLabels(); + const maxHeight = this.maxHeightForXAxis(); + + minHeight = minHeight > maxHeight ? maxHeight : minHeight; + + return { + minHeight: minHeight, + maxHeight: maxHeight, + }; + }; + + maxHeightForXAxis = () => { + if (this.props.chartType == "PIE_CHART") { + return 0; + } else { + return this.maxHeightForXAxisLabels() + this.defaultHeightForXAxisName; + } + }; + + maxHeightForXAxisLabels = (): number => { + let labelsHeight: number; + + if (this.props.labelOrientation == LabelOrientation.AUTO) { + labelsHeight = this.defaultHeightForXAxisLabels; + } else { labelsHeight = this.widthForXAxisLabels(); } return labelsHeight + this.gapBetweenLabelAndName; }; + minHeightForLabels() { + if (this.props.chartType == "PIE_CHART") { + return 0; + } + + let labelsHeight: number; + if (this.props.labelOrientation == LabelOrientation.AUTO) { + labelsHeight = this.defaultHeightForXAxisLabels; + } else { + labelsHeight = this.defaultHeightForRotatedLabels; + } + return ( + labelsHeight + + this.gapBetweenLabelAndName + + this.defaultHeightForXAxisName + ); + } + widthForXAxisLabels = () => { - switch (this.labelOrientation) { - case LabelOrientation.SLANT: { - return 50; + switch (this.props.labelOrientation) { + case LabelOrientation.AUTO: { + return 0; } default: { - return 60; + const longestLabelKey = labelKeyForChart("xAxis", this.props.chartType); + return getTextWidth( + this.props.longestLabel[longestLabelKey], + this.props.font, + ); } } }; diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.test.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.test.ts index e53ce80bec..f7142e1b50 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.test.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.test.ts @@ -1,18 +1,70 @@ import { EChartsYAxisLayoutBuilder } from "./EChartsYAxisLayoutBuilder"; +const font = "14px Nunito Sans"; describe("EChartsYAxisLayoutBuilder", () => { + describe("maxWidthForLabels", () => { + it("returns the max width for labels to equal sum of label width in pixels and an offset", () => { + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: 100, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); + + expect(builder.maxWidthForLabels()).toEqual(16); + }); + }); + + describe("width for labels", () => { + it("if available space is greater than label width, it returns value equal to label width", () => { + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: 300, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); + + expect(builder.minimumWidth).toEqual(150); + expect(builder.maxWidthForLabels()).toEqual(16); + expect(builder.widthForLabels()).toEqual(16); + }); + + it("if available space is lesser than label width, it returns available space", () => { + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: 160, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); + + expect(builder.minimumWidth).toEqual(150); + expect(builder.maxWidthForLabels()).toEqual(16); + expect(builder.widthForLabels()).toEqual(10); + }); + }); + describe("visibility of y axis config", () => { it("shows y axis if width is more than minimum width", () => { - const width = 150; - const builder = new EChartsYAxisLayoutBuilder(width); + const widgetWidth = 160; + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: widgetWidth, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); expect(builder.minimumWidth).toEqual(150); expect(builder.showYAxisConfig()).toEqual(true); }); it("hides y axis if width is more than minimum width", () => { - const width = 149; - const builder = new EChartsYAxisLayoutBuilder(width); + const widgetWidth = 149; + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: widgetWidth, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); expect(builder.minimumWidth).toEqual(150); expect(builder.showYAxisConfig()).toEqual(false); @@ -20,18 +72,29 @@ describe("EChartsYAxisLayoutBuilder", () => { }); describe("y axis grid left offset", () => { - it("when y axis is visible, offset is 100", () => { - const width = 150; - const builder = new EChartsYAxisLayoutBuilder(width); + it("when y axis is visible, offset is equal to sum of label width and offsets", () => { + const widgetWidth = 160; + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: widgetWidth, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); expect(builder.minimumWidth).toEqual(150); expect(builder.showYAxisConfig()).toEqual(true); - expect(builder.gridLeftOffset()).toEqual(100); + expect(builder.labelsWidth).toEqual(10); + expect(builder.gridLeftOffset()).toEqual(50); }); it("when y axis is not visible, offset is 5", () => { - const width = 149; - const builder = new EChartsYAxisLayoutBuilder(width); + const widgetWidth = 149; + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: widgetWidth, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); expect(builder.minimumWidth).toEqual(150); expect(builder.showYAxisConfig()).toEqual(false); @@ -41,14 +104,20 @@ describe("EChartsYAxisLayoutBuilder", () => { describe("y axis config", () => { it("returns correct y axis config based on props", () => { - const width = 150; - const builder = new EChartsYAxisLayoutBuilder(width); + const widgetWidth = 160; + const builder = new EChartsYAxisLayoutBuilder({ + widgetWidth: widgetWidth, + chartType: "LINE_CHART", + font: font, + longestLabel: { x: "x label", y: "123456" }, + }); const expectedOutput = { show: true, - nameGap: 70, + nameGap: 20, axisLabel: { - width: 60, + width: 10, + overflow: "truncate", }, }; expect(builder.config()).toEqual(expectedOutput); diff --git a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.ts b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.ts index daba8f9f5d..75dbf338ec 100644 --- a/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.ts +++ b/app/client/src/widgets/ChartWidget/component/LayoutBuilders/EChartsYAxisLayoutBuilder.ts @@ -1,26 +1,66 @@ -export class EChartsYAxisLayoutBuilder { - minimumWidth = 150; - width: number; +import type { + ChartType, + LongestLabelParams, +} from "widgets/ChartWidget/constants"; +import { getTextWidth, labelKeyForChart } from "../helpers"; - constructor(width: number) { - this.width = width; +interface YAxisLayoutBuilderParams { + widgetWidth: number; + chartType: ChartType; + font: string; + longestLabel: LongestLabelParams; +} +export class EChartsYAxisLayoutBuilder { + props: YAxisLayoutBuilderParams; + minimumWidth = 150; + labelsWidth: number; + nameGap: number; + leftOffset: number; + + constructor(props: YAxisLayoutBuilderParams) { + this.props = props; + + this.labelsWidth = this.widthForLabels(); + this.nameGap = this.labelsWidth + 10; + this.leftOffset = this.nameGap + 30; } showYAxisConfig = () => { - return this.width >= this.minimumWidth; + return this.props.widgetWidth >= this.minimumWidth; }; gridLeftOffset = () => { - return this.showYAxisConfig() ? 100 : 5; + return this.showYAxisConfig() ? this.leftOffset : 5; }; config = () => { return { show: this.showYAxisConfig(), - nameGap: 70, + nameGap: this.nameGap, axisLabel: { - width: 60, + width: this.labelsWidth, + overflow: "truncate", }, }; }; + + widthForLabels = () => { + const availableSpace = this.props.widgetWidth - this.minimumWidth; + const maxWidth = this.maxWidthForLabels(); + + if (maxWidth < availableSpace) { + return maxWidth; + } else { + return availableSpace; + } + }; + + maxWidthForLabels = () => { + const longestLabelKey = labelKeyForChart("yAxis", this.props.chartType); + const labelWidthYAxis = getTextWidth( + this.props.longestLabel[longestLabelKey], + this.props.font, + ); + return labelWidthYAxis + 10; + }; } diff --git a/app/client/src/widgets/ChartWidget/component/helpers.ts b/app/client/src/widgets/ChartWidget/component/helpers.ts index 307f64dc28..3b393dbe84 100644 --- a/app/client/src/widgets/ChartWidget/component/helpers.ts +++ b/app/client/src/widgets/ChartWidget/component/helpers.ts @@ -60,3 +60,29 @@ export const parseOnDataPointClickForBasicCharts = ( seriesTitle: seriesName, } as ChartSelectedDataPoint; }; + +export const labelKeyForChart = ( + axisName: "xAxis" | "yAxis", + chartType: ChartType, +): "x" | "y" => { + let labelKey: "x" | "y"; + + if (axisName == "xAxis") { + labelKey = chartType == "BAR_CHART" ? "y" : "x"; + } else { + labelKey = chartType == "BAR_CHART" ? "x" : "y"; + } + return labelKey; +}; + +export const getTextWidth = (text: string, font: string) => { + const canvas = document.createElement("canvas"); + const context = canvas.getContext("2d"); + if (context) { + context.font = font; + const metrics = context.measureText(text); + return metrics.width; + } else { + return 0; + } +}; diff --git a/app/client/src/widgets/ChartWidget/component/index.tsx b/app/client/src/widgets/ChartWidget/component/index.tsx index 56b7a1eb1c..cb5cc55d0d 100644 --- a/app/client/src/widgets/ChartWidget/component/index.tsx +++ b/app/client/src/widgets/ChartWidget/component/index.tsx @@ -130,17 +130,20 @@ class ChartComponent extends React.Component< } getBasicEChartOptions = () => { - const chartData = EChartsDatasetBuilder.chartData( + const datasetBuilder = new EChartsDatasetBuilder( this.props.chartType, this.props.chartData, ); + const dataset = datasetBuilder.datasetFromData(); + const options = { ...this.echartsConfigurationBuilder.prepareEChartConfig( this.props, - chartData, + datasetBuilder.filteredChartData, + datasetBuilder.longestDataLabels(), ), dataset: { - ...EChartsDatasetBuilder.datasetFromData(chartData), + ...dataset, }, }; return options; diff --git a/app/client/src/widgets/ChartWidget/constants.ts b/app/client/src/widgets/ChartWidget/constants.ts index 804fa949d6..7393f6ed13 100644 --- a/app/client/src/widgets/ChartWidget/constants.ts +++ b/app/client/src/widgets/ChartWidget/constants.ts @@ -13,8 +13,13 @@ export type ChartType = export const XAxisCategory = "Category"; export interface ChartDataPoint { - x: any; - y: any; + x: number | string; + y: number | string; +} + +export interface LongestLabelParams { + x: string; + y: string; } export interface ChartData { @@ -43,7 +48,7 @@ export const messages = { ErrorTitle: "Error in Chart Data/Configuration", MoreDetails: "More Details", EmptyData: "No chart data to display", - Undefined: "Undefined", + Undefined: "Series", customFusionChartDeprecationMessage: "Custom Fusion Charts will stop being supported on March 1st 2024. Change the chart type to E-charts Custom to switch.", customFusionChartOptionLabel: (showDeprecationMessage: boolean) => { diff --git a/app/client/src/widgets/ChartWidget/widget/index.tsx b/app/client/src/widgets/ChartWidget/widget/index.tsx index 43557226fc..6d1a01f6e0 100644 --- a/app/client/src/widgets/ChartWidget/widget/index.tsx +++ b/app/client/src/widgets/ChartWidget/widget/index.tsx @@ -56,12 +56,9 @@ export const emptyChartData = (props: ChartWidgetProps) => { } else if (props.chartType == "CUSTOM_ECHART") { return Object.keys(props.customEChartConfig).length == 0; } else { - const seriesData = EChartsDatasetBuilder.chartData( - props.chartType, - props.chartData, - ); + const builder = new EChartsDatasetBuilder(props.chartType, props.chartData); - for (const seriesID in seriesData) { + for (const seriesID in builder.filteredChartData) { if (Object.keys(props.chartData[seriesID].data).length > 0) { return false; }