Skip to content

Commit abe3790

Browse files
michael-s-molinasadpandajoe
authored andcommitted
fix: Bar charts horizontal margin adjustment error (apache#26817)
(cherry picked from commit 84c48d1)
1 parent 5ea4d85 commit abe3790

File tree

4 files changed

+72
-17
lines changed

4 files changed

+72
-17
lines changed

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts

+1
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ export default function transformProps(
437437
yAxisTitlePosition,
438438
convertInteger(yAxisTitleMargin),
439439
convertInteger(xAxisTitleMargin),
440+
isHorizontal,
440441
);
441442

442443
const legendData = rawSeries

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts

+25-17
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@ export function getPadding(
550550
yAxisTitlePosition?: string,
551551
yAxisTitleMargin?: number,
552552
xAxisTitleMargin?: number,
553+
isHorizontal?: boolean,
553554
): {
554555
bottom: number;
555556
left: number;
@@ -560,21 +561,28 @@ export function getPadding(
560561
? TIMESERIES_CONSTANTS.yAxisLabelTopOffset
561562
: 0;
562563
const xAxisOffset = addXAxisTitleOffset ? Number(xAxisTitleMargin) || 0 : 0;
563-
return getChartPadding(showLegend, legendOrientation, margin, {
564-
top:
565-
yAxisTitlePosition && yAxisTitlePosition === 'Top'
566-
? TIMESERIES_CONSTANTS.gridOffsetTop + (Number(yAxisTitleMargin) || 0)
567-
: TIMESERIES_CONSTANTS.gridOffsetTop + yAxisOffset,
568-
bottom: zoomable
569-
? TIMESERIES_CONSTANTS.gridOffsetBottomZoomable + xAxisOffset
570-
: TIMESERIES_CONSTANTS.gridOffsetBottom + xAxisOffset,
571-
left:
572-
yAxisTitlePosition === 'Left'
573-
? TIMESERIES_CONSTANTS.gridOffsetLeft + (Number(yAxisTitleMargin) || 0)
574-
: TIMESERIES_CONSTANTS.gridOffsetLeft,
575-
right:
576-
showLegend && legendOrientation === LegendOrientation.Right
577-
? 0
578-
: TIMESERIES_CONSTANTS.gridOffsetRight,
579-
});
564+
return getChartPadding(
565+
showLegend,
566+
legendOrientation,
567+
margin,
568+
{
569+
top:
570+
yAxisTitlePosition && yAxisTitlePosition === 'Top'
571+
? TIMESERIES_CONSTANTS.gridOffsetTop + (Number(yAxisTitleMargin) || 0)
572+
: TIMESERIES_CONSTANTS.gridOffsetTop + yAxisOffset,
573+
bottom: zoomable
574+
? TIMESERIES_CONSTANTS.gridOffsetBottomZoomable + xAxisOffset
575+
: TIMESERIES_CONSTANTS.gridOffsetBottom + xAxisOffset,
576+
left:
577+
yAxisTitlePosition === 'Left'
578+
? TIMESERIES_CONSTANTS.gridOffsetLeft +
579+
(Number(yAxisTitleMargin) || 0)
580+
: TIMESERIES_CONSTANTS.gridOffsetLeft,
581+
right:
582+
showLegend && legendOrientation === LegendOrientation.Right
583+
? 0
584+
: TIMESERIES_CONSTANTS.gridOffsetRight,
585+
},
586+
isHorizontal,
587+
);
580588
}

superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts

+14
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ export function getChartPadding(
466466
orientation: LegendOrientation,
467467
margin?: string | number | null,
468468
padding?: { top?: number; bottom?: number; left?: number; right?: number },
469+
isHorizontal?: boolean,
469470
): {
470471
bottom: number;
471472
left: number;
@@ -486,6 +487,19 @@ export function getChartPadding(
486487
}
487488

488489
const { bottom = 0, left = 0, right = 0, top = 0 } = padding || {};
490+
491+
if (isHorizontal) {
492+
return {
493+
left:
494+
left + (orientation === LegendOrientation.Bottom ? legendMargin : 0),
495+
right:
496+
right + (orientation === LegendOrientation.Right ? legendMargin : 0),
497+
top: top + (orientation === LegendOrientation.Top ? legendMargin : 0),
498+
bottom:
499+
bottom + (orientation === LegendOrientation.Left ? legendMargin : 0),
500+
};
501+
}
502+
489503
return {
490504
left: left + (orientation === LegendOrientation.Left ? legendMargin : 0),
491505
right: right + (orientation === LegendOrientation.Right ? legendMargin : 0),

superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts

+32
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,14 @@ describe('getChartPadding', () => {
795795
right: 0,
796796
top: 0,
797797
});
798+
expect(
799+
getChartPadding(true, LegendOrientation.Left, 100, undefined, true),
800+
).toEqual({
801+
bottom: 100,
802+
left: 0,
803+
right: 0,
804+
top: 0,
805+
});
798806
});
799807

800808
it('should return the correct padding for right orientation', () => {
@@ -804,6 +812,14 @@ describe('getChartPadding', () => {
804812
right: 50,
805813
top: 0,
806814
});
815+
expect(
816+
getChartPadding(true, LegendOrientation.Right, 50, undefined, true),
817+
).toEqual({
818+
bottom: 0,
819+
left: 0,
820+
right: 50,
821+
top: 0,
822+
});
807823
});
808824

809825
it('should return the correct padding for top orientation', () => {
@@ -813,6 +829,14 @@ describe('getChartPadding', () => {
813829
right: 0,
814830
top: 20,
815831
});
832+
expect(
833+
getChartPadding(true, LegendOrientation.Top, 20, undefined, true),
834+
).toEqual({
835+
bottom: 0,
836+
left: 0,
837+
right: 0,
838+
top: 20,
839+
});
816840
});
817841

818842
it('should return the correct padding for bottom orientation', () => {
@@ -822,6 +846,14 @@ describe('getChartPadding', () => {
822846
right: 0,
823847
top: 0,
824848
});
849+
expect(
850+
getChartPadding(true, LegendOrientation.Bottom, 10, undefined, true),
851+
).toEqual({
852+
bottom: 0,
853+
left: 10,
854+
right: 0,
855+
top: 0,
856+
});
825857
});
826858
});
827859

0 commit comments

Comments
 (0)