From 92301a1145587bf2976818e96f81fa8a4144ba29 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Fri, 18 Apr 2025 16:07:32 +0200 Subject: [PATCH 1/2] WIP --- .../chart_types/xy_chart/specs/area_series.ts | 17 ++++- .../src/chart_types/xy_chart/specs/axis.ts | 41 ++++++++-- packages/charts/src/components/chart.tsx | 15 +++- packages/charts/src/index.ts | 2 + packages/charts/src/specs/settings.tsx | 17 ++++- packages/charts/src/specs/tooltip.ts | 27 +++++-- storybook/stories/area/1_basic.story.tsx | 30 +++++--- storybook/stories/area/2_with_time.story.tsx | 75 ++++++++++--------- 8 files changed, 156 insertions(+), 68 deletions(-) diff --git a/packages/charts/src/chart_types/xy_chart/specs/area_series.ts b/packages/charts/src/chart_types/xy_chart/specs/area_series.ts index 69584ff415..05f6e07fc2 100644 --- a/packages/charts/src/chart_types/xy_chart/specs/area_series.ts +++ b/packages/charts/src/chart_types/xy_chart/specs/area_series.ts @@ -46,10 +46,23 @@ export const AreaSeries = function ( keyof (typeof buildProps)['requires'] >, ) { - const { defaults, overrides } = buildProps; - useSpecFactory>({ ...defaults, ...stripUndefined(props), ...overrides }); + useSpecFactory>(getAreaSeriesSpec(props)); return null; }; /** @public */ export type AreaSeriesProps = ComponentProps; + +/** @internal */ +export function getAreaSeriesSpec( + props: SFProps< + AreaSeriesSpec, + keyof (typeof buildProps)['overrides'], + keyof (typeof buildProps)['defaults'], + keyof (typeof buildProps)['optionals'], + keyof (typeof buildProps)['requires'] + >, +) { + const { defaults, overrides } = buildProps; + return { ...defaults, ...stripUndefined(props), ...overrides }; +} diff --git a/packages/charts/src/chart_types/xy_chart/specs/axis.ts b/packages/charts/src/chart_types/xy_chart/specs/axis.ts index ae1b87c52d..93318d7abd 100644 --- a/packages/charts/src/chart_types/xy_chart/specs/axis.ts +++ b/packages/charts/src/chart_types/xy_chart/specs/axis.ts @@ -10,16 +10,13 @@ import type { ComponentProps } from 'react'; import { ChartType } from '../..'; import { SpecType } from '../../../specs/spec_type'; // kept as long-winded import on separate line otherwise import circularity emerges -import { specComponentFactory } from '../../../state/spec_factory'; -import { Position } from '../../../utils/common'; +import type { SFProps } from '../../../state/spec_factory'; +import { buildSFProps, useSpecFactory } from '../../../state/spec_factory'; +import { Position, stripUndefined } from '../../../utils/common'; import type { AxisSpec } from '../utils/specs'; import { DEFAULT_GLOBAL_ID } from '../utils/specs'; -/** - * Add axis spec to chart - * @public - */ -export const Axis = specComponentFactory()( +const buildProps = buildSFProps()( { chartType: ChartType.XYAxis, specType: SpecType.Axis, @@ -34,5 +31,33 @@ export const Axis = specComponentFactory()( }, ); -/** @public */ +/** @internal */ +export const Axis = function ( + props: SFProps< + AxisSpec, + keyof (typeof buildProps)['overrides'], + keyof (typeof buildProps)['defaults'], + keyof (typeof buildProps)['optionals'], + keyof (typeof buildProps)['requires'] + >, +) { + useSpecFactory(getAxisSpec(props)); + return null; +}; + +/** @internal */ export type AxisProps = ComponentProps; + +/** @internal */ +export function getAxisSpec( + props: SFProps< + AxisSpec, + keyof (typeof buildProps)['overrides'], + keyof (typeof buildProps)['defaults'], + keyof (typeof buildProps)['optionals'], + keyof (typeof buildProps)['requires'] + >, +): AxisSpec { + const { defaults, overrides } = buildProps; + return { ...defaults, ...stripUndefined(props), ...overrides }; +} diff --git a/packages/charts/src/components/chart.tsx b/packages/charts/src/components/chart.tsx index 41727854f5..2f3a018e49 100644 --- a/packages/charts/src/components/chart.tsx +++ b/packages/charts/src/components/chart.tsx @@ -7,7 +7,7 @@ */ import classNames from 'classnames'; -import type { CSSProperties, ReactNode } from 'react'; +import type { CSSProperties } from 'react'; import React, { createRef } from 'react'; import { Provider } from 'react-redux'; import type { Unsubscribe, Store } from 'redux'; @@ -23,9 +23,11 @@ import { getElementZIndex } from './portal/utils'; import { chartTypeSelectors } from '../chart_types/chart_type_selectors'; import { Colors } from '../common/colors'; import type { LegendPositionConfig, PointerEvent } from '../specs'; +import type { Spec } from '../specs/spec_type'; import { SpecsParser } from '../specs/specs_parser'; import { updateChartTitles, updateParentDimensions } from '../state/actions/chart_settings'; import { onExternalPointerEvent } from '../state/actions/events'; +import { specParsed, upsertSpec } from '../state/actions/specs'; import { onComputedZIndex } from '../state/actions/z_index'; import { createChartStore, type GlobalChartState } from '../state/chart_state'; import { getChartContainerUpdateStateSelector } from '../state/selectors/chart_container_updates'; @@ -49,7 +51,7 @@ export interface ChartProps { id?: string; title?: string; description?: string; - children?: ReactNode; + config?: Spec[]; } interface ChartState { @@ -127,6 +129,13 @@ export class Chart extends React.Component { if (newChartSize && (newChartSize.width !== prevChartSize.width || newChartSize.height !== prevChartSize.height)) { this.chartStore.dispatch(updateParentDimensions({ ...newChartSize, top: 0, left: 0 })); } + if (this.props.config) { + for (const spec of this.props.config) { + // align specs with default + this.chartStore.dispatch(upsertSpec(spec)); + } + this.chartStore.dispatch(specParsed()); + } } getPNGSnapshot( @@ -187,7 +196,7 @@ export class Chart extends React.Component { - {this.props.children} + {(this.props.config ?? []).length === 0 && {this.props.children}}
diff --git a/packages/charts/src/index.ts b/packages/charts/src/index.ts index d06d72b6d0..764dd5b977 100644 --- a/packages/charts/src/index.ts +++ b/packages/charts/src/index.ts @@ -84,6 +84,8 @@ export { LEGACY_LIGHT_THEME } from './utils/themes/legacy_light_theme'; export { LEGACY_DARK_THEME } from './utils/themes/legacy_dark_theme'; export { AMSTERDAM_LIGHT_THEME } from './utils/themes/amsterdam_light_theme'; export { AMSTERDAM_DARK_THEME } from './utils/themes/amsterdam_dark_theme'; +export { getAreaSeriesSpec } from './chart_types/xy_chart/specs/area_series'; +export { getAxisSpec } from './chart_types/xy_chart/specs/axis'; // wordcloud export { WordcloudViewModel } from './chart_types/wordcloud/layout/types/viewmodel_types'; diff --git a/packages/charts/src/specs/settings.tsx b/packages/charts/src/specs/settings.tsx index c6cb54b76e..e515367c8c 100644 --- a/packages/charts/src/specs/settings.tsx +++ b/packages/charts/src/specs/settings.tsx @@ -632,8 +632,7 @@ export const Settings = function ( keyof (typeof settingsBuildProps)['requires'] >, ) { - const { defaults, overrides } = settingsBuildProps; - useSpecFactory({ ...defaults, ...stripUndefined(props), ...overrides }); + useSpecFactory(getSettingsSpec(props)); return null; }; @@ -649,3 +648,17 @@ export function isPointerOutEvent(event: PointerEvent | null | undefined): event export function isPointerOverEvent(event: PointerEvent | null | undefined): event is PointerOverEvent { return event?.type === PointerEventType.Over; } + +/** @internal */ +export function getSettingsSpec( + props: SFProps< + SettingsSpec, + keyof (typeof settingsBuildProps)['overrides'], + keyof (typeof settingsBuildProps)['defaults'], + keyof (typeof settingsBuildProps)['optionals'], + keyof (typeof settingsBuildProps)['requires'] + >, +): SettingsSpec { + const { defaults, overrides } = settingsBuildProps; + return { ...defaults, ...stripUndefined(props), ...overrides }; +} diff --git a/packages/charts/src/specs/tooltip.ts b/packages/charts/src/specs/tooltip.ts index d0bda37b9a..ceccfddf07 100644 --- a/packages/charts/src/specs/tooltip.ts +++ b/packages/charts/src/specs/tooltip.ts @@ -289,13 +289,7 @@ export const Tooltip = function , ) { - const { defaults, overrides } = tooltipBuildProps; - // @ts-ignore - default generic value - useSpecFactory>({ - ...defaults, - ...stripUndefined(props), - ...overrides, - }); + useSpecFactory(getTooltipSpec(props)); return null; }; @@ -310,3 +304,22 @@ export type TooltipProps; + +/** @internal */ +export function getTooltipSpec( + props: SFProps< + TooltipSpec, + keyof (typeof tooltipBuildProps)['overrides'], + keyof (typeof tooltipBuildProps)['defaults'], + keyof (typeof tooltipBuildProps)['optionals'], + keyof (typeof tooltipBuildProps)['requires'] + >, +): TooltipSpec { + const { defaults, overrides } = tooltipBuildProps; + // @ts-ignore - default generic value + return { + ...defaults, + ...stripUndefined(props), + ...overrides, + }; +} diff --git a/storybook/stories/area/1_basic.story.tsx b/storybook/stories/area/1_basic.story.tsx index 0948812605..d073987350 100644 --- a/storybook/stories/area/1_basic.story.tsx +++ b/storybook/stories/area/1_basic.story.tsx @@ -8,7 +8,7 @@ import React from 'react'; -import { AreaSeries, Chart, ScaleType, Settings } from '@elastic/charts'; +import { Chart, ScaleType, getAreaSeriesSpec, getSettingsSpec } from '@elastic/charts'; import { KIBANA_METRICS } from '@elastic/charts/src/utils/data_samples/test_dataset_kibana'; import type { ChartsStory } from '../../types'; @@ -17,16 +17,22 @@ import { useBaseTheme } from '../../use_base_theme'; export const Example: ChartsStory = (_, { title, description }) => { const { data } = KIBANA_METRICS.metrics.kibana_os_load.v1; return ( - - - - + ); }; diff --git a/storybook/stories/area/2_with_time.story.tsx b/storybook/stories/area/2_with_time.story.tsx index 5eca49c665..45b193cd3c 100644 --- a/storybook/stories/area/2_with_time.story.tsx +++ b/storybook/stories/area/2_with_time.story.tsx @@ -10,15 +10,15 @@ import { number } from '@storybook/addon-knobs'; import React from 'react'; import { - AreaSeries, - Axis, Chart, + getAreaSeriesSpec, + getAxisSpec, + getSettingsSpec, + getTooltipSpec, Placement, Position, ScaleType, - Settings, timeFormatter, - Tooltip, } from '@elastic/charts'; import { isDefined } from '@elastic/charts/src/utils/common'; import { KIBANA_METRICS } from '@elastic/charts/src/utils/data_samples/test_dataset_kibana'; @@ -30,34 +30,41 @@ import { customKnobs } from '../utils/knobs'; const dateFormatter = timeFormatter('HH:mm'); export const Example: ChartsStory = (_, { title, description }) => ( - - - - - Number(d).toFixed(2)} - /> - - + Number(d).toFixed(2), + }), + getAreaSeriesSpec({ + id: 'area1', + xScaleType: ScaleType.Time, + yScaleType: ScaleType.Linear, + xAccessor: 0, + yAccessors: [1], + data: KIBANA_METRICS.metrics.kibana_os_load.v1.data, + }), + ]} + /> ); From f589dcbf7ded9e1fd6b5f25442a2d6caf9a37cbb Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Fri, 18 Apr 2025 17:42:43 +0200 Subject: [PATCH 2/2] move callback callers to middleware --- packages/charts/src/components/chart.tsx | 34 +++++++----------------- packages/charts/src/state/chart_state.ts | 21 +++++++++++++-- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/packages/charts/src/components/chart.tsx b/packages/charts/src/components/chart.tsx index 2f3a018e49..9ca2183573 100644 --- a/packages/charts/src/components/chart.tsx +++ b/packages/charts/src/components/chart.tsx @@ -10,7 +10,7 @@ import classNames from 'classnames'; import type { CSSProperties } from 'react'; import React, { createRef } from 'react'; import { Provider } from 'react-redux'; -import type { Unsubscribe, Store } from 'redux'; +import type { Store } from 'redux'; import type { OptionalKeys } from 'utility-types'; import { v4 as uuidv4 } from 'uuid'; @@ -30,13 +30,10 @@ import { onExternalPointerEvent } from '../state/actions/events'; import { specParsed, upsertSpec } from '../state/actions/specs'; import { onComputedZIndex } from '../state/actions/z_index'; import { createChartStore, type GlobalChartState } from '../state/chart_state'; -import { getChartContainerUpdateStateSelector } from '../state/selectors/chart_container_updates'; -import { getInternalChartStateSelector, chartSelectorsRegistry } from '../state/selectors/get_internal_chart_state'; -import { getInternalIsInitializedSelector, InitStatus } from '../state/selectors/get_internal_is_intialized'; +import { chartSelectorsRegistry } from '../state/selectors/get_internal_chart_state'; import type { ChartSize } from '../utils/chart_size'; import { getChartSize, getFixedChartSize } from '../utils/chart_size'; import { LayoutDirection } from '../utils/common'; -import { deepEqual } from '../utils/fast_deep_equal'; import { LIGHT_THEME } from '../utils/themes/light_theme'; /** @public */ @@ -67,8 +64,6 @@ export class Chart extends React.Component { renderer: 'canvas', }; - private unsubscribeToStore: Unsubscribe; - private chartStore: Store; private chartContainerRef: React.RefObject; @@ -92,20 +87,6 @@ export class Chart extends React.Component { paddingRight: LIGHT_THEME.chartMargins.right, displayTitles: true, }; - this.unsubscribeToStore = this.chartStore.subscribe(() => { - const state = this.chartStore.getState(); - const internalChartState = getInternalChartStateSelector(state); - if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { - return; - } - - const newState = getChartContainerUpdateStateSelector(state); - if (!deepEqual(this.state, newState)) this.setState(newState); - - if (internalChartState) { - internalChartState.eventCallbacks(state); - } - }); } componentDidMount() { @@ -113,10 +94,13 @@ export class Chart extends React.Component { const zIndex = getElementZIndex(this.chartContainerRef.current, document.body); this.chartStore.dispatch(onComputedZIndex(zIndex)); } - } - - componentWillUnmount() { - this.unsubscribeToStore(); + if (this.props.config) { + for (const spec of this.props.config) { + // align specs with default + this.chartStore.dispatch(upsertSpec(spec)); + } + this.chartStore.dispatch(specParsed()); + } } componentDidUpdate({ title, description, size }: Readonly) { diff --git a/packages/charts/src/state/chart_state.ts b/packages/charts/src/state/chart_state.ts index d0a3764150..29471892be 100644 --- a/packages/charts/src/state/chart_state.ts +++ b/packages/charts/src/state/chart_state.ts @@ -7,7 +7,7 @@ */ import type { ActionReducerMapBuilder } from '@reduxjs/toolkit'; -import { configureStore, createSlice } from '@reduxjs/toolkit'; +import { configureStore, createListenerMiddleware, createSlice } from '@reduxjs/toolkit'; import { onChartRendered } from './actions/chart'; import { updateParentDimensions, updateChartTitles } from './actions/chart_settings'; @@ -24,6 +24,8 @@ import { handleDOMElementActions, handleTooltipActions, } from './reducers/interactions'; +import { getInternalChartStateSelector } from './selectors/get_internal_chart_state'; +import { getInternalIsInitializedSelector, InitStatus } from './selectors/get_internal_is_intialized'; import { getInitialPointerState } from './utils/get_initial_pointer_state'; import { getInitialTooltipState } from './utils/get_initial_tooltip_state'; import type { Color } from '../common/colors'; @@ -165,6 +167,21 @@ const createChartSlice = (initialState: ChartSliceState) => }, }); +// provides a middleware to send events to callbacks +const callbackListenerMiddleware = createListenerMiddleware(); +callbackListenerMiddleware.startListening({ + predicate: (_, currentState) => { + return getInternalIsInitializedSelector(currentState) === InitStatus.Initialized; + }, + effect: (_, listenerApi) => { + const state = listenerApi.getOriginalState(); + const internalChartState = getInternalChartStateSelector(state); + if (internalChartState) { + internalChartState.eventCallbacks(state); + } + }, +}); + /** @internal */ export const createChartStore = (chartId: string, title?: string, description?: string) => { const initialState = getInitialState(chartId, title, description); @@ -175,7 +192,7 @@ export const createChartStore = (chartId: string, title?: string, description?: getDefaultMiddleware({ // TODO https://github.com/elastic/elastic-charts/issues/2078 serializableCheck: false, - }), + }).prepend(callbackListenerMiddleware.middleware), }); };