Skip to content

Commit

Permalink
fix: remove double rendering (#693)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markov00 authored Jun 11, 2020
1 parent 5581c3c commit ebf2748
Show file tree
Hide file tree
Showing 30 changed files with 288 additions and 110 deletions.
8 changes: 8 additions & 0 deletions .playground/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
height: 100%;*/
/* overflow-x: hidden; */
}

#root {
height: 500px;
}

.chart {
background: white;
/*display: inline-block;
Expand All @@ -40,14 +45,17 @@
width: 100%;
overflow: auto;
}

.page {
padding: 100px;
}

label {
display: block;
}
</style>
</head>

<body>
<div class="page">
<div id="root"></div>
Expand Down
72 changes: 67 additions & 5 deletions .playground/playground.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
}}
/>
);
}
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
28 changes: 28 additions & 0 deletions scripts/setup_enzyme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/chart_types/goal_chart/state/chart_state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
4 changes: 2 additions & 2 deletions src/chart_types/partition_chart/renderer/canvas/partition.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/chart_types/partition_chart/state/chart_state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/chart_types/xy_chart/domains/x_domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()];
Expand Down
15 changes: 4 additions & 11 deletions src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -226,7 +219,7 @@ const DEFAULT_PROPS: ReactiveChartStateProps = {
};

const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => {
if (!getInternalIsInitializedSelector(state)) {
if (getInternalIsInitializedSelector(state) !== InitStatus.Initialized) {
return DEFAULT_PROPS;
}

Expand Down
Loading

0 comments on commit ebf2748

Please sign in to comment.