Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove double rendering #693

Merged
merged 12 commits into from
Jun 11, 2020
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