Skip to content

Commit ea4d39f

Browse files
bernardobelchioralexfauquette
bernardobelchior
and
alexfauquette
authored
[charts] Fix zoom discard inconsistency (#19535)
Co-authored-by: alex <[email protected]>
1 parent 5b67f45 commit ea4d39f

File tree

11 files changed

+670
-215
lines changed

11 files changed

+670
-215
lines changed

‎docs/data/visual-regression-tests/ZoomSliderPreview.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import * as React from 'react';
2+
3+
import { BarChartPro } from '@mui/x-charts-pro/BarChartPro';
4+
import { countryData } from '../charts/dataset/countryData';
5+
import { shareOfRenewables } from '../charts/dataset/shareOfRenewables';
6+
7+
const percentageFormatter = new Intl.NumberFormat(undefined, {
8+
style: 'percent',
9+
minimumSignificantDigits: 1,
10+
maximumSignificantDigits: 3,
11+
});
12+
13+
const sortedShareOfRenewables = shareOfRenewables.toSorted(
14+
(a, b) => a.renewablesPercentage - b.renewablesPercentage,
15+
);
16+
const barXAxis = {
17+
id: 'x',
18+
data: sortedShareOfRenewables.map((d) => countryData[d.code].country),
19+
tickLabelStyle: { angle: -45 },
20+
height: 90,
21+
};
22+
const barSettings = {
23+
series: [
24+
{
25+
data: sortedShareOfRenewables.map((d) => d.renewablesPercentage / 100),
26+
valueFormatter: (v) => percentageFormatter.format(v),
27+
},
28+
],
29+
height: 400,
30+
};
31+
32+
export default function ZoomSliderPreview() {
33+
return (
34+
<BarChartPro
35+
{...barSettings}
36+
xAxis={[
37+
{
38+
...barXAxis,
39+
zoom: { filterMode: 'discard', slider: { enabled: true, preview: true } },
40+
},
41+
]}
42+
zoomData={[{ axisId: 'x', start: 10, end: 30 }]}
43+
/>
44+
);
45+
}

‎docs/data/visual-regression-tests/ZoomSliderPreview.tsx‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import * as React from 'react';
2+
import { XAxis } from '@mui/x-charts/models';
3+
import { BarChartPro, BarChartProProps } from '@mui/x-charts-pro/BarChartPro';
4+
import { countryData } from '../charts/dataset/countryData';
5+
import { shareOfRenewables } from '../charts/dataset/shareOfRenewables';
6+
7+
const percentageFormatter = new Intl.NumberFormat(undefined, {
8+
style: 'percent',
9+
minimumSignificantDigits: 1,
10+
maximumSignificantDigits: 3,
11+
});
12+
13+
const sortedShareOfRenewables = shareOfRenewables.toSorted(
14+
(a, b) => a.renewablesPercentage - b.renewablesPercentage,
15+
);
16+
const barXAxis = {
17+
id: 'x',
18+
data: sortedShareOfRenewables.map((d) => countryData[d.code].country),
19+
tickLabelStyle: { angle: -45 },
20+
height: 90,
21+
} satisfies XAxis<'band'>;
22+
const barSettings = {
23+
series: [
24+
{
25+
data: sortedShareOfRenewables.map((d) => d.renewablesPercentage / 100),
26+
valueFormatter: (v: number | null) => percentageFormatter.format(v!),
27+
},
28+
],
29+
height: 400,
30+
} satisfies Partial<BarChartProProps>;
31+
32+
export default function ZoomSliderPreview() {
33+
return (
34+
<BarChartPro
35+
{...barSettings}
36+
xAxis={[
37+
{
38+
...barXAxis,
39+
zoom: { filterMode: 'discard', slider: { enabled: true, preview: true } },
40+
},
41+
]}
42+
zoomData={[{ axisId: 'x', start: 10, end: 30 }]}
43+
/>
44+
);
45+
}

‎docs/data/visual-regression-tests/ZoomSliderPreview.tsx.preview‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<BarChartPro
2+
{...barSettings}
3+
xAxis={[
4+
{
5+
...barXAxis,
6+
zoom: { filterMode: 'discard', slider: { enabled: true, preview: true } },
7+
},
8+
]}
9+
zoomData={[{ axisId: 'x', start: 10, end: 30 }]}
10+
/>

‎packages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/computeAxisValue.ts‎

Lines changed: 91 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { scaleBand, scalePoint, ScaleSymLog } from '@mui/x-charts-vendor/d3-scale';
21
import { createScalarFormatter } from '../../../defaultValueFormatters';
3-
import { ScaleName } from '../../../../models';
2+
import { ContinuousScaleName, ScaleName } from '../../../../models';
43
import {
54
ChartsXAxisProps,
65
ChartsAxisProps,
@@ -12,22 +11,22 @@ import {
1211
DefaultedYAxis,
1312
DefaultedAxis,
1413
AxisValueFormatterContext,
15-
isSymlogScaleConfig,
14+
ComputedAxis,
1615
} from '../../../../models/axis';
1716
import { CartesianChartSeriesType, ChartSeriesType } from '../../../../models/seriesType/config';
18-
import { getColorScale, getOrdinalColorScale } from '../../../colorScale';
19-
import { getTickNumber, scaleTickNumberByRange } from '../../../ticks';
17+
import { getColorScale, getOrdinalColorScale, getSequentialColorScale } from '../../../colorScale';
18+
import { scaleTickNumberByRange } from '../../../ticks';
2019
import { getScale } from '../../../getScale';
2120
import { isDateData, createDateFormatter } from '../../../dateHelpers';
22-
import { zoomScaleRange } from './zoom';
2321
import { getAxisExtremum } from './getAxisExtremum';
2422
import type { ChartDrawingArea } from '../../../../hooks';
2523
import { ChartSeriesConfig } from '../../models/seriesConfig';
2624
import { ComputedAxisConfig, DefaultizedZoomOptions } from './useChartCartesianAxis.types';
2725
import { ProcessedSeries } from '../../corePlugins/useChartSeries/useChartSeries.types';
2826
import { GetZoomAxisFilters, ZoomData } from './zoom.types';
2927
import { getAxisTriggerTooltip } from './getAxisTriggerTooltip';
30-
import { getAxisDomainLimit } from './getAxisDomainLimit';
28+
import { applyDomainLimit, getDomainLimit, ScaleDefinition } from './getAxisScale';
29+
import { isBandScale, isOrdinalScale } from '../../../scaleGuards';
3130

3231
function getRange(
3332
drawingArea: ChartDrawingArea,
@@ -51,6 +50,7 @@ export type ComputeResult<T extends ChartsAxisProps> = {
5150
};
5251

5352
type ComputeCommonParams<T extends ChartSeriesType = ChartSeriesType> = {
53+
scales: Record<AxisId, ScaleDefinition>;
5454
drawingArea: ChartDrawingArea;
5555
formattedSeries: ProcessedSeries<T>;
5656
seriesConfig: ChartSeriesConfig<T>;
@@ -76,6 +76,7 @@ export function computeAxisValue<T extends ChartSeriesType>(
7676
},
7777
): ComputeResult<ChartsXAxisProps>;
7878
export function computeAxisValue<T extends ChartSeriesType>({
79+
scales,
7980
drawingArea,
8081
formattedSeries,
8182
axis: allAxis,
@@ -106,138 +107,137 @@ export function computeAxisValue<T extends ChartSeriesType>({
106107
const completeAxis: ComputedAxisConfig<ChartsAxisProps> = {};
107108
allAxis.forEach((eachAxis, axisIndex) => {
108109
const axis = eachAxis as Readonly<DefaultedAxis<ScaleName, any, Readonly<ChartsAxisProps>>>;
110+
const scaleDefinition = scales[axis.id];
111+
let scale = scaleDefinition.scale;
109112
const zoomOption = zoomOptions?.[axis.id];
110113
const zoom = zoomMap?.get(axis.id);
111114
const zoomRange: [number, number] = zoom ? [zoom.start, zoom.end] : [0, 100];
112115
const range = getRange(drawingArea, axisDirection, axis.reverse ?? false);
113116

114-
const [minData, maxData] = getAxisExtremum(
115-
axis,
116-
axisDirection,
117-
seriesConfig as ChartSeriesConfig<CartesianChartSeriesType>,
118-
axisIndex,
119-
formattedSeries,
120-
zoom === undefined && !zoomOption ? getFilters : undefined, // Do not apply filtering if zoom is already defined.
121-
);
122-
123117
const triggerTooltip = !axis.ignoreTooltip && axisIdsTriggeringTooltip.has(axis.id);
124118

125119
const data = axis.data ?? [];
126120

127-
if (isBandScaleConfig(axis)) {
128-
const categoryGapRatio = axis.categoryGapRatio ?? DEFAULT_CATEGORY_GAP_RATIO;
129-
const barGapRatio = axis.barGapRatio ?? DEFAULT_BAR_GAP_RATIO;
121+
if (isOrdinalScale(scale)) {
130122
// Reverse range because ordinal scales are presented from top to bottom on y-axis
131123
const scaleRange = axisDirection === 'y' ? [range[1], range[0]] : range;
132-
const zoomedRange = zoomScaleRange(scaleRange, zoomRange);
133-
134-
completeAxis[axis.id] = {
135-
offset: 0,
136-
height: 0,
137-
categoryGapRatio,
138-
barGapRatio,
139-
triggerTooltip,
140-
...axis,
141-
data,
142-
scale: scaleBand(axis.data!, zoomedRange)
143-
.paddingInner(categoryGapRatio)
144-
.paddingOuter(categoryGapRatio / 2),
145-
tickNumber: axis.data!.length,
146-
colorScale:
147-
axis.colorMap &&
148-
(axis.colorMap.type === 'ordinal'
149-
? getOrdinalColorScale({ values: axis.data, ...axis.colorMap })
150-
: getColorScale(axis.colorMap)),
151-
};
152124

153-
if (isDateData(axis.data)) {
154-
const dateFormatter = createDateFormatter(axis.data, scaleRange, axis.tickNumber);
155-
completeAxis[axis.id].valueFormatter = axis.valueFormatter ?? dateFormatter;
125+
if (isBandScale(scale) && isBandScaleConfig(axis)) {
126+
const categoryGapRatio = axis.categoryGapRatio ?? DEFAULT_CATEGORY_GAP_RATIO;
127+
const barGapRatio = axis.barGapRatio ?? DEFAULT_BAR_GAP_RATIO;
128+
129+
completeAxis[axis.id] = {
130+
offset: 0,
131+
height: 0,
132+
categoryGapRatio,
133+
barGapRatio,
134+
triggerTooltip,
135+
...axis,
136+
data,
137+
scale,
138+
tickNumber: axis.data!.length,
139+
colorScale:
140+
axis.colorMap &&
141+
(axis.colorMap.type === 'ordinal'
142+
? getOrdinalColorScale({ values: axis.data, ...axis.colorMap })
143+
: getColorScale(axis.colorMap)),
144+
};
145+
}
146+
147+
if (isPointScaleConfig(axis)) {
148+
completeAxis[axis.id] = {
149+
offset: 0,
150+
height: 0,
151+
triggerTooltip,
152+
...axis,
153+
data,
154+
scale,
155+
tickNumber: axis.data!.length,
156+
colorScale:
157+
axis.colorMap &&
158+
(axis.colorMap.type === 'ordinal'
159+
? getOrdinalColorScale({ values: axis.data, ...axis.colorMap })
160+
: getColorScale(axis.colorMap)),
161+
};
156162
}
157-
}
158-
if (isPointScaleConfig(axis)) {
159-
const scaleRange = axisDirection === 'y' ? [...range].reverse() : range;
160-
const zoomedRange = zoomScaleRange(scaleRange, zoomRange);
161-
162-
completeAxis[axis.id] = {
163-
offset: 0,
164-
height: 0,
165-
triggerTooltip,
166-
...axis,
167-
data,
168-
scale: scalePoint(axis.data!, zoomedRange),
169-
tickNumber: axis.data!.length,
170-
colorScale:
171-
axis.colorMap &&
172-
(axis.colorMap.type === 'ordinal'
173-
? getOrdinalColorScale({ values: axis.data, ...axis.colorMap })
174-
: getColorScale(axis.colorMap)),
175-
};
176163

177164
if (isDateData(axis.data)) {
178165
const dateFormatter = createDateFormatter(axis.data, scaleRange, axis.tickNumber);
179166
completeAxis[axis.id].valueFormatter = axis.valueFormatter ?? dateFormatter;
180167
}
168+
169+
return;
181170
}
182171

183172
if (axis.scaleType === 'band' || axis.scaleType === 'point') {
184173
// Could be merged with the two previous "if conditions" but then TS does not get that `axis.scaleType` can't be `band` or `point`.
185174
return;
186175
}
187176

188-
const scaleType = axis.scaleType ?? ('linear' as const);
189-
190-
const domainLimit = preferStrictDomainInLineCharts
191-
? getAxisDomainLimit(axis, axisDirection, axisIndex, formattedSeries)
192-
: (axis.domainLimit ?? 'nice');
193-
194-
const axisExtremums = [axis.min ?? minData, axis.max ?? maxData];
195-
196-
if (typeof domainLimit === 'function') {
197-
const { min, max } = domainLimit(minData, maxData);
198-
axisExtremums[0] = min;
199-
axisExtremums[1] = max;
200-
}
201-
202-
const rawTickNumber = getTickNumber({ ...axis, range, domain: axisExtremums });
177+
const rawTickNumber = scaleDefinition.tickNumber!;
178+
const continuousAxis = axis as Readonly<
179+
DefaultedAxis<ContinuousScaleName, any, Readonly<ChartsAxisProps>>
180+
>;
181+
const scaleType = continuousAxis.scaleType ?? ('linear' as const);
203182
const tickNumber = scaleTickNumberByRange(rawTickNumber, zoomRange);
204183

205-
const zoomedRange = zoomScaleRange(range, zoomRange);
206-
207-
const scale = getScale(scaleType, axisExtremums, zoomedRange);
184+
const filter = zoom === undefined && !zoomOption ? getFilters : undefined; // Do not apply filtering if zoom is already defined.
185+
if (filter) {
186+
const [minData, maxData] = getAxisExtremum(
187+
axis,
188+
axisDirection,
189+
seriesConfig as ChartSeriesConfig<CartesianChartSeriesType>,
190+
axisIndex,
191+
formattedSeries,
192+
filter,
193+
);
194+
scale = scale.copy();
195+
scale.domain([minData, maxData]);
196+
197+
const domainLimit = getDomainLimit(
198+
axis,
199+
axisDirection,
200+
axisIndex,
201+
formattedSeries,
202+
preferStrictDomainInLineCharts,
203+
);
204+
205+
const axisExtrema = [axis.min ?? minData, axis.max ?? maxData];
206+
207+
if (typeof domainLimit === 'function') {
208+
const { min, max } = domainLimit(minData, maxData);
209+
axisExtrema[0] = min;
210+
axisExtrema[1] = max;
211+
}
208212

209-
if (isSymlogScaleConfig(axis) && axis.constant != null) {
210-
(scale as ScaleSymLog<number, number>).constant(axis.constant);
213+
scale.domain(axisExtrema);
214+
applyDomainLimit(scale, axis, domainLimit, rawTickNumber);
211215
}
212216

213-
const finalScale = domainLimit === 'nice' ? scale.nice(rawTickNumber) : scale;
214-
const [minDomain, maxDomain] = finalScale.domain();
215-
const domain = [axis.min ?? minDomain, axis.max ?? maxDomain];
216-
217217
completeAxis[axis.id] = {
218218
offset: 0,
219219
height: 0,
220220
triggerTooltip,
221-
...axis,
221+
...continuousAxis,
222222
data,
223-
scaleType: scaleType as any,
224-
scale: finalScale.domain(domain) as any,
223+
scaleType,
224+
scale,
225225
tickNumber,
226-
colorScale: axis.colorMap && getColorScale(axis.colorMap),
226+
colorScale: continuousAxis.colorMap && getSequentialColorScale(continuousAxis.colorMap),
227227
valueFormatter:
228228
axis.valueFormatter ??
229229
(createScalarFormatter(
230230
tickNumber,
231231
getScale(
232-
scaleType,
232+
scaleType as ContinuousScaleName,
233233
range.map((v) => scale.invert(v)),
234234
range,
235235
),
236236
) as <TScaleName extends ScaleName>(
237237
value: any,
238238
context: AxisValueFormatterContext<TScaleName>,
239239
) => string),
240-
};
240+
} as ComputedAxis<ContinuousScaleName, any, ChartsAxisProps>;
241241
});
242242
return {
243243
axis: completeAxis,

0 commit comments

Comments
 (0)