Skip to content

Commit b01efa7

Browse files
author
Mat Brown
committed
Move responsibility for displaying spinner into ErrorReport
The question of whether to show a spinner can reasonably be considered a responsibility of the ErrorReport, so push it down to that component and allow the ErrorReportContainer to figure out whether it should show up. Allows us to make the Output dumber.
1 parent 5fc9eb4 commit b01efa7

4 files changed

Lines changed: 22 additions & 19 deletions

File tree

src/components/ErrorReport.jsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,16 @@ import classnames from 'classnames';
44
import find from 'lodash/find';
55
import ErrorList from './ErrorList';
66

7-
function ErrorReport({errors, onErrorClick}) {
7+
function ErrorReport({errors, isValidating, onErrorClick}) {
8+
if (isValidating) {
9+
return <div className="output__delayed-error-overlay" />;
10+
}
11+
12+
const hasErrors = Boolean(find(errors, list => list.items.length));
13+
if (!hasErrors) {
14+
return null;
15+
}
16+
817
const isDocked = Boolean(find(errors, {state: 'runtime-error'}));
918
const {html, css, javascript} = errors;
1019

@@ -41,6 +50,7 @@ ErrorReport.propTypes = {
4150
html: PropTypes.object.isRequired,
4251
javascript: PropTypes.object.isRequired,
4352
}).isRequired,
53+
isValidating: PropTypes.bool.isRequired,
4454
onErrorClick: PropTypes.func.isRequired,
4555
};
4656

src/components/Output.jsx

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,7 @@ import {t} from 'i18next';
44
import classnames from 'classnames';
55
import {ErrorReport, Preview} from '../containers';
66

7-
const errorStates = new Set(['validation-error', 'runtime-error']);
8-
97
class Output extends React.Component {
10-
_renderErrors() {
11-
if (this.props.validationState === 'validating') {
12-
return <div className="output__delayed-error-overlay" />;
13-
}
14-
15-
if (errorStates.has(this.props.validationState)) {
16-
return <ErrorReport />;
17-
}
18-
19-
return null;
20-
}
21-
228
render() {
239
const {
2410
isDraggingColumnDivider,
@@ -46,7 +32,7 @@ class Output extends React.Component {
4632
{t('workspace.components.output')}
4733
</div>
4834
<Preview />
49-
{this._renderErrors()}
35+
<ErrorReport />
5036
</div>
5137
</div>
5238
);
@@ -57,7 +43,6 @@ Output.propTypes = {
5743
isDraggingColumnDivider: PropTypes.bool.isRequired,
5844
isHidden: PropTypes.bool.isRequired,
5945
style: PropTypes.object.isRequired,
60-
validationState: PropTypes.string.isRequired,
6146
onHide: PropTypes.func.isRequired,
6247
onRef: PropTypes.func.isRequired,
6348
};

src/components/Workspace.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,6 @@ class Workspace extends React.Component {
228228
isDraggingColumnDivider={isDraggingColumnDivider}
229229
isHidden={includes(hiddenUIComponents, 'output')}
230230
style={{flex: rowsFlex[1]}}
231-
validationState={this._getOverallValidationState()}
232231
onHide={
233232
partial(this._handleComponentHide,
234233
'output')

src/containers/ErrorReport.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,16 @@ import {ErrorReport} from '../components';
33
import {focusLine} from '../actions';
44

55
function mapStateToProps(state) {
6-
return {errors: state.get('errors').toJS()};
6+
return {
7+
errors: state.get('errors').toJS(),
8+
isValidating: Boolean(
9+
state.getIn(['ui', 'editors', 'typing']) &&
10+
state.get('errors').find(
11+
error => error.get('state') === 'validation-error',
12+
) ||
13+
state.get('errors').find(error => error.get('state') === 'validating'),
14+
),
15+
};
716
}
817

918
function mapDispatchToProps(dispatch) {

0 commit comments

Comments
 (0)