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
elastic · Jun 11, 2020 · ebf2748 · ebf2748
1 parent 5581c3c
commit ebf2748
Show file tree

Hide file tree

Showing 30 changed files with 288 additions and 110 deletions.
diff --git a/.playground/index.html b/.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;
       }
     </style>
   </head>
+
   <body>
     <div class="page">
       <div id="root"></div>

diff --git a/.playground/playground.tsx b/.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 }) => (
+  <div id="colorPicker">
+    <span>Custom Color Picker</span>
+    <button
+      id="change"
+      type="button"
+      onClick={() => {
+        setTimeout(() => {
+          onClose();
+          setColors(customColor);
+        }, 0);
+      }}
+    >
+      {customColor}
+    </button>
+    <button id="close" type="button" onClick={onClose}>
+      close
+    </button>
+  </div>
+);
+function LegendColorPickerMock(props: { onLegendItemClick: () => void; customColor: string }) {
+  const data = dg.generateGroupedSeries(10, 4, 'split');
+  const [color, setColor] = useState('red');
+
+  return (
+    <>
+      <button
+        type="button"
+        onClick={() => {
+          setColor('violet');
+        }}
+      >
+        change
+      </button>
+      <Chart className="chart">
+        <Settings
+          showLegend
+          onLegendItemClick={props.onLegendItemClick}
+          legendColorPicker={legendColorPickerFn(setColor, props.customColor)}
+        />
+        <BarSeries
+          id="areas"
+          xScaleType={ScaleType.Linear}
+          yScaleType={ScaleType.Linear}
+          xAccessor="x"
+          yAccessors={['y']}
+          splitSeriesAccessors={['g']}
+          color={color}
+          data={data}
+        />
+      </Chart>
+    </>
+  );
+}
 
 export class Playground extends React.Component {
   render() {
     return (
-      <div className="testing">
-        <div className="chart">{Example()}</div>
-      </div>
+      <LegendColorPickerMock
+        customColor="blue"
+        onLegendItemClick={() => {
+          // npo
+        }}
+      />
     );
   }
 }
diff --git a/package.json b/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/scripts/setup_enzyme.ts b/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/src/chart_types/goal_chart/renderer/canvas/connected_component.tsx b/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/src/chart_types/goal_chart/state/chart_state.tsx b/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/src/chart_types/partition_chart/layout/viewmodel/viewmodel.ts b/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/src/chart_types/partition_chart/renderer/canvas/partition.tsx b/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/src/chart_types/partition_chart/renderer/dom/highlighter_hover.tsx b/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/src/chart_types/partition_chart/renderer/dom/highlighter_legend.tsx b/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/src/chart_types/partition_chart/state/chart_state.tsx b/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/src/chart_types/xy_chart/domains/x_domain.ts b/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/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/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<XYChartProps> {
       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 (
-        <div className="echReactiveChart_unavailable">
-          <p>No data to display</p>
-        </div>
-      );
-    }
     return (
       <canvas
         ref={forwardStageRef}
@@ -226,7 +219,7 @@ const DEFAULT_PROPS: ReactiveChartStateProps = {
 };
 
 const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => {
-  if (!getInternalIsInitializedSelector(state)) {
+  if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) {
     return DEFAULT_PROPS;
   }