From e4fe143ba6c46e2d5b26aaaeadc9a10268c33a21 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Thu, 22 Mar 2018 15:09:26 -0400
Subject: [PATCH] Changes to hash should happen through project loader hoc.
 Changes to the current project id and current project data should happen in a
 reducer since they are part of global state. That way, the load button can
 affect the global project data as well.

---
 src/containers/gui.jsx                  | 13 +++--
 src/containers/import-modal.jsx         | 13 ++---
 src/containers/load-button.jsx          | 32 +++---------
 src/containers/paint-editor-wrapper.jsx |  2 +-
 src/containers/record-modal.jsx         |  2 +-
 src/containers/save-button.jsx          |  2 +-
 src/containers/sound-editor.jsx         |  8 +--
 src/containers/sprite-selector-item.jsx |  2 +-
 src/containers/stage-selector.jsx       |  4 +-
 src/examples/blocks-only.jsx            |  2 +-
 src/examples/compatibility-testing.jsx  |  2 +-
 src/examples/player.jsx                 |  2 +-
 src/lib/project-loader-hoc.jsx          | 65 +++++++++++++++++++------
 src/lib/vm-listener-hoc.jsx             |  2 +-
 src/reducers/vm.js                      | 35 +++++++++++--
 15 files changed, 118 insertions(+), 68 deletions(-)

