Skip to content

Commit 3973b21

Browse files
committed
fix: (1) Fix: fix bar overflow issue on category axis with boundaryGap: false - use the newly added containShape handler. (2) Fix regression: Drop the previous containShape strategy on bar series, use a simple strategy for better compatibility. (3) Fix regression: Fix new added bar clip feature on single data item case. (4) Fix regression: Clarify code.
1 parent 25bb7da commit 3973b21

26 files changed

Lines changed: 818 additions & 321 deletions

src/chart/bar/BarView.ts

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -93,33 +93,6 @@ type RealtimeSortConfig = {
9393
// Return a number, based on which the ordinal sorted.
9494
type OrderMapping = (dataIndex: number) => number;
9595

96-
function createClipArea(coord: CoordSysOfBar, data: SeriesData) {
97-
const coordSysClipArea = coord.getArea && coord.getArea();
98-
if (isCoordinateSystemType<Cartesian2D>(coord, 'cartesian2d')) {
99-
const baseAxis = coord.getBaseAxis();
100-
// When boundaryGap is false in category axis, bar may exceed the grid.
101-
// We should not clip this part.
102-
// See test/bar2.html
103-
// FIXME:
104-
// When bar series lay out on `boundaryGap: false`, the effect is not
105-
// preferable - edge bars will overflow the Cartesian area.
106-
// It may be optimized following the way used in candlestick
107-
// (using `registerAxisContainShapeHandler`).
108-
if (baseAxis.type === 'category' && !baseAxis.onBand) {
109-
const expandWidth = data.getLayout('bandWidth');
110-
if (baseAxis.isHorizontal()) {
111-
(coordSysClipArea as CartesianCoordArea).x -= expandWidth;
112-
(coordSysClipArea as CartesianCoordArea).width += expandWidth * 2;
113-
}
114-
else {
115-
(coordSysClipArea as CartesianCoordArea).y -= expandWidth;
116-
(coordSysClipArea as CartesianCoordArea).height += expandWidth * 2;
117-
}
118-
}
119-
}
120-
121-
return coordSysClipArea as PolarCoordArea | CartesianCoordArea;
122-
}
12396

12497
class BarView extends ChartView {
12598
static type = 'bar' as const;
@@ -226,7 +199,8 @@ class BarView extends ChartView {
226199
}
227200

228201
const needsClip = seriesModel.get('clip', true) || realtimeSortCfg;
229-
const coordSysClipArea = createClipArea(coord, data);
202+
// Both `cartesian2d` and `polar` has `getArea()`.
203+
const coordSysClipArea = coord.getArea();
230204
// If there is clipPath created in large mode. Remove it.
231205
group.removeClipPath();
232206
// We don't use clipPath in normal mode because we needs a perfect animation

src/chart/helper/axisSnippets.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,21 @@ import { ComponentSubType } from '../../util/types';
2929

3030
/**
3131
* Require `requireAxisStatistics`.
32+
*
33+
* Simply expand `Scale` extent by half bandWidth.
34+
* Do nothing if an `OrdinalScale` has `boundaryGap: true`.
3235
*/
3336
export function createBandWidthBasedAxisContainShapeHandler(axisStatKey: AxisStatKey): AxisContainShapeHandler {
37+
38+
// [AXIS_CONTAIN_SHAPE_COMMON_STRATEGY]:
39+
// The calculation below is based on a proportion mapping
40+
// from `[barsBoundVal[0], barsBoundVal[1]]` to `[minValNew, maxValNew]`:
41+
// |------|------------------------------|---|
42+
// barsBoundVal[0] minValOld maxValOld barsBoundVal[1]
43+
// |----|----------------------|--|
44+
// minValNew minValOld maxValOld maxValNew
45+
// (Note: `|---|` above represents "pixels" rather than "data".)
46+
3447
return function (axis, scale, ecModel) {
3548
const bandWidthResult = calcBandWidth(axis, {fromStat: {key: axisStatKey}});
3649
const invRatio = bandWidthResult.invRatio;

src/component/dataZoom/AxisProxy.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ interface MinMaxSpan {
4949
export interface AxisProxyWindow {
5050
// NOTE: May include non-effective portion.
5151
value: number[];
52+
// Although `dataZoom` effectively calculates based on "noZoomMapMM" (where
53+
// containShape is considered), `dataZoom` labels always show `noZoomEffMM`.
5254
noZoomEffMM: ScaleRawExtentResultForZoom['noZoomEffMM'];
5355
percent: number[];
5456
// Percent invert from "value window", which may be slightly different from "percent window" due to some
@@ -375,10 +377,10 @@ class AxisProxy {
375377
// `axisHelper.getScaleExtent`. But the different just affects the experience a
376378
// little when zooming. So it will not be fixed until some users require it strongly.
377379
if (percent[0] !== 0) {
378-
rawExtentInfo.setZoomMinMax(0, value[0]);
380+
rawExtentInfo.setZoomMM(0, value[0]);
379381
}
380382
if (percent[1] !== 100) {
381-
rawExtentInfo.setZoomMinMax(1, value[1]);
383+
rawExtentInfo.setZoomMM(1, value[1]);
382384
}
383385
}
384386

src/coord/Axis.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,6 @@ function makeExtentWithBands(axis: Axis): number[] {
278278
const margin = size / (axis.scale as OrdinalScale).count() / 2;
279279
extent[0] += margin;
280280
extent[1] -= margin;
281-
282281
}
283282
return extent;
284283
}

src/coord/axisAlignTicks.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ import type Axis from './Axis';
3838

3939
/**
4040
* NOTE: See the summary of the process of extent determination in the comment of `scaleMapper.setExtent`.
41+
*
42+
* @see SCALE_EXTENT_CONSTRUCTION for the full processing flow.
4143
*/
4244
export function scaleCalcAlign(
4345
targetAxis: Axis,

src/coord/axisBand.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,21 +125,23 @@ function calcBandWidthForCategoryAxis(
125125
scale: Scale
126126
): void {
127127
const axisExtent = axis.getExtent();
128-
const dataExtent = scale.getExtent();
129128
const pxSpan = mathAbs(axisExtent[1] - axisExtent[0]);
130-
const linearScaleSpan = dataExtent[1] - dataExtent[0];
129+
// See the reason on BAND_WIDTH_USED_LINEAR_SCALE_SPAN.
130+
const linearScaleSpan = getScaleLinearSpanForMapping(scale);
131131
const onBand = axis.onBand;
132132

133133
let len = linearScaleSpan + (onBand ? 1 : 0);
134134
// Fix #2728, avoid NaN when only one data.
135135
len === 0 && (len = 1);
136136

137137
out.w = pxSpan / len;
138-
if (!onBand) {
138+
// NOTE:
139+
// - When `linearScaleSpan === 0`, no need to expand extent.
140+
// - `onBand: true` (`boundaryGap: true`) does not need to support `containShape`,
141+
// thereby no `invRatio`.
142+
if (!onBand && linearScaleSpan && pxSpan) {
139143
out.invRatio = linearScaleSpan / pxSpan;
140144
}
141-
// `onBand: true` (`boundaryGap: true`) does not need to support `containShape`,
142-
// thereby no `invRatio`.
143145
}
144146

145147
/**
@@ -185,7 +187,18 @@ function calcBandWidthForNumericAxis(
185187
const axisExtent = axis.getExtent();
186188
// Always use a new pxSpan because it may be changed in `grid` contain label calculation.
187189
const pxSpan = mathAbs(axisExtent[1] - axisExtent[0]);
190+
191+
// [BAND_WIDTH_USED_LINEAR_SCALE_SPAN]
192+
// Here we deliberately use `getScaleLinearSpanForMapping` rather than `scale.getExtent()`,
193+
// because band width should always respect to the currently specified extent (e.g., specified by
194+
// `calcContainShape`). Otherwise, the result may incorrect, especially when data count is small.
195+
// For example, when "containShape" is calculating, no `SCALE_EXTENT_KIND_MAPPING` is set, so here only
196+
// `SCALE_EXTENT_KIND_EFFECTIVE` is returned, say, `[3, 5]`, based on which a `SCALE_EXTENT_KIND_MAPPING`
197+
// is calculated, say `[2.5, 5.5]` (expanded by `0.5`). Then when rendering, that `SCALE_EXTENT_KIND_MAPPING`
198+
// is returned here.
199+
// See AXIS_CONTAIN_SHAPE_COMMON_STRATEGY for more details.
188200
const linearScaleSpan = getScaleLinearSpanForMapping(scale);
201+
189202
// `linearScaleSpan` may be `0` or `Infinity` or `NaN`, since normalizers like
190203
// `intervalScaleEnsureValidExtent` may not have been called yet.
191204
if (isNullableNumberFinite(linearScaleSpan) && linearScaleSpan > 0

src/coord/axisCommonTypes.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,11 @@ export interface AxisBaseOptionCommon extends ComponentOption,
142142
breakLabelLayout?: {
143143
moveOverlap?: 'auto' | boolean;
144144
}
145+
146+
// The axis contains the series shapes if possible, instead of overlowing at the edges.
147+
// null/undefined means `true`.
148+
// Not available on category axis with `boundaryGap: true`, but available with `boundaryGap: false`.
149+
containShape?: boolean;
145150
}
146151

147152
/**
@@ -155,11 +160,6 @@ export interface NumericAxisBaseOptionCommon extends AxisBaseOptionCommon {
155160

156161
boundaryGap?: NumericAxisBoundaryGapOption;
157162

158-
// The axis contains the series shapes if possible, instead of overlowing at the edges.
159-
// Key is series type, like 'bar', 'pictorialBar'.
160-
// null/undefined means `true`.
161-
containShape?: boolean;
162-
163163
/**
164164
* AxisTick and axisLabel and splitLine are calculated based on splitNumber.
165165
*/

src/coord/axisHelper.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,3 +374,7 @@ export function getTickValueOutermost(scale: Scale, tick: ScaleTick): number {
374374
? scale.getRawOrdinalNumber(tick.value)
375375
: tick.value;
376376
}
377+
378+
export function isAxisOnBand(scale: Scale, axisModel: AxisBaseModel): boolean {
379+
return isOrdinalScale(scale) && !!(axisModel as AxisBaseModel<CategoryAxisBaseOption>).get('boundaryGap');
380+
}

src/coord/axisNiceTicks.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ import type LogScale from '../scale/Log';
3535
import Scale from '../scale/Scale';
3636
import {
3737
adoptScaleExtentKindMapping, adoptScaleRawExtentInfoAndPrepare,
38-
ScaleExtentFixMinMax
38+
ScaleExtentFixMinMax,
39+
ScaleRawExtentResultFinal
3940
} from './scaleRawExtentInfo';
4041
import { getScaleLinearSpanEffective } from '../scale/scaleMapper';
4142
import { NullUndefined } from '../util/types';
@@ -58,7 +59,7 @@ function calcNiceForIntervalOrLogScale(
5859
const oldOutermostExtent = isTargetLogScale ? scale.getExtent() : null;
5960
const oldIntervalExtent = intervalStub.getExtent();
6061

61-
let newIntervalExtent = intervalScaleEnsureValidExtent(oldIntervalExtent, fixMinMax);
62+
let newIntervalExtent = intervalScaleEnsureValidExtent(oldIntervalExtent, fixMinMax, opt.rawExtentResult);
6263

6364
intervalStub.setExtent(newIntervalExtent[0], newIntervalExtent[1]);
6465
newIntervalExtent = intervalStub.getExtent();
@@ -183,13 +184,16 @@ type ScaleCalcNiceMethodOpt = {
183184
minInterval?: number;
184185
maxInterval?: number;
185186
fixMinMax?: ScaleExtentFixMinMax;
187+
rawExtentResult?: ScaleRawExtentResultFinal;
186188
};
187189

188190
/**
189191
* NOTE: See the summary of the process of extent determination in the comment of `scaleMapper.setExtent`.
190192
*
191193
* Calculate a "nice" extent and "nice" ticks configs based on the current scale extent and ec options.
192194
* scale extent will be modified, and config may be set to the scale.
195+
*
196+
* @see SCALE_EXTENT_CONSTRUCTION for the full processing flow.
193197
*/
194198
export function scaleCalcNice(
195199
axisLike: {
@@ -209,6 +213,9 @@ export function scaleCalcNice(
209213
scaleCalcNice2(scale, model, axis, ecModel, null);
210214
}
211215

216+
/**
217+
* @see SCALE_EXTENT_CONSTRUCTION for the full processing flow.
218+
*/
212219
export function scaleCalcNice2(
213220
scale: Scale,
214221
model: AxisBaseModel<NumericAxisBaseOptionCommon>,
@@ -237,12 +244,13 @@ export function scaleCalcNice2(
237244
splitNumber: model.get('splitNumber'),
238245
fixMinMax: rawExtentResult.fixMM,
239246
minInterval: isIntervalOrTime ? model.get('minInterval') : null,
240-
maxInterval: isIntervalOrTime ? model.get('maxInterval') : null
247+
maxInterval: isIntervalOrTime ? model.get('maxInterval') : null,
248+
rawExtentResult
241249
});
242250
}
243251

244252
if (axis && ecModel) {
245-
adoptScaleExtentKindMapping(scale, rawExtentResult);
253+
adoptScaleExtentKindMapping(axis, scale, rawExtentResult);
246254
}
247255

248256
if (__DEV__) {

src/coord/cartesian/Grid.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
SCALE_VALUE_POSITION_KIND_EDGE,
3939
SCALE_VALUE_POSITION_KIND_INSIDE,
4040
getTickValueOutermost,
41+
isAxisOnBand,
4142
} from '../../coord/axisHelper';
4243
import Cartesian2D, {cartesian2DDimensions} from './Cartesian2D';
4344
import Axis2D from './Axis2D';
@@ -493,8 +494,7 @@ class Grid implements CoordinateSystemMaster {
493494
axisPosition
494495
);
495496

496-
const isCategory = axis.type === 'category';
497-
axis.onBand = isCategory && (axisModel as AxisBaseModel<CategoryAxisBaseOption>).get('boundaryGap');
497+
axis.onBand = isAxisOnBand(axis.scale, axisModel);
498498
axis.inverse = axisModel.get('inverse');
499499

500500
// Inject axis into axisModel

0 commit comments

Comments
 (0)