From dc2fea3dd6970f3271b54ac98bb6febc748aa034 Mon Sep 17 00:00:00 2001
From: DD Liu <liudi@media.mit.edu>
Date: Thu, 27 Jul 2017 16:41:41 -0400
Subject: [PATCH] some clean up in react code'

---
 src/components/paint-editor.jsx      | 36 +++++++++++++++++-----------
 src/containers/paint-editor.jsx      | 20 +++++-----------
 src/containers/paper-canvas.jsx      | 21 ++++++++++++----
 src/containers/tools/brush-tool.jsx  | 15 ++++++------
 src/containers/tools/eraser-tool.jsx | 10 ++++----
 5 files changed, 55 insertions(+), 47 deletions(-)

diff --git a/src/components/paint-editor.jsx b/src/components/paint-editor.jsx
index 1bad869e..68661388 100644
--- a/src/components/paint-editor.jsx
+++ b/src/components/paint-editor.jsx
@@ -1,29 +1,37 @@
-import PropTypes from 'prop-types';
+import bindAll from 'lodash.bindall';
 import React from 'react';
 import PaperCanvas from '../containers/paper-canvas.jsx';
 import BrushTool from '../containers/tools/brush-tool.jsx';
 import EraserTool from '../containers/tools/eraser-tool.jsx';
-import ToolTypes from '../tools/tool-types.js';
 
 class PaintEditorComponent extends React.Component {
+    constructor (props) {
+        super(props);
+        bindAll(this, [
+            'setCanvas'
+        ]);
+        this.state = {};
+    }
+    setCanvas (canvas) {
+        this.setState({canvas: canvas});
+    }
     render () {
+        // Tools can't work without a canvas, so we might as well not render them until we have it
+        if (this.state.canvas) {
+            return (
+                <div>
+                    <PaperCanvas canvasRef={this.setCanvas} />
+                    <BrushTool canvas={this.state.canvas} />
+                    <EraserTool canvas={this.state.canvas} />
+                </div>
+            );
+        }
         return (
             <div>
-                <PaperCanvas
-                    ref={canvas => {
-                        this.canvas = canvas;
-                    }}
-                    tool={this.props.tool}
-                />
-                <BrushTool canvas={this.canvas} />
-                <EraserTool canvas={this.canvas} />
+                <PaperCanvas canvasRef={this.setCanvas} />
             </div>
         );
     }
 }
 
-PaintEditorComponent.propTypes = {
-    tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
-};
-
 export default PaintEditorComponent;
diff --git a/src/containers/paint-editor.jsx b/src/containers/paint-editor.jsx
index 12d40e54..78eae432 100644
--- a/src/containers/paint-editor.jsx
+++ b/src/containers/paint-editor.jsx
@@ -13,35 +13,27 @@ class PaintEditor extends React.Component {
         document.removeEventListener('keydown', this.props.onKeyPress);
     }
     render () {
-        const {
-            onKeyPress, // eslint-disable-line no-unused-vars
-            ...props
-        } = this.props;
         return (
-            <PaintEditorComponent {...props} />
+            <PaintEditorComponent />
         );
     }
 }
 
 PaintEditor.propTypes = {
-    onKeyPress: PropTypes.func.isRequired,
-    tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
+    onKeyPress: PropTypes.func.isRequired
 };
 
