Skip to content
This repository was archived by the owner on May 15, 2019. It is now read-only.

Commit ef6197e

Browse files
committed
Fix up a number of issues in react-components.
Summary: - Removed all lint errors - Removed all usage of Underscore (this makes it so that react-components.js no longer depends on corelibs-legacy.js!) - Upgrade to use the latest version of React - Upgraded all dev dependencies to latest versions - Fixed issues in react-live-editor and brought in that code: Khan/react-live-editor@9a805f8 Test Plan: - Confirmed that "make test" passes and runs without warnings. - Confirmed that "make" runs. - Confirmed that the docs/index.html page loads without errors or warnings * * i18n.js seems to produce errors in the demo page, specifically: "Invariant Violation: $_(...): A valid React element (or null) must be returned. You may have returned undefined, an array or some other invalid object." It works fine in the tests, I'm not sure what's up! The implementation also seems to be the same as what we have in webapp, I'm assuming that it's an issue with the code editor and how the code is being dynamically compiled. Reviewers: emily, alex Reviewed By: alex Differential Revision: https://phabricator.khanacademy.org/D32383
1 parent cde969e commit ef6197e

39 files changed

+919
-945
lines changed

.babelrc

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"presets": ["latest", "react"]
3+
}

.gitmodules

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
[submodule "react-live-editor"]
22
path = react-live-editor
3-
url = https://github.com/joelburget/react-live-editor.git
3+
url = https://github.com/Khan/react-live-editor

Makefile

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,16 @@ pages: docs
2222
git checkout master
2323

