diff --git a/src/components/menu-bar/menu-bar.jsx b/src/components/menu-bar/menu-bar.jsx index 2dff0b97877..82cf2af945d 100644 --- a/src/components/menu-bar/menu-bar.jsx +++ b/src/components/menu-bar/menu-bar.jsx @@ -776,31 +776,19 @@ class MenuBar extends React.Component { /> )} - {(toggleCloudVariables, {enabled, canUseCloudVariables}) => ( - - {canUseCloudVariables ? ( - enabled ? ( - - ) : ( - - ) + {(toggleCloudVariables, enabled) => ( + + {enabled ? ( + ) : ( )} diff --git a/src/containers/tw-cloud-toggler.jsx b/src/containers/tw-cloud-toggler.jsx index 5e761374fbf..f679727f260 100644 --- a/src/containers/tw-cloud-toggler.jsx +++ b/src/containers/tw-cloud-toggler.jsx @@ -1,24 +1,8 @@ import bindAll from 'lodash.bindall'; import PropTypes from 'prop-types'; import React from 'react'; -import {defineMessages, injectIntl, intlShape} from 'react-intl'; import {connect} from 'react-redux'; import {setCloud} from '../reducers/tw'; -import isScratchDesktop from '../lib/isScratchDesktop'; - -const messages = defineMessages({ - cloudUnavailableAlert: { - defaultMessage: 'Cannot use cloud variables, most likely because you opened the editor.', - // eslint-disable-next-line max-len - description: 'Message displayed when clicking on the option to toggle cloud variables when cloud variables are not available', - id: 'tw.menuBar.cloudUnavailableAlert' - }, - cloudUnavailableDesktop: { - defaultMessage: 'Cannot use cloud variables in desktop app.', - description: 'Message displayed when clicking on the option to toggle cloud variables in desktop app', - id: 'tw.menuBar.cloudUnavailableDesktop' - } -}); class CloudVariablesToggler extends React.Component { constructor (props) { @@ -28,47 +12,28 @@ class CloudVariablesToggler extends React.Component { ]); } toggleCloudVariables () { - if (!this.props.canUseCloudVariables) { - const message = this.props.intl.formatMessage( - isScratchDesktop() ? messages.cloudUnavailableDesktop : messages.cloudUnavailableAlert - ); - // eslint-disable-next-line no-alert - alert(message); - return; - } this.props.onCloudChange(!this.props.enabled); } render () { - const { - /* eslint-disable no-unused-vars */ - children, - /* eslint-enable no-unused-vars */ - ...props - } = this.props; - return this.props.children(this.toggleCloudVariables, props); + return this.props.children(this.toggleCloudVariables, this.props.enabled); } } CloudVariablesToggler.propTypes = { - intl: intlShape, children: PropTypes.func, enabled: PropTypes.bool, - username: PropTypes.string, - onCloudChange: PropTypes.func, - canUseCloudVariables: PropTypes.bool + onCloudChange: PropTypes.func }; const mapStateToProps = state => ({ - username: state.scratchGui.tw.username, - enabled: state.scratchGui.tw.cloud, - canUseCloudVariables: !state.scratchGui.mode.hasEverEnteredEditor + enabled: state.scratchGui.tw.cloud }); const mapDispatchToProps = dispatch => ({ onCloudChange: enabled => dispatch(setCloud(enabled)) }); -export default injectIntl(connect( +export default connect( mapStateToProps, mapDispatchToProps -)(CloudVariablesToggler)); +)(CloudVariablesToggler); diff --git a/src/lib/cloud-manager-hoc.jsx b/src/lib/cloud-manager-hoc.jsx index 078f9acf112..0fb188806be 100644 --- a/src/lib/cloud-manager-hoc.jsx +++ b/src/lib/cloud-manager-hoc.jsx @@ -7,6 +7,7 @@ import VM from 'scratch-vm'; import CloudProvider from '../lib/cloud-provider'; import { + getIsShowingProject, getIsShowingWithId } from '../reducers/project-state'; @@ -54,7 +55,7 @@ const cloudManagerHOC = function (WrappedComponent) { // when loading a new project e.g. via file upload // (and eventually move it out of the vm.clear function) - if (this.shouldReconnect(this.props, prevProps)) { + if (this.shouldConsiderReconnecting(this.props, prevProps)) { this.disconnectFromCloud(); if (this.shouldConnect(this.props)) { this.connectToCloud(); @@ -67,6 +68,7 @@ const cloudManagerHOC = function (WrappedComponent) { } if (this.shouldDisconnect(this.props, prevProps)) { + this.shouldDisconnect(this.props, prevProps); this.disconnectFromCloud(); } } @@ -86,7 +88,7 @@ const cloudManagerHOC = function (WrappedComponent) { } shouldConnect (props) { return !this.isConnected() && this.canUseCloud(props) && - props.isShowingWithId && props.vm.runtime.hasCloudData() && + props.isShowingProject && props.vm.runtime.hasCloudData() && props.canModifyCloudData; } shouldDisconnect (props, prevProps) { @@ -94,16 +96,15 @@ const cloudManagerHOC = function (WrappedComponent) { ( // Can no longer use cloud or cloud provider info is now stale !this.canUseCloud(props) || !props.vm.runtime.hasCloudData() || - (props.projectId !== prevProps.projectId) || - // tw: username changes are handled in "reconnect" - // (props.username !== prevProps.username) || + props.projectId !== prevProps.projectId || // Editing someone else's project !props.canModifyCloudData ); } - shouldReconnect (props, prevProps) { + shouldConsiderReconnecting (props, prevProps) { return this.isConnected() && ( props.username !== prevProps.username || + props.projectId !== prevProps.projectId || props.reduxCloudHost !== prevProps.reduxCloudHost ); } @@ -151,7 +152,7 @@ const cloudManagerHOC = function (WrappedComponent) { projectId, username, hasCloudPermission, - isShowingWithId, + isShowingProject, onShowCloudInfo, onInvalidUsername, /* eslint-enable no-unused-vars */ @@ -174,7 +175,7 @@ const cloudManagerHOC = function (WrappedComponent) { reduxCloudHost: PropTypes.string, onSetReduxCloudHost: PropTypes.func, hasCloudPermission: PropTypes.bool, - isShowingWithId: PropTypes.bool.isRequired, + isShowingProject: PropTypes.bool.isRequired, onInvalidUsername: PropTypes.func, onShowCloudInfo: PropTypes.func, projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), @@ -190,14 +191,20 @@ const cloudManagerHOC = function (WrappedComponent) { const mapStateToProps = (state, ownProps) => { const loadingState = state.scratchGui.projectState.loadingState; + const baseProjectId = getIsShowingWithId(loadingState) || !getIsShowingProject(loadingState) ? ( + state.scratchGui.projectState.projectId + ) : ( + `@gui/${state.scratchGui.projectTitle}` + ); return { - reduxCloudHost: state.scratchGui.tw.cloudHost, - isShowingWithId: getIsShowingWithId(loadingState), - projectId: state.scratchGui.projectState.projectId, - // if you're editing someone else's project, you can't modify cloud data - canModifyCloudData: (!state.scratchGui.mode.hasEverEnteredEditor || ownProps.canSave) && - // possible security concern if the program attempts to encode webcam data over cloud variables - !(DISABLE_WITH_VIDEO_SENSING && ownProps.vm.extensionManager.isExtensionLoaded('videoSensing')) + isShowingProject: getIsShowingProject(loadingState), + projectId: state.scratchGui.mode.hasEverEnteredEditor ? `@editor/${baseProjectId}` : baseProjectId, + hasCloudPermission: state.scratchGui.tw ? state.scratchGui.tw.cloud : false, + username: state.scratchGui.tw ? state.scratchGui.tw.username : '', + canModifyCloudData: !( + DISABLE_WITH_VIDEO_SENSING && ownProps.vm.extensionManager.isExtensionLoaded('videoSensing') + ), + reduxCloudHost: state.scratchGui.tw.cloudHost }; }; diff --git a/src/lib/vm-manager-hoc.jsx b/src/lib/vm-manager-hoc.jsx index 1bdcab90180..672573d2216 100644 --- a/src/lib/vm-manager-hoc.jsx +++ b/src/lib/vm-manager-hoc.jsx @@ -122,7 +122,6 @@ const vmManagerHOC = function (WrappedComponent) { onLoadedProject: PropTypes.func, onSetProjectUnchanged: PropTypes.func, projectData: PropTypes.oneOfType([PropTypes.object, PropTypes.string]), - projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), username: PropTypes.string, vm: PropTypes.instanceOf(VM).isRequired }; @@ -135,7 +134,6 @@ const vmManagerHOC = function (WrappedComponent) { locale: state.locales.locale, messages: state.locales.messages, projectData: state.scratchGui.projectState.projectData, - projectId: state.scratchGui.projectState.projectId, loadingState: loadingState, isPlayerOnly: state.scratchGui.mode.isPlayerOnly, isStarted: state.scratchGui.vmStatus.started diff --git a/test/unit/util/cloud-manager-hoc.test.jsx b/test/unit/util/cloud-manager-hoc.test.jsx index 773fb38494c..bfb207f76b4 100644 --- a/test/unit/util/cloud-manager-hoc.test.jsx +++ b/test/unit/util/cloud-manager-hoc.test.jsx @@ -185,7 +185,7 @@ describe.skip('CloudManagerHOC', () => { vm.emit('HAS_CLOUD_DATA_UPDATE', true); mounted.setProps({ - isShowingWithId: true, + isShowingProject: true, loadingState: LoadingState.SHOWING_WITH_ID }); expect(vm.setCloudProvider.mock.calls.length).toBe(1); @@ -194,7 +194,7 @@ describe.skip('CloudManagerHOC', () => { expect(onShowCloudInfo).not.toHaveBeenCalled(); }); - test('projectId change should not trigger cloudProvider connection unless isShowingWithId becomes true', () => { + test('projectId change should not trigger cloudProvider connection unless isShowingProject becomes true', () => { const Component = () =>
; const WrappedComponent = cloudManagerHOC(Component); const mounted = mountWithIntl( @@ -212,7 +212,7 @@ describe.skip('CloudManagerHOC', () => { expect(vm.setCloudProvider.mock.calls.length).toBe(0); expect(CloudProvider).not.toHaveBeenCalled(); mounted.setProps({ - isShowingWithId: true, + isShowingProject: true, loadingState: LoadingState.SHOWING_WITH_ID }); expect(vm.setCloudProvider.mock.calls.length).toBe(1); @@ -265,7 +265,8 @@ describe.skip('CloudManagerHOC', () => { projectId: 'a different id' }); - expect(vm.setCloudProvider.mock.calls.length).toBe(2); + // should be 3 not 2 -- one to connect initially, one to disconnect, one to reconnect + expect(vm.setCloudProvider.mock.calls.length).toBe(3); expect(vm.setCloudProvider).toHaveBeenCalledWith(null); expect(requestCloseConnection).toHaveBeenCalledTimes(1);