From 1a9bbb92d5614f180018432253049688db810272 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Thu, 11 Jun 2020 14:54:05 +0200 Subject: [PATCH] fix: remove double rendering (#693) Fix the double rendering caused by ignoring the fact that the container size > 0 needs to be part of the required flags to initialize the rendering process. fix #690 --- packages/osd-charts/.playground/index.html | 8 +++ .../osd-charts/.playground/playground.tsx | 72 +++++++++++++++++-- packages/osd-charts/package.json | 1 + packages/osd-charts/scripts/setup_enzyme.ts | 28 ++++++++ .../renderer/canvas/connected_component.tsx | 4 +- .../goal_chart/state/chart_state.tsx | 3 +- .../layout/viewmodel/viewmodel.ts | 1 - .../renderer/canvas/partition.tsx | 4 +- .../renderer/dom/highlighter_hover.tsx | 4 +- .../renderer/dom/highlighter_legend.tsx | 4 +- .../partition_chart/state/chart_state.tsx | 3 +- .../chart_types/xy_chart/domains/x_domain.ts | 2 +- .../xy_chart/renderer/canvas/xy_chart.tsx | 15 ++-- .../renderer/dom/annotations/annotations.tsx | 61 ++++++++-------- .../xy_chart/renderer/dom/brush.tsx | 4 +- .../xy_chart/renderer/dom/crosshair.tsx | 4 +- .../xy_chart/renderer/dom/highlighter.tsx | 4 +- .../xy_chart/state/chart_state.tsx | 3 +- .../__snapshots__/chart.test.tsx.snap | 3 + .../osd-charts/src/components/chart.test.tsx | 28 ++++++-- packages/osd-charts/src/components/chart.tsx | 6 +- .../src/components/chart_background.tsx | 4 +- .../src/components/chart_container.tsx | 25 ++++--- .../src/components/chart_resizer.tsx | 36 +++++----- .../src/components/legend/legend.test.tsx | 2 +- .../src/components/legend/legend.tsx | 4 +- .../src/components/tooltip/tooltip.tsx | 4 +- packages/osd-charts/src/state/chart_state.ts | 6 +- .../selectors/get_internal_is_intialized.ts | 36 ++++++++-- packages/osd-charts/yarn.lock | 19 +++++ 30 files changed, 288 insertions(+), 110 deletions(-) create mode 100644 packages/osd-charts/src/components/__snapshots__/chart.test.tsx.snap diff --git a/packages/osd-charts/.playground/index.html b/packages/osd-charts/.playground/index.html index 4ac6cbb7609c..4f0a5647705a 100644 --- a/packages/osd-charts/.playground/index.html +++ b/packages/osd-charts/.playground/index.html @@ -24,6 +24,11 @@ height: 100%;*/ /* overflow-x: hidden; */ } + + #root { + height: 500px; + } + .chart { background: white; /*display: inline-block; @@ -40,14 +45,17 @@ width: 100%; overflow: auto; } + .page { padding: 100px; } + label { display: block; } +
diff --git a/packages/osd-charts/.playground/playground.tsx b/packages/osd-charts/.playground/playground.tsx index 0fa64ff4c054..e19342b18789 100644 --- a/packages/osd-charts/.playground/playground.tsx +++ b/packages/osd-charts/.playground/playground.tsx @@ -17,16 +17,78 @@ * under the License. */ -import React from 'react'; +import React, { useState } from 'react'; -import { Example } from '../stories/bar/23_bar_chart_2y2g'; +import { Chart, BarSeries, LegendColorPicker, Settings, ScaleType } from '../src'; +import { SeededDataGenerator } from '../src/mocks/utils'; + +const dg = new SeededDataGenerator(); + +type SetColorFn = (color: string) => void; +const legendColorPickerFn = (setColors: SetColorFn, customColor: string): LegendColorPicker => ({ onClose }) => ( +
+ Custom Color Picker + + +
+); +function LegendColorPickerMock(props: { onLegendItemClick: () => void; customColor: string }) { + const data = dg.generateGroupedSeries(10, 4, 'split'); + const [color, setColor] = useState('red'); + + return ( + <> + + + + + + + ); +} export class Playground extends React.Component { render() { return ( -
-
{Example()}
-
+ { + // npo + }} + /> ); } } diff --git a/packages/osd-charts/package.json b/packages/osd-charts/package.json index 32ff10f206f2..e3d778a62a9e 100644 --- a/packages/osd-charts/package.json +++ b/packages/osd-charts/package.json @@ -119,6 +119,7 @@ "@types/jest": "^25.2.1", "@types/jest-environment-puppeteer": "^4.3.1", "@types/jest-image-snapshot": "^2.12.0", + "@types/jsdom": "^16.2.3", "@types/lodash": "^4.14.121", "@types/luxon": "^1.11.1", "@types/moment-timezone": "^0.5.12", diff --git a/packages/osd-charts/scripts/setup_enzyme.ts b/packages/osd-charts/scripts/setup_enzyme.ts index 2d7f0276b1e5..916f7291a1d6 100644 --- a/packages/osd-charts/scripts/setup_enzyme.ts +++ b/packages/osd-charts/scripts/setup_enzyme.ts @@ -23,3 +23,31 @@ import Adapter from 'enzyme-adapter-react-16'; configure({ adapter: new Adapter() }); process.env.RNG_SEED = 'jest-unit-tests'; + +/** + * Mocking RAF and ResizeObserver to missing RAF and RO in jsdom + */ + +window.requestAnimationFrame = (callback) => { + callback(0); + return 0; +}; + +type ResizeObserverMockCallback = (entries: Array<{ contentRect: { width: number; height: number } }>) => void; +class ResizeObserverMock { + callback: ResizeObserverMockCallback; + constructor(callback: ResizeObserverMockCallback) { + this.callback = callback; + } + + observe() { + this.callback([{ contentRect: { width: 200, height: 200 } }]); + } + + unobserve() { } + + disconnect() { } +} + +// @ts-ignore +window.ResizeObserver = ResizeObserverMock; diff --git a/packages/osd-charts/src/chart_types/goal_chart/renderer/canvas/connected_component.tsx b/packages/osd-charts/src/chart_types/goal_chart/renderer/canvas/connected_component.tsx index 92a803adb704..c6bcd34a13dd 100644 --- a/packages/osd-charts/src/chart_types/goal_chart/renderer/canvas/connected_component.tsx +++ b/packages/osd-charts/src/chart_types/goal_chart/renderer/canvas/connected_component.tsx @@ -23,7 +23,7 @@ import { bindActionCreators, Dispatch } from 'redux'; import { onChartRendered } from '../../../../state/actions/chart'; import { GlobalChartState } from '../../../../state/chart_state'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized'; import { Dimensions } from '../../../../utils/dimensions'; import { BulletViewModel, nullShapeViewModel, ShapeViewModel } from '../../layout/types/viewmodel_types'; import { geometries } from '../../state/selectors/geometries'; @@ -159,7 +159,7 @@ const DEFAULT_PROPS: ReactiveChartStateProps = { }; const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return DEFAULT_PROPS; } return { diff --git a/packages/osd-charts/src/chart_types/goal_chart/state/chart_state.tsx b/packages/osd-charts/src/chart_types/goal_chart/state/chart_state.tsx index 9e05560f7923..85306023a982 100644 --- a/packages/osd-charts/src/chart_types/goal_chart/state/chart_state.tsx +++ b/packages/osd-charts/src/chart_types/goal_chart/state/chart_state.tsx @@ -23,6 +23,7 @@ import { ChartTypes } from '../..'; import { LegendItem } from '../../../commons/legend'; import { Tooltip } from '../../../components/tooltip'; import { InternalChartState, GlobalChartState, BackwardRef } from '../../../state/chart_state'; +import { InitStatus } from '../../../state/selectors/get_internal_is_intialized'; import { LegendItemLabel } from '../../../state/selectors/get_legend_items_labels'; import { Goal } from '../renderer/canvas/connected_component'; import { getSpecOrNull } from './selectors/goal_spec'; @@ -51,7 +52,7 @@ export class GoalState implements InternalChartState { } isInitialized(globalState: GlobalChartState) { - return globalState.specsInitialized && getSpecOrNull(globalState) !== null; + return getSpecOrNull(globalState) !== null ? InitStatus.Initialized : InitStatus.ChartNotInitialized; } isBrushAvailable() { diff --git a/packages/osd-charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts b/packages/osd-charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts index 06fbe358b1a0..692b823ea79c 100644 --- a/packages/osd-charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts +++ b/packages/osd-charts/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts @@ -202,7 +202,6 @@ export function shapeViewModel( partitionLayout, sectorLineWidth, } = config; - const innerWidth = width * (1 - Math.min(1, margin.left + margin.right)); const innerHeight = height * (1 - Math.min(1, margin.top + margin.bottom)); diff --git a/packages/osd-charts/src/chart_types/partition_chart/renderer/canvas/partition.tsx b/packages/osd-charts/src/chart_types/partition_chart/renderer/canvas/partition.tsx index b427a0352d63..e92b2d72345a 100644 --- a/packages/osd-charts/src/chart_types/partition_chart/renderer/canvas/partition.tsx +++ b/packages/osd-charts/src/chart_types/partition_chart/renderer/canvas/partition.tsx @@ -24,7 +24,7 @@ import { bindActionCreators, Dispatch } from 'redux'; import { onChartRendered } from '../../../../state/actions/chart'; import { GlobalChartState } from '../../../../state/chart_state'; import { getChartContainerDimensionsSelector } from '../../../../state/selectors/get_chart_container_dimensions'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized'; import { Dimensions } from '../../../../utils/dimensions'; import { nullShapeViewModel, QuadViewModel, ShapeViewModel } from '../../layout/types/viewmodel_types'; import { INPUT_KEY } from '../../layout/utils/group_by_rollup'; @@ -175,7 +175,7 @@ const DEFAULT_PROPS: ReactiveChartStateProps = { }; const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return DEFAULT_PROPS; } return { diff --git a/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_hover.tsx b/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_hover.tsx index c446c9c957fc..246c19319726 100644 --- a/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_hover.tsx +++ b/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_hover.tsx @@ -21,13 +21,13 @@ import { connect } from 'react-redux'; import { GlobalChartState } from '../../../../state/chart_state'; import { getChartContainerDimensionsSelector } from '../../../../state/selectors/get_chart_container_dimensions'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized'; import { partitionGeometries } from '../../state/selectors/geometries'; import { getPickedShapes } from '../../state/selectors/picked_shapes'; import { HighlighterComponent, HighlighterProps, DEFAULT_PROPS } from './highlighter'; const hoverMapStateToProps = (state: GlobalChartState): HighlighterProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return DEFAULT_PROPS; } diff --git a/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_legend.tsx b/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_legend.tsx index 19bed6569c8c..76dc11b11826 100644 --- a/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_legend.tsx +++ b/packages/osd-charts/src/chart_types/partition_chart/renderer/dom/highlighter_legend.tsx @@ -21,13 +21,13 @@ import { connect } from 'react-redux'; import { GlobalChartState } from '../../../../state/chart_state'; import { getChartContainerDimensionsSelector } from '../../../../state/selectors/get_chart_container_dimensions'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized'; import { partitionGeometries } from '../../state/selectors/geometries'; import { getHighlightedSectorsSelector } from '../../state/selectors/get_highlighted_shapes'; import { HighlighterComponent, HighlighterProps, DEFAULT_PROPS } from './highlighter'; const legendMapStateToProps = (state: GlobalChartState): HighlighterProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return DEFAULT_PROPS; } diff --git a/packages/osd-charts/src/chart_types/partition_chart/state/chart_state.tsx b/packages/osd-charts/src/chart_types/partition_chart/state/chart_state.tsx index bc242a2838c1..cd2c2a5006d1 100644 --- a/packages/osd-charts/src/chart_types/partition_chart/state/chart_state.tsx +++ b/packages/osd-charts/src/chart_types/partition_chart/state/chart_state.tsx @@ -22,6 +22,7 @@ import React, { RefObject } from 'react'; import { ChartTypes } from '../..'; import { Tooltip } from '../../../components/tooltip'; import { InternalChartState, GlobalChartState, BackwardRef } from '../../../state/chart_state'; +import { InitStatus } from '../../../state/selectors/get_internal_is_intialized'; import { Partition } from '../renderer/canvas/partition'; import { HighlighterFromHover } from '../renderer/dom/highlighter_hover'; import { HighlighterFromLegend } from '../renderer/dom/highlighter_legend'; @@ -51,7 +52,7 @@ export class PartitionState implements InternalChartState { } isInitialized(globalState: GlobalChartState) { - return globalState.specsInitialized && getPieSpec(globalState) !== null; + return getPieSpec(globalState) !== null ? InitStatus.Initialized : InitStatus.SpecNotInitialized; } isBrushAvailable() { diff --git a/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts b/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts index 667a847b44e0..e66cfb86adf5 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts +++ b/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts @@ -47,7 +47,7 @@ export function mergeXDomain( ): XDomain { const mainXScaleType = convertXScaleTypes(specs); if (!mainXScaleType) { - throw new Error('Cannot merge the domain. Missing X scale types'); + throw new Error(`Cannot merge the domain. Missing X scale types ${JSON.stringify(specs)}`); } const values = [...xValues.values()]; diff --git a/packages/osd-charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/packages/osd-charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx index 09e2d1f4b0de..8a30aa192916 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx +++ b/packages/osd-charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx @@ -27,7 +27,7 @@ import { GlobalChartState } from '../../../../state/chart_state'; import { getChartContainerDimensionsSelector } from '../../../../state/selectors/get_chart_container_dimensions'; import { getChartRotationSelector } from '../../../../state/selectors/get_chart_rotation'; import { getChartThemeSelector } from '../../../../state/selectors/get_chart_theme'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized'; import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; import { Rotation } from '../../../../utils/commons'; import { Dimensions } from '../../../../utils/dimensions'; @@ -144,19 +144,12 @@ class XYChartComponent extends React.Component { isChartEmpty, chartContainerDimensions: { width, height }, } = this.props; - if (!initialized || width === 0 || height === 0) { + + if (!initialized || isChartEmpty) { this.ctx = null; return null; } - if (isChartEmpty) { - this.ctx = null; - return ( -
-

No data to display

-
- ); - } return ( { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return DEFAULT_PROPS; } diff --git a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/annotations/annotations.tsx b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/annotations/annotations.tsx index 15f4cda3f574..5b490ce8f787 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/annotations/annotations.tsx +++ b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/annotations/annotations.tsx @@ -23,7 +23,7 @@ import { bindActionCreators, Dispatch } from 'redux'; import { onPointerMove as onPointerMoveAction } from '../../../../../state/actions/mouse'; import { GlobalChartState, BackwardRef } from '../../../../../state/chart_state'; -import { getInternalIsInitializedSelector } from '../../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../../state/selectors/get_internal_is_intialized'; import { Dimensions } from '../../../../../utils/dimensions'; import { AnnotationId } from '../../../../../utils/ids'; import { AnnotationLineProps } from '../../../annotations/line/types'; @@ -56,6 +56,33 @@ interface AnnotationsOwnProps { type AnnotationsProps = AnnotationsDispatchProps & AnnotationsStateProps & AnnotationsOwnProps; +function renderAnnotationLineMarkers( + chartDimensions: Dimensions, + annotationLines: AnnotationLineProps[], + id: AnnotationId, +) { + return annotationLines.reduce((markers, { marker }: AnnotationLineProps, index: number) => { + if (!marker) { + return markers; + } + + const { icon, color, position } = marker; + const style = { + color, + top: chartDimensions.top + position.top, + left: chartDimensions.left + position.left, + }; + + markers.push( + // eslint-disable-next-line react/no-array-index-key +
+ {icon} +
, + ); + + return markers; + }, []); +} const AnnotationsComponent = ({ tooltipState, isChartEmpty, @@ -66,32 +93,6 @@ const AnnotationsComponent = ({ chartId, onPointerMove, }: AnnotationsProps) => { - const renderAnnotationLineMarkers = useCallback( - (annotationLines: AnnotationLineProps[], id: AnnotationId) => - annotationLines.reduce((markers, { marker }: AnnotationLineProps, index: number) => { - if (!marker) { - return markers; - } - - const { icon, color, position } = marker; - const style = { - color, - top: chartDimensions.top + position.top, - left: chartDimensions.left + position.left, - }; - - markers.push( - // eslint-disable-next-line react/no-array-index-key -
- {icon} -
, - ); - - return markers; - }, []), - [], // eslint-disable-line react-hooks/exhaustive-deps - ); - const renderAnnotationMarkers = useCallback((): JSX.Element[] => { const markers: JSX.Element[] = []; @@ -103,13 +104,13 @@ const AnnotationsComponent = ({ if (isLineAnnotation(annotationSpec)) { const annotationLines = dimensions as AnnotationLineProps[]; - const lineMarkers = renderAnnotationLineMarkers(annotationLines, id); + const lineMarkers = renderAnnotationLineMarkers(chartDimensions, annotationLines, id); markers.push(...lineMarkers); } }); return markers; - }, [annotationDimensions, annotationSpecs, renderAnnotationLineMarkers]); + }, [chartDimensions, annotationDimensions, annotationSpecs]); const onScroll = useCallback(() => { onPointerMove({ x: -1, y: -1 }, new Date().getTime()); @@ -138,7 +139,7 @@ const mapDispatchToProps = (dispatch: Dispatch): AnnotationsDispatchProps => bindActionCreators({ onPointerMove: onPointerMoveAction }, dispatch); const mapStateToProps = (state: GlobalChartState): AnnotationsStateProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return { isChartEmpty: true, chartDimensions: { top: 0, left: 0, width: 0, height: 0 }, diff --git a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/brush.tsx b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/brush.tsx index 129fa9a5a95b..d803115920f3 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/brush.tsx +++ b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/brush.tsx @@ -23,7 +23,7 @@ import { connect } from 'react-redux'; import { clearCanvas, withContext, withClip } from '../../../../renderers/canvas'; import { GlobalChartState } from '../../../../state/chart_state'; import { getChartContainerDimensionsSelector } from '../../../../state/selectors/get_chart_container_dimensions'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized'; import { Dimensions } from '../../../../utils/dimensions'; import { computeChartDimensionsSelector } from '../../state/selectors/compute_chart_dimensions'; import { getBrushAreaSelector } from '../../state/selectors/get_brush_area'; @@ -141,7 +141,7 @@ class BrushToolComponent extends React.Component { } const mapStateToProps = (state: GlobalChartState): Props => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return { initialized: false, isBrushing: false, diff --git a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/crosshair.tsx b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/crosshair.tsx index 0db4204fc9b6..7ee917ddad2e 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/crosshair.tsx +++ b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/crosshair.tsx @@ -24,7 +24,7 @@ import { TooltipType } from '../../../../specs'; import { GlobalChartState } from '../../../../state/chart_state'; import { getChartRotationSelector } from '../../../../state/selectors/get_chart_rotation'; import { getChartThemeSelector } from '../../../../state/selectors/get_chart_theme'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized'; import { Rotation } from '../../../../utils/commons'; import { Dimensions } from '../../../../utils/dimensions'; import { LIGHT_THEME } from '../../../../utils/themes/light_theme'; @@ -116,7 +116,7 @@ class CrosshairComponent extends React.Component { } const mapStateToProps = (state: GlobalChartState): CrosshairProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return { theme: LIGHT_THEME, chartRotation: 0, diff --git a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/highlighter.tsx b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/highlighter.tsx index cc6fdb9ef424..a72e9b391cb7 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/highlighter.tsx +++ b/packages/osd-charts/src/chart_types/xy_chart/renderer/dom/highlighter.tsx @@ -22,7 +22,7 @@ import { connect } from 'react-redux'; import { GlobalChartState } from '../../../../state/chart_state'; import { getChartRotationSelector } from '../../../../state/selectors/get_chart_rotation'; -import { getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; +import { InitStatus, getInternalIsInitializedSelector } from '../../../../state/selectors/get_internal_is_intialized'; import { Rotation } from '../../../../utils/commons'; import { Dimensions } from '../../../../utils/dimensions'; import { isPointGeometry, IndexedGeometry } from '../../../../utils/geometry'; @@ -94,7 +94,7 @@ class HighlighterComponent extends React.Component { } const mapStateToProps = (state: GlobalChartState): HighlighterProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return { initialized: false, chartId: state.chartId, diff --git a/packages/osd-charts/src/chart_types/xy_chart/state/chart_state.tsx b/packages/osd-charts/src/chart_types/xy_chart/state/chart_state.tsx index a7b5251a3824..33b2c07008ff 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/state/chart_state.tsx +++ b/packages/osd-charts/src/chart_types/xy_chart/state/chart_state.tsx @@ -24,6 +24,7 @@ import { LegendItemExtraValues } from '../../../commons/legend'; import { SeriesKey } from '../../../commons/series_id'; import { Tooltip } from '../../../components/tooltip'; import { InternalChartState, GlobalChartState, BackwardRef } from '../../../state/chart_state'; +import { InitStatus } from '../../../state/selectors/get_internal_is_intialized'; import { htmlIdGenerator } from '../../../utils/commons'; import { XYChart } from '../renderer/canvas/xy_chart'; import { Annotations } from '../renderer/dom/annotations'; @@ -69,7 +70,7 @@ export class XYAxisChartState implements InternalChartState { } isInitialized(globalState: GlobalChartState) { - return globalState.specsInitialized && getSeriesSpecsSelector(globalState).length > 0; + return getSeriesSpecsSelector(globalState).length > 0 ? InitStatus.Initialized : InitStatus.SpecNotInitialized; } isBrushAvailable(globalState: GlobalChartState) { diff --git a/packages/osd-charts/src/components/__snapshots__/chart.test.tsx.snap b/packages/osd-charts/src/components/__snapshots__/chart.test.tsx.snap new file mode 100644 index 000000000000..00df8cd9addd --- /dev/null +++ b/packages/osd-charts/src/components/__snapshots__/chart.test.tsx.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Chart should render the legend name test 1`] = `"
  • test
"`; diff --git a/packages/osd-charts/src/components/chart.test.tsx b/packages/osd-charts/src/components/chart.test.tsx index f131857ee3f6..182e73244d19 100644 --- a/packages/osd-charts/src/components/chart.test.tsx +++ b/packages/osd-charts/src/components/chart.test.tsx @@ -17,24 +17,44 @@ * under the License. */ -import { render } from 'enzyme'; +import { mount } from 'enzyme'; import React from 'react'; -import { Settings } from '../specs'; +import { Settings, BarSeries } from '../specs'; import { Chart } from './chart'; describe('Chart', () => { it('should render \'No data to display\' without series', () => { - const wrapper = render(); + const wrapper = mount(); expect(wrapper.text()).toContain('No data to display'); }); it('should render \'No data to display\' with settings but without series', () => { - const wrapper = render( + const wrapper = mount( , ); expect(wrapper.text()).toContain('No data to display'); }); + + it('should render \'No data to display\' with an empty dataset', () => { + const wrapper = mount( + + + + , + ); + expect(wrapper.text()).toContain('No data to display'); + }); + + it('should render the legend name test', () => { + const wrapper = mount( + + + + , + ); + expect(wrapper.html()).toMatchSnapshot(); + }); }); diff --git a/packages/osd-charts/src/components/chart.tsx b/packages/osd-charts/src/components/chart.tsx index 7fe2ae72ffa8..d144d70ea5b0 100644 --- a/packages/osd-charts/src/components/chart.tsx +++ b/packages/osd-charts/src/components/chart.tsx @@ -28,7 +28,7 @@ import { PointerEvent } from '../specs'; import { SpecsParser } from '../specs/specs_parser'; import { onExternalPointerEvent } from '../state/actions/events'; import { chartStoreReducer, GlobalChartState } from '../state/chart_state'; -import { getInternalIsInitializedSelector } from '../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../state/selectors/get_internal_is_intialized'; import { getSettingsSpecSelector } from '../state/selectors/get_settings_specs'; import { ChartSize, getChartSize } from '../utils/chart_size'; import { Position } from '../utils/commons'; @@ -68,7 +68,7 @@ export class Chart extends React.Component { this.chartContainerRef = createRef(); this.chartStageRef = createRef(); - const id = uuid.v4(); + const id = props.id ?? uuid.v4(); const storeReducer = chartStoreReducer(id); const enhancers = typeof window !== 'undefined' && (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ ? (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({ trace: true, name: `@elastic/charts (id: ${id})` })() @@ -80,7 +80,7 @@ export class Chart extends React.Component { }; this.unsubscribeToStore = this.chartStore.subscribe(() => { const state = this.chartStore.getState(); - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return; } diff --git a/packages/osd-charts/src/components/chart_background.tsx b/packages/osd-charts/src/components/chart_background.tsx index 359859248851..d6157dd3e2e4 100644 --- a/packages/osd-charts/src/components/chart_background.tsx +++ b/packages/osd-charts/src/components/chart_background.tsx @@ -21,7 +21,7 @@ import { connect } from 'react-redux'; import { GlobalChartState } from '../state/chart_state'; import { getChartThemeSelector } from '../state/selectors/get_chart_theme'; -import { getInternalIsInitializedSelector } from '../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../state/selectors/get_internal_is_intialized'; interface ChartBackgroundProps { backgroundColor: string; @@ -37,7 +37,7 @@ export class ChartBackgroundComponent extends React.Component { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return { backgroundColor: 'transparent', }; diff --git a/packages/osd-charts/src/components/chart_container.tsx b/packages/osd-charts/src/components/chart_container.tsx index dcb8a115a40c..6a8799248468 100644 --- a/packages/osd-charts/src/components/chart_container.tsx +++ b/packages/osd-charts/src/components/chart_container.tsx @@ -28,13 +28,13 @@ import { getInternalChartRendererSelector } from '../state/selectors/get_chart_t import { getInternalPointerCursor } from '../state/selectors/get_internal_cursor_pointer'; import { getInternalIsBrushingSelector } from '../state/selectors/get_internal_is_brushing'; import { getInternalIsBrushingAvailableSelector } from '../state/selectors/get_internal_is_brushing_available'; -import { getInternalIsInitializedSelector } from '../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../state/selectors/get_internal_is_intialized'; import { getSettingsSpecSelector } from '../state/selectors/get_settings_specs'; import { isInternalChartEmptySelector } from '../state/selectors/is_chart_empty'; import { deepEqual } from '../utils/fast_deep_equal'; interface ChartContainerComponentStateProps { - initialized: boolean; + initialized: InitStatus; isChartEmpty?: boolean; pointerCursor: string; isBrushing: boolean; @@ -143,8 +143,15 @@ class ChartContainerComponent extends React.Component { }; render() { - const { initialized } = this.props; - if (!initialized) { + const { initialized, isChartEmpty } = this.props; + if ( + initialized === InitStatus.ParentSizeInvalid + || initialized === InitStatus.SpecNotInitialized + || initialized === InitStatus.ChartNotInitialized + ) { + return null; + } + if (initialized === InitStatus.MissingChartType || isChartEmpty === true) { return (

No data to display

@@ -179,10 +186,12 @@ const mapDispatchToProps = (dispatch: Dispatch): ChartContainerComponentDispatch dispatch, ); const mapStateToProps = (state: GlobalChartState): ChartContainerComponentStateProps => { - if (!getInternalIsInitializedSelector(state)) { + const status = getInternalIsInitializedSelector(state); + + if (status !== InitStatus.Initialized) { return { - initialized: false, - isChartEmpty: true, + initialized: status, + isChartEmpty: undefined, pointerCursor: 'default', isBrushingAvailable: false, isBrushing: false, @@ -191,7 +200,7 @@ const mapStateToProps = (state: GlobalChartState): ChartContainerComponentStateP } return { - initialized: true, + initialized: status, isChartEmpty: isInternalChartEmptySelector(state), pointerCursor: getInternalPointerCursor(state), isBrushingAvailable: getInternalIsBrushingAvailableSelector(state), diff --git a/packages/osd-charts/src/components/chart_resizer.tsx b/packages/osd-charts/src/components/chart_resizer.tsx index d09dd5ca4067..9ab1a8834786 100644 --- a/packages/osd-charts/src/components/chart_resizer.tsx +++ b/packages/osd-charts/src/components/chart_resizer.tsx @@ -23,6 +23,7 @@ import { Dispatch, bindActionCreators } from 'redux'; import ResizeObserver from 'resize-observer-polyfill'; import { debounce } from 'ts-debounce'; +import { isDefined } from '../chart_types/xy_chart/state/utils'; import { updateParentDimensions } from '../state/actions/chart_settings'; import { GlobalChartState } from '../state/chart_state'; import { getSettingsSpecSelector } from '../state/selectors/get_settings_specs'; @@ -38,26 +39,28 @@ interface ResizerDispatchProps { type ResizerProps = ResizerStateProps & ResizerDispatchProps; +const DEFAULT_RESIZE_DEBOUNCE = 200; + class Resizer extends React.Component { private initialResizeComplete = false; private containerRef: RefObject; private ro: ResizeObserver; private animationFrameID: number | null; + private onResizeDebounced: (entries: ResizeObserverEntry[]) => void; constructor(props: ResizerProps) { super(props); this.containerRef = React.createRef(); this.ro = new ResizeObserver(this.handleResize); this.animationFrameID = null; + this.onResizeDebounced = () => { }; } componentDidMount() { this.onResizeDebounced = debounce(this.onResize, this.props.resizeDebounce); if (this.containerRef.current) { - const { clientWidth, clientHeight } = this.containerRef.current; - this.props.updateParentDimensions({ width: clientWidth, height: clientHeight, top: 0, left: 0 }); + this.ro.observe(this.containerRef.current as Element); } - this.ro.observe(this.containerRef.current as Element); } componentWillUnmount() { @@ -67,21 +70,11 @@ class Resizer extends React.Component { this.ro.disconnect(); } - private onResizeDebounced: (entries: ResizeObserverEntry[]) => void = () => {}; - private handleResize = (entries: ResizeObserverEntry[]) => { - if (this.initialResizeComplete) { - this.onResizeDebounced(entries); - } else { - this.initialResizeComplete = true; - this.onResize(entries); - } - }; - onResize = (entries: ResizeObserverEntry[]) => { if (!Array.isArray(entries)) { return; } - if (!entries.length || !entries[0]) { + if (entries.length === 0 || !entries[0]) { return; } const { width, height } = entries[0].contentRect; @@ -90,6 +83,15 @@ class Resizer extends React.Component { }); }; + handleResize = (entries: ResizeObserverEntry[]) => { + if (this.initialResizeComplete) { + this.onResizeDebounced(entries); + } else { + this.initialResizeComplete = true; + this.onResize(entries); + } + }; + render() { return
; } @@ -105,9 +107,11 @@ const mapDispatchToProps = (dispatch: Dispatch): ResizerDispatchProps => const mapStateToProps = (state: GlobalChartState): ResizerStateProps => { const settings = getSettingsSpecSelector(state); - const resizeDebounce = settings.resizeDebounce === undefined || settings.resizeDebounce === null ? 200 : settings.resizeDebounce; + const resizeDebounce = settings.resizeDebounce === undefined || settings.resizeDebounce === null + ? 200 : settings.resizeDebounce; return { - resizeDebounce, + resizeDebounce: + !isDefined(resizeDebounce) || Number.isNaN(resizeDebounce) ? DEFAULT_RESIZE_DEBOUNCE : resizeDebounce, }; }; diff --git a/packages/osd-charts/src/components/legend/legend.test.tsx b/packages/osd-charts/src/components/legend/legend.test.tsx index 68e98ebb6aea..b3eedfeef88c 100644 --- a/packages/osd-charts/src/components/legend/legend.test.tsx +++ b/packages/osd-charts/src/components/legend/legend.test.tsx @@ -156,7 +156,7 @@ describe('Legend', () => { class LegendColorPickerMock extends Component< { onLegendItemClick: () => void; customColor: string }, { colors: string[] } - > { + > { state = { colors: ['red'], }; diff --git a/packages/osd-charts/src/components/legend/legend.tsx b/packages/osd-charts/src/components/legend/legend.tsx index 364655b795d6..c5a0340921ea 100644 --- a/packages/osd-charts/src/components/legend/legend.tsx +++ b/packages/osd-charts/src/components/legend/legend.tsx @@ -33,7 +33,7 @@ import { } from '../../state/actions/legend'; import { GlobalChartState } from '../../state/chart_state'; import { getChartThemeSelector } from '../../state/selectors/get_chart_theme'; -import { getInternalIsInitializedSelector } from '../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../state/selectors/get_internal_is_intialized'; import { getLegendItemsSelector } from '../../state/selectors/get_legend_items'; import { getLegendExtraValuesSelector } from '../../state/selectors/get_legend_items_values'; import { getLegendSizeSelector } from '../../state/selectors/get_legend_size'; @@ -146,7 +146,7 @@ const EMPTY_DEFAULT_STATE = { showExtra: false, }; const mapStateToProps = (state: GlobalChartState): LegendStateProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return EMPTY_DEFAULT_STATE; } const { diff --git a/packages/osd-charts/src/components/tooltip/tooltip.tsx b/packages/osd-charts/src/components/tooltip/tooltip.tsx index f66c6a4968c8..9a8f9703f173 100644 --- a/packages/osd-charts/src/components/tooltip/tooltip.tsx +++ b/packages/osd-charts/src/components/tooltip/tooltip.tsx @@ -27,7 +27,7 @@ import { onPointerMove } from '../../state/actions/mouse'; import { GlobalChartState, BackwardRef } from '../../state/chart_state'; import { getChartRotationSelector } from '../../state/selectors/get_chart_rotation'; import { getChartThemeSelector } from '../../state/selectors/get_chart_theme'; -import { getInternalIsInitializedSelector } from '../../state/selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from '../../state/selectors/get_internal_is_intialized'; import { getInternalIsTooltipVisibleSelector } from '../../state/selectors/get_internal_is_tooltip_visible'; import { getInternalTooltipAnchorPositionSelector } from '../../state/selectors/get_internal_tooltip_anchor_position'; import { getInternalTooltipInfoSelector } from '../../state/selectors/get_internal_tooltip_info'; @@ -223,7 +223,7 @@ const mapDispatchToProps = (dispatch: Dispatch): TooltipDispatchProps => bindActionCreators({ onPointerMove }, dispatch); const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return HIDDEN_TOOLTIP_PROPS; } return { diff --git a/packages/osd-charts/src/state/chart_state.ts b/packages/osd-charts/src/state/chart_state.ts index 4024e2f4025c..d66c0dc1ce97 100644 --- a/packages/osd-charts/src/state/chart_state.ts +++ b/packages/osd-charts/src/state/chart_state.ts @@ -38,7 +38,7 @@ import { SET_PERSISTED_COLOR, SET_TEMPORARY_COLOR, CLEAR_TEMPORARY_COLORS } from import { EXTERNAL_POINTER_EVENT } from './actions/events'; import { SPEC_PARSED, SPEC_UNMOUNTED, UPSERT_SPEC, REMOVE_SPEC, SPEC_PARSING } from './actions/specs'; import { interactionsReducer } from './reducers/interactions'; -import { getInternalIsInitializedSelector } from './selectors/get_internal_is_intialized'; +import { getInternalIsInitializedSelector, InitStatus } from './selectors/get_internal_is_intialized'; import { getLegendItemsSelector } from './selectors/get_legend_items'; import { LegendItemLabel } from './selectors/get_legend_items_labels'; @@ -53,7 +53,7 @@ export interface InternalChartState { * The chart type */ chartType: ChartTypes; - isInitialized(globalState: GlobalChartState): boolean; + isInitialized(globalState: GlobalChartState): InitStatus; /** * Returns a JSX element with the chart rendered (lenged excluded) * @param containerRef @@ -379,7 +379,7 @@ export const chartStoreReducer = (chartId: string) => { }, }; default: - if (!getInternalIsInitializedSelector(state)) { + if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) { return state; } return { diff --git a/packages/osd-charts/src/state/selectors/get_internal_is_intialized.ts b/packages/osd-charts/src/state/selectors/get_internal_is_intialized.ts index eea59e1e3027..45055b5d9f7e 100644 --- a/packages/osd-charts/src/state/selectors/get_internal_is_intialized.ts +++ b/packages/osd-charts/src/state/selectors/get_internal_is_intialized.ts @@ -17,12 +17,40 @@ * under the License. */ +import { $Values } from 'utility-types'; + import { GlobalChartState } from '../chart_state'; + +export const InitStatus = Object.freeze({ + ParentSizeInvalid: 'ParentSizeInvalid' as const, + SpecNotInitialized: 'SpecNotInitialized' as const, + MissingChartType: 'MissingChartType' as const, + ChartNotInitialized: 'ChartNotInitialized' as const, + Initialized: 'Initialized' as const, +}); + +export type InitStatus = $Values; + /** @internal */ -export const getInternalIsInitializedSelector = (state: GlobalChartState): boolean => { - if (state.internalChartState) { - return state.internalChartState.isInitialized(state); +export const getInternalIsInitializedSelector = (state: GlobalChartState): InitStatus => { + const { + parentDimensions: { width, height }, + specsInitialized, + internalChartState, + } = state; + + if (!specsInitialized) { + return InitStatus.SpecNotInitialized; } - return false; + + if (!internalChartState) { + return InitStatus.MissingChartType; + } + + if (width <= 0 || height <= 0) { + return InitStatus.ParentSizeInvalid; + } + + return internalChartState.isInitialized(state); }; diff --git a/packages/osd-charts/yarn.lock b/packages/osd-charts/yarn.lock index 13eea8d14d6b..1012cc28a685 100644 --- a/packages/osd-charts/yarn.lock +++ b/packages/osd-charts/yarn.lock @@ -4989,6 +4989,15 @@ jest-diff "^25.2.1" pretty-format "^25.2.1" +"@types/jsdom@^16.2.3": + version "16.2.3" + resolved "https://registry.yarnpkg.com/@types/jsdom/-/jsdom-16.2.3.tgz#c6feadfe0836389b27f9c911cde82cd32e91c537" + integrity sha512-BREatezSn74rmLIDksuqGNFUTi9HNAWWQXYpFBFLK9U6wlMCO4M0QCa8CMpDsZQuqxSO9XifVLT5Q1P0vgKLqw== + dependencies: + "@types/node" "*" + "@types/parse5" "*" + "@types/tough-cookie" "*" + "@types/json-schema@^7.0.3": version "7.0.3" resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.3.tgz#bdfd69d61e464dcc81b25159c270d75a73c1a636" @@ -5046,6 +5055,11 @@ resolved "https://registry.yarnpkg.com/@types/parse-json/-/parse-json-4.0.0.tgz#2f8bb441434d163b35fb8ffdccd7138927ffb8c0" integrity sha512-//oorEZjL6sbPcKUaCdIGlIUeH26mgzimjBB77G6XRgnDl/L5wOnpyBGRe/Mmf5CVW3PwEBE1NjiMZ/ssFh4wA== +"@types/parse5@*": + version "5.0.3" + resolved "https://registry.yarnpkg.com/@types/parse5/-/parse5-5.0.3.tgz#e7b5aebbac150f8b5fdd4a46e7f0bd8e65e19109" + integrity sha512-kUNnecmtkunAoQ3CnjmMkzNU/gtxG8guhi+Fk2U/kOpIKjIMKnXGp4IJCgQJrXSgMsWYimYG4TGjz/UzbGEBTw== + "@types/prettier@^1.19.0": version "1.19.1" resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-1.19.1.tgz#33509849f8e679e4add158959fdb086440e9553f" @@ -5170,6 +5184,11 @@ "@storybook/react" "^5.2.0" "@types/react" "*" +"@types/tough-cookie@*": + version "4.0.0" + resolved "https://registry.yarnpkg.com/@types/tough-cookie/-/tough-cookie-4.0.0.tgz#fef1904e4668b6e5ecee60c52cc6a078ffa6697d" + integrity sha512-I99sngh224D0M7XgW1s120zxCt3VYQ3IQsuw3P3jbq5GG4yc79+ZjyKznyOGIQrflfylLgcfekeZW/vk0yng6A== + "@types/unist@^2.0.0", "@types/unist@^2.0.2": version "2.0.3" resolved "https://registry.yarnpkg.com/@types/unist/-/unist-2.0.3.tgz#9c088679876f374eb5983f150d4787aa6fb32d7e"