From 004b586cad6b2c4461e3c31dd0875296b4fd2157 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Fri, 23 Mar 2018 10:06:54 -0400
Subject: [PATCH] Revert "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."

This reverts commit e4fe143ba6c46e2d5b26aaaeadc9a10268c33a21.
---
 src/containers/gui.jsx                  | 18 +++----
 src/containers/import-modal.jsx         | 13 +++--
 src/containers/load-button.jsx          | 34 ++++++++++---
 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/modals.js                  | 11 +++++
 src/reducers/vm.js                      | 35 ++-----------
 16 files changed, 87 insertions(+), 117 deletions(-)

diff --git a/src/containers/gui.jsx b/src/containers/gui.jsx
index a5729f901..85b3a7acd 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.setState({loading: false}, () => {
+                    this.props.vm.setCompatibilityMode(true);
+                    this.props.vm.start();
+                });
             })
             .catch(e => {
                 // Need to catch this error and update component state so that
@@ -42,7 +42,6 @@ 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)
@@ -65,13 +64,14 @@ class GUI extends React.Component {
         const {
             children,
             fetchingProject,
+            loadingStateVisible,
             projectData, // eslint-disable-line no-unused-vars
             vm,
             ...componentProps
         } = this.props;
         return (
             <GUIComponent
-                loading={fetchingProject || this.state.loading}
+                loading={fetchingProject || this.state.loading || loadingStateVisible}
                 vm={vm}
                 {...componentProps}
             >
@@ -86,9 +86,9 @@ GUI.propTypes = {
     feedbackFormVisible: PropTypes.bool,
     fetchingProject: PropTypes.bool,
     importInfoVisible: PropTypes.bool,
+    loadingStateVisible: PropTypes.bool,
     previewInfoVisible: PropTypes.bool,
-    // eslint-disable-line react/forbid-prop-types
-    projectData: PropTypes.oneOfType([PropTypes.object, PropTypes.string]),
+    projectData: PropTypes.string,
     vm: PropTypes.instanceOf(VM)
 };
 
@@ -100,8 +100,8 @@ const mapStateToProps = state => ({
     costumesTabVisible: state.editorTab.activeTabIndex === COSTUMES_TAB_INDEX,
     feedbackFormVisible: state.modals.feedbackForm,
     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 33305f619..0731098f1 100644
--- a/src/containers/import-modal.jsx
+++ b/src/containers/import-modal.jsx
@@ -9,7 +9,6 @@ import {
     closeImportInfo,
     openPreviewInfo
 } from '../reducers/modals';
-import {setProjectId} from '../reducers/vm';
 
 class ImportModal extends React.Component {
     constructor (props) {
@@ -36,7 +35,11 @@ class ImportModal extends React.Component {
         const projectId = this.validate(inputValue);
 
         if (projectId) {
-            this.props.setProjectId(projectId);
+            const projectLink = document.createElement('a');
+            document.body.appendChild(projectLink);
+            projectLink.href = `#${projectId}`;
+            projectLink.click();
+            document.body.removeChild(projectLink);
             this.props.onViewProject();
         } else {
             this.setState({
@@ -84,8 +87,7 @@ class ImportModal extends React.Component {
 ImportModal.propTypes = {
     onBack: PropTypes.func.isRequired,
     onCancel: PropTypes.func.isRequired,
-    onViewProject: PropTypes.func,
-    setProjectId: PropTypes.func
+    onViewProject: PropTypes.func
 };
 
 const mapStateToProps = () => ({});
@@ -100,9 +102,6 @@ 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 a5e90a255..e52a6735d 100644
--- a/src/containers/load-button.jsx
+++ b/src/containers/load-button.jsx
@@ -4,7 +4,11 @@ import React from 'react';
 import {connect} from 'react-redux';
 
 import LoadButtonComponent from '../components/load-button/load-button.jsx';
-import {setProjectData} from '../reducers/vm';
+
+import {
+    openLoadingProject,
+    closeLoadingProject
+} from '../reducers/modals';
 
 class LoadButton extends React.Component {
     constructor (props) {
@@ -20,8 +24,17 @@ 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.setNewProjectData(reader.result, null);
+        reader.onload = () => this.props.vm.loadProject(reader.result)
+            .then(() => {
+                this.props.closeLoadingState();
+            })
+            .catch(error => {
+                this.setState({loadingError: true, errorMessage: error});
+            });
         reader.readAsArrayBuffer(e.target.files[0]);
     }
     handleClick () {
@@ -33,7 +46,9 @@ class LoadButton extends React.Component {
     render () {
         if (this.state.loadingError) throw new Error(`Failed to load project: ${this.state.errorMessage}`);
         const {
-            setNewProjectData, // eslint-disable-line no-unused-vars
+            closeLoadingState, // eslint-disable-line no-unused-vars
+            openLoadingState, // eslint-disable-line no-unused-vars
+            vm, // eslint-disable-line no-unused-vars
             ...props
         } = this.props;
         return (
@@ -48,13 +63,20 @@ class LoadButton extends React.Component {
 }
 
 LoadButton.propTypes = {
-    setNewProjectData: PropTypes.func
+    closeLoadingState: PropTypes.func,
+    openLoadingState: PropTypes.func,
+    vm: PropTypes.shape({
+        loadProject: PropTypes.func
+    })
 };
 
-const mapStateToProps = () => ({});
+const mapStateToProps = state => ({
+    vm: state.vm
+});
 
 const mapDispatchToProps = dispatch => ({
-    setNewProjectData: (projectData, projectId) => dispatch(setProjectData(projectData, projectId))
+    closeLoadingState: () => dispatch(closeLoadingProject()),
+    openLoadingState: () => dispatch(openLoadingProject())
 });
 
 export default connect(
diff --git a/src/containers/paint-editor-wrapper.jsx b/src/containers/paint-editor-wrapper.jsx
index 00787f68e..52bb5708f 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
+        vm: state.vm
     };
 };
 
diff --git a/src/containers/record-modal.jsx b/src/containers/record-modal.jsx
index 45f847807..9c4311539 100644
--- a/src/containers/record-modal.jsx
+++ b/src/containers/record-modal.jsx
@@ -135,7 +135,7 @@ RecordModal.propTypes = {
 };
 
 const mapStateToProps = state => ({
-    vm: state.vm.vm
+    vm: state.vm
 });
 
 const mapDispatchToProps = dispatch => ({
diff --git a/src/containers/save-button.jsx b/src/containers/save-button.jsx
index bef371708..fc8bcf40a 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
+    vm: state.vm
 });
 
 export default connect(
diff --git a/src/containers/sound-editor.jsx b/src/containers/sound-editor.jsx
index acf745131..de10ebc2c 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.vm.editingTarget.sprite;
+    const sprite = state.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.vm.editingTarget.sprite.sounds[index];
-    const audioBuffer = state.vm.vm.getSoundBuffer(index);
+    const sound = state.vm.editingTarget.sprite.sounds[index];
+    const audioBuffer = state.vm.getSoundBuffer(index);
     return {
         soundId: sound.soundId,
         sampleRate: audioBuffer.sampleRate,
         samples: audioBuffer.getChannelData(0),
         name: sound.name,
-        vm: state.vm.vm
+        vm: state.vm
     };
 };
 
diff --git a/src/containers/sprite-selector-item.jsx b/src/containers/sprite-selector-item.jsx
index 519e2f99a..edb124b2c 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.vm.runtime.storage.get(assetId).encodeDataURI()),
+    costumeURL: costumeURL || (assetId && state.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 6174a31d3..bf749c12a 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.vm.runtime.storage.get(assetId).encodeDataURI(),
-    vm: state.vm.vm
+    url: assetId && state.vm.runtime.storage.get(assetId).encodeDataURI(),
+    vm: state.vm
 });
 
 const mapDispatchToProps = dispatch => ({
diff --git a/src/examples/blocks-only.jsx b/src/examples/blocks-only.jsx
index 0a961129c..8d5159e82 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.vm});
+const mapStateToProps = state => ({vm: state.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 0e05ced12..3fa8e71d9 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.vm});
+const mapStateToProps = state => ({vm: state.vm});
 
 const VMStage = connect(mapStateToProps)(Stage);
 const VMControls = connect(mapStateToProps)(Controls);
diff --git a/src/examples/player.jsx b/src/examples/player.jsx
index 85ea90f2a..ab50527cd 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.vm});
+const mapStateToProps = state => ({vm: state.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 fdf79edcf..2c20b9fed 100644
--- a/src/lib/project-loader-hoc.jsx
+++ b/src/lib/project-loader-hoc.jsx
@@ -1,13 +1,9 @@
 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
@@ -20,6 +16,8 @@ const ProjectLoaderHOC = function (WrappedComponent) {
             this.fetchProjectId = this.fetchProjectId.bind(this);
             this.updateProject = this.updateProject.bind(this);
             this.state = {
+                projectId: null,
+                projectData: null,
                 fetchingProject: false
             };
         }
@@ -27,25 +25,15 @@ const ProjectLoaderHOC = function (WrappedComponent) {
             window.addEventListener('hashchange', this.updateProject);
             this.updateProject();
         }
-        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}` : '.');
-                if (projectId === null) return; // load button triggered this and is already calling setProjectData
+        componentWillUpdate (nextProps, nextState) {
+            if (this.state.projectId !== nextState.projectId) {
                 this.setState({fetchingProject: true}, () => {
                     storage
-                        .load(storage.AssetType.Project, projectId, storage.DataFormat.JSON)
-                        .then(projectAsset => {
-                            if (!projectAsset) return;
-
-                            this.setState({
-                                fetchingProject: false
-                            });
-
-                            this.props.setNewProjectData(projectAsset.data, projectId);
-                        })
+                        .load(storage.AssetType.Project, this.state.projectId, storage.DataFormat.JSON)
+                        .then(projectAsset => projectAsset && this.setState({
+                            projectData: projectAsset.data.toString(),
+                            fetchingProject: false
+                        }))
                         .catch(err => log.error(err));
                 });
             }
@@ -58,9 +46,9 @@ const ProjectLoaderHOC = function (WrappedComponent) {
         }
         updateProject () {
             let projectId = this.fetchProjectId();
-            if (projectId !== this.props.projectId) {
+            if (projectId !== this.state.projectId) {
                 if (projectId.length < 1) projectId = 0;
-                this.props.setNewProjectId(projectId);
+                this.setState({projectId: projectId});
 
                 if (projectId !== 0) {
                     analytics.event({
@@ -73,42 +61,21 @@ const ProjectLoaderHOC = function (WrappedComponent) {
             }
         }
         render () {
-            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;
+            if (!this.state.projectData) return null;
             return (
                 <WrappedComponent
                     fetchingProject={this.state.fetchingProject}
-                    {...props}
+                    projectData={this.state.projectData}
+                    {...this.props}
                 />
             );
         }
     }
 
-    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);
+    return ProjectLoaderComponent;
 };
 
+
 export {
     ProjectLoaderHOC as default
 };
diff --git a/src/lib/vm-listener-hoc.jsx b/src/lib/vm-listener-hoc.jsx
index dbc520093..8e7a0ca17 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,
+        vm: state.vm,
         hoveredSprite: state.hoveredTarget.sprite,
         editingTarget: state.targets.editingTarget
     });
diff --git a/src/reducers/modals.js b/src/reducers/modals.js
index ba038ff34..2a3772e59 100644
--- a/src/reducers/modals.js
+++ b/src/reducers/modals.js
@@ -8,6 +8,7 @@ const MODAL_COSTUME_LIBRARY = 'costumeLibrary';
 const MODAL_EXTENSION_LIBRARY = 'extensionLibrary';
 const MODAL_FEEDBACK_FORM = 'feedbackForm';
 const MODAL_IMPORT_INFO = 'importInfo';
+const MODAL_LOADING_PROJECT = 'loadingProject';
 const MODAL_PREVIEW_INFO = 'previewInfo';
 const MODAL_SOUND_LIBRARY = 'soundLibrary';
 const MODAL_SPRITE_LIBRARY = 'spriteLibrary';
@@ -20,6 +21,7 @@ const initialState = {
     [MODAL_EXTENSION_LIBRARY]: false,
     [MODAL_FEEDBACK_FORM]: false,
     [MODAL_IMPORT_INFO]: false,
+    [MODAL_LOADING_PROJECT]: false,
     [MODAL_PREVIEW_INFO]: true,
     [MODAL_SOUND_LIBRARY]: false,
     [MODAL_SPRITE_LIBRARY]: false,
@@ -73,6 +75,10 @@ const openImportInfo = function () {
     analytics.pageview('modals/import');
     return openModal(MODAL_IMPORT_INFO);
 };
+const openLoadingProject = function () {
+    analytics.pageview('modals/loading');
+    return openModal(MODAL_LOADING_PROJECT);
+};
 const openPreviewInfo = function () {
     analytics.pageview('/modals/preview');
     return openModal(MODAL_PREVIEW_INFO);
@@ -104,6 +110,9 @@ const closeFeedbackForm = function () {
 const closeImportInfo = function () {
     return closeModal(MODAL_IMPORT_INFO);
 };
+const closeLoadingProject = function () {
+    return closeModal(MODAL_LOADING_PROJECT);
+};
 const closePreviewInfo = function () {
     return closeModal(MODAL_PREVIEW_INFO);
 };
@@ -123,6 +132,7 @@ export {
     openExtensionLibrary,
     openFeedbackForm,
     openImportInfo,
+    openLoadingProject,
     openPreviewInfo,
     openSoundLibrary,
     openSpriteLibrary,
@@ -132,6 +142,7 @@ export {
     closeExtensionLibrary,
     closeFeedbackForm,
     closeImportInfo,
+    closeLoadingProject,
     closePreviewInfo,
     closeSpriteLibrary,
     closeSoundLibrary,
diff --git a/src/reducers/vm.js b/src/reducers/vm.js
index 1f96dc400..dd70f3f4f 100644
--- a/src/reducers/vm.js
+++ b/src/reducers/vm.js
@@ -2,29 +2,15 @@ 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 = {
-    vm: defaultVM,
-    projectData: null,
-    projectId: null // 0 means default project, null or undefined means project from local file
-};
+const initialState = defaultVM;
 
 const reducer = function (state, action) {
     if (typeof state === 'undefined') state = initialState;
     switch (action.type) {
     case SET_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
-        });
+        return action.vm;
     default:
         return state;
     }
@@ -35,22 +21,7 @@ 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,
-    setProjectId,
-    setProjectData
+    setVM
 };
-- 
GitLab