Skip to content

Commit 18a23a8

Browse files
committed
refactor(scale): For readability and maintainability (1) Migrate calcNiceTicks and calcNiceExtent from Scale class override member functions to plain functions, similar to axisAlignTicks. Previously it's hard to modify and error-prone. (2) Remove unnecessary override on Scale class hierarchy and limit override usage, which is difficult to understand and error-prone. (3) Simplify the inheritance - shift LogScale and TimeScale inheritance from IntervalScale to Scale. (4) Clarify the impl of IntervalScale - uniform the parameters setting entry for both "nice ticks" and "align ticks".
1 parent a6ab245 commit 18a23a8

25 files changed

Lines changed: 703 additions & 523 deletions

src/component/dataZoom/AxisProxy.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,7 @@ class AxisProxy {
458458

459459
const axisModel = this.getAxisModel();
460460

461-
const window = this._window;
462-
if (!window) {
463-
return;
464-
}
465-
const {percent, value} = window;
461+
const {percent, value} = this._window;
466462

467463
// For value axis, if min/max/scale are not set, we just use the extent obtained
468464
// by series data, which may be a little different from the extent calculated by

src/component/timeline/SliderTimelineView.ts

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import * as graphic from '../../util/graphic';
2323
import { createTextStyle } from '../../label/labelStyle';
2424
import * as layout from '../../util/layout';
2525
import TimelineView from './TimelineView';
26-
import TimelineAxis from './TimelineAxis';
26+
import TimelineAxis, { TimelineAxisType } from './TimelineAxis';
2727
import {createSymbol, normalizeSymbolOffset, normalizeSymbolSize} from '../../util/symbol';
2828
import * as numberUtil from '../../util/number';
2929
import GlobalModel from '../../model/Global';
@@ -35,17 +35,16 @@ import TimelineModel, { TimelineDataItemOption, TimelineCheckpointStyle } from '
3535
import { TimelineChangePayload, TimelinePlayChangePayload } from './timelineAction';
3636
import Model from '../../model/Model';
3737
import { PathProps, PathStyleProps } from 'zrender/src/graphic/Path';
38-
import Scale from '../../scale/Scale';
39-
import OrdinalScale from '../../scale/Ordinal';
40-
import TimeScale from '../../scale/Time';
41-
import IntervalScale from '../../scale/Interval';
4238
import { VectorArray } from 'zrender/src/core/vector';
4339
import { parsePercent } from 'zrender/src/contain/text';
4440
import { makeInner } from '../../util/model';
4541
import { getECData } from '../../util/innerStore';
4642
import { enableHoverEmphasis } from '../../util/states';
4743
import { createTooltipMarkup } from '../tooltip/tooltipMarkup';
4844
import Displayable from 'zrender/src/graphic/Displayable';
45+
import { createScaleByModel } from '../../coord/axisHelper';
46+
import { OptionAxisType } from '../../coord/axisCommonTypes';
47+
import { scaleCalcNiceReal } from '../../coord/axisNiceTicks';
4948

5049
const PI = Math.PI;
5150

@@ -338,9 +337,12 @@ class SliderTimelineView extends TimelineView {
338337

339338
private _createAxis(layoutInfo: LayoutInfo, timelineModel: SliderTimelineModel) {
340339
const data = timelineModel.getData();
341-
const axisType = timelineModel.get('axisType');
340+
let axisType = timelineModel.get('axisType') || timelineModel.get('type') as TimelineAxisType;
341+
if (axisType !== 'category' && axisType !== 'time') {
342+
axisType = 'value';
343+
}
342344

343-
const scale = createScaleByModel(timelineModel, axisType);
345+
const scale = createScaleByModel(timelineModel, axisType as OptionAxisType);
344346

345347
// Customize scale. The `tickValue` is `dataIndex`.
346348
scale.getTicks = function () {
@@ -351,7 +353,7 @@ class SliderTimelineView extends TimelineView {
351353

352354
const dataExtent = data.getDataExtent('value');
353355
scale.setExtent(dataExtent[0], dataExtent[1]);
354-
scale.calcNiceTicks();
356+
scaleCalcNiceReal(scale, {fixMinMax: [true, true]});
355357

356358
const axis = new TimelineAxis('value', scale, layoutInfo.axisExtent as [number, number], axisType);
357359
axis.model = timelineModel;
@@ -730,28 +732,6 @@ class SliderTimelineView extends TimelineView {
730732
}
731733
}
732734

733-
function createScaleByModel(model: SliderTimelineModel, axisType?: string): Scale {
734-
axisType = axisType || model.get('type');
735-
if (axisType) {
736-
switch (axisType) {
737-
// Buildin scale
738-
case 'category':
739-
return new OrdinalScale({
740-
ordinalMeta: model.getCategories(),
741-
extent: [Infinity, -Infinity]
742-
});
743-
case 'time':
744-
return new TimeScale({
745-
locale: model.ecModel.getLocaleModel(),
746-
useUTC: model.ecModel.get('useUTC')
747-
});
748-
default:
749-
// default to be value
750-
return new IntervalScale();
751-
}
752-
}
753-
}
754-
755735

756736
function getViewRect(model: SliderTimelineModel, api: ExtensionAPI) {
757737
return layout.getLayoutRect(

src/component/timeline/TimelineAxis.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ import TimelineModel from './TimelineModel';
2323
import { LabelOption } from '../../util/types';
2424
import Model from '../../model/Model';
2525

26+
export type TimelineAxisType = 'category' | 'time' | 'value';
27+
2628
/**
2729
* Extend axis 2d
2830
*/
2931
class TimelineAxis extends Axis {
3032

31-
type: 'category' | 'time' | 'value';
33+
type: TimelineAxisType;
3234

3335
// @ts-ignore
3436
model: TimelineModel;
@@ -37,7 +39,7 @@ class TimelineAxis extends Axis {
3739
dim: string,
3840
scale: Scale,
3941
coordExtent: [number, number],
40-
axisType: 'category' | 'time' | 'value'
42+
axisType: TimelineAxisType
4143
) {
4244
super(dim, scale, coordExtent);
4345
this.type = axisType || 'value';

src/component/timeline/TimelineModel.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,6 @@ class TimelineModel extends ComponentModel<TimelineOption> {
292292
return this._data;
293293
}
294294

295-
/**
296-
* @public
297-
* @return {Array.<string>} categoreis
298-
*/
299295
getCategories() {
300296
if (this.get('axisType') === 'category') {
301297
return this._names.slice();

src/coord/axisAlignTicks.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export function alignScaleTicks(
113113
// At least one nice segment is present, and not-nice segments are only present on
114114
// the start and/or the end.
115115
// In this case, ticks corresponding to nice segments are made "nice".
116-
const alignToInterval = alignToScaleLinear.getInterval();
116+
const alignToInterval = alignToScaleLinear.getConfig().interval;
117117
t0 = (
118118
1 - (alignToTicks[0].value - alignToExpNiceTicks[0].value) / alignToInterval
119119
) % 1;
@@ -282,23 +282,28 @@ export function alignScaleTicks(
282282
const currIntervalCount = mathRound((maxNice - minNice) / interval);
283283
if (currIntervalCount <= alignToNiceSegCount) {
284284
const moreCount = alignToNiceSegCount - currIntervalCount;
285-
// Consider cases that negative series data do not make sense (or vice versa), users can
286-
// simply specify `xxxAxis.min/max: 0` to achieve that. But we still optimize it for some
287-
// common default cases whenever possible, especially when ec option `xxxAxis.scale: false`
288-
// (the default), it is usually unexpected if negative (or positive) ticks are introduced.
285+
// Consider cases that negative tick do not make sense (or vice versa), users can simply
286+
// specify `xxxAxis.min/max: 0` to avoid negative. But we still automatically handle it
287+
// for some common cases whenever possible:
288+
// - When ec option is `xxxAxis.scale: false` (the default), it is usually unexpected if
289+
// negative (or positive) ticks are introduced.
290+
// - In LogScale, series data are usually either all > 1 or all < 1, rather than both,
291+
// that is, logarithm result is typically either all positive or all negative.
289292
let moreCountPair: number[];
290-
const needCrossZero = targetExtentInfo.needCrossZero;
291-
if (needCrossZero && targetExtent[0] === 0) {
293+
const mayEnhanceZero = targetExtentInfo.needCrossZero || isTargetLogScale;
294+
// `bounds < 0` or `bounds > 0` may require more complex handling, so we only auto handle
295+
// `bounds === 0`.
296+
if (mayEnhanceZero && targetExtent[0] === 0) {
292297
// 0 has been included in extent and all positive.
293298
moreCountPair = [0, moreCount];
294299
}
295-
else if (needCrossZero && targetExtent[1] === 0) {
300+
else if (mayEnhanceZero && targetExtent[1] === 0) {
296301
// 0 has been included in extent and all negative.
297302
moreCountPair = [moreCount, 0];
298303
}
299304
else {
300-
// Try to arrange tick in the middle as possible corresponding to the given `alignTo`
301-
// ticks, which is especially preferable in `LogScale`.
305+
// Try to center ticks in axis space whenever possible, which is especially preferable
306+
// in `LogScale`.
302307
const lessHalfCount = mathFloor(moreCount / 2);
303308
moreCountPair = moreCount % 2 === 0 ? [lessHalfCount, lessHalfCount]
304309
: (min + max) < (targetExtent[0] + targetExtent[1]) ? [lessHalfCount, lessHalfCount + 1]
@@ -325,9 +330,9 @@ export function alignScaleTicks(
325330
]
326331
: [];
327332

328-
// NOTE: Must in setExtent -> setInterval order.
333+
// NOTE: Must in setExtent -> setConfigs order.
329334
targetScaleLinear.setExtent(min, max);
330-
targetScaleLinear.setInterval({
335+
targetScaleLinear.setConfig({
331336
// Even in LogScale, `interval` should not be in log space.
332337
interval,
333338
intervalCount,

src/coord/axisCommonTypes.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ import type { PrimaryTimeUnit } from '../util/time';
3434
export const AXIS_TYPES = {value: 1, category: 1, time: 1, log: 1} as const;
3535
export type OptionAxisType = keyof typeof AXIS_TYPES;
3636

37+
// `scale/Ordinal` | `scale/Interval` | `scale/Log` | `scale/Time`
38+
export type AxisScaleType = 'ordinal' | 'interval' | 'log' | 'time';
39+
3740
export interface AxisBaseOptionCommon extends ComponentOption,
3841
AnimationOptionMixin {
3942
type?: OptionAxisType;

src/coord/axisHelper.ts

Lines changed: 21 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
AxisLabelCategoryFormatter,
4242
AxisLabelValueFormatter,
4343
AxisLabelFormatterExtraParams,
44+
OptionAxisType,
4445
} from './axisCommonTypes';
4546
import CartesianAxisModel from './cartesian/AxisModel';
4647
import SeriesData from '../data/SeriesData';
@@ -50,7 +51,8 @@ import { ensureScaleRawExtentInfo, ScaleRawExtentResult } from './scaleRawExtent
5051
import { parseTimeAxisLabelFormatter } from '../util/time';
5152
import { getScaleBreakHelper } from '../scale/break';
5253
import { error } from '../util/log';
53-
import { isIntervalScale, isTimeScale } from '../scale/helper';
54+
import { isTimeScale } from '../scale/helper';
55+
import { AxisModelExtendedInCreator } from './axisModelCreator';
5456

5557

5658
type BarWidthAndOffset = ReturnType<typeof makeColumnLayout>;
@@ -157,43 +159,22 @@ function adjustScaleForOverflow(
157159
return {min: min, max: max};
158160
}
159161

160-
export function niceScaleExtent(
161-
scale: Scale,
162-
inModel: AxisBaseModel,
163-
// Typically: data extent from all series on this axis, which can be obtained by
164-
// `scale.unionExtentFromData(...); scale.getExtent();`.
165-
dataExtent: number[],
166-
): void {
167-
const model = inModel as AxisBaseModel<LogAxisBaseOption>;
168-
const extentInfo = adoptScaleExtentOptionAndPrepare(scale, model, dataExtent);
169-
170-
const isInterval = isIntervalScale(scale);
171-
const isIntervalOrTime = isInterval || isTimeScale(scale);
172-
173-
scale.setBreaksFromOption(retrieveAxisBreaksOption(model));
174-
scale.setExtent(extentInfo.min, extentInfo.max);
175-
scale.calcNiceExtent({
176-
splitNumber: model.get('splitNumber'),
177-
fixMinMax: [extentInfo.minFixed, extentInfo.maxFixed],
178-
minInterval: isIntervalOrTime ? model.get('minInterval') : null,
179-
maxInterval: isIntervalOrTime ? model.get('maxInterval') : null
180-
});
181-
182-
// If some one specified the min, max. And the default calculated interval
183-
// is not good enough. He can specify the interval. It is often appeared
184-
// in angle axis with angle 0 - 360. Interval calculated in interval scale is hard
185-
// to be 60.
186-
// In `xxxAxis.type: 'log'`, ec option `xxxAxis.interval` requires a logarithm-applied
187-
// value rather than a value in the raw scale.
188-
const interval = model.get('interval');
189-
if (interval != null && (scale as IntervalScale).setInterval) {
190-
(scale as IntervalScale).setInterval({interval});
191-
}
192-
}
193-
194-
export function createScaleByModel(model: AxisBaseModel): Scale {
195-
const axisType = model.get('type');
196-
switch (axisType) {
162+
export function createScaleByModel(
163+
model:
164+
Model<
165+
// Expect `Pick<AxisBaseOptionCommon, 'type'>`,
166+
// but be lenient for user's invalid input.
167+
{type?: string}
168+
& Pick<LogAxisBaseOption, 'logBase'>
169+
>
170+
& Partial<Pick<
171+
AxisModelExtendedInCreator,
172+
'getOrdinalMeta' | 'getCategories'
173+
>>,
174+
axisType?: OptionAxisType
175+
): Scale {
176+
const type = axisType || model.get('type');
177+
switch (type) {
197178
case 'category':
198179
return new OrdinalScale({
199180
ordinalMeta: model.getOrdinalMeta
@@ -208,10 +189,10 @@ export function createScaleByModel(model: AxisBaseModel): Scale {
208189
});
209190
case 'log':
210191
// See also #3749
211-
return new LogScale((model as AxisBaseModel<LogAxisBaseOption>).get('logBase'));
192+
return new LogScale(model.get('logBase'));
212193
default:
213194
// case 'value'/'interval', or others.
214-
return new (Scale.getClass(axisType) || IntervalScale)();
195+
return new (Scale.getClass(type) || IntervalScale)();
215196
}
216197
}
217198

0 commit comments

Comments
 (0)