From 716c979d0f4aed5faa24321cb690b8605281ae23 Mon Sep 17 00:00:00 2001
From: Ray Schamp <ray@scratch.mit.edu>
Date: Sun, 16 Oct 2016 19:03:54 -0400
Subject: [PATCH] Recoupling refactor
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Confine the logic for adding/removing event listeners between the VM and the components to the relevant components.  While this tightens the coupling between the component and the environment, it makes using the individual components more self-contained.
---
 src/components/blocks.js |  63 ++++++++++++++++++--
 src/components/gui.js    |  29 +++-------
 src/components/stage.js  |  91 +++++++++++++++++++++++++----
 src/lib/vm-manager.js    | 121 ++++++++++++++-------------------------
 4 files changed, 190 insertions(+), 114 deletions(-)

diff --git a/src/components/blocks.js b/src/components/blocks.js
index 0d05a95f0..56c30596c 100644
--- a/src/components/blocks.js
+++ b/src/components/blocks.js
@@ -1,16 +1,73 @@
+const bindAll = require('lodash.bindall');
 const defaultsDeep = require('lodash.defaultsdeep');
 const React = require('react');
 const ScratchBlocks = require('scratch-blocks');
 
 class Blocks extends React.Component {
+    constructor (props) {
+        super(props);
+        bindAll(this, [
+            'attachVM',
+            'detachVM',
+            'onStackGlowOn',
+            'onStackGlowOff',
+            'onBlockGlowOn',
+            'onBlockGlowOff',
+            'onVisualReport',
+            'onWorkspaceUpdate'
+        ]);
+    }
     componentDidMount () {
         let workspaceConfig = defaultsDeep({}, Blocks.defaultOptions, this.props.options);
         this.workspace = ScratchBlocks.inject(this.refs.scratchBlocks, workspaceConfig);
-        this.props.onReceiveWorkspace(this.workspace);
+        this.attachVM();
     }
     componentWillUnmount () {
+        this.detachVM();
         this.workspace.dispose();
     }
+    attachVM () {
+        this.workspace.addChangeListener(this.props.vm.blockListener);
+        this.workspace.getFlyout().getWorkspace().addChangeListener(
+            this.props.vm.flyoutBlockListener
+        );
+        this.props.vm.on('STACK_GLOW_ON', this.onStackGlowOn);
+        this.props.vm.on('STACK_GLOW_OFF', this.onStackGlowOff);
+        this.props.vm.on('BLOCK_GLOW_ON', this.onBlockGlowOn);
+        this.props.vm.on('BLOCK_GLOW_OFF', this.onBlockGlowOff);
+        this.props.vm.on('VISUAL_REPORT', this.onVisualReport);
+        this.props.vm.on('workspaceUpdate', this.onWorkspaceUpdate);
+    }
+    detachVM () {
+        this.props.vm.off('STACK_GLOW_ON', this.onStackGlowOn);
+        this.props.vm.off('STACK_GLOW_OFF', this.onStackGlowOff);
+        this.props.vm.off('BLOCK_GLOW_ON', this.onBlockGlowOn);
+        this.props.vm.off('BLOCK_GLOW_OFF', this.onBlockGlowOff);
+        this.props.vm.off('VISUAL_REPORT', this.onVisualReport);
+        this.props.vm.off('workspaceUpdate', this.onWorkspaceUpdate);
+    }
+    onStackGlowOn (data) {
+        this.workspace.glowStack(data.id, true);
+    }
+    onStackGlowOff (data) {
+        this.workspace.glowStack(data.id, false);
+    }
+    onBlockGlowOn (data) {
+        this.workspace.glowBlock(data.id, true);
+    }
+    onBlockGlowOff (data) {
+        this.workspace.glowBlock(data.id, false);
+    }
+    onVisualReport (data) {
+        this.workspace.reportValue(data.id, data.value);
+    }
+    onWorkspaceUpdate (data) {
+        ScratchBlocks.Events.disable();
+        this.workspace.clear();
+        let dom = ScratchBlocks.Xml.textToDom(data.xml);
+        ScratchBlocks.Xml.domToWorkspace(dom, this.workspace);
+        ScratchBlocks.Events.enable();
+    }
     render () {
         return (
             <div
@@ -29,10 +86,7 @@ class Blocks extends React.Component {
 }
 
 Blocks.propTypes = {
-    onReceiveWorkspace: React.PropTypes.func,
     options: React.PropTypes.shape({
-        // The toolbox is actually an element, but React doesn't agree :/
-        toolbox: React.PropTypes.object,
         media: React.PropTypes.string,
         zoom: React.PropTypes.shape({
             controls: React.PropTypes.boolean,
@@ -72,7 +126,6 @@ Blocks.defaultOptions = {
 };
 
 Blocks.defaultProps = {
-    onReceiveWorkspace: function () {},
     options: Blocks.defaultOptions
 };
 
diff --git a/src/components/gui.js b/src/components/gui.js
index 4ae1463d6..b21acc0cc 100644
--- a/src/components/gui.js
+++ b/src/components/gui.js
@@ -1,4 +1,3 @@
-const bindAll = require('lodash.bindall');
 const React = require('react');
 
 const Blocks = require('./blocks');
@@ -12,35 +11,24 @@ const VMManager = require('../lib/vm-manager');
 class GUI extends React.Component {
     constructor (props) {
         super(props);
-        bindAll(this, ['animate', 'onReceiveRenderer', 'onReceiveWorkspace']);
-        this.state = {};
         this.vmManager = new VMManager(this.props.vm);
     }
     componentDidMount () {
         this.vmManager.attachKeyboardEvents();
         this.props.vm.loadProject(this.props.projectData);
         this.props.vm.start();
-        requestAnimationFrame(this.animate);
+        this.vmManager.startAnimation();
+    }
+    componentWillUnmount () {
+        this.vmManager.detachKeyboardEvents();
+        this.props.vm.stopAll();
+        this.vmManager.stopAnimation();
     }
     componentWillReceiveProps (nextProps) {
         if (this.props.projectData !== nextProps.projectData) {
             this.props.vm.loadProject(nextProps.projectData);
         }
     }
-    animate () {
-        this.props.vm.animationFrame();
-        requestAnimationFrame(this.animate);
-    }
-    onReceiveRenderer (renderer, stage) {
-        this.renderer = renderer;
-        this.stage = stage;
-        this.props.vm.attachRenderer(this.renderer);
-        this.vmManager.attachMouseEvents(this.stage);
-    }
-    onReceiveWorkspace (workspace) {
-        this.workspace = workspace;
-        this.vmManager.attachWorkspace(this.workspace);
-    }
     render () {
         return (
             <div
@@ -55,16 +43,13 @@ class GUI extends React.Component {
             >
                 <GreenFlag vm={this.props.vm} />
                 <StopAll vm={this.props.vm} />
-                <Stage
-                    onReceiveRenderer={this.onReceiveRenderer}
-                />
+                <Stage vm={this.props.vm} />
                 <SpriteSelector vm={this.props.vm} />
                 <Blocks
                     options={{
                         media: this.props.basePath + 'static/blocks-media/'
                     }}
                     vm={this.props.vm}
-                    onReceiveWorkspace={this.onReceiveWorkspace}
                 />
             </div>
         );
diff --git a/src/components/stage.js b/src/components/stage.js
index b4570100a..40601d2fa 100644
--- a/src/components/stage.js
+++ b/src/components/stage.js
@@ -5,18 +5,89 @@ const Renderer = require('scratch-render');
 class Stage extends React.Component {
     constructor (props) {
         super(props);
-        bindAll(this, ['initRenderer']);
+        bindAll(this, [
+            'attachMouseEvents',
+            'detachMouseEvents',
+            'onMouseUp',
+            'onMouseMove',
+            'onMouseDown'
+        ]);
     }
-    initRenderer (stage) {
-        this.stage = stage;
-        this.renderer = new Renderer(stage);
-        this.props.onReceiveRenderer(this.renderer, this.stage);
+    componentDidMount () {
+        this.renderer = new Renderer(this.canvas);
+        this.props.vm.attachRenderer(this.renderer);
+        this.attachMouseEvents(this.canvas);
+    }
+    componentWillUnmount () {
+        this.detachMouseEvents(this.canvas);
+    }
+    attachMouseEvents (canvas) {
+        document.addEventListener('mousemove', this.onMouseMove);
+        canvas.addEventListener('mouseup', this.onMouseUp);
+        canvas.addEventListener('mousedown', this.onMouseDown);
+    }
+    detachMouseEvents (canvas) {
+        document.removeEventListener('mousemove', this.onMouseMove);
+        canvas.removeEventListener('mouseup', this.onMouseUp);
+        canvas.removeEventListener('mousedown', this.onMouseDown);
+    }
+    onMouseMove (e) {
+        let rect = this.canvas.getBoundingClientRect();
+        let coordinates = {
+            x: e.clientX - rect.left,
+            y: e.clientY - rect.top,
+            canvasWidth: rect.width,
+            canvasHeight: rect.height
+        };
+        this.props.vm.postIOData('mouse', coordinates);
+    }
+    onMouseUp (e) {
+        let rect = this.canvas.getBoundingClientRect();
+        let data = {
+            isDown: false,
+            x: e.clientX - rect.left,
+            y: e.clientY - rect.top,
+            canvasWidth: rect.width,
+            canvasHeight: rect.height
+        };
+        this.props.vm.postIOData('mouse', data);
+        e.preventDefault();
+    }
+    onMouseDown (e) {
+        let rect = this.canvas.getBoundingClientRect();
+        let data = {
+            isDown: true,
+            x: e.clientX - rect.left,
+            y: e.clientY - rect.top,
+            canvasWidth: rect.width,
+            canvasHeight: rect.height
+        };
+        this.props.vm.postIOData('mouse', data);
+        e.preventDefault();
     }
+    render () {
+        return (
+            <StageCanvas
+                {... this.props}
+                canvasRef={canvas => this.canvas = canvas}
+            />
+        );
+    }
+}
+
+Stage.propTypes = {
+    vm: React.PropTypes.shape({
+        attachRenderer: React.PropTypes.func,
+        postIOData: React.PropTypes.func
+    }).isRequired
+};
+
+class StageCanvas extends React.Component {
     render () {
         return (
             <canvas
                 className="scratch-stage"
-                ref={this.initRenderer}
+                ref={this.props.canvasRef}
                 style={{
                     position: 'absolute',
                     top: 10,
@@ -29,14 +100,14 @@ class Stage extends React.Component {
     }
 }
 
-Stage.propTypes = {
-    onReceiveRenderer: React.PropTypes.func,
+StageCanvas.propTypes = {
+    canvasRef: React.PropTypes.func,
     width: React.PropTypes.number,
     height: React.PropTypes.number
 };
 
-Stage.defaultProps = {
-    onReceiveRenderer: function () {},
+StageCanvas.defaultProps = {
+    canvasRef: function () {},
     width: 480,
     height: 360
 };
diff --git a/src/lib/vm-manager.js b/src/lib/vm-manager.js
index eb3c4c3b9..4216b2876 100644
--- a/src/lib/vm-manager.js
+++ b/src/lib/vm-manager.js
@@ -1,92 +1,59 @@
 const bindAll = require('lodash.bindall');
-const ScratchBlocks = require('scratch-blocks');
 
 class VMManager {
     constructor (vm) {
-        bindAll(
-            this,
-            ['attachWorkspace', 'attachMouseEvents', 'attachKeyboardEvents']
-        );
+        bindAll(this, [
+            'attachKeyboardEvents',
+            'detachKeyboardEvents',
+            'onKeyDown',
+            'onKeyUp',
+            'animate',
+            'startAnimation',
+            'stopAnimation'
+        ]);
         this.vm = vm;
     }
-    attachWorkspace (workspace) {
-        workspace.addChangeListener(this.vm.blockListener);
-        var flyoutWorkspace = workspace.getFlyout().getWorkspace();
-        flyoutWorkspace.addChangeListener(this.vm.flyoutBlockListener);
-        this.vm.on('STACK_GLOW_ON', data => workspace.glowStack(data.id, true));
-        this.vm.on('STACK_GLOW_OFF', data => workspace.glowStack(data.id, false));
-        this.vm.on('BLOCK_GLOW_ON', data => workspace.glowBlock(data.id, true));
-        this.vm.on('BLOCK_GLOW_OFF', data => workspace.glowBlock(data.id, false));
-        this.vm.on('VISUAL_REPORT', data => workspace.reportValue(data.id, data.value));
-        this.vm.on('workspaceUpdate', data => {
-            ScratchBlocks.Events.disable();
-            workspace.clear();
-            let dom = ScratchBlocks.Xml.textToDom(data.xml);
-            ScratchBlocks.Xml.domToWorkspace(dom, workspace);
-            ScratchBlocks.Events.enable();
-        });
+    startAnimation () {
+        this.animationFrame = requestAnimationFrame(this.animate);
     }
-    attachMouseEvents (stage) {
-        document.addEventListener('mousemove', e => {
-            let rect = stage.getBoundingClientRect();
-            let coordinates = {
-                x: e.clientX - rect.left,
-                y: e.clientY - rect.top,
-                canvasWidth: rect.width,
-                canvasHeight: rect.height
-            };
-            this.vm.postIOData('mouse', coordinates);
-        });
-        stage.addEventListener('mousedown', e => {
-            let rect = stage.getBoundingClientRect();
-            let data = {
-                isDown: true,
-                x: e.clientX - rect.left,
-                y: e.clientY - rect.top,
-                canvasWidth: rect.width,
-                canvasHeight: rect.height
-            };
-            this.vm.postIOData('mouse', data);
-            e.preventDefault();
-        });
-        stage.addEventListener('mouseup', e => {
-            let rect = stage.getBoundingClientRect();
-            let data = {
-                isDown: false,
-                x: e.clientX - rect.left,
-                y: e.clientY - rect.top,
-                canvasWidth: rect.width,
-                canvasHeight: rect.height
-            };
-            this.vm.postIOData('mouse', data);
-            e.preventDefault();
-        });
+    stopAnimation () {
+        cancelAnimationFrame(this.animationFrame);
+    }
+    animate () {
+        this.vm.animationFrame();
+        this.animationFrame = requestAnimationFrame(this.animate);
     }
     attachKeyboardEvents () {
         // Feed keyboard events as VM I/O events.
-        document.addEventListener('keydown', e => {
-            // Don't capture keys intended for Blockly inputs.
-            if (e.target != document && e.target != document.body) {
-                return;
-            }
-            this.vm.postIOData('keyboard', {
-                keyCode: e.keyCode,
-                isDown: true
-            });
-            e.preventDefault();
+        document.addEventListener('keydown', this.onKeyDown);
+        document.addEventListener('keyup', this.onKeyUp);
+    }
+    detachKeyboardEvents () {
+        document.removeEventListener('keydown', this.onKeyDown);
+        document.removeEventListener('keyup', this.onKeyUp);
+    }
+    onKeyDown (e) {
+        // Don't capture keys intended for Blockly inputs.
+        if (e.target != document && e.target != document.body) {
+            return;
+        }
+        this.vm.postIOData('keyboard', {
+            keyCode: e.keyCode,
+            isDown: true
         });
-        document.addEventListener('keyup', e => {
-            // Always capture up events,
-            // even those that have switched to other targets.
-            this.vm.postIOData('keyboard', {
-                keyCode: e.keyCode,
-                isDown: false
-            });
-            // E.g., prevent scroll.
-            if (e.target != document && e.target != document.body) {
-                e.preventDefault();
-            }
+        e.preventDefault();
+    }
+    onKeyUp (e) {
+        // Always capture up events,
+        // even those that have switched to other targets.
+        this.vm.postIOData('keyboard', {
+            keyCode: e.keyCode,
+            isDown: false
         });
+        // E.g., prevent scroll.
+        if (e.target != document && e.target != document.body) {
+            e.preventDefault();
+        }
     }
 }
 
-- 
GitLab