2424
bundle.js: jsdeps
25-
./node_modules/.bin/browserify -t [ reactify --es6 ] js/*.jsx -o bundle.js
25+
./node_modules/.bin/browserify -t [ babelify --presets [ es2015 react ] ] js/*.jsx -o bundle.js
2626

2727
docs/index.html: pydeps
2828
./make_template.py
2929

3030
docs/preview-bundle.js: jsdeps
31-
./node_modules/.bin/browserify -d -t [ reactify --es6 ] reactify-components.jsx -o docs/preview-bundle.js
31+
./node_modules/.bin/browserify -d -t [ babelify --presets [ es2015 react ] ] reactify-components.jsx -o docs/preview-bundle.js
3232

3333
watch-preview: jsdeps
34-
./node_modules/.bin/watchify -d -t [ reactify --es6 ] js/*.jsx reactify-components.jsx -o docs/preview-bundle.js
34+
./node_modules/.bin/watchify -d -t [ babelify --presets [ es2015 react ] ] js/*.jsx reactify-components.jsx -o docs/preview-bundle.js
3535

3636
test: jsdeps
37-
./node_modules/.bin/mocha --reporter spec --compilers jsx:test/compiler.js -r test/test-helper.js test/*test.jsx
37+
./node_modules/.bin/mocha --reporter spec --compilers jsx:babel-register -r test/test-helper.js test/*test.jsx

examples/i18n.jsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ var translated = <$_ first="Motoko" last="Kusanagi">
22
Hello, %(first)s %(last)s!
33
</$_>;
44

5-
var link = <a href="javascript:void 0;" onClick={this._markTooHard}>
5+
var link = <a href="javascript:void 0;">
66
<$_>Click here</$_>
77
</a>;
88

js/backbone-mixin.jsx

+22-15
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
/* TODO(emily): fix these lint errors (http://eslint.org/docs/rules): */
2-
/* eslint-disable comma-dangle, no-var */
3-
/* To fix, remove an entry above, run ka-lint, and fix errors. */
4-
5-
var _ = require('underscore');
61
/* WARNING - DEPRECATED
72
*
83
* We recommend that you don't use this mixin. It's not idiomatic react and
@@ -15,10 +10,10 @@ var _ = require('underscore');
1510
* BackboneMixin - automatic binding and unbinding for react classes mirroring
1611
* backbone models and views. Example:
1712
*
18-
* var Model = Backbone.Model.extend({ ... });
19-
* var Collection = Backbone.Collection.extend({ ... });
13+
* const Model = Backbone.Model.extend({ ... });
14+
* const Collection = Backbone.Collection.extend({ ... });
2015
*
21-
* var Example = React.createClass({
16+
* const Example = React.createClass({
2217
* mixins: [BackboneMixin],
2318
* getBackboneModels: function() {
2419
* return [this.model, this.collection];
@@ -30,7 +25,7 @@ var _ = require('underscore');
3025
*
3126
* This binds *and* unbinds the events.
3227
*/
33-
var BackboneMixin = {
28+
const BackboneMixin = {
3429
componentDidMount: function() {
3530
this._backboneModels = this.getBackboneModels();
3631
this._validateModelArray(this._backboneModels);
@@ -44,11 +39,23 @@ var BackboneMixin = {
4439

4540
// The backbone models may have changed - rebind to the new ones
4641
componentDidUpdate: function(nextProps, nextState) {
47-
var previousModels = this._backboneModels;
48-
var currentModels = this._backboneModels = this.getBackboneModels();
42+
const previousModels = this._backboneModels;
43+
const currentModels = this._backboneModels = this.getBackboneModels();
44+
45+
const oldModels = [];
46+
const newModels = [];
47+
48+
for (const model of previousModels) {
49+
if (currentModels.indexOf(model) < 0) {
50+
oldModels.push(model);
51+
}
52+
}
4953

50-
var oldModels = _(previousModels).difference(currentModels);
51-
var newModels = _(currentModels).difference(previousModels);
54+
for (const model of currentModels) {
55+
if (previousModels.indexOf(model) < 0) {
56+
newModels.push(model);
57+
}
58+
}
5259

5360
this._unbind(oldModels);
5461
this._bind(newModels);
@@ -78,11 +85,11 @@ var BackboneMixin = {
7885
},
7986

8087
_validateModelArray: function(backboneModels) {
81-
if (!_.isArray(backboneModels)) {
88+
if (!Array.isArray(backboneModels)) {
8289
throw new Error("getBackboneModels must return an array. " +
8390
"get this " + backboneModels + " out of here.");
8491
}
85-
}
92+
},
8693
};
8794

8895
module.exports = BackboneMixin;

js/blur-input.jsx

+16-16
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
/* TODO(emily): fix these lint errors (http://eslint.org/docs/rules): */
2-
/* eslint-disable comma-dangle, no-var, react/jsx-closing-bracket-location, react/jsx-sort-prop-types, react/sort-comp */
3-
/* To fix, remove an entry above, run ka-lint, and fix errors. */
4-
5-
var React = require("react");
1+
const React = require("react");
62

73
/* You know when you want to propagate input to a parent...
84
* but then that parent does something with the input...
@@ -16,22 +12,16 @@ var React = require("react");
1612
* Enough melodrama. Its an input that only sends changes
1713
* to its parent on blur.
1814
*/
19-
var BlurInput = React.createClass({
15+
const BlurInput = React.createClass({
2016
propTypes: {
17+
className: React.PropTypes.string,
18+
style: React.PropTypes.any,
2119
value: React.PropTypes.string.isRequired,
22-
onChange: React.PropTypes.func.isRequired
20+
onChange: React.PropTypes.func.isRequired,
2321
},
2422
getInitialState: function() {
2523
return {value: this.props.value};
2624
},
27-
render: function() {
28-
return <input
29-
{...this.props}
30-
type="text"
31-
value={this.state.value}
32-
onChange={this.handleChange}
33-
onBlur={this.handleBlur} />;
34-
},
3525
componentWillReceiveProps: function(nextProps) {
3626
this.setState({value: nextProps.value});
3727
},
@@ -40,7 +30,17 @@ var BlurInput = React.createClass({
4030
},
4131
handleBlur: function(e) {
4232
this.props.onChange(e.target.value);
43-
}
33+
},
34+
render: function() {
35+
return <input
36+
className={this.props.className}
37+
style={this.props.style}
38+
type="text"
39+
value={this.state.value}
40+
onChange={this.handleChange}
41+
onBlur={this.handleBlur}
42+
/>;
43+
},
4444
});
4545

4646
module.exports = BlurInput;

js/button-group.jsx

+38-42
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,3 @@
1-
/* TODO(emily): fix these lint errors (http://eslint.org/docs/rules): */
2-
/* eslint-disable comma-dangle, indent, no-var, react/jsx-closing-bracket-location, react/jsx-indent-props, react/jsx-sort-prop-types, react/sort-comp */
3-
/* To fix, remove an entry above, run ka-lint, and fix errors. */
4-
5-
var React = require('react');
6-
var ReactDOM = require("react-dom");
7-
var _ = require('underscore');
8-
var styles = require('./styles.js');
9-
var css = require("aphrodite").css;
10-
111
/* ButtonGroup is an aesthetically pleasing group of buttons.
122
*
133
* The class requires these properties:
@@ -27,66 +17,72 @@ var css = require("aphrodite").css;
2717
* Requires stylesheets/perseus-admin-package/editor.less to look nice.
2818
*/
2919

30-
var ButtonGroup = React.createClass({
20+
const React = require('react');
21+
const ReactDOM = require("react-dom");
22+
const styles = require('./styles.js');
23+
const css = require("aphrodite").css;
24+
25+
const ButtonGroup = React.createClass({
3126
propTypes: {
3227
value: React.PropTypes.any,
3328
buttons: React.PropTypes.arrayOf(React.PropTypes.shape({
3429
value: React.PropTypes.any.isRequired,
3530
content: React.PropTypes.node,
36-
title: React.PropTypes.string
31+
title: React.PropTypes.string,
3732
})).isRequired,
3833
onChange: React.PropTypes.func.isRequired,
39-
allowEmpty: React.PropTypes.bool
34+
allowEmpty: React.PropTypes.bool,
4035
},
4136

4237
getDefaultProps: function() {
4338
return {
4439
value: null,
45-
allowEmpty: true
40+
allowEmpty: true,
4641
};
4742
},
4843

49-
render: function() {
50-
var value = this.props.value;
51-
var buttons = _(this.props.buttons).map((button, i) => {
52-
return <button title={button.title}
53-
type="button"
54-
id={"" + i}
55-
ref={"button" + i}
56-
key={"" + i}
57-
className={css(
58-
styles.button.buttonStyle,
59-
button.value === value &&
60-
styles.button.selectedStyle
61-
)}
62-
onClick={this.toggleSelect.bind(this, button.value)}>
63-
{button.content || "" + button.value}
64-
</button>;
65-
});
66-
67-
var outerStyle = {
68-
display: 'inline-block',
69-
};
70-
return <div style={outerStyle}>
71-
{buttons}
72-
</div>;
73-
},
74-
7544
focus: function() {
7645
ReactDOM.findDOMNode(this).focus();
7746
return true;
7847
},
7948

8049
toggleSelect: function(newValue) {
81-
var value = this.props.value;
50+
const value = this.props.value;
8251

8352
if (this.props.allowEmpty) {
8453
// Select the new button or unselect if it's already selected
8554
this.props.onChange(value !== newValue ? newValue : null);
8655
} else {
8756
this.props.onChange(newValue);
8857
}
89-
}
58+
},
59+
60+
render: function() {
61+
const value = this.props.value;
62+
const buttons = this.props.buttons.map((button, i) => {
63+
return <button title={button.title}
64+
type="button"
65+
id={"" + i}
66+
ref={"button" + i}
67+
key={"" + i}
68+
className={css(
69+
styles.button.buttonStyle,
70+
button.value === value &&
71+
styles.button.selectedStyle
72+
)}
73+
onClick={this.toggleSelect.bind(this, button.value)}
74+
>
75+
{button.content || "" + button.value}
76+
</button>;
77+
});
78+
79+
const outerStyle = {
80+
display: 'inline-block',
81+
};
82+
return <div style={outerStyle}>
83+
{buttons}
84+
</div>;
85+
},
9086
});
9187

9288
module.exports = ButtonGroup;

js/drag-target.jsx

+26-29
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,3 @@
1-
/* TODO(emily): fix these lint errors (http://eslint.org/docs/rules): */
2-
/* eslint-disable comma-dangle, no-var, react/jsx-closing-bracket-location, react/jsx-sort-prop-types, react/prop-types, react/sort-comp */
3-
/* To fix, remove an entry above, run ka-lint, and fix errors. */
4-
5-
var React = require('react');
6-
var _ = require('underscore');
7-
81
/* This component makes its children a drag target. Example:
92
*
103
* <DragTarget onDrop={this.handleDrop}>Drag to me</DragTarget>
@@ -28,36 +21,25 @@ var _ = require('underscore');
2821
// Other extensions:
2922
// * custom styles for global drag and dragOver
3023
// * only respond to certain types of drags (only images for instance)!
31-
var DragTarget = React.createClass({
24+
25+
const React = require('react');
26+
27+
const DragTarget = React.createClass({
3228
propTypes: {
3329
onDrop: React.PropTypes.func.isRequired,
3430
component: React.PropTypes.any, // component type
35-
shouldDragHighlight: React.PropTypes.func
36-
},
37-
render: function() {
38-
var opacity = this.state.dragHover ? {"opacity": 0.3} : {};
39-
var Component = this.props.component;
40-
41-
return (
42-
<Component
43-
{...this.props}
44-
style={_.extend({}, this.props.style, opacity)}
45-
onDrop={this.handleDrop}
46-
onDragEnd={this.handleDragEnd}
47-
onDragOver={this.handleDragOver}
48-
onDragEnter={this.handleDragEnter}
49-
onDragLeave={this.handleDragLeave} />
50-
);
51-
},
52-
getInitialState: function() {
53-
return {dragHover: false};
31+
shouldDragHighlight: React.PropTypes.func,
32+
style: React.PropTypes.any,
5433
},
5534
getDefaultProps: function() {
5635
return {
5736
component: "div",
58-
shouldDragHighlight: () => true
37+
shouldDragHighlight: () => true,
5938
};
6039
},
40+
getInitialState: function() {
41+
return {dragHover: false};
42+
},
6143
handleDrop: function(e) {
6244
e.stopPropagation();
6345
e.preventDefault();
@@ -75,7 +57,22 @@ var DragTarget = React.createClass({
7557
},
7658
handleDragEnter: function(e) {
7759
this.setState({dragHover: this.props.shouldDragHighlight(e)});
78-
}
60+
},
61+
render: function() {
62+
const opacity = this.state.dragHover ? {"opacity": 0.3} : {};
63+
const Component = this.props.component;
64+
65+
return (
66+
<Component
67+
style={Object.assign({}, this.props.style, opacity)}
68+
onDrop={this.handleDrop}
69+
onDragEnd={this.handleDragEnd}
70+
onDragOver={this.handleDragOver}
71+
onDragEnter={this.handleDragEnter}
72+
onDragLeave={this.handleDragLeave}
73+
/>
74+
);
75+
},
7976
});
8077

8178
module.exports = DragTarget;

0 commit comments

Comments
 (0)