Merge pull request #5168 from benjiwheeler/clear-title-on-default-project

Refactor title handling

src/components/gui/gui.jsx

@@ -94,7 +94,6 @@ const GUIComponent = props => {
         onLogOut,
         onOpenRegistration,
         onToggleLoginOpen,
-        onUpdateProjectTitle,
         onActivateCostumesTab,
         onActivateSoundsTab,
         onActivateTab,
@@ -228,7 +227,6 @@ const GUIComponent = props => {
                     onSeeCommunity={onSeeCommunity}
                     onShare={onShare}
                     onToggleLoginOpen={onToggleLoginOpen}
-                    onUpdateProjectTitle={onUpdateProjectTitle}
                 />
                 <Box className={styles.bodyWrapper}>
                     <Box className={styles.flexWrapper}>
@@ -406,7 +404,6 @@ GUIComponent.propTypes = {
     onTelemetryModalOptIn: PropTypes.func,
     onTelemetryModalOptOut: PropTypes.func,
     onToggleLoginOpen: PropTypes.func,
-    onUpdateProjectTitle: PropTypes.func,
     renderLogin: PropTypes.func,
     showComingSoon: PropTypes.bool,
     soundsTabVisible: PropTypes.bool,
@@ -433,7 +430,6 @@ GUIComponent.defaultProps = {
     isCreating: false,
     isShared: false,
     loading: false,
-    onUpdateProjectTitle: () => {},
     showComingSoon: false,
     stageSizeMode: STAGE_SIZE_MODES.large
 };

src/components/menu-bar/menu-bar.jsx

@@ -395,7 +395,6 @@ class MenuBar extends React.Component {
                                         <SBFileUploader
                                             canSave={this.props.canSave}
                                             userOwnsProject={this.props.userOwnsProject}
-                                            onUpdateProjectTitle={this.props.onUpdateProjectTitle}
                                         >
                                             {(className, renderFileInput, handleLoadProject) => (
                                                 <MenuItem
@@ -495,7 +494,6 @@ class MenuBar extends React.Component {
                             >
                                 <ProjectTitleInput
                                     className={classNames(styles.titleFieldGrowable)}
-                                    onUpdateProjectTitle={this.props.onUpdateProjectTitle}
                                 />
                             </MenuBarItemTooltip>
                         </div>
@@ -746,7 +744,6 @@ MenuBar.propTypes = {
     onSeeCommunity: PropTypes.func,
     onShare: PropTypes.func,
     onToggleLoginOpen: PropTypes.func,
-    onUpdateProjectTitle: PropTypes.func,
     projectTitle: PropTypes.string,
     renderLogin: PropTypes.func,
     sessionExists: PropTypes.bool,

src/components/menu-bar/project-title-input.jsx

@@ -1,9 +1,9 @@
 import classNames from 'classnames';
 import {connect} from 'react-redux';
 import PropTypes from 'prop-types';
-import bindAll from 'lodash.bindall';
 import React from 'react';
 import {defineMessages, intlShape, injectIntl} from 'react-intl';
+import {setProjectTitle} from '../../reducers/project-title';
 
 import BufferedInputHOC from '../forms/buffered-input-hoc.jsx';
 import Input from '../forms/input.jsx';
@@ -19,39 +19,27 @@ const messages = defineMessages({
     }
 });
 
-class ProjectTitleInput extends React.Component {
-    constructor (props) {
-        super(props);
-        bindAll(this, [
-            'handleUpdateProjectTitle'
-        ]);
-    }
-    // call onUpdateProjectTitle if it is defined (only defined when gui
-    // is used within scratch-www)
-    handleUpdateProjectTitle (newTitle) {
-        if (this.props.onUpdateProjectTitle) {
-            this.props.onUpdateProjectTitle(newTitle);
-        }
-    }
-    render () {
-        return (
-            <BufferedInput
-                className={classNames(styles.titleField, this.props.className)}
-                maxLength="100"
-                placeholder={this.props.intl.formatMessage(messages.projectTitlePlaceholder)}
-                tabIndex="0"
-                type="text"
-                value={this.props.projectTitle}
-                onSubmit={this.handleUpdateProjectTitle}
-            />
-        );
-    }
-}
+const ProjectTitleInput = ({
+    className,
+    intl,
+    onSubmit,
+    projectTitle
+}) => (
+    <BufferedInput
+        className={classNames(styles.titleField, className)}
+        maxLength="100"
+        placeholder={intl.formatMessage(messages.projectTitlePlaceholder)}
+        tabIndex="0"
+        type="text"
+        value={projectTitle}
+        onSubmit={onSubmit}
+    />
+);
 
 ProjectTitleInput.propTypes = {
     className: PropTypes.string,
     intl: intlShape.isRequired,
-    onUpdateProjectTitle: PropTypes.func,
+    onSubmit: PropTypes.func,
     projectTitle: PropTypes.string
 };
 
@@ -59,7 +47,9 @@ const mapStateToProps = state => ({
     projectTitle: state.scratchGui.projectTitle
 });
 
-const mapDispatchToProps = () => ({});
+const mapDispatchToProps = dispatch => ({
+    onSubmit: title => dispatch(setProjectTitle(title))
+});
 
 export default injectIntl(connect(
     mapStateToProps,

src/containers/gui.jsx

@@ -4,14 +4,13 @@ import {compose} from 'redux';
 import {connect} from 'react-redux';
 import ReactModal from 'react-modal';
 import VM from 'scratch-vm';
-import {defineMessages, injectIntl, intlShape} from 'react-intl';
+import {injectIntl, intlShape} from 'react-intl';
 
 import ErrorBoundaryHOC from '../lib/error-boundary-hoc.jsx';
 import {
     getIsError,
     getIsShowingProject
 } from '../reducers/project-state';
-import {setProjectTitle} from '../reducers/project-title';
 import {
     activateTab,
     BLOCKS_TAB_INDEX,
@@ -29,6 +28,7 @@ import {
 import FontLoaderHOC from '../lib/font-loader-hoc.jsx';
 import LocalizationHOC from '../lib/localization-hoc.jsx';
 import ProjectFetcherHOC from '../lib/project-fetcher-hoc.jsx';
+import TitledHOC from '../lib/titled-hoc.jsx';
 import ProjectSaverHOC from '../lib/project-saver-hoc.jsx';
 import QueryParserHOC from '../lib/query-parser-hoc.jsx';
 import storage from '../lib/storage';
@@ -39,18 +39,9 @@ import cloudManagerHOC from '../lib/cloud-manager-hoc.jsx';
 import GUIComponent from '../components/gui/gui.jsx';
 import {setIsScratchDesktop} from '../lib/isScratchDesktop.js';
 
-const messages = defineMessages({
-    defaultProjectTitle: {
-        id: 'gui.gui.defaultProjectTitle',
-        description: 'Default title for project',
-        defaultMessage: 'Scratch Project'
-    }
-});
-
 class GUI extends React.Component {
     componentDidMount () {
         setIsScratchDesktop(this.props.isScratchDesktop);
-        this.setReduxTitle(this.props.projectTitle);
         this.props.onStorageInit(storage);
         this.props.onVmInit(this.props.vm);
     }
@@ -58,24 +49,12 @@ class GUI extends React.Component {
         if (this.props.projectId !== prevProps.projectId && this.props.projectId !== null) {
             this.props.onUpdateProjectId(this.props.projectId);
         }
-        if (this.props.projectTitle !== prevProps.projectTitle) {
-            this.setReduxTitle(this.props.projectTitle);
-        }
         if (this.props.isShowingProject && !prevProps.isShowingProject) {
             // this only notifies container when a project changes from not yet loaded to loaded
             // At this time the project view in www doesn't need to know when a project is unloaded
             this.props.onProjectLoaded();
         }
     }
-    setReduxTitle (newTitle) {
-        if (newTitle === null || typeof newTitle === 'undefined') {
-            this.props.onUpdateReduxProjectTitle(
-                this.props.intl.formatMessage(messages.defaultProjectTitle)
-            );
-        } else {
-            this.props.onUpdateReduxProjectTitle(newTitle);
-        }
-    }
     render () {
         if (this.props.isError) {
             throw new Error(
@@ -92,11 +71,9 @@ class GUI extends React.Component {
             onProjectLoaded,
             onStorageInit,
             onUpdateProjectId,
-            onUpdateReduxProjectTitle,
             onVmInit,
             projectHost,
             projectId,
-            projectTitle,
             /* eslint-enable no-unused-vars */
             children,
             fetchingProject,
@@ -131,12 +108,9 @@ GUI.propTypes = {
     onSeeCommunity: PropTypes.func,
     onStorageInit: PropTypes.func,
     onUpdateProjectId: PropTypes.func,
-    onUpdateProjectTitle: PropTypes.func,
-    onUpdateReduxProjectTitle: PropTypes.func,
     onVmInit: PropTypes.func,
     projectHost: PropTypes.string,
     projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
-    projectTitle: PropTypes.string,
     telemetryModalVisible: PropTypes.bool,
     vm: PropTypes.instanceOf(VM).isRequired
 };
@@ -186,8 +160,7 @@ const mapDispatchToProps = dispatch => ({
     onActivateSoundsTab: () => dispatch(activateTab(SOUNDS_TAB_INDEX)),
     onRequestCloseBackdropLibrary: () => dispatch(closeBackdropLibrary()),
     onRequestCloseCostumeLibrary: () => dispatch(closeCostumeLibrary()),
-    onRequestCloseTelemetryModal: () => dispatch(closeTelemetryModal()),
-    onUpdateReduxProjectTitle: title => dispatch(setProjectTitle(title))
+    onRequestCloseTelemetryModal: () => dispatch(closeTelemetryModal())
 });
 
 const ConnectedGUI = injectIntl(connect(
@@ -204,6 +177,7 @@ const WrappedGui = compose(
     FontLoaderHOC,
     QueryParserHOC,
     ProjectFetcherHOC,
+    TitledHOC,
     ProjectSaverHOC,
     vmListenerHOC,
     vmManagerHOC,

src/containers/sb-file-uploader.jsx

@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import React from 'react';
 import {connect} from 'react-redux';
 import {defineMessages, injectIntl, intlShape} from 'react-intl';
+import {setProjectTitle} from '../reducers/project-title';
 
 import log from '../lib/log';
 import sharedMessages from '../lib/shared-messages';
@@ -131,7 +132,7 @@ class SBFileUploader extends React.Component {
                     // This is necessary in case the user wants to reload a project
                     if (filename) {
                         const uploadedProjectTitle = this.getProjectTitleFromFilename(filename);
-                        this.props.onUpdateProjectTitle(uploadedProjectTitle);
+                        this.props.onReceivedProjectTitle(uploadedProjectTitle);
                     }
                     this.resetFileInput();
                 })
@@ -179,9 +180,9 @@ SBFileUploader.propTypes = {
     loadingState: PropTypes.oneOf(LoadingStates),
     onLoadingFinished: PropTypes.func,
     onLoadingStarted: PropTypes.func,
-    onUpdateProjectTitle: PropTypes.func,
     projectChanged: PropTypes.bool,
     requestProjectUpload: PropTypes.func,
+    onReceivedProjectTitle: PropTypes.func,
     userOwnsProject: PropTypes.bool,
     vm: PropTypes.shape({
         loadProject: PropTypes.func
@@ -209,7 +210,8 @@ const mapDispatchToProps = (dispatch, ownProps) => ({
         dispatch(closeFileMenu());
     },
     requestProjectUpload: loadingState => dispatch(requestProjectUpload(loadingState)),
-    onLoadingStarted: () => dispatch(openLoadingProject())
+    onLoadingStarted: () => dispatch(openLoadingProject()),
+    onReceivedProjectTitle: title => dispatch(setProjectTitle(title))
 });
 
 // Allow incoming props to override redux-provided props. Used to mock in tests.

src/lib/titled-hoc.jsx

@@ -1,5 +1,18 @@
+import PropTypes from 'prop-types';
 import React from 'react';
-import bindAll from 'lodash.bindall';
+import {connect} from 'react-redux';
+import {defineMessages, injectIntl, intlShape} from 'react-intl';
+
+import {getIsShowingWithoutId} from '../reducers/project-state';
+import {setProjectTitle} from '../reducers/project-title';
+
+const messages = defineMessages({
+    defaultProjectTitle: {
+        id: 'gui.gui.defaultProjectTitle',
+        description: 'Default title for project',
+        defaultMessage: 'Scratch Project'
+    }
+});
 
 /* Higher Order Component to get and set the project title
  * @param {React.Component} WrappedComponent component to receive project title related props
@@ -7,31 +20,79 @@ import bindAll from 'lodash.bindall';
  */
 const TitledHOC = function (WrappedComponent) {
     class TitledComponent extends React.Component {
-        constructor (props) {
-            super(props);
-            bindAll(this, [
-                'handleUpdateProjectTitle'
-            ]);
-            this.state = {
-                projectTitle: null
-            };
+        componentDidMount () {
+            this.handleReceivedProjectTitle(this.props.projectTitle);
         }
-        handleUpdateProjectTitle (newTitle) {
-            this.setState({projectTitle: newTitle});
+        componentDidUpdate (prevProps) {
+            if (this.props.projectTitle !== prevProps.projectTitle) {
+                this.handleReceivedProjectTitle(this.props.projectTitle);
+            }
+            // if the projectTitle hasn't changed, but the reduxProjectTitle
+            // HAS changed, we need to report that change to the projectTitle's owner
+            if (this.props.reduxProjectTitle !== prevProps.reduxProjectTitle &&
+                this.props.reduxProjectTitle !== this.props.projectTitle) {
+                this.props.onUpdateProjectTitle(this.props.reduxProjectTitle);
+            }
+        }
+        handleReceivedProjectTitle (requestedTitle) {
+            let newTitle = requestedTitle;
+            if (newTitle === null || typeof newTitle === 'undefined') {
+                newTitle = this.props.intl.formatMessage(messages.defaultProjectTitle);
+            }
+            this.props.onChangedProjectTitle(newTitle);
         }
         render () {
+            const {
+                /* eslint-disable no-unused-vars */
+                intl,
+                isShowingWithoutId,
+                onChangedProjectTitle,
+                // for children, we replace onUpdateProjectTitle with our own
+                onUpdateProjectTitle,
+                // we don't pass projectTitle prop to children -- they must use
+                // redux value
+                projectTitle,
+                reduxProjectTitle,
+                /* eslint-enable no-unused-vars */
+                ...componentProps
+            } = this.props;
             return (
                 <WrappedComponent
-                    canEditTitle
-                    projectTitle={this.state.projectTitle}
-                    onUpdateProjectTitle={this.handleUpdateProjectTitle}
-                    {...this.props}
+                    {...componentProps}
                 />
             );
         }
     }
 
-    return TitledComponent;
+    TitledComponent.propTypes = {
+        intl: intlShape,
+        isShowingWithoutId: PropTypes.bool,
+        onChangedProjectTitle: PropTypes.func,
+        onUpdateProjectTitle: PropTypes.func,
+        projectTitle: PropTypes.string,
+        reduxProjectTitle: PropTypes.string
+    };
+
+    TitledComponent.defaultProps = {
+        onUpdateProjectTitle: () => {}
+    };
+
+    const mapStateToProps = state => {
+        const loadingState = state.scratchGui.projectState.loadingState;
+        return {
+            isShowingWithoutId: getIsShowingWithoutId(loadingState),
+            reduxProjectTitle: state.scratchGui.projectTitle
+        };
+    };
+
+    const mapDispatchToProps = dispatch => ({
+        onChangedProjectTitle: title => dispatch(setProjectTitle(title))
+    });
+
+    return injectIntl(connect(
+        mapStateToProps,
+        mapDispatchToProps,
+    )(TitledComponent));
 };
 
 export {

src/playground/player.jsx

@@ -9,7 +9,6 @@ import Box from '../components/box/box.jsx';
 import GUI from '../containers/gui.jsx';
 import HashParserHOC from '../lib/hash-parser-hoc.jsx';
 import AppStateHOC from '../lib/app-state-hoc.jsx';
-import TitledHOC from '../lib/titled-hoc.jsx';
 
 import {setPlayer} from '../reducers/mode';
 
@@ -24,6 +23,7 @@ const Player = ({isPlayerOnly, onSeeInside, projectId}) => (
     <Box className={classNames(isPlayerOnly ? styles.stageOnly : styles.editor)}>
         {isPlayerOnly && <button onClick={onSeeInside}>{'See inside'}</button>}
         <GUI
+            canEditTitle
             enableCommunity
             isPlayerOnly={isPlayerOnly}
             projectId={projectId}
@@ -55,8 +55,7 @@ const ConnectedPlayer = connect(
 // ability to compose reducers.
 const WrappedPlayer = compose(
     AppStateHOC,
-    HashParserHOC,
-    TitledHOC
+    HashParserHOC
 )(ConnectedPlayer);
 
 const appTarget = document.createElement('div');

src/playground/render-gui.jsx

@@ -5,7 +5,6 @@ import {compose} from 'redux';
 import AppStateHOC from '../lib/app-state-hoc.jsx';
 import GUI from '../containers/gui.jsx';
 import HashParserHOC from '../lib/hash-parser-hoc.jsx';
-import TitledHOC from '../lib/titled-hoc.jsx';
 import log from '../lib/log.js';
 
 const onClickLogo = () => {
@@ -37,8 +36,7 @@ export default appTarget => {
     // ability to compose reducers.
     const WrappedGui = compose(
         AppStateHOC,
-        HashParserHOC,
-        TitledHOC
+        HashParserHOC
     )(GUI);
 
     // TODO a hack for testing the backpack, allow backpack host to be set by url param
@@ -67,6 +65,7 @@ export default appTarget => {
         // important: this is checking whether `simulateScratchDesktop` is truthy, not just defined!
         simulateScratchDesktop ?
             <WrappedGui
+                canEditTitle
                 isScratchDesktop
                 showTelemetryModal
                 canSave={false}
@@ -75,6 +74,7 @@ export default appTarget => {
                 onTelemetryModalOptOut={handleTelemetryModalOptOut}
             /> :
             <WrappedGui
+                canEditTitle
                 backpackVisible
                 showComingSoon
                 backpackHost={backpackHost}

test/unit/containers/sb-file-uploader.test.jsx

@@ -11,7 +11,6 @@ describe('SBFileUploader Container', () => {
     const mockStore = configureStore();
     let onLoadingFinished;
     let onLoadingStarted;
-    let onUpdateProjectTitle;
     let store;
 
     // Wrap this in a function so it gets test specific states and can be reused.
@@ -20,7 +19,6 @@ describe('SBFileUploader Container', () => {
             <SBFileUploader
                 onLoadingFinished={onLoadingFinished}
                 onLoadingStarted={onLoadingStarted}
-                onUpdateProjectTitle={onUpdateProjectTitle}
             >
                 {(renderFileInput, loadProject) => (
                     <div
@@ -40,7 +38,6 @@ describe('SBFileUploader Container', () => {
                 vm: {}
             }
         });
-        onUpdateProjectTitle = jest.fn();
         onLoadingFinished = jest.fn();
         onLoadingStarted = jest.fn();
     });