Skip to content

Commit

Permalink
Merge pull request #213 from Quartz/error-messaging-refactor
Browse files Browse the repository at this point in the history
refactor error messaging, create ErrorStore, add tests
  • Loading branch information
yanofsky committed Feb 23, 2016
2 parents 50c1f48 + 0eb8b6a commit 3b34357
Show file tree
Hide file tree
Showing 20 changed files with 532 additions and 113 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,12 @@
"lodash": "^4.0.0",
"markdown": "^0.5.0",
"react": "^0.14.5",
"react-addons-pure-render-mixin": "^0.14.5",
"react-addons-shallow-compare": "^0.14.5",
"react-addons-update": "^0.14.5",
"react-addons-pure-render-mixin": "^0.14.5",
"react-dom": "^0.14.5",
"save-svg-as-png": "^1.0.2",
"sizeof": "^1.0.0",
"sugar-date": "^1.5.1"
},
"devDependencies": {
Expand Down
9 changes: 9 additions & 0 deletions src/js/charts/cb-xy/parse-xy.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ function parseXY(config, _chartProps, callback, parseOpts) {

var chartSettings = map(bySeries.series, function(dataSeries, i) {
var settings;

if (chartProps.chartSettings[i]) {
settings = chartProps.chartSettings[i];
} else {
Expand All @@ -60,31 +61,39 @@ function parseXY(config, _chartProps, callback, parseOpts) {

// add data points to relevant scale
if (settings.altAxis === false) {

var _computed = _scaleComputed.primaryScale;
_computed.data = _computed.data.concat(values);
_computed.count += 1;
if (settings.type == "column") {
_computed.hasColumn = true;
}

} else {

var _computed = _scaleComputed.secondaryScale;
_computed.data = _computed.data.concat(values);
_computed.count += 1;
if (settings.type == "column") {
_computed.hasColumn = true;
}

}

return settings;

});

labels.values = map(bySeries.series, function(dataSeries, i) {

if (labels.values[i]) {
return assign({}, { name: chartSettings[i].label}, labels.values[i]);
} else {
return {
name: dataSeries.name
};
}

});

var maxPrecision = 5;
Expand Down
12 changes: 11 additions & 1 deletion src/js/components/Chartbuilder.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var PropTypes = React.PropTypes;
var ChartPropertiesStore = require("../stores/ChartPropertiesStore");
var ChartMetadataStore = require("../stores/ChartMetadataStore");
var SessionStore = require("../stores/SessionStore");
var ErrorStore = require("../stores/ErrorStore");

/*
* Global React components that are used irrespective of chart type
Expand All @@ -22,6 +23,7 @@ var Canvas = require("./Canvas.jsx");
var ChartExport = require("./ChartExport.jsx");
var ChartMetadata = require("./ChartMetadata.jsx");
var ChartTypeSelector = require("./ChartTypeSelector.jsx");
var ErrorDisplay = require("./ErrorDisplay.jsx");
var RendererWrapper = require("./RendererWrapper.jsx");
var LocalStorageTimer = require("./LocalStorageTimer.jsx");

Expand All @@ -48,6 +50,7 @@ function getStateFromStores() {
return {
chartProps: ChartPropertiesStore.getAll(),
metadata: ChartMetadataStore.getAll(),
errors: ErrorStore.getAll(),
session: SessionStore.getAll()
};
}
Expand Down Expand Up @@ -103,13 +106,15 @@ var Chartbuilder = React.createClass({
componentDidMount: function() {
ChartPropertiesStore.addChangeListener(this._onChange);
ChartMetadataStore.addChangeListener(this._onChange);
ErrorStore.addChangeListener(this._onChange);
SessionStore.addChangeListener(this._onChange);
},

/* Remove listeners on component unmount */
componentWillUnmount: function() {
ChartPropertiesStore.removeChangeListener(this._onChange);
ChartMetadataStore.removeChangeListener(this._onChange);
ErrorStore.removeChangeListener(this._onChange);
SessionStore.removeChangeListener(this._onChange);
},

Expand Down Expand Up @@ -183,6 +188,7 @@ var Chartbuilder = React.createClass({
timerOn={this.state.session.timerOn}
/>
<Editor
errors={this.state.errors}
chartProps={this.state.chartProps}
numColors={numColors}
/>
Expand All @@ -193,11 +199,15 @@ var Chartbuilder = React.createClass({
additionalComponents={this.props.additionalComponents.metadata}
/>
{mobileOverrides}
<ErrorDisplay
stepNumber={String(editorSteps + 3)}
messages={this.state.errors.messages}
/>
<ChartExport
data={this.state.chartProps.data}
svgWrapperClassName={svgWrapperClassName.desktop}
metadata={this.state.metadata}
stepNumber={String(editorSteps + 3)}
stepNumber={String(editorSteps + 4)}
additionalComponents={this.props.additionalComponents.misc}
model={this.state}
/>
Expand Down
51 changes: 51 additions & 0 deletions src/js/components/ErrorDisplay.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
var React = require("react");
var PropTypes = React.PropTypes;
var ErrorMessage = require("./shared/ErrorMessage.jsx");

var ErrorDisplay = React.createClass({

propTypes: {
type: PropTypes.number,
text: PropTypes.string
},

_renderErrorMessages: function() {
if (this.props.messages.length === 0) {
return (
<ErrorMessage
type={1}
text="No errors here!"
/>
);
} else {
return this.props.messages.map(function(msg, i) {
return (
<ErrorMessage
key={i}
type={msg.type}
text={msg.text}
/>
);
});
}

},

render: function() {

var errorMessages = this._renderErrorMessages();

return (
<div className="error-display">
<h2>
<span className="step-number">{this.props.stepNumber}</span>
<span>Everything look good?</span>
</h2>
{errorMessages}
</div>
);
}

});

module.exports = ErrorDisplay;
2 changes: 1 addition & 1 deletion src/js/components/RendererWrapper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ var RendererWrapper = React.createClass({
},

shouldComponentUpdate: function(nextProps, nextState) {
if (!nextProps.model.chartProps.input.valid) {
if (!nextProps.model.errors.valid) {
return false;
}
return true;
Expand Down
11 changes: 6 additions & 5 deletions src/js/components/chart-grid/ChartGridEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,7 @@ var ChartGridEditor = React.createClass({

propTypes: {
chartProps: PropTypes.shape({
input: PropTypes.shape({
raw: PropTypes.string,
status: PropTypes.string,
valid: PropTypes.bool
}).isRequired,
input: PropTypes.object.isRequired,
chartSettings: PropTypes.array,
data: PropTypes.array,
scale: PropTypes.shape({
Expand Down Expand Up @@ -132,6 +128,10 @@ var ChartGridEditor = React.createClass({
)
}

var inputErrors = this.props.errors.messages.filter(function(e) {
return e.location === "input";
});

return (
<div className="chartgrid-editor">
<div className="editor-options">
Expand All @@ -140,6 +140,7 @@ var ChartGridEditor = React.createClass({
<span>Input your data</span>
</h2>
<DataInput
errors={inputErrors}
chartProps={chartProps}
className="data-input"
/>
Expand Down
14 changes: 8 additions & 6 deletions src/js/components/chart-xy/XYEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,9 @@ var axisOptions = [
var XYEditor = React.createClass({

propTypes: {
errors: PropTypes.object,
chartProps: PropTypes.shape({
input: PropTypes.shape({
raw: PropTypes.string,
status: PropTypes.string,
valid: PropTypes.bool
}).isRequired,
input: PropTypes.object.isRequired,
chartSettings: PropTypes.array,
data: PropTypes.array,
scale: PropTypes.object,
Expand Down Expand Up @@ -140,9 +137,13 @@ var XYEditor = React.createClass({
stepNumber="5"
onUpdate={this._handlePropUpdate.bind(null, "scale")}
/>
)
);
}

var inputErrors = this.props.errors.messages.filter(function(e) {
return e.location === "input";
});

return (
<div className="xy-editor">
<div className="editor-options">
Expand All @@ -151,6 +152,7 @@ var XYEditor = React.createClass({
<span>Input your data</span>
</h2>
<DataInput
errors={inputErrors}
chartProps={chartProps}
className="data-input"
/>
Expand Down
Loading

0 comments on commit 3b34357

Please sign in to comment.