code review

This commit is contained in:
DD Liu 2017-07-27 00:34:33 -04:00
parent dc683f1d82
commit a4891d22b0
10 changed files with 40 additions and 51 deletions

View file

@ -1,3 +1,4 @@
{ {
"plugins": ["transform-object-rest-spread"],
"presets": ["es2015", "react"] "presets": ["es2015", "react"]
} }

View file

@ -9,7 +9,7 @@
"deploy": "touch playground/.nojekyll && gh-pages -t -d playground -m \"Build for $(git log --pretty=format:%H -n1)\"", "deploy": "touch playground/.nojekyll && gh-pages -t -d playground -m \"Build for $(git log --pretty=format:%H -n1)\"",
"lint": "eslint . --ext .js,.jsx", "lint": "eslint . --ext .js,.jsx",
"start": "webpack-dev-server", "start": "webpack-dev-server",
"test": "npm run lint && npm run build && npm run unit-test", "test": "npm run lint && npm run build && npm run unit",
"unit": "jest", "unit": "jest",
"watch": "webpack --progress --colors --watch" "watch": "webpack --progress --colors --watch"
}, },
@ -44,15 +44,17 @@
"gh-pages": "github:rschamp/gh-pages#publish-branch-to-subfolder", "gh-pages": "github:rschamp/gh-pages#publish-branch-to-subfolder",
"html-webpack-plugin": "2.28.0", "html-webpack-plugin": "2.28.0",
"jest": "^20.0.4", "jest": "^20.0.4",
"keymirror": "0.1.1",
"lodash.bindall": "4.4.0",
"lodash.defaultsdeep": "4.6.0", "lodash.defaultsdeep": "4.6.0",
"minilog": "3.1.0", "minilog": "3.1.0",
"mkdirp": "^0.5.1", "mkdirp": "^0.5.1",
"paper": "^0.11.4", "paper": "0.11.4",
"postcss-import": "^10.0.0", "postcss-import": "^10.0.0",
"postcss-loader": "^2.0.5", "postcss-loader": "^2.0.5",
"postcss-simple-vars": "^4.0.0", "postcss-simple-vars": "^4.0.0",
"prop-types": "^15.5.10", "prop-types": "^15.5.10",
"react": "15.5.4", "react": "15.6.1",
"react-dom": "15.5.4", "react-dom": "15.5.4",
"react-intl": "2.3.0", "react-intl": "2.3.0",
"react-redux": "5.0.5", "react-redux": "5.0.5",

View file

