You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/02/01 16:43:11 UTC
(superset) 04/09: fix: Bar charts horizontal margin adjustment error (#26817)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 89527c6c977d3ccdaa6848a99edb8c57850117b3
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Mon Jan 29 15:25:16 2024 -0500
fix: Bar charts horizontal margin adjustment error (#26817)
(cherry picked from commit 84c48d11d8b3bef244823643804f5fd3d6e3ca86)
---
.../src/Timeseries/transformProps.ts | 1 +
.../src/Timeseries/transformers.ts | 42 +++++++++++++---------
.../plugin-chart-echarts/src/utils/series.ts | 14 ++++++++
.../plugin-chart-echarts/test/utils/series.test.ts | 32 +++++++++++++++++
4 files changed, 72 insertions(+), 17 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index 519323a62e..0f91fb4a00 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -435,6 +435,7 @@ export default function transformProps(
yAxisTitlePosition,
convertInteger(yAxisTitleMargin),
convertInteger(xAxisTitleMargin),
+ isHorizontal,
);
const legendData = rawSeries
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
index 0bcc5baf8d..1e274ca875 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
@@ -554,6 +554,7 @@ export function getPadding(
yAxisTitlePosition?: string,
yAxisTitleMargin?: number,
xAxisTitleMargin?: number,
+ isHorizontal?: boolean,
): {
bottom: number;
left: number;
@@ -564,23 +565,30 @@ export function getPadding(
? TIMESERIES_CONSTANTS.yAxisLabelTopOffset
: 0;
const xAxisOffset = addXAxisTitleOffset ? Number(xAxisTitleMargin) || 0 : 0;
- return getChartPadding(showLegend, legendOrientation, margin, {
- top:
- yAxisTitlePosition && yAxisTitlePosition === 'Top'
- ? TIMESERIES_CONSTANTS.gridOffsetTop + (Number(yAxisTitleMargin) || 0)
- : TIMESERIES_CONSTANTS.gridOffsetTop + yAxisOffset,
- bottom: zoomable
- ? TIMESERIES_CONSTANTS.gridOffsetBottomZoomable + xAxisOffset
- : TIMESERIES_CONSTANTS.gridOffsetBottom + xAxisOffset,
- left:
- yAxisTitlePosition === 'Left'
- ? TIMESERIES_CONSTANTS.gridOffsetLeft + (Number(yAxisTitleMargin) || 0)
- : TIMESERIES_CONSTANTS.gridOffsetLeft,
- right:
- showLegend && legendOrientation === LegendOrientation.Right
- ? 0
- : TIMESERIES_CONSTANTS.gridOffsetRight,
- });
+ return getChartPadding(
+ showLegend,
+ legendOrientation,
+ margin,
+ {
+ top:
+ yAxisTitlePosition && yAxisTitlePosition === 'Top'
+ ? TIMESERIES_CONSTANTS.gridOffsetTop + (Number(yAxisTitleMargin) || 0)
+ : TIMESERIES_CONSTANTS.gridOffsetTop + yAxisOffset,
+ bottom: zoomable
+ ? TIMESERIES_CONSTANTS.gridOffsetBottomZoomable + xAxisOffset
+ : TIMESERIES_CONSTANTS.gridOffsetBottom + xAxisOffset,
+ left:
+ yAxisTitlePosition === 'Left'
+ ? TIMESERIES_CONSTANTS.gridOffsetLeft +
+ (Number(yAxisTitleMargin) || 0)
+ : TIMESERIES_CONSTANTS.gridOffsetLeft,
+ right:
+ showLegend && legendOrientation === LegendOrientation.Right
+ ? 0
+ : TIMESERIES_CONSTANTS.gridOffsetRight,
+ },
+ isHorizontal,
+ );
}
export function getTooltipTimeFormatter(
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index 6a51e7cbf7..8c87bf4333 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -466,6 +466,7 @@ export function getChartPadding(
orientation: LegendOrientation,
margin?: string | number | null,
padding?: { top?: number; bottom?: number; left?: number; right?: number },
+ isHorizontal?: boolean,
): {
bottom: number;
left: number;
@@ -486,6 +487,19 @@ export function getChartPadding(
}
const { bottom = 0, left = 0, right = 0, top = 0 } = padding || {};
+
+ if (isHorizontal) {
+ return {
+ left:
+ left + (orientation === LegendOrientation.Bottom ? legendMargin : 0),
+ right:
+ right + (orientation === LegendOrientation.Right ? legendMargin : 0),
+ top: top + (orientation === LegendOrientation.Top ? legendMargin : 0),
+ bottom:
+ bottom + (orientation === LegendOrientation.Left ? legendMargin : 0),
+ };
+ }
+
return {
left: left + (orientation === LegendOrientation.Left ? legendMargin : 0),
right: right + (orientation === LegendOrientation.Right ? legendMargin : 0),
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
index 3d7d21c8d0..10768a47f8 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -795,6 +795,14 @@ describe('getChartPadding', () => {
right: 0,
top: 0,
});
+ expect(
+ getChartPadding(true, LegendOrientation.Left, 100, undefined, true),
+ ).toEqual({
+ bottom: 100,
+ left: 0,
+ right: 0,
+ top: 0,
+ });
});
it('should return the correct padding for right orientation', () => {
@@ -804,6 +812,14 @@ describe('getChartPadding', () => {
right: 50,
top: 0,
});
+ expect(
+ getChartPadding(true, LegendOrientation.Right, 50, undefined, true),
+ ).toEqual({
+ bottom: 0,
+ left: 0,
+ right: 50,
+ top: 0,
+ });
});
it('should return the correct padding for top orientation', () => {
@@ -813,6 +829,14 @@ describe('getChartPadding', () => {
right: 0,
top: 20,
});
+ expect(
+ getChartPadding(true, LegendOrientation.Top, 20, undefined, true),
+ ).toEqual({
+ bottom: 0,
+ left: 0,
+ right: 0,
+ top: 20,
+ });
});
it('should return the correct padding for bottom orientation', () => {
@@ -822,6 +846,14 @@ describe('getChartPadding', () => {
right: 0,
top: 0,
});
+ expect(
+ getChartPadding(true, LegendOrientation.Bottom, 10, undefined, true),
+ ).toEqual({
+ bottom: 0,
+ left: 10,
+ right: 0,
+ top: 0,
+ });
});
});