-const mapStateToProps = state => ({
-    tool: state.tool
-});
 const mapDispatchToProps = dispatch => ({
-    onKeyPress: e => {
-        if (e.key === 'e') {
+    onKeyPress: event => {
+        if (event.key === 'e') {
             dispatch(ToolsReducer.changeTool(ToolTypes.ERASER));
-        } else if (e.key === 'b') {
+        } else if (event.key === 'b') {
             dispatch(ToolsReducer.changeTool(ToolTypes.BRUSH));
         }
     }
 });
 
 export default connect(
-    mapStateToProps,
+    null,
     mapDispatchToProps
 )(PaintEditor);
diff --git a/src/containers/paper-canvas.jsx b/src/containers/paper-canvas.jsx
index fed3620a..a3b524ff 100644
--- a/src/containers/paper-canvas.jsx
+++ b/src/containers/paper-canvas.jsx
@@ -1,9 +1,15 @@
+import bindAll from 'lodash.bindall';
 import PropTypes from 'prop-types';
 import React from 'react';
 import paper from 'paper';
-import ToolTypes from '../tools/tool-types.js';
 
 class PaperCanvas extends React.Component {
+    constructor (props) {
+        super(props);
+        bindAll(this, [
+            'setCanvas'
+        ]);
+    }
     componentDidMount () {
         paper.setup(this.canvas);
         // Create a Paper.js Path to draw a line into it:
@@ -22,19 +28,24 @@ class PaperCanvas extends React.Component {
     componentWillUnmount () {
         paper.remove();
     }
+    setCanvas (canvas) {
+        debugger;
+        this.canvas = canvas;
+        if (this.props.canvasRef) {
+            this.props.canvasRef(canvas);
+        }
+    }
     render () {
         return (
             <canvas
-                ref={canvas => {
-                    this.canvas = canvas;
-                }}
+                ref={this.setCanvas}
             />
         );
     }
 }
 
 PaperCanvas.propTypes = {
-    tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
+    canvasRef: PropTypes.func
 };
 
 export default PaperCanvas;
diff --git a/src/containers/tools/brush-tool.jsx b/src/containers/tools/brush-tool.jsx
index 53bc763c..00c1b285 100644
--- a/src/containers/tools/brush-tool.jsx
+++ b/src/containers/tools/brush-tool.jsx
@@ -22,8 +22,7 @@ class BrushTool extends React.Component {
     }
     componentDidMount () {
         if (this.props.tool === BrushTool.TOOL_TYPE) {
-            debugger;
-            this.activateTool();
+            this.activateTool(this.props);
         }
     }
     componentWillReceiveProps (nextProps) {
@@ -39,6 +38,7 @@ class BrushTool extends React.Component {
         return false; // Logic only component
     }
     activateTool () {
+        // TODO: This is temporary until a component that provides the brush size is hooked up
         this.props.canvas.addEventListener('mousewheel', this.onScroll);
 
         this.tool = new paper.Tool();
@@ -57,6 +57,7 @@ class BrushTool extends React.Component {
     }
     deactivateTool () {
         this.props.canvas.removeEventListener('mousewheel', this.onScroll);
+        this.blob.deactivateTool();
     }
     onScroll (event) {
         if (event.deltaY < 0) {
@@ -64,11 +65,11 @@ class BrushTool extends React.Component {
         } else if (event.deltaY > 0 && this.props.brushToolState.brushSize > 1) {
             this.props.changeBrushSize(this.props.brushToolState.brushSize - 1);
         }
-        return false;
+        return true;
     }
     render () {
         return (
-            <div>Brush Tool </div>
+            <div>Brush Tool</div>
         );
     }
 }
@@ -77,11 +78,9 @@ BrushTool.propTypes = {
     brushToolState: PropTypes.shape({
         brushSize: PropTypes.number.isRequired
     }),
-    canvas: PropTypes.element,
+    canvas: PropTypes.instanceOf(Element).isRequired,
     changeBrushSize: PropTypes.func.isRequired,
-    tool: PropTypes.shape({
-        name: PropTypes.string.isRequired
-    })
+    tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
 };
 
 const mapStateToProps = state => ({
diff --git a/src/containers/tools/eraser-tool.jsx b/src/containers/tools/eraser-tool.jsx
index 9ee9fa6a..aa0e1268 100644
--- a/src/containers/tools/eraser-tool.jsx
+++ b/src/containers/tools/eraser-tool.jsx
@@ -49,29 +49,27 @@ class EraserTool extends React.Component {
         this.blob.deactivateTool();
     }
     onScroll (event) {
+        event.preventDefault();
         if (event.deltaY < 0) {
             this.props.changeBrushSize(this.props.eraserToolState.brushSize + 1);
         } else if (event.deltaY > 0 && this.props.eraserToolState.brushSize > 1) {
             this.props.changeBrushSize(this.props.eraserToolState.brushSize - 1);
         }
-        return false;
     }
     render () {
         return (
-            <div>Eraser Tool </div>
+            <div>Eraser Tool</div>
         );
     }
 }
 
 EraserTool.propTypes = {
-    canvas: PropTypes.element,
+    canvas: PropTypes.instanceOf(Element).isRequired,
     changeBrushSize: PropTypes.func.isRequired,
     eraserToolState: PropTypes.shape({
         brushSize: PropTypes.number.isRequired
     }),
-    tool: PropTypes.shape({
-        name: PropTypes.string.isRequired
-    })
+    tool: PropTypes.oneOf(Object.keys(ToolTypes)).isRequired
 };
 
 const mapStateToProps = state => ({