plainheart commented on code in PR #19941:
URL: https://github.com/apache/echarts/pull/19941#discussion_r2034227889
##########
src/coord/cartesian/Axis2D.ts:
##########
@@ -66,6 +67,11 @@ class Axis2D extends Axis {
*/
grid: Grid;
+ /**
+ * See the comment of `JitterStorable`.
+ */
+ jitterStore: JitterData[] = [];
Review Comment:
Remove.
##########
src/util/jitter.ts:
##########
@@ -0,0 +1,129 @@
+import type Axis from '../coord/Axis';
+import type { AxisBaseModel } from '../coord/AxisBaseModel';
+import Axis2D from '../coord/cartesian/Axis2D';
+import type SingleAxis from '../coord/single/SingleAxis';
+import type SeriesModel from '../model/Series';
+import { makeInner } from './model';
+
+export function needFixJitter(seriesModel: SeriesModel, axis: Axis): boolean {
+ const { coordinateSystem } = seriesModel;
+ const { type: coordType } = coordinateSystem;
Review Comment:
I saw this comment was marked as resolved, but I didn't see the null check
for `seriesModel.coordinateSystem`. And I don't think the alias deconstruction
is necessary.
```ts
const coordinateSystem = seriesModel.coordinateSystem;
const coordType = coordinateSystem && coordinateSystem.type;
const baseAxis = coordinateSystem && coordinateSystem.getBaseAxis &&
coordinateSystem.getBaseAxis();
const scaleType = baseAxis && baseAxis.scale;
// ...
```
##########
src/coord/single/SingleAxis.ts:
##########
@@ -47,6 +48,11 @@ class SingleAxis extends Axis {
model: SingleAxisModel;
+ /**
+ * See the comment of `JitterStorable`.
+ */
+ jitterStore: JitterData[] = [];
+
Review Comment:
Remove.
##########
src/chart/scatter/install.ts:
##########
@@ -22,13 +22,16 @@ import ScatterSeriesModel from './ScatterSeries';
import ScatterView from './ScatterView';
import {install as installGridSimple} from
'../../component/grid/installSimple';
import layoutPoints from '../../layout/points';
+import jitterLayout from './jitterLayout';
export function install(registers: EChartsExtensionInstallRegisters) {
// In case developer forget to include grid component
use(installGridSimple);
registers.registerSeriesModel(ScatterSeriesModel);
+
registers.registerChartView(ScatterView);
- registers.registerLayout(layoutPoints('scatter'));
+ registers.registerLayout(layoutPoints('scatter'));
+ registers.registerLayout(registers.PRIORITY.VISUAL.POST_CHART_LAYOUT,
jitterLayout);
Review Comment:
I agree with @100pah ([comment
link](https://github.com/apache/echarts/pull/19941/files#r1836853985)).
Extracting the jitter layout to an installable feature is also easy and won't
increase the library's already large size.
##########
src/coord/cartesian/Axis2D.ts:
##########
@@ -39,7 +40,7 @@ interface Axis2D {
*/
toGlobalCoord(coord: number): number;
}
-class Axis2D extends Axis {
+class Axis2D extends Axis implements JitterStorable {
Review Comment:
I didn't see any usage of the `jitterStore` member of the `JitterStorable`
interface. It can be removed safely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