@ -1,6 +1,7 @@
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import React from 'react'; import React from 'react';
import PaperCanvas from '../containers/paper-canvas.jsx'; import PaperCanvas from '../containers/paper-canvas.jsx';
import ToolTypes from '../tools/tool-types.js';
const PaintEditorComponent = props => ( const PaintEditorComponent = props => (
<PaperCanvas <PaperCanvas
@ -9,9 +10,7 @@ const PaintEditorComponent = props => (
); );
PaintEditorComponent.propTypes = { PaintEditorComponent.propTypes = {
tool: PropTypes.shape({ tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
name: PropTypes.string.isRequired
})
}; };
export default PaintEditorComponent; export default PaintEditorComponent;

View file

@ -7,26 +7,25 @@ import {connect} from 'react-redux';
class PaintEditor extends React.Component { class PaintEditor extends React.Component {
componentDidMount () { componentDidMount () {
const onKeyPress = this.props.onKeyPress; document.addEventListener('keydown', this.props.onKeyPress);
document.onkeydown = function (e) { }
e = e || window.event; componentWillUnmount () {
onKeyPress(e); document.removeEventListener('keydown', this.props.onKeyPress);
};
} }
render () { render () {
const {
onKeyPress, // eslint-disable-line no-unused-vars
...props
} = this.props;
return ( return (
<PaintEditorComponent <PaintEditorComponent {...props} />
tool={this.props.tool}
/>
); );
} }
} }
PaintEditor.propTypes = { PaintEditor.propTypes = {
onKeyPress: PropTypes.func.isRequired, onKeyPress: PropTypes.func.isRequired,
tool: PropTypes.shape({ tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
name: PropTypes.string.isRequired
})
}; };
const mapStateToProps = state => ({ const mapStateToProps = state => ({

View file

@ -4,13 +4,8 @@ import paper from 'paper';
import ToolTypes from '../tools/tool-types.js'; import ToolTypes from '../tools/tool-types.js';
class PaperCanvas extends React.Component { class PaperCanvas extends React.Component {
constructor (props) {
super(props);
this.state = {
};
}
componentDidMount () { componentDidMount () {
paper.setup('paper-canvas'); paper.setup(this.canvas);
// Create a Paper.js Path to draw a line into it: // Create a Paper.js Path to draw a line into it:
const path = new paper.Path(); const path = new paper.Path();
// Give the stroke a color // Give the stroke a color
@ -25,23 +20,26 @@ class PaperCanvas extends React.Component {
paper.view.draw(); paper.view.draw();
} }
componentWillReceiveProps (nextProps) { componentWillReceiveProps (nextProps) {
if (nextProps.tool !== this.props.tool && nextProps.tool instanceof ToolTypes) { if (nextProps.tool !== this.props.tool) {
// TODO switch tool // TODO switch tool
} }
} }
componentWillUnmount () { componentWillUnmount () {
paper.remove();
} }
render () { render () {
return ( return (
<canvas id="paper-canvas" /> <canvas
ref={canvas => {
this.canvas = canvas;
}}
/>
); );
} }
} }
PaperCanvas.propTypes = { PaperCanvas.propTypes = {
tool: PropTypes.shape({ tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
name: PropTypes.string.isRequired
})
}; };
export default PaperCanvas; export default PaperCanvas;

View file

@ -1,4 +1,4 @@
import minilog from 'minilog'; import minilog from 'minilog';
minilog.enable(); minilog.enable();
export default minilog('paint-editor'); export default minilog('scratch-paint');

View file

@ -2,15 +2,12 @@ import React from 'react';
import ReactDOM from 'react-dom'; import ReactDOM from 'react-dom';
import PaintEditor from '..'; import PaintEditor from '..';
import {Provider} from 'react-redux'; import {Provider} from 'react-redux';
import {createStore, applyMiddleware} from 'redux'; import {createStore} from 'redux';
import throttle from 'redux-throttle';
import reducer from '../reducers/combine-reducers'; import reducer from '../reducers/combine-reducers';
const appTarget = document.createElement('div'); const appTarget = document.createElement('div');
document.body.appendChild(appTarget); document.body.appendChild(appTarget);
const store = applyMiddleware( const store = createStore(
throttle(300, {leading: true, trailing: true})
)(createStore)(
reducer, reducer,
window.__REDUX_DEVTOOLS_EXTENSION__ && window.__REDUX_DEVTOOLS_EXTENSION__() window.__REDUX_DEVTOOLS_EXTENSION__ && window.__REDUX_DEVTOOLS_EXTENSION__()
); );

View file

@ -8,10 +8,10 @@ const reducer = function (state, action) {
if (typeof state === 'undefined') state = initialState; if (typeof state === 'undefined') state = initialState;
switch (action.type) { switch (action.type) {
case CHANGE_TOOL: case CHANGE_TOOL:
if (action.tool instanceof ToolTypes) { if (action.tool in ToolTypes) {
return action.tool; return action.tool;
} }
log.warn(`Warning: Tool type does not exist: ${action.tool}`); log.warn(`Tool type does not exist: ${action.tool}`);
/* falls through */ /* falls through */
default: default:
return state; return state;
@ -22,10 +22,7 @@ const reducer = function (state, action) {
reducer.changeTool = function (tool) { reducer.changeTool = function (tool) {
return { return {
type: CHANGE_TOOL, type: CHANGE_TOOL,
tool: tool, tool: tool
meta: {
throttle: 30
}
}; };
}; };

View file

@ -1,12 +1,8 @@
class ToolTypes { import keyMirror from 'keymirror';
constructor (name) {
this.name = name; const ToolTypes = keyMirror({
} BRUSH: null,
toString () { ERASER: null
return `ToolTypes.${this.name}`; });
}
}
ToolTypes.BRUSH = new ToolTypes('BRUSH');
ToolTypes.ERASER = new ToolTypes('ERASER');
export default ToolTypes; export default ToolTypes;

View file

@ -4,7 +4,7 @@ import reducer from '../../src/reducers/tools';
test('initialState', () => { test('initialState', () => {
let defaultState; let defaultState;
expect(reducer(defaultState /* state */, {type: 'anything'} /* action */) instanceof ToolTypes).toBeTruthy(); expect(reducer(defaultState /* state */, {type: 'anything'} /* action */) in ToolTypes).toBeTruthy();
}); });
test('changeTool', () => { test('changeTool', () => {