From 0d8d79666c191f7c98c762e483b4c6fce58998b0 Mon Sep 17 00:00:00 2001 From: GarboMuffin Date: Sun, 13 Nov 2022 02:12:32 -0600 Subject: [PATCH 1/4] Remove projectId from vm-manager-hoc Was leaking to lower HOCs --- src/lib/vm-manager-hoc.jsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/vm-manager-hoc.jsx b/src/lib/vm-manager-hoc.jsx index f548773fe07..e07bf9ce0ae 100644 --- a/src/lib/vm-manager-hoc.jsx +++ b/src/lib/vm-manager-hoc.jsx @@ -117,7 +117,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 }; @@ -130,7 +129,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 From 86204cc2c86bcfccf4b3a9cc705597b35f6be105 Mon Sep 17 00:00:00 2001 From: GarboMuffin Date: Sat, 19 Nov 2022 23:32:29 -0600 Subject: [PATCH 2/4] Allow using cloud variables in projects loaded from files and in the editor For projects from files, we generate a room ID from the title (similar to the packager). In the editor, we prefix the room ID so that cloud variables still work but remain separate from variables outside of the editor. Closes https://github.com/TurboWarp/scratch-gui/issues/645 --- src/lib/cloud-manager-hoc.jsx | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/lib/cloud-manager-hoc.jsx b/src/lib/cloud-manager-hoc.jsx index 75661e603a6..3b180dd5e74 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'; @@ -43,8 +44,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) - // tw: handle cases where we should explicitly close and reconnect() in the same update - if (this.shouldReconnect(this.props, prevProps)) { + if (this.shouldConsiderReconnecting(this.props, prevProps)) { this.disconnectFromCloud(); if (this.shouldConnect(this.props)) { this.connectToCloud(); @@ -68,7 +68,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) { @@ -83,10 +83,11 @@ const cloudManagerHOC = function (WrappedComponent) { !props.canModifyCloudData ); } - // tw: handle cases where we should explicitly close and reconnect() in the same update - shouldReconnect (props, prevProps) { - // reconnect when username changes - return this.isConnected() && props.username !== prevProps.username; + shouldConsiderReconnecting (props, prevProps) { + return this.isConnected() && ( + props.username !== prevProps.username || + props.projectId !== prevProps.projectId + ); } isConnected () { return this.cloudProvider && !!this.cloudProvider.connection; @@ -126,7 +127,7 @@ const cloudManagerHOC = function (WrappedComponent) { projectId, username, hasCloudPermission, - isShowingWithId, + isShowingProject, onShowCloudInfo, onInvalidUsername, /* eslint-enable no-unused-vars */ @@ -147,7 +148,7 @@ const cloudManagerHOC = function (WrappedComponent) { canModifyCloudData: PropTypes.bool.isRequired, cloudHost: PropTypes.string, hasCloudPermission: PropTypes.bool, - isShowingWithId: PropTypes.bool.isRequired, + isShowingProject: PropTypes.bool.isRequired, onInvalidUsername: PropTypes.func, onShowCloudInfo: PropTypes.func, projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), @@ -161,14 +162,19 @@ const cloudManagerHOC = function (WrappedComponent) { username: null }; - const mapStateToProps = (state, ownProps) => { + const mapStateToProps = state => { const loadingState = state.scratchGui.projectState.loadingState; + const baseProjectId = getIsShowingWithId(loadingState) ? ( + state.scratchGui.projectState.projectId + ) : ( + `@gui/${state.scratchGui.projectTitle}` + ); return { - isShowingWithId: getIsShowingWithId(loadingState), - projectId: state.scratchGui.projectState.projectId, + 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: (!state.scratchGui.mode.hasEverEnteredEditor || ownProps.canSave) + canModifyCloudData: true }; }; From 6f3c3a9bce01e9cdd0ae15157ef50418c1337662 Mon Sep 17 00:00:00 2001 From: GarboMuffin Date: Sat, 19 Nov 2022 23:36:55 -0600 Subject: [PATCH 3/4] Update menu bar cloud variable toggle Previous commit makes cloud variables available in the editor --- src/components/menu-bar/menu-bar.jsx | 33 +++++++++----------------- src/containers/tw-cloud-toggler.jsx | 35 ++++------------------------ 2 files changed, 16 insertions(+), 52 deletions(-) diff --git a/src/components/menu-bar/menu-bar.jsx b/src/components/menu-bar/menu-bar.jsx index bd3bcb3fde2..1c38514f558 100644 --- a/src/components/menu-bar/menu-bar.jsx +++ b/src/components/menu-bar/menu-bar.jsx @@ -733,30 +733,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 ad48bcc00a5..f679727f260 100644 --- a/src/containers/tw-cloud-toggler.jsx +++ b/src/containers/tw-cloud-toggler.jsx @@ -1,18 +1,9 @@ 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'; -const messages = defineMessages({ - cloudUnavailableAlert: { - defaultMessage: 'Cannot use cloud variables, most likely because you opened the editor.', - description: 'Message displayed when clicking on the option to toggle cloud variables when cloud variables are not available', - id: 'tw.menuBar.cloudUnavailableAlert' - } -}); - class CloudVariablesToggler extends React.Component { constructor (props) { super(props); @@ -21,44 +12,28 @@ class CloudVariablesToggler extends React.Component { ]); } toggleCloudVariables () { - if (!this.props.canUseCloudVariables) { - // eslint-disable-next-line no-alert - alert(this.props.intl.formatMessage(messages.cloudUnavailableAlert)); - 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); From 14080a3ed50dadcc8355cf2090d277db6db8e2f7 Mon Sep 17 00:00:00 2001 From: GarboMuffin Date: Sat, 19 Nov 2022 23:54:46 -0600 Subject: [PATCH 4/4] Fix tests --- test/unit/util/cloud-manager-hoc.test.jsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/unit/util/cloud-manager-hoc.test.jsx b/test/unit/util/cloud-manager-hoc.test.jsx index ae75311bb29..b457722299a 100644 --- a/test/unit/util/cloud-manager-hoc.test.jsx +++ b/test/unit/util/cloud-manager-hoc.test.jsx @@ -140,7 +140,7 @@ describe('CloudManagerHOC', () => { expect(CloudProvider).not.toHaveBeenCalled(); }); - test('if the isShowingWithId prop becomes true, it sets the cloud provider on the vm', () => { + test('if the isShowingProject prop becomes true, it sets the cloud provider on the vm', () => { const Component = () =>
; const WrappedComponent = cloudManagerHOC(Component); const onShowCloudInfo = jest.fn(); @@ -162,7 +162,7 @@ describe('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); @@ -171,7 +171,7 @@ describe('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( @@ -189,7 +189,7 @@ describe('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); @@ -242,7 +242,8 @@ describe('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);