Skip to content

Commit 52ceb92

Browse files
committed
fix: (1) Fix dataZoom AxisProxy can not be cleared when dataZoom option changed. (2) Fix onZero on double value axis and dataZoom is applied on a base axis - onZero should be disabled by default.
1 parent bdec91e commit 52ceb92

15 files changed

Lines changed: 190 additions & 65 deletions

File tree

src/component/dataZoom/AxisProxy.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { isOrdinalScale, isTimeScale } from '../../scale/helper';
3535
import {
3636
AXIS_EXTENT_INFO_BUILD_FROM_DATA_ZOOM, scaleRawExtentInfoReallyCreate,
3737
} from '../../coord/scaleRawExtentInfo';
38+
import { suppressOnAxisZero } from '../../coord/axisHelper';
3839

3940

4041
interface MinMaxSpan {
@@ -56,6 +57,8 @@ interface AxisProxyWindow {
5657
}
5758

5859
/**
60+
* NOTICE: Its lifetime is different from `Axis` instance. It is recreated in each run of "ec prepare".
61+
*
5962
* Operate single axis.
6063
* One axis can only operated by one axis operator.
6164
* Different dataZoomModels may be defined to operate the same axis.
@@ -156,7 +159,8 @@ class AxisProxy {
156159
const dataExtent = this._dataExtent;
157160
const axis = this.getAxisModel().axis;
158161
const scale = axis.scale;
159-
const rangePropMode = this._dataZoomModel.getRangePropMode();
162+
const dataZoomModel = this._dataZoomModel;
163+
const rangePropMode = dataZoomModel.getRangePropMode();
160164
const percentExtent = [0, 100];
161165
const percentWindow = [] as unknown as [number, number];
162166
const valueWindow = [] as unknown as [number, number];
@@ -262,6 +266,13 @@ class AxisProxy {
262266
const pxSpan = mathAbs(pxExtent[1] - pxExtent[0]);
263267
const precision = isScaleOrdinalOrTime
264268
? 0
269+
// NOTICE: We deliberately do not allow specifying this precision by users, until real requirements
270+
// occur. Otherwise, unnecessary complexity and bad case may be introduced. A small precision may
271+
// cause the rounded ends overflow the expected min/max significantly. And this precision effectively
272+
// determines the size of a roaming step, and a big step would likely constantly cut through series
273+
// shapes in an unexpected place and cause visual artifacts (e.g., for bar series). Although
274+
// theroetically that defect can be resolved by introducing extra spaces between axis min/max tick
275+
// and axis boundary (see `SCALE_EXTENT_KIND_MAPPING`), it's complicated and unnecessary.
265276
: getAcceptableTickPrecision(valueWindow, pxSpan, 0.5);
266277
each([[0, mathCeil], [1, mathFloor]] as const, function ([idx, ceilOrFloor]) {
267278
if (!needRound[idx] || !isFinite(precision)) {
@@ -324,6 +335,8 @@ class AxisProxy {
324335
const axis = this.getAxisModel().axis;
325336
scaleRawExtentInfoReallyCreate(this.ecModel, axis, AXIS_EXTENT_INFO_BUILD_FROM_DATA_ZOOM);
326337

338+
suppressOnAxisZero(axis, {dz: true});
339+
327340
const rawExtentInfo = axis.scale.rawExtentInfo;
328341
this._dataExtent = rawExtentInfo.makeForZoom();
329342

src/component/dataZoom/DataZoomModel.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import {
2424
LayoutOrient,
2525
ComponentOption,
2626
LabelOption,
27-
NullUndefined
2827
} from '../../util/types';
2928
import Model from '../../model/Model';
3029
import GlobalModel from '../../model/Global';

src/component/dataZoom/SliderZoomModel.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,11 @@ export interface SliderDataZoomOption
107107
* Height of handle rect. Can be a percent string relative to the slider height.
108108
*/
109109
moveHandleSize?: number
110-
110+
/**
111+
* The precision only used on displayed labels.
112+
* NOTICE: Specifying the "value precision" or "roaming step" is not allowed.
113+
* `getAcceptableTickPrecision` is used for that. See `AxisProxy` for reasons.
114+
*/
111115
labelPrecision?: number | 'auto'
112116

113117
labelFormatter?: string | ((value: number, valueStr: string) => string)

src/component/dataZoom/dataZoomProcessor.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,12 @@
2020
import {createHashMap, each} from 'zrender/src/core/util';
2121
import SeriesModel from '../../model/Series';
2222
import DataZoomModel from './DataZoomModel';
23-
import { getAxisMainType, DataZoomAxisDimension, DataZoomExtendedAxisBaseModel, getAlignTo } from './helper';
23+
import {
24+
getAxisMainType, DataZoomAxisDimension, getAlignTo, getAxisProxyFromModel, setAxisProxyToModel
25+
} from './helper';
2426
import AxisProxy from './AxisProxy';
2527
import { StageHandler } from '../../util/types';
28+
import { AxisBaseModel } from '../../coord/AxisBaseModel';
2629

2730

2831
const dataZoomProcessor: StageHandler = {
@@ -36,31 +39,27 @@ const dataZoomProcessor: StageHandler = {
3639
cb: (
3740
axisDim: DataZoomAxisDimension,
3841
axisIndex: number,
39-
axisModel: DataZoomExtendedAxisBaseModel,
42+
axisModel: AxisBaseModel,
4043
dataZoomModel: DataZoomModel
4144
) => void
4245
) {
4346
ecModel.eachComponent('dataZoom', function (dataZoomModel: DataZoomModel) {
4447
dataZoomModel.eachTargetAxis(function (axisDim, axisIndex) {
4548
const axisModel = ecModel.getComponent(getAxisMainType(axisDim), axisIndex);
46-
cb(axisDim, axisIndex, axisModel as DataZoomExtendedAxisBaseModel, dataZoomModel);
49+
cb(axisDim, axisIndex, axisModel as AxisBaseModel, dataZoomModel);
4750
});
4851
});
4952
}
5053
// FIXME: it brings side-effect to `getTargetSeries`.
51-
// Prepare axis proxies.
52-
eachAxisModel(function (axisDim, axisIndex, axisModel, dataZoomModel) {
53-
// dispose all last axis proxy, in case that some axis are deleted.
54-
axisModel.__dzAxisProxy = null;
55-
});
5654
const proxyList: AxisProxy[] = [];
5755
eachAxisModel(function (axisDim, axisIndex, axisModel, dataZoomModel) {
5856
// Different dataZooms may control the same axis. In that case,
5957
// an axisProxy serves both of them.
60-
if (!axisModel.__dzAxisProxy) {
58+
if (!getAxisProxyFromModel(axisModel)) {
6159
// Use the first dataZoomModel as the main model of axisProxy.
62-
axisModel.__dzAxisProxy = new AxisProxy(axisDim, axisIndex, dataZoomModel, ecModel);
63-
proxyList.push(axisModel.__dzAxisProxy);
60+
const axisProxy = new AxisProxy(axisDim, axisIndex, dataZoomModel, ecModel);
61+
proxyList.push(axisProxy);
62+
setAxisProxyToModel(axisModel, axisProxy);
6463
}
6564
});
6665

@@ -135,4 +134,4 @@ const dataZoomProcessor: StageHandler = {
135134
}
136135
};
137136

138-
export default dataZoomProcessor;
137+
export default dataZoomProcessor;

src/component/dataZoom/helper.ts

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import SeriesModel from '../../model/Series';
2525
import { CoordinateSystemHostModel } from '../../coord/CoordinateSystem';
2626
import { AxisBaseModel } from '../../coord/AxisBaseModel';
2727
import type AxisProxy from './AxisProxy';
28+
import { getCachePerECPrepare, GlobalModelCachePerECPrepare, makeInner } from '../../util/model';
29+
import type ComponentModel from '../../model/Component';
2830

2931

3032
export interface DataZoomPayloadBatchItem {
@@ -37,17 +39,13 @@ export interface DataZoomPayloadBatchItem {
3739

3840
export interface DataZoomReferCoordSysInfo {
3941
model: CoordinateSystemHostModel;
40-
// Notice: if two dataZooms refer the same coordinamte system model,
41-
// (1) The axis they refered may different
42+
// Notice: if two dataZooms refer the same coordinate system model,
43+
// (1) The axis they referred may different
4244
// (2) The sequence the axisModels matters, may different in
4345
// different dataZooms.
4446
axisModels: AxisBaseModel[];
4547
}
4648

47-
export type DataZoomExtendedAxisBaseModel = AxisBaseModel & {
48-
__dzAxisProxy: AxisProxy
49-
};
50-
5149
export const DATA_ZOOM_AXIS_DIMENSIONS = [
5250
'x', 'y', 'radius', 'angle', 'single'
5351
] as const;
@@ -61,6 +59,12 @@ type DataZoomAxisIdPropName =
6159
'xAxisId' | 'yAxisId' | 'radiusAxisId' | 'angleAxisId' | 'singleAxisId';
6260
export type DataZoomCoordSysMainType = 'polar' | 'grid' | 'singleAxis';
6361

62+
const ecModelCacheInner = makeInner<{
63+
axisProxyMap: AxisProxyMap;
64+
}, GlobalModelCachePerECPrepare>();
65+
66+
type AxisProxyMap = HashMap<AxisProxy, ComponentModel['uid']>;
67+
6468
// Supported coords.
6569
// FIXME: polar has been broken (but rarely used).
6670
const SERIES_COORDS = ['cartesian2d', 'polar', 'singleAxis'] as const;
@@ -211,8 +215,28 @@ export function collectReferCoordSysModelInfo(dataZoomModel: DataZoomModel): {
211215
return coordSysInfoWrap;
212216
}
213217

214-
export function getAxisProxyFromModel(axisModel: AxisBaseModel): AxisProxy | NullUndefined {
215-
return axisModel && (axisModel as DataZoomExtendedAxisBaseModel).__dzAxisProxy;
218+
function ensureAxisProxyMap(ecModel: GlobalModel): AxisProxyMap {
219+
// Consider some axes may be deleted, and dataZoom options may be changed at and only at each run of
220+
// "ec prepare", we save axis proxies to a cache that is auto-cleared for each run of "ec prepare".
221+
const store = ecModelCacheInner(getCachePerECPrepare(ecModel));
222+
return store.axisProxyMap || (store.axisProxyMap = createHashMap());
223+
}
224+
225+
export function getAxisProxyFromModel(axisModel: AxisBaseModel | NullUndefined): AxisProxy | NullUndefined {
226+
if (!axisModel) {
227+
return;
228+
}
229+
if (__DEV__) {
230+
assert(axisModel.ecModel);
231+
}
232+
return ensureAxisProxyMap(axisModel.ecModel).get(axisModel.uid);
233+
}
234+
235+
export function setAxisProxyToModel(axisModel: AxisBaseModel, axisProxy: AxisProxy): void {
236+
if (__DEV__) {
237+
assert(axisModel.ecModel);
238+
}
239+
ensureAxisProxyMap(axisModel.ecModel).set(axisModel.uid, axisProxy);
216240
}
217241

218242
/**
@@ -221,7 +245,7 @@ export function getAxisProxyFromModel(axisModel: AxisBaseModel): AxisProxy | Nul
221245
* then do not input it into `AxisProxy#reset`.
222246
*/
223247
export function getAlignTo(dataZoomModel: DataZoomModel, axisProxy: AxisProxy): AxisProxy | NullUndefined {
224-
const alignToAxis = axisProxy.getAxisModel().axis.alignTo;
248+
const alignToAxis = axisProxy.getAxisModel().axis.__alignTo;
225249
return (
226250
alignToAxis && dataZoomModel.getAxisProxy(
227251
alignToAxis.dim as DataZoomAxisDimension,

src/coord/Axis.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,8 @@ class Axis {
8282
// `inverse` can be inferred by `extent` unless `extent[0] === extent[1]`.
8383
inverse: AxisBaseOption['inverse'] = false;
8484

85-
// Injected outside
86-
alignTo: Axis;
87-
// Injected outside.
88-
suggestNotOnZeroOfMe: boolean;
85+
// To be injected outside. May change - do not use it outside of echarts.
86+
__alignTo: Axis;
8987

9088

9189
constructor(dim: DimensionName, scale: Scale, extent: [number, number]) {

src/coord/AxisBaseModel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,5 @@ export interface AxisBaseModel<T extends AxisBaseOptionCommon = AxisBaseOptionCo
3131
AxisModelCommonMixin<T>,
3232
AxisModelExtendedInCreator {
3333

34-
axis: Axis
34+
axis: Axis;
3535
}

src/coord/axisHelper.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,21 @@ import {
5050
extentDiffers, isLogScale, isOrdinalScale
5151
} from '../scale/helper';
5252
import { AxisModelExtendedInCreator } from './axisModelCreator';
53-
import { initExtentForUnion } from '../util/model';
53+
import { initExtentForUnion, makeInner } from '../util/model';
5454
import { ComponentModel } from '../echarts.simple';
5555
import { SCALE_EXTENT_KIND_EFFECTIVE, SCALE_MAPPER_DEPTH_OUT_OF_BREAK } from '../scale/scaleMapper';
5656

5757

58+
const axisInner = makeInner<{
59+
noOnMyZero: SuppressOnAxisZeroReason;
60+
}, Axis>();
61+
62+
type SuppressOnAxisZeroReason = {
63+
dz?: boolean;
64+
base?: boolean
65+
};
66+
67+
5868
export function determineAxisType(
5969
model: Model<Pick<AxisBaseOption, 'type'>>
6070
): OptionAxisType {
@@ -137,6 +147,21 @@ export function ifAxisCrossZero(axis: Axis) {
137147
return !((min > 0 && max > 0) || (min < 0 && max < 0));
138148
}
139149

150+
export function suppressOnAxisZero(axis: Axis, reason: Partial<SuppressOnAxisZeroReason>): void {
151+
zrUtil.defaults(axisInner(axis).noOnMyZero || (axisInner(axis).noOnMyZero = {}), reason);
152+
}
153+
154+
/**
155+
* `true`: Prevent orthoganal axes from positioning at the zero point of this axis.
156+
*/
157+
export function isOnAxisZeroSuppressed(axis: Axis): boolean {
158+
const dontOnAxisZero = axisInner(axis).noOnMyZero;
159+
// Empirically, onZero causes weired effect when dataZoom is used on an "base axis". Consider
160+
// bar series as an example. And also consider when `SCALE_EXTENT_KIND_MAPPING` is used, where
161+
// the axis line is likely to cross the series shapes unexpectedly.
162+
return dontOnAxisZero && dontOnAxisZero.dz && dontOnAxisZero.base;
163+
}
164+
140165
/**
141166
* @param axis
142167
* @return Label formatter function.

src/coord/cartesian/Cartesian2D.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,11 @@ class Cartesian2D extends Cartesian<Axis2D> implements CoordinateSystem {
8888
}
8989

9090
/**
91-
* Base axis will be used on stacking.
91+
* Base axis will be used on stacking and series such as 'bar', 'pictorialBar', etc.
9292
*/
9393
getBaseAxis(): Axis2D {
94+
// PENGING: Should we allow bar series to specify a base axis when
95+
// both axes are type "value", rather than force to xAxis?
9496
return this.getAxesByScale('ordinal')[0]
9597
|| this.getAxesByScale('time')[0]
9698
|| this.getAxis('x');

src/coord/cartesian/Grid.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
shouldAxisShow,
3333
retrieveAxisBreaksOption,
3434
determineAxisType,
35+
suppressOnAxisZero,
36+
isOnAxisZeroSuppressed,
3537
} from '../../coord/axisHelper';
3638
import Cartesian2D, {cartesian2DDimensions} from './Cartesian2D';
3739
import Axis2D from './Axis2D';
@@ -74,7 +76,6 @@ import {
7476
AXIS_EXTENT_INFO_BUILD_FROM_COORD_SYS_UPDATE,
7577
scaleRawExtentInfoEnableBoxCoordSysUsage, scaleRawExtentInfoReallyCreate, scaleRawExtentInfoRequireCreate
7678
} from '../scaleRawExtentInfo';
77-
import { SCALE_EXTENT_KIND_MAPPING } from '../../scale/scaleMapper';
7879
import { hasBreaks } from '../../scale/break';
7980

8081

@@ -144,21 +145,21 @@ class Grid implements CoordinateSystemMaster {
144145

145146
for (let i = axesIndices.length - 1; i >= 0; i--) { // Reverse order
146147
const axis = axes[+axesIndices[i]];
147-
if (axis.alignTo) {
148+
if (axis.__alignTo) {
148149
axisNeedsAlign.push(axis);
149150
}
150151
else {
151152
scaleCalcNice(axis);
152153
}
153154
};
154155
each(axisNeedsAlign, axis => {
155-
if (incapableOfAlignNeedFallback(axis, axis.alignTo as Axis2D)) {
156+
if (incapableOfAlignNeedFallback(axis, axis.__alignTo as Axis2D)) {
156157
scaleCalcNice(axis);
157158
}
158159
else {
159160
scaleCalcAlign(
160161
axis,
161-
axis.alignTo.scale as IntervalScale | LogScale
162+
axis.__alignTo.scale as IntervalScale | LogScale
162163
);
163164
}
164165
});
@@ -445,6 +446,8 @@ class Grid implements CoordinateSystemMaster {
445446

446447
cartesian.addAxis(xAxis);
447448
cartesian.addAxis(yAxis);
449+
450+
suppressOnAxisZero(cartesian.getBaseAxis(), {base: true});
448451
});
449452
});
450453

@@ -666,17 +669,18 @@ function fixAxisOnZero(
666669
}
667670
}
668671

672+
/**
673+
* CAVEAT: Must not be called before `CoordinateSystem#update` due to `__dontOnMyZero`.
674+
*/
669675
function canOnZeroToAxis(
670676
onZeroOption: AxisBaseOptionCommon['axisLine']['onZero'],
671677
axis: Axis2D
672678
): boolean {
679+
// PENDING: Historical behavior: `onZero` on 'category' and 'time' axis are always disabled
680+
// even if ec option gives `onZero: true`.
673681
let can = axis && axis.type !== 'category' && axis.type !== 'time' && ifAxisCrossZero(axis);
674-
if (can && onZeroOption === 'auto') {
675-
if (axis.scale.getExtentUnsafe(SCALE_EXTENT_KIND_MAPPING, null)) {
676-
// Empirically, onZero is inappropriate when `SCALE_EXTENT_KIND_MAPPING` is
677-
// used - the axis line is likely to cross the series shapes unexpectedly.
678-
can = false;
679-
}
682+
if (can && onZeroOption === 'auto' && isOnAxisZeroSuppressed(axis)) {
683+
can = false;
680684
}
681685
// falsy value of `onZeroOption` has been handled in the previous logic.
682686
return can;
@@ -722,7 +726,7 @@ function prepareAlignToInCoordSysCreate(axes: Record<number, Axis2D>): void {
722726
}
723727
if (alignTo) {
724728
each(axisNeedsAlign, function (axis) {
725-
axis.alignTo = alignTo;
729+
axis.__alignTo = alignTo;
726730
});
727731
}
728732
}

0 commit comments

Comments
 (0)