Refactor series properties management (#776)

* Refactor series properties management

* fix bug

* fix broken tests

packages/ag-charts-community/src/chart/agChart.test.ts

@@ -152,7 +152,7 @@ describe('update', () => {
  expect(chart.subtitle?.enabled).toBe(false);
  expect((chart as any).background.fill).toBe('red');
  expect((chart as any).background.visible).toBe(false);
- expect((chart.series[0] as any).marker.shape).toBe('plus');
+ expect((chart.series[0] as any).properties.marker.shape).toBe('plus');
 
  AgCharts.updateDelta(chartProxy, {
  data: revenueProfitData,
@@ -254,15 +254,15 @@ describe('update', () => {
  expect(updatedSeries.length).toEqual(4);
  expect(updatedSeries[0]).not.toBe(createdSeries[0]);
  expect(updatedSeries[1]).not.toBe(createdSeries[1]);
- expect((updatedSeries[0] as any).marker.shape).toEqual('square');
- expect((updatedSeries[0] as any).marker.size).toEqual(10);
- expect((updatedSeries[1] as any).fill).toEqual('lime');
- expect((updatedSeries[1] as any).yKey).toEqual('profit');
- expect((updatedSeries[2] as any).fill).toEqual('cyan');
- expect((updatedSeries[2] as any).yKey).toEqual('foobar');
+ expect((updatedSeries[0].properties as any).marker.shape).toEqual('square');
+ expect((updatedSeries[0].properties as any).marker.size).toEqual(10);
+ expect((updatedSeries[1].properties as any).fill).toEqual('lime');
+ expect((updatedSeries[1].properties as any).yKey).toEqual('profit');
+ expect((updatedSeries[2].properties as any).fill).toEqual('cyan');
+ expect((updatedSeries[2].properties as any).yKey).toEqual('foobar');
  expect(updatedSeries[3]).toBeInstanceOf(AreaSeries);
- expect((updatedSeries[3] as any).xKey).toEqual('month');
- expect((updatedSeries[3] as any).yKey).toEqual('bazqux');
+ expect((updatedSeries[3].properties as any).xKey).toEqual('month');
+ expect((updatedSeries[3].properties as any).yKey).toEqual('bazqux');
 
  AgCharts.update(chartProxy, {
  data: revenueProfitData,
@@ -336,10 +336,10 @@ describe('update', () => {
  expect(updatedSeries3[0]).toBeInstanceOf(BarSeries);
  expect(updatedSeries3[1]).toBeInstanceOf(BarSeries);
  expect(updatedSeries3[2]).toBeInstanceOf(LineSeries);
- expect((updatedSeries3[0] as any).yKey).toEqual('profit');
- expect((updatedSeries3[1] as any).yKey).toEqual('foobar');
- expect((updatedSeries3[2] as any).yKey).toEqual('revenue');
- expect((updatedSeries3[2] as any).marker.size).toEqual(10);
+ expect((updatedSeries3[0].properties as any).yKey).toEqual('profit');
+ expect((updatedSeries3[1].properties as any).yKey).toEqual('foobar');
+ expect((updatedSeries3[2].properties as any).yKey).toEqual('revenue');
+ expect((updatedSeries3[2].properties as any).marker.size).toEqual(10);
 
  const lineSeries = updatedSeries3[1];
 

packages/ag-charts-community/src/chart/agChartV2.ts

@@ -43,8 +43,8 @@ import {
  optionsType,
 } from './mapping/types';
 import { PolarChart } from './polarChart';
-import { PieTitle } from './series/polar/pieSeries';
 import type { Series } from './series/series';
+import type { SeriesGrouping } from './series/seriesStateManager';
 
 const debug = Debug.create(true, 'opts');
 
@@ -484,18 +484,18 @@ function applySeries(chart: Chart, options: AgChartOptions) {
  return false;
  }
 
- const keysToConsider = ['type', 'direction', 'xKey', 'yKey', 'sizeKey', 'angleKey', 'stacked', 'stackGroup'];
+ const keysToConsider = ['direction', 'xKey', 'yKey', 'sizeKey', 'angleKey', 'stacked', 'stackGroup'];
 
  let matchingTypes = chart.series.length === optSeries.length;
  for (let i = 0; i < chart.series.length && matchingTypes; i++) {
+ matchingTypes &&= chart.series[i].type === (optSeries[i] as any).type;
  for (const key of keysToConsider) {
- matchingTypes &&= (chart.series[i] as any)[key] === (optSeries[i] as any)[key];
+ matchingTypes &&= (chart.series[i].properties as any)[key] === (optSeries[i] as any)[key];
  }
  }
 
  // Try to optimise series updates if series count and types didn't change.
  if (matchingTypes) {
- const moduleContext = chart.getModuleContext();
  chart.series.forEach((s, i) => {
  const previousOpts = chart.processedOptions?.series?.[i] ?? {};
  const seriesDiff = jsonDiff(previousOpts, optSeries[i] ?? {});
@@ -506,7 +506,7 @@ function applySeries(chart: Chart, options: AgChartOptions) {
 
  debug(`AgChartV2.applySeries() - applying series diff idx ${i}`, seriesDiff);
 
- applySeriesValues(s as any, moduleContext, seriesDiff, { path: `series[${i}]`, index: i });
+ applySeriesValues(s as any, seriesDiff);
  s.markNodeDataDirty();
  });
 
@@ -556,16 +556,14 @@ function createSeries(chart: Chart, options: SeriesOptionsTypes[]): Series<any>[
  const series: Series<any>[] = [];
  const moduleContext = chart.getModuleContext();
 
- let index = 0;
  for (const seriesOptions of options ?? []) {
- const path = `series[${index++}]`;
  const type = seriesOptions.type ?? 'unknown';
  if (isEnterpriseSeriesType(type) && !isEnterpriseSeriesTypeLoaded(type)) {
  continue;
  }
  const seriesInstance = getSeries(type, moduleContext);
  applySeriesOptionModules(seriesInstance, seriesOptions);
- applySeriesValues(seriesInstance, moduleContext, seriesOptions, { path, index });
+ applySeriesValues(seriesInstance, seriesOptions);
  series.push(seriesInstance);
  }
 
@@ -650,53 +648,23 @@ function applyOptionValues<T extends object, S>(
  return jsonApply<T, any>(target, options, applyOpts);
 }
 
-function applySeriesValues(
- target: Series<any>,
- moduleContext: ModuleContext,
- options?: AgBaseSeriesOptions<any>,
- { path, index }: { path?: string; index?: number } = {}
-): Series<any> {
- const skip: string[] = ['series[].listeners', 'series[].seriesGrouping'];
- const jsonApplyOptions = getJsonApplyOptions(moduleContext);
- const ctrs = jsonApplyOptions.constructors ?? {};
- // Allow context to be injected and meet the type requirements
- class PieTitleWithContext extends PieTitle {
- constructor() {
- super(moduleContext);
- }
+function applySeriesValues(target: Series<any>, options: AgBaseSeriesOptions<any>): Series<any> {
+ target.properties.set(options);
+ target.data = options.data;
+ if ('errorBar' in target && 'errorBar' in options) {
+ (target.errorBar as any).properties.set(options.errorBar);
  }
- const seriesTypeOverrides = {
- constructors: {
- ...ctrs,
- title: target.type === 'pie' ? PieTitleWithContext : ctrs['title'],
- },
- };
-
- const applyOpts = {
- ...jsonApplyOptions,
- ...seriesTypeOverrides,
- skip: ['series[].type', ...(skip ?? [])],
- ...(path ? { path } : {}),
- idx: index ?? -1,
- };
-
- const result = jsonApply(target, options, applyOpts);
 
  if (options?.listeners != null) {
  registerListeners(target, options.listeners);
  }
 
- const { seriesGrouping } = options as any;
- if ('seriesGrouping' in (options ?? {})) {
- if (seriesGrouping) {
- target.seriesGrouping = Object.freeze({
- ...(target.seriesGrouping ?? {}),
- ...seriesGrouping,
- });
- } else {
- target.seriesGrouping = seriesGrouping;
- }
+ if ('seriesGrouping' in options) {
+ const seriesGrouping = options.seriesGrouping as SeriesGrouping | undefined;
+ target.seriesGrouping = seriesGrouping
+ ? Object.freeze({ ...target.seriesGrouping, ...seriesGrouping })
+ : undefined;
  }
 
- return result;
+ return target;
 }

packages/ag-charts-community/src/chart/axis/axis.ts

@@ -259,6 +259,7 @@ export abstract class Axis<S extends Scale<D, number, TickInterval<S>> = Scale<a
  ) {
  this.refreshScale();
 
+ this._titleCaption.registerInteraction(this.moduleCtx);
  this._titleCaption.node.rotation = -Math.PI / 2;
  this.axisGroup.appendChild(this._titleCaption.node);
 
@@ -388,7 +389,7 @@ export abstract class Axis<S extends Scale<D, number, TickInterval<S>> = Scale<a
 
  @Validate(predicateWithMessage((title) => typeof title == 'object', 'Title object'), { optional: true })
  public title?: AxisTitle = undefined;
- protected _titleCaption = new Caption(this.moduleCtx);
+ protected _titleCaption = new Caption();
 
  private setDomain() {
  const {
@@ -712,7 +713,7 @@ export abstract class Axis<S extends Scale<D, number, TickInterval<S>> = Scale<a
 
  const { title } = this;
  if (title?.enabled) {
- const caption = new Caption(this.moduleCtx);
+ const caption = new Caption();
  const spacing = BBox.merge(boxes).width;
  this.setTitleProps(caption, { spacing });
  const titleNode = caption.node;

packages/ag-charts-community/src/chart/axis/axisTitle.ts

@@ -20,19 +20,19 @@ import { Caption } from '../caption';
 
 export class AxisTitle implements AgAxisCaptionOptions {
  @Validate(BOOLEAN)
- enabled = false;
+ enabled: boolean = false;
 
  @Validate(STRING, { optional: true })
- text?: string = undefined;
+ text?: string;
 
  @Validate(POSITIVE_NUMBER, { optional: true })
  spacing?: number = Caption.SMALL_PADDING;
 
  @Validate(FONT_STYLE, { optional: true })
- fontStyle?: FontStyle = undefined;
+ fontStyle?: FontStyle;
 
  @Validate(FONT_WEIGHT, { optional: true })
- fontWeight?: FontWeight = undefined;
+ fontWeight?: FontWeight;
 
  @Validate(POSITIVE_NUMBER)
  fontSize: number = 10;
@@ -47,5 +47,5 @@ export class AxisTitle implements AgAxisCaptionOptions {
  wrapping: TextWrap = 'always';
 
  @Validate(FUNCTION, { optional: true })
- formatter?: (params: AgAxisCaptionFormatterParams) => string = undefined;
+ formatter?: (params: AgAxisCaptionFormatterParams) => string;
 }

packages/ag-charts-community/src/chart/caption.ts

@@ -3,6 +3,7 @@ import type { FontStyle, FontWeight, TextWrap } from '../options/agChartOptions'
 import { PointerEvents } from '../scene/node';
 import { Text } from '../scene/shape/text';
 import { createId } from '../util/id';
+import { BaseProperties } from '../util/properties';
 import { ProxyPropertyOnWrite } from '../util/proxy';
 import {
  BOOLEAN,
@@ -17,27 +18,30 @@ import {
 import type { InteractionEvent } from './interaction/interactionManager';
 import { toTooltipHtml } from './tooltip/tooltip';
 
-export class Caption {
+export class Caption extends BaseProperties {
  static readonly SMALL_PADDING = 10;
  static readonly LARGE_PADDING = 20;
 
  readonly id = createId(this);
- readonly node: Text = new Text();
+ readonly node = new Text().setProperties({
+ textAlign: 'center',
+ pointerEvents: PointerEvents.None,
+ });
 
  @Validate(BOOLEAN)
  enabled = false;
 
  @Validate(STRING, { optional: true })
  @ProxyPropertyOnWrite('node')
- text?: string = undefined;
+ text?: string;
 
  @Validate(FONT_STYLE, { optional: true })
  @ProxyPropertyOnWrite('node')
- fontStyle: FontStyle | undefined;
+ fontStyle?: FontStyle;
 
  @Validate(FONT_WEIGHT, { optional: true })
  @ProxyPropertyOnWrite('node')
- fontWeight: FontWeight | undefined;
+ fontWeight?: FontWeight;
 
  @Validate(POSITIVE_NUMBER)
  @ProxyPropertyOnWrite('node')
@@ -49,7 +53,7 @@ export class Caption {
 
  @Validate(COLOR_STRING, { optional: true })
  @ProxyPropertyOnWrite('node', 'fill')
- color: string | undefined;
+ color?: string;
 
  @Validate(POSITIVE_NUMBER, { optional: true })
  spacing?: number;
@@ -58,24 +62,18 @@ export class Caption {
  lineHeight?: number;
 
  @Validate(POSITIVE_NUMBER, { optional: true })
- maxWidth?: number = undefined;
+ maxWidth?: number;
 
  @Validate(POSITIVE_NUMBER, { optional: true })
- maxHeight?: number = undefined;
+ maxHeight?: number;
 
  @Validate(TEXT_WRAP)
  wrapping: TextWrap = 'always';
 
- private truncated: boolean = false;
+ private truncated = false;
 
- private destroyFns: Function[] = [];
-
- constructor(protected readonly moduleCtx: ModuleContext) {
- const node = this.node;
- node.textAlign = 'center';
- node.pointerEvents = PointerEvents.None;
-
- this.destroyFns.push(moduleCtx.interactionManager.addListener('hover', (e) => this.handleMouseMove(e)));
+ registerInteraction(moduleCtx: ModuleContext) {
+ return moduleCtx.interactionManager.addListener('hover', (event) => this.handleMouseMove(moduleCtx, event));
  }
 
  computeTextWrap(containerWidth: number, containerHeight: number) {
@@ -91,9 +89,8 @@ export class Caption {
  this.truncated = truncated;
  }
 
- handleMouseMove(event: InteractionEvent<'hover'>) {
- const { enabled } = this;
- if (!enabled) {
+ handleMouseMove(moduleCtx: ModuleContext, event: InteractionEvent<'hover'>) {
+ if (!this.enabled) {
  return;
  }
 
@@ -102,7 +99,7 @@ export class Caption {
  const pointerInsideCaption = this.node.visible && bbox.containsPoint(offsetX, offsetY);
 
  if (!pointerInsideCaption) {
- this.moduleCtx.tooltipManager.removeTooltip(this.id);
+ moduleCtx.tooltipManager.removeTooltip(this.id);
  return;
  }
 
@@ -111,11 +108,11 @@ export class Caption {
  event.consume();
 
  if (!this.truncated) {
- this.moduleCtx.tooltipManager.removeTooltip(this.id);
+ moduleCtx.tooltipManager.removeTooltip(this.id);
  return;
  }
 
- this.moduleCtx.tooltipManager.updateTooltip(
+ moduleCtx.tooltipManager.updateTooltip(
  this.id,
  { pageX, pageY, offsetX, offsetY, event, showArrow: false, addCustomClass: false },
  toTooltipHtml({ content: this.text })

packages/ag-charts-community/src/chart/cartesianChart.test.ts

@@ -223,7 +223,7 @@ describe('CartesianChart', () => {
  await waitForChartStability(chart);
 
  const seriesImpl = chart.series.find(
- (v: any) => v.yKey === yKey || v.yKeys?.some((s) => s.includes(yKey))
+ (v: any) => v.properties.yKey === yKey || v.properties.yKeys?.some((s) => s.includes(yKey))
  );
  if (seriesImpl == null) fail('No seriesImpl found');