Skip to content

Commit

Permalink
fix(plugin-chart-echarts): use scale for truncating x-axis (apache#26269
Browse files Browse the repository at this point in the history
)
  • Loading branch information
villebro authored Dec 14, 2023
1 parent 429e2a3 commit 67468c4
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import { EchartsTimeseriesSeriesType } from '../Timeseries/types';
import {
legendSection,
richTooltipSection,
truncateXAxis,
xAxisBounds,
xAxisLabelRotation,
} from '../controls';

Expand Down Expand Up @@ -333,6 +335,8 @@ const config: ControlPanelConfig = {
},
},
],
[truncateXAxis],
[xAxisBounds],
[
{
name: 'truncateYAxis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,32 +20,32 @@
import { invert } from 'lodash';
import {
AnnotationLayer,
buildCustomFormatters,
CategoricalColorNamespace,
CurrencyFormatter,
ensureIsArray,
GenericDataType,
getCustomFormatter,
getNumberFormatter,
getXAxisLabel,
isDefined,
isEventAnnotationLayer,
isFormulaAnnotationLayer,
isIntervalAnnotationLayer,
isPhysicalColumn,
isTimeseriesAnnotationLayer,
QueryFormData,
QueryFormMetric,
TimeseriesChartDataResponseResult,
TimeseriesDataRecord,
getXAxisLabel,
isPhysicalColumn,
isDefined,
ensureIsArray,
buildCustomFormatters,
ValueFormatter,
QueryFormMetric,
getCustomFormatter,
CurrencyFormatter,
} from '@superset-ui/core';
import { getOriginalSeries } from '@superset-ui/chart-controls';
import { EChartsCoreOption, SeriesOption } from 'echarts';
import {
DEFAULT_FORM_DATA,
EchartsMixedTimeseriesFormData,
EchartsMixedTimeseriesChartTransformedProps,
EchartsMixedTimeseriesFormData,
EchartsMixedTimeseriesProps,
} from './types';
import {
Expand All @@ -55,14 +55,15 @@ import {
} from '../types';
import { parseAxisBound } from '../utils/controls';
import {
getOverMaxHiddenFormatter,
dedupSeries,
extractDataTotalValues,
extractSeries,
extractShowValueIndexes,
getAxisType,
getColtypesMapping,
getLegendProps,
extractDataTotalValues,
extractShowValueIndexes,
getMinAndMaxFromBounds,
getOverMaxHiddenFormatter,
} from '../utils/series';
import {
extractAnnotationLabels,
Expand All @@ -84,7 +85,7 @@ import {
transformSeries,
transformTimeseriesAnnotation,
} from '../Timeseries/transformers';
import { TIMESERIES_CONSTANTS, TIMEGRAIN_TO_TIMESTAMP } from '../constants';
import { TIMEGRAIN_TO_TIMESTAMP, TIMESERIES_CONSTANTS } from '../constants';
import { getDefaultTooltip } from '../utils/tooltip';
import {
getTooltipTimeFormatter,
Expand Down Expand Up @@ -166,6 +167,7 @@ export default function transformProps(
showValueB,
stack,
stackB,
truncateXAxis,
truncateYAxis,
tooltipTimeFormat,
yAxisFormat,
Expand All @@ -181,6 +183,7 @@ export default function transformProps(
zoomable,
richTooltip,
tooltipSortByMetric,
xAxisBounds,
xAxisLabelRotation,
groupby,
groupbyB,
Expand Down Expand Up @@ -345,7 +348,8 @@ export default function transformProps(
});

// yAxisBounds need to be parsed to replace incompatible values with undefined
let [min, max] = (yAxisBounds || []).map(parseAxisBound);
const [xAxisMin, xAxisMax] = (xAxisBounds || []).map(parseAxisBound);
let [yAxisMin, yAxisMax] = (yAxisBounds || []).map(parseAxisBound);
let [minSecondary, maxSecondary] = (yAxisBoundsSecondary || []).map(
parseAxisBound,
);
Expand Down Expand Up @@ -386,7 +390,7 @@ export default function transformProps(
formatter:
seriesType === EchartsTimeseriesSeriesType.Bar
? getOverMaxHiddenFormatter({
max,
max: yAxisMax,
formatter: seriesFormatter,
})
: seriesFormatter,
Expand Down Expand Up @@ -447,8 +451,8 @@ export default function transformProps(

// default to 0-100% range when doing row-level contribution chart
if (contributionMode === 'row' && stack) {
if (min === undefined) min = 0;
if (max === undefined) max = 1;
if (yAxisMin === undefined) yAxisMin = 0;
if (yAxisMax === undefined) yAxisMax = 1;
if (minSecondary === undefined) minSecondary = 0;
if (maxSecondary === undefined) maxSecondary = 1;
}
Expand Down Expand Up @@ -499,13 +503,23 @@ export default function transformProps(
xAxisType === 'time' && timeGrainSqla
? TIMEGRAIN_TO_TIMESTAMP[timeGrainSqla]
: 0,
...getMinAndMaxFromBounds(
xAxisType,
truncateXAxis,
xAxisMin,
xAxisMax,
seriesType === EchartsTimeseriesSeriesType.Bar ||
seriesTypeB === EchartsTimeseriesSeriesType.Bar
? EchartsTimeseriesSeriesType.Bar
: undefined,
),
},
yAxis: [
{
...defaultYAxis,
type: logAxis ? 'log' : 'value',
min,
max,
min: yAxisMin,
max: yAxisMax,
minorTick: { show: true },
minorSplitLine: { show: minorSplitLine },
axisLabel: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export const DEFAULT_FORM_DATA: EchartsMixedTimeseriesFormData = {
yAxisFormatSecondary: TIMESERIES_DEFAULTS.yAxisFormat,
yAxisTitleSecondary: DEFAULT_TITLE_FORM_DATA.yAxisTitle,
tooltipTimeFormat: TIMESERIES_DEFAULTS.tooltipTimeFormat,
xAxisBounds: TIMESERIES_DEFAULTS.xAxisBounds,
xAxisTimeFormat: TIMESERIES_DEFAULTS.xAxisTimeFormat,
area: TIMESERIES_DEFAULTS.area,
areaB: TIMESERIES_DEFAULTS.area,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,13 @@ export default function transformProps(
xAxisType === AxisType.time && timeGrainSqla
? TIMEGRAIN_TO_TIMESTAMP[timeGrainSqla]
: 0,
...getMinAndMaxFromBounds(xAxisType, truncateXAxis, xAxisMin, xAxisMax),
...getMinAndMaxFromBounds(
xAxisType,
truncateXAxis,
xAxisMin,
xAxisMax,
seriesType,
),
};

let yAxis: any = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import { validateNumber } from '@superset-ui/core';

// eslint-disable-next-line import/prefer-default-export
export function parseAxisBound(
bound?: string | number | null,
): number | undefined {
Expand Down
40 changes: 32 additions & 8 deletions superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ import {
StackControlsValue,
TIMESERIES_CONSTANTS,
} from '../constants';
import { LegendOrientation, LegendType, StackType } from '../types';
import {
EchartsTimeseriesSeriesType,
LegendOrientation,
LegendType,
StackType,
} from '../types';
import { defaultLegendPadding } from '../defaults';

function isDefined<T>(value: T | undefined | null): boolean {
Expand Down Expand Up @@ -547,16 +552,35 @@ export function calculateLowerLogTick(minPositiveValue: number) {
return Math.pow(10, logBase10);
}

type BoundsType = {
min?: number | 'dataMin';
max?: number | 'dataMax';
scale?: true;
};

export function getMinAndMaxFromBounds(
axisType: AxisType,
truncateAxis: boolean,
min?: number,
max?: number,
): { min: number | 'dataMin'; max: number | 'dataMax' } | {} {
return truncateAxis && axisType === AxisType.value
? {
min: min === undefined ? 'dataMin' : min,
max: max === undefined ? 'dataMax' : max,
}
: {};
seriesType?: EchartsTimeseriesSeriesType,
): BoundsType | {} {
if (axisType === AxisType.value && truncateAxis) {
const ret: BoundsType = {};
if (seriesType === EchartsTimeseriesSeriesType.Bar) {
ret.scale = true;
}
if (min !== undefined) {
ret.min = min;
} else if (seriesType !== EchartsTimeseriesSeriesType.Bar) {
ret.min = 'dataMin';
}
if (max !== undefined) {
ret.max = max;
} else if (seriesType !== EchartsTimeseriesSeriesType.Bar) {
ret.max = 'dataMax';
}
return ret;
}
return {};
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ import {
sortAndFilterSeries,
sortRows,
} from '../../src/utils/series';
import { LegendOrientation, LegendType } from '../../src/types';
import {
EchartsTimeseriesSeriesType,
LegendOrientation,
LegendType,
} from '../../src/types';
import { defaultLegendPadding } from '../../src/defaults';
import { NULL_STRING } from '../../src/constants';

Expand Down Expand Up @@ -885,28 +889,109 @@ test('getAxisType', () => {
});

test('getMinAndMaxFromBounds returns empty object when not truncating', () => {
expect(getMinAndMaxFromBounds(AxisType.value, false, 10, 100)).toEqual({});
expect(
getMinAndMaxFromBounds(
AxisType.value,
false,
10,
100,
EchartsTimeseriesSeriesType.Bar,
),
).toEqual({});
});

test('getMinAndMaxFromBounds returns empty object for categorical axis', () => {
expect(
getMinAndMaxFromBounds(
AxisType.category,
false,
10,
100,
EchartsTimeseriesSeriesType.Bar,
),
).toEqual({});
});

test('getMinAndMaxFromBounds returns empty object for time axis', () => {
expect(
getMinAndMaxFromBounds(
AxisType.time,
false,
10,
100,
EchartsTimeseriesSeriesType.Bar,
),
).toEqual({});
});

test('getMinAndMaxFromBounds returns automatic bounds when truncating', () => {
test('getMinAndMaxFromBounds returns dataMin/dataMax for non-bar charts', () => {
expect(
getMinAndMaxFromBounds(AxisType.value, true, undefined, undefined),
getMinAndMaxFromBounds(
AxisType.value,
true,
undefined,
undefined,
EchartsTimeseriesSeriesType.Line,
),
).toEqual({
min: 'dataMin',
max: 'dataMax',
});
});

test('getMinAndMaxFromBounds returns automatic upper bound when truncating', () => {
expect(getMinAndMaxFromBounds(AxisType.value, true, 10, undefined)).toEqual({
test('getMinAndMaxFromBounds returns bound without scale for non-bar charts', () => {
expect(
getMinAndMaxFromBounds(
AxisType.value,
true,
10,
undefined,
EchartsTimeseriesSeriesType.Line,
),
).toEqual({
min: 10,
max: 'dataMax',
});
});

test('getMinAndMaxFromBounds returns scale when truncating without bounds', () => {
expect(
getMinAndMaxFromBounds(
AxisType.value,
true,
undefined,
undefined,
EchartsTimeseriesSeriesType.Bar,
),
).toEqual({ scale: true });
});

test('getMinAndMaxFromBounds returns automatic upper bound when truncating', () => {
expect(
getMinAndMaxFromBounds(
AxisType.value,
true,
10,
undefined,
EchartsTimeseriesSeriesType.Bar,
),
).toEqual({
min: 10,
scale: true,
});
});

test('getMinAndMaxFromBounds returns automatic lower bound when truncating', () => {
expect(getMinAndMaxFromBounds(AxisType.value, true, undefined, 100)).toEqual({
min: 'dataMin',
expect(
getMinAndMaxFromBounds(
AxisType.value,
true,
undefined,
100,
EchartsTimeseriesSeriesType.Bar,
),
).toEqual({
max: 100,
scale: true,
});
});

0 comments on commit 67468c4

Please sign in to comment.