Skip to content

Merging Hub into Editor #176

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Dec 19, 2017
Merged

Merging Hub into Editor #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
71ba63b
merging hub into editor
nicolaskruchten Dec 19, 2017
e1ddddc
adding react back in
nicolaskruchten Dec 19, 2017
83f8d07
fixing some tests
nicolaskruchten Dec 19, 2017
947a74e
bugfix
nicolaskruchten Dec 19, 2017
aec613c
fixing all tests
nicolaskruchten Dec 19, 2017
dceb164
explanatory comments
nicolaskruchten Dec 19, 2017
90a60f3
PR feedback
nicolaskruchten Dec 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions examples/async-data/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions examples/async-data/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
"plotly.js": "^1.31.2",
"react": "^15.6.1",
"react-dom": "^15.6.1",
"react-plotly.js": "^1.0.4",
"react-plotly.js-editor": "^0.1.0",
"react-scripts": "1.0.17"
},
"scripts": {
Expand Down
108 changes: 46 additions & 62 deletions examples/async-data/src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import './App.css';
import 'react-plotly.js-editor/lib/react-plotly.js-editor.css';
import 'react-select/dist/react-select.css';
import PlotlyEditor, {
EDITOR_ACTIONS,
Hub,
dereference,
} from 'react-plotly.js-editor';
import PlotlyEditor, {dereference} from 'react-plotly.js-editor';
import React, {Component} from 'react';
import createPlotComponent from 'react-plotly.js/factory';
import ee from 'event-emitter';
Expand Down Expand Up @@ -49,6 +45,7 @@ class Backend {
ee(Backend.prototype);

const backend = new Backend({
// eslint-disable-next-line no-magic-numbers
dataSources: {col1: [1, 2, 3], col2: [4, 3, 2], col3: [17, 13, 9]},
delay: 10,
});
Expand All @@ -57,38 +54,25 @@ class App extends Component {
constructor() {
super();

// A basic starting plotly.js figure object. Instead of assigning
const figure = {
// This object is passed to Plotly.js, which then causes it to be
// overwritten with a full DOM node that contains data, layout, _fullData,
// _fullLayout etc in handlePlotUpdate()
const graphDiv = {
data: [{type: 'scatter'}],
layout: {title: 'Room readings'},
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this, when the plot component updates, it sets graphDiv to the actual dom element:
https://github.com/plotly/react-plotly.js/blob/master/src/factory.js#L167

it's that actual dom element that we're passing around and deriving all our values from, so making it seem like it is not the case is confusing to me

@bpostlethwaite is this not the case, am I missing something here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I don't think I understand how that state.figure used to work and update, what was making it update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plotly.js would basically copy the figure.data and figure.layout references into graphDiv and pass it to the Editor via the Hub... Now it's just more explicit that the App has a handle on graphDiv: it partially-constructs it (i.e. gives it data and layout) and then Plotly.js adds all the other stuff it needs to (basically by overwriting it every time it updates)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but to me it seems like the graphDiv that plotly.js passes is the actual dom div element.. when looking at the code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/plotly/react-plotly.js/blob/master/src/factory.js#L189

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re the code here, we could also just have an initialFigure = {data, layout} type thing, with the initial value of graphDiv as null... I had it this way earlier and it worked as well, if we want to make it really explicit what's happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what note would you add?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that graphDiv is later on replaced by a full dom node with all the other properties of plotly.js

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re the code here, we could also just have an initialFigure = {data, layout} type thing, with the initial value of graphDiv as null
I think I prefer that

Then no note is really needed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment per your suggestion. The reason I removed the initialFigure thing is that in render() it looked really weird to have <Plot data={this.initialFigure.data} layout={this.initialFigure.layout} ... /> because it seemed like it would never get new data.

// Store the figure, dataSource and dataSourceOptions in state.
this.state = {figure, dataSources: {}};

// Instantiate a Hub object. The hub is a convenience module that updates
// the applies Editor updates to the figure object. After an update it
// will call the onUpdate function with the editor and plot revision numbers.
// We set these in our state and pass them to react-plotly.js-editor and
// react-plotly.js plot component respectively. This is necessary because
// the plotly.js library will mutate the figure object with user interactions.
// The hub listens for events from plotly.js and alerts us to the mutation here.
// The Editor follows the same mutation pattern (for good or ill) and the Hub
// will pick up editor results in the same way.
this.hub = new Hub({
debug: true,
onUpdate: ({editorRevision, plotRevision}) =>
this.setState(prevState => ({
...prevState,
editorRevision,
plotRevision,
})),
});
this.state = {
graphDiv,
editorRevision: 0,
plotRevision: 0,
dataSources: {},
};

this.getChartingData = this.getChartingData.bind(this);
this.setChartingData = this.setChartingData.bind(this);
this.setChartingDataOptions = this.setChartingDataOptions.bind(this);
this.onEditorUpdate = this.onEditorUpdate.bind(this);
}

componentDidMount() {
Expand All @@ -98,18 +82,18 @@ class App extends Component {
this.getChartingDataColumnsNames();
}

componentDidUnmount() {
componentWillUnmount() {
backend.off('ChartingDataColumnNames', this.setChartingDataOptions);
backend.off('ChartingData', this.setChartingData);
}

setChartingDataOptions(columnNames) {
const dataSourceOptions = columnNames.map(name => ({
value: name,
label: name,
}));
this.setState(prevState => ({...prevState, dataSourceOptions}));
this.hub.editorSourcesUpdated();
this.setState({
dataSourceOptions: columnNames.map(name => ({
value: name,
label: name,
})),
});
}

getChartingDataColumnsNames() {
Expand All @@ -127,35 +111,34 @@ class App extends Component {

setChartingData({columnName, data}) {
if (Array.isArray(data) && data.length) {
const {dataSources, ...otherState} = this.state;
dataSources[columnName] = data;
dereference(this.state.figure.data, dataSources);
this.setState({
dataSources,
...otherState,
this.setState(({dataSources, graphDiv}) => {
const newDataSources = {...dataSources, [columnName]: data};
dereference(graphDiv.data, newDataSources);
return {dataSources: newDataSources, graphDiv};
});
this.hub.figureUpdated();
}
}

onEditorUpdate(event) {
const {type, payload} = event;
if (type === EDITOR_ACTIONS.UPDATE_TRACES) {
const {update} = payload;
if (update) {
for (const key in update) {
if (key.substr(key.length - 3) === 'src') {
const columnId = update[key];
const data = this.state.dataSources[columnId];
if (!Array.isArray(data).length || !data.length) {
this.getChartingData(columnId);
}
handleEditorUpdateTraces({update}) {
if (update) {
for (const key in update) {
if (key.substr(key.length - 3) === 'src') {
const columnId = update[key];
const data = this.state.dataSources[columnId];
if (!Array.isArray(data).length || !data.length) {
this.getChartingData(columnId);
}
}
}
}
}

this.hub.handleEditorUpdate(event);
handlePlotUpdate(graphDiv) {
this.setState(({editorRevision: x}) => ({editorRevision: x + 1, graphDiv}));
}

handleEditorUpdate() {
this.setState(({plotRevision: x}) => ({plotRevision: x + 1}));
}

render() {
Expand All @@ -165,17 +148,18 @@ class App extends Component {
locale="en"
dataSources={this.state.dataSources}
dataSourceOptions={this.state.dataSourceOptions}
graphDiv={this.hub.graphDiv}
onUpdate={this.onEditorUpdate}
plotly={plotly}
graphDiv={this.state.graphDiv}
onUpdate={this.handleEditorUpdate.bind(this)}
onUpdateTraces={this.handleEditorUpdateTraces.bind(this)}
revision={this.state.editorRevision}
plotly={plotly}
/>
<Plot
debug
data={this.state.figure.data}
layout={this.state.figure.layout}
onUpdate={this.hub.handlePlotUpdate}
onInitialized={this.hub.handlePlotInitialized}
data={this.state.graphDiv.data}
layout={this.state.graphDiv.layout}
onUpdate={this.handlePlotUpdate.bind(this)}
onInitialized={this.handlePlotUpdate.bind(this)}
revision={this.state.plotRevision}
/>
</div>
Expand Down
54 changes: 24 additions & 30 deletions examples/simple/src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, {Component} from 'react';
import plotly from 'plotly.js/dist/plotly-basic';
import createPlotComponent from 'react-plotly.js/factory';
import PlotlyEditor, {Hub, dereference} from 'react-plotly.js-editor';
import PlotlyEditor, {dereference} from 'react-plotly.js-editor';
import 'react-plotly.js-editor/lib/react-plotly.js-editor.css';
import 'react-select/dist/react-select.css';

Expand All @@ -12,46 +12,40 @@ class App extends Component {
super();

// DataSources are a mapping of column or data source ids to data arrays.
// eslint-disable-next-line no-magic-numbers
const dataSources = {col1: [1, 2, 3], col2: [4, 3, 2], col3: [17, 13, 9]};
const dataSourceOptions = [
{value: 'col1', label: 'CO2'},
{value: 'col2', label: 'NO2'},
{value: 'col3', label: 'SiO2'},
];

// A basic starting plotly.js figure object. Instead of assigning
const figure = {
// This object is passed to Plotly.js, which then causes it to be
// overwritten with a full DOM node that contains data, layout, _fullData,
// _fullLayout etc in handlePlotUpdate()
const graphDiv = {
data: [{type: 'scatter', xsrc: 'col1', ysrc: 'col2'}],
layout: {title: 'Room readings'},
};

dereference(figure.data, dataSources);
dereference(graphDiv.data, dataSources);

// Store the figure, dataSource and dataSourceOptions in state.
this.state = {
figure,
graphDiv,
editorRevision: 0,
plotRevision: 0,
dataSources,
dataSourceOptions,
};
}

handlePlotUpdate(graphDiv) {
this.setState(({editorRevision: x}) => ({editorRevision: x + 1, graphDiv}));
}

// Instantiate a Hub object. The hub is a convenience module that updates
// the applies Editor updates to the figure object. After an update it
// will call the onUpdate function with the editor and plot revision numbers.
// We set these in our state and pass them to react-plotly.js-editor and
// react-plotly.js plot component respectively. This is necessary because
// the plotly.js library will mutate the figure object with user interactions.
// The hub listens for events from plotly.js and alerts us to the mutation here.
// The Editor follows the same mutation pattern (for good or ill) and the Hub
// will pick up editor results in the same way.
this.hub = new Hub({
debug: true,
onUpdate: ({editorRevision, plotRevision}) =>
this.setState(prevState => ({
...prevState,
editorRevision,
plotRevision,
})),
});
handleEditorUpdate() {
this.setState(({plotRevision: x}) => ({plotRevision: x + 1}));
}

render() {
Expand All @@ -61,18 +55,18 @@ class App extends Component {
locale="en"
dataSources={this.state.dataSources}
dataSourceOptions={this.state.dataSourceOptions}
graphDiv={this.hub.graphDiv}
onUpdate={this.hub.handleEditorUpdate}
plotly={plotly}
graphDiv={this.state.graphDiv}
onUpdate={this.handleEditorUpdate.bind(this)}
revision={this.state.editorRevision}
plotly={plotly}
/>
<div className="plotlyPlot">
<Plot
debug
data={this.state.figure.data}
layout={this.state.figure.layout}
onUpdate={this.hub.handlePlotUpdate}
onInitialized={this.hub.handlePlotInitialized}
data={this.state.graphDiv.data}
layout={this.state.graphDiv.layout}
onUpdate={this.handlePlotUpdate.bind(this)}
onInitialized={this.handlePlotUpdate.bind(this)}
revision={this.state.plotRevision}
/>
</div>
Expand Down
Loading