diff --git a/src/containers/gui.jsx b/src/containers/gui.jsx
index 85b3a7acd..62ded94da 100644
--- a/src/containers/gui.jsx
+++ b/src/containers/gui.jsx
@@ -28,12 +28,12 @@ class GUI extends React.Component {
     componentDidMount () {
         this.audioEngine = new AudioEngine();
         this.props.vm.attachAudioEngine(this.audioEngine);
+        this.props.vm.setCompatibilityMode(true);
+        this.props.vm.start();
+        if (!this.props.projectData) return;
         this.props.vm.loadProject(this.props.projectData)
             .then(() => {
-                this.setState({loading: false}, () => {
-                    this.props.vm.setCompatibilityMode(true);
-                    this.props.vm.start();
-                });
+                this.setState({loading: false});
             })
             .catch(e => {
                 // Need to catch this error and update component state so that
@@ -42,6 +42,7 @@ class GUI extends React.Component {
             });
     }
     componentWillReceiveProps (nextProps) {
+        if (!nextProps.projectData) return;
         if (this.props.projectData !== nextProps.projectData) {
             this.setState({loading: true}, () => {
                 this.props.vm.loadProject(nextProps.projectData)
@@ -88,7 +89,8 @@ GUI.propTypes = {
     importInfoVisible: PropTypes.bool,
     loadingStateVisible: PropTypes.bool,
     previewInfoVisible: PropTypes.bool,
-    projectData: PropTypes.string,
+    // eslint-disable-line react/forbid-prop-types
+    projectData: PropTypes.oneOfType([PropTypes.object, PropTypes.string]),
     vm: PropTypes.instanceOf(VM)
 };
 
@@ -102,6 +104,7 @@ const mapStateToProps = state => ({
     importInfoVisible: state.modals.importInfo,
     loadingStateVisible: state.modals.loadingProject,
     previewInfoVisible: state.modals.previewInfo,
+    projectData: state.vm.projectData,
     soundsTabVisible: state.editorTab.activeTabIndex === SOUNDS_TAB_INDEX
 });
 
diff --git a/src/containers/import-modal.jsx b/src/containers/import-modal.jsx
index 0731098f1..33305f619 100644
--- a/src/containers/import-modal.jsx
+++ b/src/containers/import-modal.jsx
@@ -9,6 +9,7 @@ import {
     closeImportInfo,
     openPreviewInfo
 } from '../reducers/modals';
+import {setProjectId} from '../reducers/vm';
 
 class ImportModal extends React.Component {
     constructor (props) {
@@ -35,11 +36,7 @@ class ImportModal extends React.Component {
         const projectId = this.validate(inputValue);
 
         if (projectId) {
-            const projectLink = document.createElement('a');
-            document.body.appendChild(projectLink);
-            projectLink.href = `#${projectId}`;
-            projectLink.click();
-            document.body.removeChild(projectLink);
+            this.props.setProjectId(projectId);
             this.props.onViewProject();
         } else {
             this.setState({
@@ -87,7 +84,8 @@ class ImportModal extends React.Component {
 ImportModal.propTypes = {
     onBack: PropTypes.func.isRequired,
     onCancel: PropTypes.func.isRequired,
-    onViewProject: PropTypes.func
+    onViewProject: PropTypes.func,
+    setProjectId: PropTypes.func
 };
 
 const mapStateToProps = () => ({});
@@ -102,6 +100,9 @@ const mapDispatchToProps = dispatch => ({
     },
     onViewProject: () => {
         dispatch(closeImportInfo());
+    },
+    setProjectId: projectId => {
+        dispatch(setProjectId(projectId));
     }
 });
 
diff --git a/src/containers/load-button.jsx b/src/containers/load-button.jsx
index e52a6735d..3587bfbdb 100644
--- a/src/containers/load-button.jsx
+++ b/src/containers/load-button.jsx
@@ -5,10 +5,8 @@ import {connect} from 'react-redux';
 
 import LoadButtonComponent from '../components/load-button/load-button.jsx';
 
-import {
-    openLoadingProject,
-    closeLoadingProject
-} from '../reducers/modals';
+import {openLoadingProject} from '../reducers/modals';
+import {setProjectData} from '../reducers/vm';
 
 class LoadButton extends React.Component {
     constructor (props) {
@@ -25,16 +23,8 @@ class LoadButton extends React.Component {
     }
     handleChange (e) {
         this.props.openLoadingState();
-        // Remove the hash if any (without triggering a hash change event or a reload)
-        history.replaceState({}, document.title, '.');
         const reader = new FileReader();
-        reader.onload = () => this.props.vm.loadProject(reader.result)
-            .then(() => {
-                this.props.closeLoadingState();
-            })
-            .catch(error => {
-                this.setState({loadingError: true, errorMessage: error});
-            });
+        reader.onload = () => this.props.setNewProjectData(reader.result, null);
         reader.readAsArrayBuffer(e.target.files[0]);
     }
     handleClick () {
@@ -46,9 +36,8 @@ class LoadButton extends React.Component {
     render () {
         if (this.state.loadingError) throw new Error(`Failed to load project: ${this.state.errorMessage}`);
         const {
-            closeLoadingState, // eslint-disable-line no-unused-vars
             openLoadingState, // eslint-disable-line no-unused-vars
-            vm, // eslint-disable-line no-unused-vars
+            setNewProjectData, // eslint-disable-line no-unused-vars
             ...props
         } = this.props;
         return (
@@ -63,20 +52,15 @@ class LoadButton extends React.Component {
 }
 
 LoadButton.propTypes = {
-    closeLoadingState: PropTypes.func,
     openLoadingState: PropTypes.func,
-    vm: PropTypes.shape({
-        loadProject: PropTypes.func
-    })
+    setNewProjectData: PropTypes.func
 };
 
-const mapStateToProps = state => ({
-    vm: state.vm
-});
+const mapStateToProps = () => ({});
 
 const mapDispatchToProps = dispatch => ({
-    closeLoadingState: () => dispatch(closeLoadingProject()),
-    openLoadingState: () => dispatch(openLoadingProject())
+    openLoadingState: () => dispatch(openLoadingProject()),
+    setNewProjectData: (projectData, projectId) => dispatch(setProjectData(projectData, projectId))
 });
 
 export default connect(
diff --git a/src/containers/paint-editor-wrapper.jsx b/src/containers/paint-editor-wrapper.jsx
index 52bb5708f..00787f68e 100644
--- a/src/containers/paint-editor-wrapper.jsx
+++ b/src/containers/paint-editor-wrapper.jsx
@@ -60,7 +60,7 @@ const mapStateToProps = (state, {selectedCostumeIndex}) => {
         rotationCenterX: costume && costume.rotationCenterX,
         rotationCenterY: costume && costume.rotationCenterY,
         svgId: editingTarget && `${editingTarget}${costume.skinId}`,
-        vm: state.vm
+        vm: state.vm.vm
     };
 };
 
diff --git a/src/containers/record-modal.jsx b/src/containers/record-modal.jsx
index 8e4b9d249..ee05f298b 100644
--- a/src/containers/record-modal.jsx
+++ b/src/containers/record-modal.jsx
@@ -133,7 +133,7 @@ RecordModal.propTypes = {
 };
 
 const mapStateToProps = state => ({
-    vm: state.vm
+    vm: state.vm.vm
 });
 
 const mapDispatchToProps = dispatch => ({
diff --git a/src/containers/save-button.jsx b/src/containers/save-button.jsx
index fc8bcf40a..bef371708 100644
--- a/src/containers/save-button.jsx
+++ b/src/containers/save-button.jsx
@@ -63,7 +63,7 @@ SaveButton.propTypes = {
 };
 
 const mapStateToProps = state => ({
-    vm: state.vm
+    vm: state.vm.vm
 });
 
 export default connect(
diff --git a/src/containers/sound-editor.jsx b/src/containers/sound-editor.jsx
index de10ebc2c..acf745131 100644
--- a/src/containers/sound-editor.jsx
+++ b/src/containers/sound-editor.jsx
@@ -211,17 +211,17 @@ SoundEditor.propTypes = {
 };
 
 const mapStateToProps = (state, {soundIndex}) => {
-    const sprite = state.vm.editingTarget.sprite;
+    const sprite = state.vm.vm.editingTarget.sprite;
     // Make sure the sound index doesn't go out of range.
     const index = soundIndex < sprite.sounds.length ? soundIndex : sprite.sounds.length - 1;
-    const sound = state.vm.editingTarget.sprite.sounds[index];
-    const audioBuffer = state.vm.getSoundBuffer(index);
+    const sound = state.vm.vm.editingTarget.sprite.sounds[index];
+    const audioBuffer = state.vm.vm.getSoundBuffer(index);
     return {
         soundId: sound.soundId,
         sampleRate: audioBuffer.sampleRate,
         samples: audioBuffer.getChannelData(0),
         name: sound.name,
-        vm: state.vm
+        vm: state.vm.vm
     };
 };
 
diff --git a/src/containers/sprite-selector-item.jsx b/src/containers/sprite-selector-item.jsx
index edb124b2c..519e2f99a 100644
--- a/src/containers/sprite-selector-item.jsx
+++ b/src/containers/sprite-selector-item.jsx
@@ -79,7 +79,7 @@ SpriteSelectorItem.propTypes = {
 };
 
 const mapStateToProps = (state, {assetId, costumeURL, id}) => ({
-    costumeURL: costumeURL || (assetId && state.vm.runtime.storage.get(assetId).encodeDataURI()),
+    costumeURL: costumeURL || (assetId && state.vm.vm.runtime.storage.get(assetId).encodeDataURI()),
     receivedBlocks: state.hoveredTarget.receivedBlocks &&
             state.hoveredTarget.sprite === id
 });
diff --git a/src/containers/stage-selector.jsx b/src/containers/stage-selector.jsx
index bf749c12a..6174a31d3 100644
--- a/src/containers/stage-selector.jsx
+++ b/src/containers/stage-selector.jsx
@@ -78,8 +78,8 @@ StageSelector.propTypes = {
 };
 
 const mapStateToProps = (state, {assetId}) => ({
-    url: assetId && state.vm.runtime.storage.get(assetId).encodeDataURI(),
-    vm: state.vm
+    url: assetId && state.vm.vm.runtime.storage.get(assetId).encodeDataURI(),
+    vm: state.vm.vm
 });
 
 const mapDispatchToProps = dispatch => ({
diff --git a/src/examples/blocks-only.jsx b/src/examples/blocks-only.jsx
index 8d5159e82..0a961129c 100644
--- a/src/examples/blocks-only.jsx
+++ b/src/examples/blocks-only.jsx
@@ -10,7 +10,7 @@ import ProjectLoaderHOC from '../lib/project-loader-hoc.jsx';
 
 import styles from './blocks-only.css';
 
-const mapStateToProps = state => ({vm: state.vm});
+const mapStateToProps = state => ({vm: state.vm.vm});
 
 const VMBlocks = connect(mapStateToProps)(Blocks);
 const VMControls = connect(mapStateToProps)(Controls);
diff --git a/src/examples/compatibility-testing.jsx b/src/examples/compatibility-testing.jsx
index 3fa8e71d9..0e05ced12 100644
--- a/src/examples/compatibility-testing.jsx
+++ b/src/examples/compatibility-testing.jsx
@@ -9,7 +9,7 @@ import Box from '../components/box/box.jsx';
 import GUI from '../containers/gui.jsx';
 import ProjectLoaderHOC from '../lib/project-loader-hoc.jsx';
 
-const mapStateToProps = state => ({vm: state.vm});
+const mapStateToProps = state => ({vm: state.vm.vm});
 
 const VMStage = connect(mapStateToProps)(Stage);
 const VMControls = connect(mapStateToProps)(Controls);
diff --git a/src/examples/player.jsx b/src/examples/player.jsx
index ab50527cd..85ea90f2a 100644
--- a/src/examples/player.jsx
+++ b/src/examples/player.jsx
@@ -11,7 +11,7 @@ import ProjectLoaderHOC from '../lib/project-loader-hoc.jsx';
 
 import './player.css';
 
-const mapStateToProps = state => ({vm: state.vm});
+const mapStateToProps = state => ({vm: state.vm.vm});
 
 const VMStage = connect(mapStateToProps)(Stage);
 const VMControls = connect(mapStateToProps)(Controls);
diff --git a/src/lib/project-loader-hoc.jsx b/src/lib/project-loader-hoc.jsx
index 2c20b9fed..1f096f660 100644
--- a/src/lib/project-loader-hoc.jsx
+++ b/src/lib/project-loader-hoc.jsx
@@ -1,9 +1,13 @@
 import React from 'react';
+import PropTypes from 'prop-types';
+import {connect} from 'react-redux';
 
 import analytics from './analytics';
 import log from './log';
 import storage from './storage';
 
+import {setProjectId, setProjectData} from '../reducers/vm';
+
 /* Higher Order Component to provide behavior for loading projects by id from
  * the window's hash (#this part in the url)
  * @param {React.Component} WrappedComponent component to receive projectData prop
@@ -16,8 +20,6 @@ const ProjectLoaderHOC = function (WrappedComponent) {
             this.fetchProjectId = this.fetchProjectId.bind(this);
             this.updateProject = this.updateProject.bind(this);
             this.state = {
-                projectId: null,
-                projectData: null,
                 fetchingProject: false
             };
         }
@@ -25,15 +27,25 @@ const ProjectLoaderHOC = function (WrappedComponent) {
             window.addEventListener('hashchange', this.updateProject);
             this.updateProject();
         }
-        componentWillUpdate (nextProps, nextState) {
-            if (this.state.projectId !== nextState.projectId) {
+        componentWillUpdate (nextProps) {
+            const projectId = nextProps.projectId;
+            if (this.props.projectId !== projectId) {
+                // Replace URL hash without triggering a hash change event
+                history.replaceState({}, document.title,
+                    projectId ? `./#${projectId}` : '.');
+
                 this.setState({fetchingProject: true}, () => {
                     storage
-                        .load(storage.AssetType.Project, this.state.projectId, storage.DataFormat.JSON)
-                        .then(projectAsset => projectAsset && this.setState({
-                            projectData: projectAsset.data.toString(),
-                            fetchingProject: false
-                        }))
+                        .load(storage.AssetType.Project, projectId, storage.DataFormat.JSON)
+                        .then(projectAsset => {
+                            if (!projectAsset) return;
+
+                            this.setState({
+                                fetchingProject: false
+                            });
+
+                            this.props.setNewProjectData(projectAsset.data, projectId);
+                        })
                         .catch(err => log.error(err));
                 });
             }
@@ -46,9 +58,9 @@ const ProjectLoaderHOC = function (WrappedComponent) {
         }
         updateProject () {
             let projectId = this.fetchProjectId();
-            if (projectId !== this.state.projectId) {
+            if (projectId !== this.props.projectId) {
                 if (projectId.length < 1) projectId = 0;
-                this.setState({projectId: projectId});
+                this.props.setNewProjectId(projectId);
 
                 if (projectId !== 0) {
                     analytics.event({
@@ -61,20 +73,41 @@ const ProjectLoaderHOC = function (WrappedComponent) {
             }
         }
         render () {
-            if (!this.state.projectData) return null;
+            const {
+                projectId, // eslint-disable-line no-unused-vars
+                setNewProjectData, // eslint-disable-line no-unused-vars
+                setNewProjectId, // eslint-disable-line no-unused-vars
+                ...props
+            } = this.props;
             return (
                 <WrappedComponent
                     fetchingProject={this.state.fetchingProject}
-                    projectData={this.state.projectData}
-                    {...this.props}
+                    {...props}
                 />
             );
         }
     }
 
-    return ProjectLoaderComponent;
-};
+    ProjectLoaderComponent.propTypes = {
+        projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
+        setNewProjectData: PropTypes.func,
+        setNewProjectId: PropTypes.func
+    };
+
+    const mapStateToProps = state => ({
+        projectId: state.vm.projectId
+    });
 
+    const mapDispatchToProps = dispatch => ({
+        setNewProjectId: projectId => dispatch(setProjectId(projectId)),
+        setNewProjectData: (projectData, projectId) => dispatch(setProjectData(projectData, projectId))
+    });
+
+    return connect(
+        mapStateToProps,
+        mapDispatchToProps,
+    )(ProjectLoaderComponent);
+};
 
 export {
     ProjectLoaderHOC as default
diff --git a/src/lib/vm-listener-hoc.jsx b/src/lib/vm-listener-hoc.jsx
index 8e7a0ca17..dbc520093 100644
--- a/src/lib/vm-listener-hoc.jsx
+++ b/src/lib/vm-listener-hoc.jsx
@@ -99,7 +99,7 @@ const vmListenerHOC = function (WrappedComponent) {
         attachKeyboardEvents: true
     };
     const mapStateToProps = state => ({
-        vm: state.vm,
+        vm: state.vm.vm,
         hoveredSprite: state.hoveredTarget.sprite,
         editingTarget: state.targets.editingTarget
     });
diff --git a/src/reducers/vm.js b/src/reducers/vm.js
index dd70f3f4f..1f96dc400 100644
--- a/src/reducers/vm.js
+++ b/src/reducers/vm.js
@@ -2,15 +2,29 @@ import VM from 'scratch-vm';
 import storage from '../lib/storage';
 
 const SET_VM = 'scratch-gui/vm/SET_VM';
+const SET_PROJECT_ID = 'scratch-gui/vm/SET_PROJECT_ID';
+const SET_PROJECT_DATA = 'scratch-gui/vm/SET_PROJECT_DATA';
+
 const defaultVM = new VM();
 defaultVM.attachStorage(storage);
-const initialState = defaultVM;
+const initialState = {
+    vm: defaultVM,
+    projectData: null,
+    projectId: null // 0 means default project, null or undefined means project from local file
+};
 
 const reducer = function (state, action) {
     if (typeof state === 'undefined') state = initialState;
     switch (action.type) {
     case SET_VM:
-        return action.vm;
+        return Object.assign({}, state, {vm: action.vm});
+    case SET_PROJECT_ID:
+        return Object.assign({}, state, {projectId: action.projectId});
+    case SET_PROJECT_DATA:
+        return Object.assign({}, state, {
+            projectData: action.projectData,
+            projectId: action.projectId
+        });
     default:
         return state;
     }
@@ -21,7 +35,22 @@ const setVM = function (vm) {
         vm: vm
     };
 };
+const setProjectId = function (projectId) {
+    return {
+        type: SET_PROJECT_ID,
+        projectId: projectId
+    };
+};
+const setProjectData = function (projectData, projectId) {
+    return {
+        type: SET_PROJECT_DATA,
+        projectData: projectData,
+        projectId: projectId
+    };
+};
 export {
     reducer as default,
-    setVM
+    setVM,
+    setProjectId,
+    setProjectData
 };
-- 
GitLab