From f2291d963cfe82b9d714d99bc75703f419a10f96 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 30 Apr 2019 09:38:39 -0400 Subject: [PATCH 1/5] continuing sb file uploader refactor into hoc draft of refactor to make custom confirm modal work for upload; need to further edit sb-file-uploader Add support for custom confirm modal to sb-file-uploader Show custom confirm modal when confirming project upload got custom upload refactor of file upload working, and tested failure case cleaned up debug code, make project file upload work simplified logic around canceling file upload removed old sb-file-uploader.jsx; fixed linter errors reverted two components to use old strings, not shared (can do that refactor another time) removed unnecessary confirm container removed stray comment moved, updated sb file uploader tests, related project state tests removed custom modal, to reduce scope of changes sb file uploader test uses intl --- src/components/gui/gui.jsx | 3 + src/components/menu-bar/menu-bar.jsx | 21 +- src/containers/gui.jsx | 2 + src/containers/sb-file-uploader.jsx | 226 ---------------- src/lib/sb-file-uploader-hoc.jsx | 241 ++++++++++++++++++ src/reducers/project-state.js | 19 +- .../unit/containers/sb-file-uploader.test.jsx | 84 ------ .../reducers/project-state-reducer.test.js | 112 +++++--- test/unit/util/project-saver-hoc.test.jsx | 2 +- test/unit/util/sb-file-uploader-hoc.test.jsx | 108 ++++++++ 10 files changed, 454 insertions(+), 364 deletions(-) delete mode 100644 src/containers/sb-file-uploader.jsx create mode 100644 src/lib/sb-file-uploader-hoc.jsx delete mode 100644 test/unit/containers/sb-file-uploader.test.jsx create mode 100644 test/unit/util/sb-file-uploader-hoc.test.jsx diff --git a/src/components/gui/gui.jsx b/src/components/gui/gui.jsx index 9846eed75..8524b2ba3 100644 --- a/src/components/gui/gui.jsx +++ b/src/components/gui/gui.jsx @@ -105,6 +105,7 @@ const GUIComponent = props => { onRequestCloseTelemetryModal, onSeeCommunity, onShare, + onStartSelectingFileUpload, onTelemetryModalCancel, onTelemetryModalOptIn, onTelemetryModalOptOut, @@ -226,6 +227,7 @@ const GUIComponent = props => { onProjectTelemetryEvent={onProjectTelemetryEvent} onSeeCommunity={onSeeCommunity} onShare={onShare} + onStartSelectingFileUpload={onStartSelectingFileUpload} onToggleLoginOpen={onToggleLoginOpen} /> @@ -399,6 +401,7 @@ GUIComponent.propTypes = { onRequestCloseTelemetryModal: PropTypes.func, onSeeCommunity: PropTypes.func, onShare: PropTypes.func, + onStartSelectingFileUpload: PropTypes.func, onTabSelect: PropTypes.func, onTelemetryModalCancel: PropTypes.func, onTelemetryModalOptIn: PropTypes.func, diff --git a/src/components/menu-bar/menu-bar.jsx b/src/components/menu-bar/menu-bar.jsx index 98bc128a9..05a5294bc 100644 --- a/src/components/menu-bar/menu-bar.jsx +++ b/src/components/menu-bar/menu-bar.jsx @@ -17,7 +17,6 @@ import {ComingSoonTooltip} from '../coming-soon/coming-soon.jsx'; import Divider from '../divider/divider.jsx'; import LanguageSelector from '../../containers/language-selector.jsx'; import SaveStatus from './save-status.jsx'; -import SBFileUploader from '../../containers/sb-file-uploader.jsx'; import ProjectWatcher from '../../containers/project-watcher.jsx'; import MenuBarMenu from './menu-bar-menu.jsx'; import {MenuItem, MenuSection} from '../menu/menu.jsx'; @@ -392,22 +391,11 @@ class MenuBar extends React.Component { )} - - {(className, renderFileInput, handleLoadProject) => ( - - {/* eslint-disable max-len */} - {this.props.intl.formatMessage(sharedMessages.loadFromComputerTitle)} - {/* eslint-enable max-len */} - {renderFileInput()} - - )} - + {this.props.intl.formatMessage(sharedMessages.loadFromComputerTitle)} + {(className, downloadProjectCallback) => ( {(className, renderFileInput, handleLoadProject) => ( - * - * {renderFileInput()} - * - * )} - */ - -const messages = defineMessages({ - loadError: { - id: 'gui.projectLoader.loadError', - defaultMessage: 'The project file that was selected failed to load.', - description: 'An error that displays when a local project file fails to load.' - } -}); - -class SBFileUploader extends React.Component { - constructor (props) { - super(props); - bindAll(this, [ - 'getProjectTitleFromFilename', - 'renderFileInput', - 'setFileInput', - 'handleChange', - 'handleClick', - 'onload', - 'resetFileInput' - ]); - } - componentWillMount () { - this.reader = new FileReader(); - this.reader.onload = this.onload; - this.resetFileInput(); - } - componentDidUpdate (prevProps) { - if (this.props.isLoadingUpload && !prevProps.isLoadingUpload && this.fileToUpload && this.reader) { - this.reader.readAsArrayBuffer(this.fileToUpload); - } - } - componentWillUnmount () { - this.reader = null; - this.resetFileInput(); - } - resetFileInput () { - this.fileToUpload = null; - if (this.fileInput) { - this.fileInput.value = null; - } - } - getProjectTitleFromFilename (fileInputFilename) { - if (!fileInputFilename) return ''; - // only parse title with valid scratch project extensions - // (.sb, .sb2, and .sb3) - const matches = fileInputFilename.match(/^(.*)\.sb[23]?$/); - if (!matches) return ''; - return matches[1].substring(0, 100); // truncate project title to max 100 chars - } - // called when user has finished selecting a file to upload - handleChange (e) { - const { - intl, - isShowingWithoutId, - loadingState, - projectChanged, - userOwnsProject - } = this.props; - - const thisFileInput = e.target; - if (thisFileInput.files) { // Don't attempt to load if no file was selected - this.fileToUpload = thisFileInput.files[0]; - - // If user owns the project, or user has changed the project, - // we must confirm with the user that they really intend to replace it. - // (If they don't own the project and haven't changed it, no need to confirm.) - let uploadAllowed = true; - if (userOwnsProject || (projectChanged && isShowingWithoutId)) { - uploadAllowed = confirm( // eslint-disable-line no-alert - intl.formatMessage(sharedMessages.replaceProjectWarning) - ); - } - if (uploadAllowed) { - this.props.requestProjectUpload(loadingState); - } else { - this.props.closeFileMenu(); - } - } - } - // called when file upload raw data is available in the reader - onload () { - if (this.reader) { - this.props.onLoadingStarted(); - const filename = this.fileToUpload && this.fileToUpload.name; - this.props.vm.loadProject(this.reader.result) - .then(() => { - this.props.onLoadingFinished(this.props.loadingState, true); - // Reset the file input after project is loaded - // This is necessary in case the user wants to reload a project - if (filename) { - const uploadedProjectTitle = this.getProjectTitleFromFilename(filename); - this.props.onReceivedProjectTitle(uploadedProjectTitle); - } - this.resetFileInput(); - }) - .catch(error => { - log.warn(error); - alert(this.props.intl.formatMessage(messages.loadError)); // eslint-disable-line no-alert - this.props.onLoadingFinished(this.props.loadingState, false); - // Reset the file input after project is loaded - // This is necessary in case the user wants to reload a project - this.resetFileInput(); - }); - } - } - handleClick () { - // open filesystem browsing window - this.fileInput.click(); - } - setFileInput (input) { - this.fileInput = input; - } - renderFileInput () { - return ( - - ); - } - render () { - return this.props.children(this.props.className, this.renderFileInput, this.handleClick); - } -} - -SBFileUploader.propTypes = { - canSave: PropTypes.bool, // eslint-disable-line react/no-unused-prop-types - children: PropTypes.func, - className: PropTypes.string, - closeFileMenu: PropTypes.func, - intl: intlShape.isRequired, - isLoadingUpload: PropTypes.bool, - isShowingWithoutId: PropTypes.bool, - loadingState: PropTypes.oneOf(LoadingStates), - onLoadingFinished: PropTypes.func, - onLoadingStarted: PropTypes.func, - projectChanged: PropTypes.bool, - requestProjectUpload: PropTypes.func, - onReceivedProjectTitle: PropTypes.func, - userOwnsProject: PropTypes.bool, - vm: PropTypes.shape({ - loadProject: PropTypes.func - }) -}; -SBFileUploader.defaultProps = { - className: '' -}; -const mapStateToProps = state => { - const loadingState = state.scratchGui.projectState.loadingState; - return { - isLoadingUpload: getIsLoadingUpload(loadingState), - isShowingWithoutId: getIsShowingWithoutId(loadingState), - loadingState: loadingState, - projectChanged: state.scratchGui.projectChanged, - vm: state.scratchGui.vm - }; -}; - -const mapDispatchToProps = (dispatch, ownProps) => ({ - closeFileMenu: () => dispatch(closeFileMenu()), - onLoadingFinished: (loadingState, success) => { - dispatch(onLoadedProject(loadingState, ownProps.canSave, success)); - dispatch(closeLoadingProject()); - dispatch(closeFileMenu()); - }, - requestProjectUpload: loadingState => dispatch(requestProjectUpload(loadingState)), - onLoadingStarted: () => dispatch(openLoadingProject()), - onReceivedProjectTitle: title => dispatch(setProjectTitle(title)) -}); - -// Allow incoming props to override redux-provided props. Used to mock in tests. -const mergeProps = (stateProps, dispatchProps, ownProps) => Object.assign( - {}, stateProps, dispatchProps, ownProps -); - -export default connect( - mapStateToProps, - mapDispatchToProps, - mergeProps -)(injectIntl(SBFileUploader)); diff --git a/src/lib/sb-file-uploader-hoc.jsx b/src/lib/sb-file-uploader-hoc.jsx new file mode 100644 index 000000000..90dd1ff53 --- /dev/null +++ b/src/lib/sb-file-uploader-hoc.jsx @@ -0,0 +1,241 @@ +import bindAll from 'lodash.bindall'; +import React from 'react'; +import PropTypes from 'prop-types'; +import {defineMessages, intlShape, injectIntl} from 'react-intl'; +import {connect} from 'react-redux'; +import log from '../lib/log'; +import sharedMessages from './shared-messages'; + +import { + LoadingStates, + getIsLoadingUpload, + getIsShowingWithoutId, + onLoadedProject, + requestProjectUpload +} from '../reducers/project-state'; +import { + openLoadingProject, + closeLoadingProject +} from '../reducers/modals'; +import { + closeFileMenu +} from '../reducers/menus'; + +const messages = defineMessages({ + loadError: { + id: 'gui.projectLoader.loadError', + defaultMessage: 'The project file that was selected failed to load.', + description: 'An error that displays when a local project file fails to load.' + } +}); + +/** + * Higher Order Component to provide behavior for loading local project files into editor. + * @param {React.Component} WrappedComponent the component to add project file loading functionality to + * @returns {React.Component} WrappedComponent with project file loading functionality added + * + * + * + * + */ +const SBFileUploaderHOC = function (WrappedComponent) { + class SBFileUploaderComponent extends React.Component { + constructor (props) { + super(props); + bindAll(this, [ + 'getProjectTitleFromFilename', + 'handleStartSelectingFileUpload', + 'setFileInput', + 'handleChange', + 'onload', + 'resetFileInput' + ]); + } + componentWillMount () { + this.reader = new FileReader(); + this.reader.onload = this.onload; + this.resetFileInput(); + } + componentDidUpdate (prevProps) { + if (this.props.isLoadingUpload && !prevProps.isLoadingUpload) { + if (this.fileToUpload && this.reader) { + this.reader.readAsArrayBuffer(this.fileToUpload); + } else { + this.props.cancelFileUpload(this.props.loadingState); + } + } + } + componentWillUnmount () { + this.reader = null; + this.resetFileInput(); + } + resetFileInput () { + this.fileToUpload = null; + if (this.fileInput) { + this.fileInput.value = null; + } + } + getProjectTitleFromFilename (fileInputFilename) { + if (!fileInputFilename) return ''; + // only parse title with valid scratch project extensions + // (.sb, .sb2, and .sb3) + const matches = fileInputFilename.match(/^(.*)\.sb[23]?$/); + if (!matches) return ''; + return matches[1].substring(0, 100); // truncate project title to max 100 chars + } + // called when user has finished selecting a file to upload + handleChange (e) { + const { + intl, + isShowingWithoutId, + loadingState, + projectChanged, + userOwnsProject + } = this.props; + + const thisFileInput = e.target; + if (thisFileInput.files) { // Don't attempt to load if no file was selected + this.fileToUpload = thisFileInput.files[0]; + + // If user owns the project, or user has changed the project, + // we must confirm with the user that they really intend to replace it. + // (If they don't own the project and haven't changed it, no need to confirm.) + let uploadAllowed = true; + if (userOwnsProject || (projectChanged && isShowingWithoutId)) { + uploadAllowed = confirm( // eslint-disable-line no-alert + intl.formatMessage(sharedMessages.replaceProjectWarning) + ); + } + if (uploadAllowed) { + this.props.requestProjectUpload(loadingState); + } else { + this.resetFileInput(); + } + this.props.closeFileMenu(); + } + } + // called when file upload raw data is available in the reader + onload () { + if (this.reader) { + this.props.onLoadingStarted(); + const filename = this.fileToUpload && this.fileToUpload.name; + this.props.vm.loadProject(this.reader.result) + .then(() => { + this.props.onLoadingFinished(this.props.loadingState, true); + // Reset the file input after project is loaded + // This is necessary in case the user wants to reload a project + if (filename) { + const uploadedProjectTitle = this.getProjectTitleFromFilename(filename); + this.props.onUpdateProjectTitle(uploadedProjectTitle); + } + this.resetFileInput(); + }) + .catch(error => { + log.warn(error); + this.props.intl.formatMessage(messages.loadError); // eslint-disable-line no-alert + this.props.onLoadingFinished(this.props.loadingState, false); + // Reset the file input after project is loaded + // This is necessary in case the user wants to reload a project + this.resetFileInput(); + }); + } + } + handleStartSelectingFileUpload () { + // open filesystem browsing window + this.fileInput.click(); + } + setFileInput (input) { + this.fileInput = input; + } + render () { + const fileInput = ( + + ); + const { + /* eslint-disable no-unused-vars */ + closeFileMenu: closeFileMenuProp, + isLoadingUpload, + isShowingWithoutId, + loadingState, + onLoadingFinished, + onLoadingStarted, + projectChanged, + requestProjectUpload: requestProjectUploadProp, + userOwnsProject, + /* eslint-enable no-unused-vars */ + ...componentProps + } = this.props; + return ( + + + {fileInput} + + ); + } + } + + SBFileUploaderComponent.propTypes = { + canSave: PropTypes.bool, + cancelFileUpload: PropTypes.func, + closeFileMenu: PropTypes.func, + intl: intlShape.isRequired, + isLoadingUpload: PropTypes.bool, + isShowingWithoutId: PropTypes.bool, + loadingState: PropTypes.oneOf(LoadingStates), + onLoadingFinished: PropTypes.func, + onLoadingStarted: PropTypes.func, + onUpdateProjectTitle: PropTypes.func, + projectChanged: PropTypes.bool, + requestProjectUpload: PropTypes.func, + userOwnsProject: PropTypes.bool, + vm: PropTypes.shape({ + loadProject: PropTypes.func + }) + }; + const mapStateToProps = (state, ownProps) => { + const loadingState = state.scratchGui.projectState.loadingState; + const user = state.session && state.session.session && state.session.session.user; + return { + isLoadingUpload: getIsLoadingUpload(loadingState), + isShowingWithoutId: getIsShowingWithoutId(loadingState), + loadingState: loadingState, + projectChanged: state.scratchGui.projectChanged, + userOwnsProject: ownProps.authorUsername && user && + (ownProps.authorUsername === user.username) + // vm: state.scratchGui.vm + }; + }; + const mapDispatchToProps = (dispatch, ownProps) => ({ + cancelFileUpload: loadingState => dispatch(onLoadedProject(loadingState, false, false)), + closeFileMenu: () => dispatch(closeFileMenu()), + onLoadingFinished: (loadingState, success) => { + dispatch(onLoadedProject(loadingState, ownProps.canSave, success)); + dispatch(closeLoadingProject()); + dispatch(closeFileMenu()); + }, + onLoadingStarted: () => dispatch(openLoadingProject()), + requestProjectUpload: loadingState => dispatch(requestProjectUpload(loadingState)) + }); + // Allow incoming props to override redux-provided props. Used to mock in tests. + const mergeProps = (stateProps, dispatchProps, ownProps) => Object.assign( + {}, stateProps, dispatchProps, ownProps + ); + return injectIntl(connect( + mapStateToProps, + mapDispatchToProps, + mergeProps + )(SBFileUploaderComponent)); +}; + +export { + SBFileUploaderHOC as default +}; diff --git a/src/reducers/project-state.js b/src/reducers/project-state.js index 91e51bbe4..a26d7c971 100644 --- a/src/reducers/project-state.js +++ b/src/reducers/project-state.js @@ -17,7 +17,7 @@ const START_AUTO_UPDATING = 'scratch-gui/project-state/START_AUTO_UPDATING'; const START_CREATING_NEW = 'scratch-gui/project-state/START_CREATING_NEW'; const START_ERROR = 'scratch-gui/project-state/START_ERROR'; const START_FETCHING_NEW = 'scratch-gui/project-state/START_FETCHING_NEW'; -const START_LOADING_VM_FILE_UPLOAD = 'scratch-gui/project-state/START_LOADING_FILE_UPLOAD'; +const START_LOADING_VM_FILE_UPLOAD = 'scratch-gui/project-state/START_LOADING_VM_FILE_UPLOAD'; const START_MANUAL_UPDATING = 'scratch-gui/project-state/START_MANUAL_UPDATING'; const START_REMIXING = 'scratch-gui/project-state/START_REMIXING'; const START_UPDATING_BEFORE_CREATING_COPY = 'scratch-gui/project-state/START_UPDATING_BEFORE_CREATING_COPY'; @@ -435,10 +435,21 @@ const onLoadedProject = (loadingState, canSave, success) => { default: return; } + } else { + switch (loadingState) { + case LoadingState.LOADING_VM_WITH_ID: + case LoadingState.LOADING_VM_FILE_UPLOAD: + return { + type: RETURN_TO_SHOWING + }; + case LoadingState.LOADING_VM_NEW_DEFAULT: + return { + type: START_ERROR + }; + default: + return; + } } - return { - type: RETURN_TO_SHOWING - }; }; const doneUpdatingProject = loadingState => { diff --git a/test/unit/containers/sb-file-uploader.test.jsx b/test/unit/containers/sb-file-uploader.test.jsx deleted file mode 100644 index c757114b4..000000000 --- a/test/unit/containers/sb-file-uploader.test.jsx +++ /dev/null @@ -1,84 +0,0 @@ -import React from 'react'; - -import {shallowWithIntl} from '../../helpers/intl-helpers.jsx'; -import configureStore from 'redux-mock-store'; -import SBFileUploader from '../../../src/containers/sb-file-uploader'; -import {LoadingState} from '../../../src/reducers/project-state'; - -jest.mock('react-ga'); // must mock this entire library, or lib/analytics causes error - -describe('SBFileUploader Container', () => { - const mockStore = configureStore(); - let onLoadingFinished; - let onLoadingStarted; - let store; - - // Wrap this in a function so it gets test specific states and can be reused. - const getContainer = function () { - return ( - - {(renderFileInput, loadProject) => ( -
- )} - - ); - }; - - beforeEach(() => { - store = mockStore({ - scratchGui: { - projectState: { - loadingState: LoadingState.SHOWING_WITH_ID - }, - vm: {} - } - }); - onLoadingFinished = jest.fn(); - onLoadingStarted = jest.fn(); - }); - - test('correctly sets title with .sb3 filename', () => { - const wrapper = shallowWithIntl(getContainer(), {context: {store}}); - const instance = wrapper - .dive() // unwrap redux Connect(InjectIntl(SBFileUploader)) - .dive() // unwrap InjectIntl(SBFileUploader) - .instance(); // SBFileUploader - const projectName = instance.getProjectTitleFromFilename('my project is great.sb3'); - expect(projectName).toBe('my project is great'); - }); - - test('correctly sets title with .sb2 filename', () => { - const wrapper = shallowWithIntl(getContainer(), {context: {store}}); - const instance = wrapper - .dive() // unwrap redux Connect(InjectIntl(SBFileUploader)) - .dive() // unwrap InjectIntl(SBFileUploader) - .instance(); // SBFileUploader - const projectName = instance.getProjectTitleFromFilename('my project is great.sb2'); - expect(projectName).toBe('my project is great'); - }); - - test('correctly sets title with .sb filename', () => { - const wrapper = shallowWithIntl(getContainer(), {context: {store}}); - const instance = wrapper - .dive() // unwrap redux Connect(InjectIntl(SBFileUploader)) - .dive() // unwrap InjectIntl(SBFileUploader) - .instance(); // SBFileUploader - const projectName = instance.getProjectTitleFromFilename('my project is great.sb'); - expect(projectName).toBe('my project is great'); - }); - - test('sets blank title with filename with no extension', () => { - const wrapper = shallowWithIntl(getContainer(), {context: {store}}); - const instance = wrapper - .dive() // unwrap redux Connect(InjectIntl(SBFileUploader)) - .dive() // unwrap InjectIntl(SBFileUploader) - .instance(); // SBFileUploader - const projectName = instance.getProjectTitleFromFilename('my project is great'); - expect(projectName).toBe(''); - }); -}); diff --git a/test/unit/reducers/project-state-reducer.test.js b/test/unit/reducers/project-state-reducer.test.js index 2683c5bee..6938085a3 100644 --- a/test/unit/reducers/project-state-reducer.test.js +++ b/test/unit/reducers/project-state-reducer.test.js @@ -92,25 +92,9 @@ test('onFetchedProjectData new loads project data into vm', () => { expect(resultState.projectData).toBe('1010101'); }); -test('onLoadedProject upload, with canSave false, shows without id', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_FILE_UPLOAD - }; - const action = onLoadedProject(initialState.loadingState, false, true); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); -}); +// onLoadedProject: LOADING_VM_WITH_ID -test('onLoadedProject upload, with canSave true, prepares to save', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_FILE_UPLOAD - }; - const action = onLoadedProject(initialState.loadingState, true, true); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.AUTO_UPDATING); -}); - -test('onLoadedProject with id shows with id', () => { +test('onLoadedProject (LOADING_VM_WITH_ID, true, true) shows with id', () => { const initialState = { loadingState: LoadingState.LOADING_VM_WITH_ID }; @@ -119,25 +103,26 @@ test('onLoadedProject with id shows with id', () => { expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); }); -test('onLoadedProject new shows without id', () => { +test('onLoadedProject (LOADING_VM_WITH_ID, false, true) shows with id', () => { const initialState = { - loadingState: LoadingState.LOADING_VM_NEW_DEFAULT - }; - const action = onLoadedProject(initialState.loadingState, false, true); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); -}); - -test('onLoadedProject new, to save shows without id', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_NEW_DEFAULT + loadingState: LoadingState.LOADING_VM_WITH_ID }; const action = onLoadedProject(initialState.loadingState, true, true); const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); }); -test('onLoadedProject with success false, no project id, shows without id', () => { +test('onLoadedProject (LOADING_VM_WITH_ID, false, false), with project id, shows with id', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_WITH_ID, + projectId: '12345' + }; + const action = onLoadedProject(initialState.loadingState, false, false); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); +}); + +test('onLoadedProject (LOADING_VM_WITH_ID, false, false), with no project id, shows without id', () => { const initialState = { loadingState: LoadingState.LOADING_VM_WITH_ID, projectId: null @@ -147,9 +132,29 @@ test('onLoadedProject with success false, no project id, shows without id', () = expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); -test('onLoadedProject with success false, valid project id, shows with id', () => { +// onLoadedProject: LOADING_VM_FILE_UPLOAD + +test('onLoadedProject(LOADING_VM_FILE_UPLOAD, true, true) prepares to save', () => { const initialState = { - loadingState: LoadingState.LOADING_VM_WITH_ID, + loadingState: LoadingState.LOADING_VM_FILE_UPLOAD + }; + const action = onLoadedProject(initialState.loadingState, true, true); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.AUTO_UPDATING); +}); + +test('onLoadedProject (LOADING_VM_FILE_UPLOAD, false, true) shows without id', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_FILE_UPLOAD + }; + const action = onLoadedProject(initialState.loadingState, false, true); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); +}); + +test('onLoadedProject (LOADING_VM_FILE_UPLOAD, false, false), with project id, shows with id', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_FILE_UPLOAD, projectId: '12345' }; const action = onLoadedProject(initialState.loadingState, false, false); @@ -157,6 +162,47 @@ test('onLoadedProject with success false, valid project id, shows with id', () = expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); }); +test('onLoadedProject (LOADING_VM_FILE_UPLOAD, false, false), with no project id, shows without id', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_FILE_UPLOAD, + projectId: null + }; + const action = onLoadedProject(initialState.loadingState, false, false); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); +}); + +// onLoadedProject: LOADING_VM_NEW_DEFAULT + +test('onLoadedProject (LOADING_VM_NEW_DEFAULT, true, true) shows without id', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_NEW_DEFAULT + }; + const action = onLoadedProject(initialState.loadingState, true, true); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); +}); + +test('onLoadedProject (LOADING_VM_NEW_DEFAULT, false, true) shows without id', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_NEW_DEFAULT + }; + const action = onLoadedProject(initialState.loadingState, false, true); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); +}); + +test('onLoadedProject (LOADING_VM_NEW_DEFAULT, false, false) shows error', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_NEW_DEFAULT + }; + const action = onLoadedProject(initialState.loadingState, false, false); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.ERROR); +}); + +// doneUpdatingProject + test('doneUpdatingProject with id shows with id', () => { const initialState = { loadingState: LoadingState.MANUAL_UPDATING diff --git a/test/unit/util/project-saver-hoc.test.jsx b/test/unit/util/project-saver-hoc.test.jsx index 0ec3190bf..f5ac7b680 100644 --- a/test/unit/util/project-saver-hoc.test.jsx +++ b/test/unit/util/project-saver-hoc.test.jsx @@ -56,7 +56,7 @@ describe('projectSaverHOC', () => { expect(mockedUpdateProject).toHaveBeenCalled(); }); - test('if canSave is alreatdy true and we show a project with an id, project will NOT be saved', () => { + test('if canSave is already true and we show a project with an id, project will NOT be saved', () => { const mockedSaveProject = jest.fn(); const Component = () =>
; const WrappedComponent = projectSaverHOC(Component); diff --git a/test/unit/util/sb-file-uploader-hoc.test.jsx b/test/unit/util/sb-file-uploader-hoc.test.jsx new file mode 100644 index 000000000..c200157c9 --- /dev/null +++ b/test/unit/util/sb-file-uploader-hoc.test.jsx @@ -0,0 +1,108 @@ +import 'web-audio-test-api'; + +import React from 'react'; +import configureStore from 'redux-mock-store'; +import {mountWithIntl, shallowWithIntl} from '../../helpers/intl-helpers.jsx'; +import {LoadingState} from '../../../src/reducers/project-state'; +import VM from 'scratch-vm'; + +import SBFileUploaderHOC from '../../../src/lib/sb-file-uploader-hoc.jsx'; + +describe('SBFileUploaderHOC', () => { + const mockStore = configureStore(); + let store; + let vm; + + // Wrap this in a function so it gets test specific states and can be reused. + const getContainer = function () { + const Component = () =>
; + return SBFileUploaderHOC(Component); + }; + + const shallowMountWithContext = component => ( + shallowWithIntl(component, {context: {store}}) + ); + + const unwrappedInstance = () => { + const WrappedComponent = getContainer(); + // default starting state: looking at a project you created, not logged in + const wrapper = shallowMountWithContext( + + ); + return wrapper + .dive() // unwrap intl + .dive() // unwrap redux Connect(SBFileUploaderComponent) + .instance(); // SBFileUploaderComponent + }; + + beforeEach(() => { + vm = new VM(); + store = mockStore({ + scratchGui: { + projectState: { + loadingState: LoadingState.SHOWING_WITHOUT_ID + }, + vm: {} + }, + locales: { + locale: 'en' + } + }); + }); + + test('correctly sets title with .sb3 filename', () => { + const projectName = unwrappedInstance().getProjectTitleFromFilename('my project is great.sb3'); + expect(projectName).toBe('my project is great'); + }); + + test('correctly sets title with .sb2 filename', () => { + const projectName = unwrappedInstance().getProjectTitleFromFilename('my project is great.sb2'); + expect(projectName).toBe('my project is great'); + }); + + test('correctly sets title with .sb filename', () => { + const projectName = unwrappedInstance().getProjectTitleFromFilename('my project is great.sb'); + expect(projectName).toBe('my project is great'); + }); + + test('sets blank title with filename with no extension', () => { + const projectName = unwrappedInstance().getProjectTitleFromFilename('my project is great'); + expect(projectName).toBe(''); + }); + + test('if isLoadingUpload becomes true, without fileToUpload set, will call cancelFileUpload', () => { + const mockedCancelFileUpload = jest.fn(); + const WrappedComponent = getContainer(); + const mounted = mountWithIntl( + + ); + mounted.setProps({ + isLoadingUpload: true + }); + expect(mockedCancelFileUpload).toHaveBeenCalled(); + }); +}); From efed4d3721674d2a2361be1fadfbdd5a54c014d1 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 28 Jan 2020 14:38:20 -0500 Subject: [PATCH 2/5] file input is dynamically added to DOM, not rendered --- src/lib/sb-file-uploader-hoc.jsx | 144 ++++++++++++++++++------------- 1 file changed, 86 insertions(+), 58 deletions(-) diff --git a/src/lib/sb-file-uploader-hoc.jsx b/src/lib/sb-file-uploader-hoc.jsx index 90dd1ff53..78f68646c 100644 --- a/src/lib/sb-file-uploader-hoc.jsx +++ b/src/lib/sb-file-uploader-hoc.jsx @@ -43,47 +43,48 @@ const SBFileUploaderHOC = function (WrappedComponent) { constructor (props) { super(props); bindAll(this, [ + 'createFileObjects', 'getProjectTitleFromFilename', + 'handleFinishedLoadingUpload', 'handleStartSelectingFileUpload', - 'setFileInput', 'handleChange', 'onload', - 'resetFileInput' + 'removeFileObjects' ]); } - componentWillMount () { - this.reader = new FileReader(); - this.reader.onload = this.onload; - this.resetFileInput(); - } componentDidUpdate (prevProps) { if (this.props.isLoadingUpload && !prevProps.isLoadingUpload) { - if (this.fileToUpload && this.reader) { - this.reader.readAsArrayBuffer(this.fileToUpload); - } else { - this.props.cancelFileUpload(this.props.loadingState); - } + this.handleFinishedLoadingUpload(); // cue step 5 below } } componentWillUnmount () { - this.reader = null; - this.resetFileInput(); + this.removeFileObjects(); } - resetFileInput () { - this.fileToUpload = null; - if (this.fileInput) { - this.fileInput.value = null; - } + // step 1: this is where the upload process begins + handleStartSelectingFileUpload () { + this.createFileObjects(); // go to step 2 } - getProjectTitleFromFilename (fileInputFilename) { - if (!fileInputFilename) return ''; - // only parse title with valid scratch project extensions - // (.sb, .sb2, and .sb3) - const matches = fileInputFilename.match(/^(.*)\.sb[23]?$/); - if (!matches) return ''; - return matches[1].substring(0, 100); // truncate project title to max 100 chars + // step 2: create a FileReader and an element, and issue a + // pseudo-click to it. That will open the file chooser dialog. + createFileObjects () { + // redo step 7, in case it got skipped last time and its objects are + // still in memory + this.removeFileObjects(); + // create fileReader + this.fileReader = new FileReader(); + this.fileReader.onload = this.onload; + // create element and add it to DOM + this.inputElement = document.createElement('input'); + this.inputElement.accept = '.sb,.sb2,.sb3'; + this.inputElement.style = 'display: none;'; + this.inputElement.type = 'file'; + this.inputElement.onchange = this.handleChange; // connects to step 3 + document.body.appendChild(this.inputElement); + // simulate a click to open file chooser dialog + this.inputElement.click(); } - // called when user has finished selecting a file to upload + // step 3: user has picked a file using the file chooser dialog. + // We don't actually load the file here, we only decide whether to do so. handleChange (e) { const { intl, @@ -92,14 +93,14 @@ const SBFileUploaderHOC = function (WrappedComponent) { projectChanged, userOwnsProject } = this.props; - const thisFileInput = e.target; if (thisFileInput.files) { // Don't attempt to load if no file was selected this.fileToUpload = thisFileInput.files[0]; // If user owns the project, or user has changed the project, - // we must confirm with the user that they really intend to replace it. - // (If they don't own the project and haven't changed it, no need to confirm.) + // we must confirm with the user that they really intend to + // replace it. (If they don't own the project and haven't + // changed it, no need to confirm.) let uploadAllowed = true; if (userOwnsProject || (projectChanged && isShowingWithoutId)) { uploadAllowed = confirm( // eslint-disable-line no-alert @@ -107,58 +108,80 @@ const SBFileUploaderHOC = function (WrappedComponent) { ); } if (uploadAllowed) { + // cues step 4 this.props.requestProjectUpload(loadingState); } else { - this.resetFileInput(); + // skips ahead to step 7 + this.removeFileObjects(); } this.props.closeFileMenu(); } } - // called when file upload raw data is available in the reader + // step 4 is below, in mapDispatchToProps + + // step 5: called from componentDidUpdate when project state shows + // that project data has finished "uploading" into the browser + handleFinishedLoadingUpload () { + if (this.fileToUpload && this.fileReader) { + // begin to read data from the file. When finished, + // cues step 6 using the reader's onload callback + this.fileReader.readAsArrayBuffer(this.fileToUpload); + } else { + this.props.cancelFileUpload(this.props.loadingState); + // skip ahead to step 7 + this.removeFileObjects(); + } + } + // used in step 6 below + getProjectTitleFromFilename (fileInputFilename) { + if (!fileInputFilename) return ''; + // only parse title with valid scratch project extensions + // (.sb, .sb2, and .sb3) + const matches = fileInputFilename.match(/^(.*)\.sb[23]?$/); + if (!matches) return ''; + return matches[1].substring(0, 100); // truncate project title to max 100 chars + } + // step 6: attached as a handler on our FileReader object; called when + // file upload raw data is available in the reader onload () { - if (this.reader) { + if (this.fileReader) { this.props.onLoadingStarted(); const filename = this.fileToUpload && this.fileToUpload.name; - this.props.vm.loadProject(this.reader.result) + this.props.vm.loadProject(this.fileReader.result) .then(() => { this.props.onLoadingFinished(this.props.loadingState, true); - // Reset the file input after project is loaded - // This is necessary in case the user wants to reload a project if (filename) { const uploadedProjectTitle = this.getProjectTitleFromFilename(filename); this.props.onUpdateProjectTitle(uploadedProjectTitle); } - this.resetFileInput(); }) .catch(error => { log.warn(error); this.props.intl.formatMessage(messages.loadError); // eslint-disable-line no-alert this.props.onLoadingFinished(this.props.loadingState, false); - // Reset the file input after project is loaded - // This is necessary in case the user wants to reload a project - this.resetFileInput(); + }) + .then(() => { + // go back to step 7: whether project loading succeeded + // or failed, reset file objects + this.removeFileObjects(); }); } } - handleStartSelectingFileUpload () { - // open filesystem browsing window - this.fileInput.click(); - } - setFileInput (input) { - this.fileInput = input; + // step 7: remove the element from the DOM and clear reader and + // fileToUpload reference, so those objects can be garbage collected + removeFileObjects () { + if (this.inputElement) { + this.inputElement.value = null; + document.body.removeChild(this.inputElement); + } + this.inputElement = null; + this.fileReader = null; + this.fileToUpload = null; } render () { - const fileInput = ( - - ); const { /* eslint-disable no-unused-vars */ + cancelFileUpload, closeFileMenu: closeFileMenuProp, isLoadingUpload, isShowingWithoutId, @@ -177,7 +200,6 @@ const SBFileUploaderHOC = function (WrappedComponent) { onStartSelectingFileUpload={this.handleStartSelectingFileUpload} {...componentProps} /> - {fileInput} ); } @@ -210,19 +232,25 @@ const SBFileUploaderHOC = function (WrappedComponent) { loadingState: loadingState, projectChanged: state.scratchGui.projectChanged, userOwnsProject: ownProps.authorUsername && user && - (ownProps.authorUsername === user.username) - // vm: state.scratchGui.vm + (ownProps.authorUsername === user.username), + vm: state.scratchGui.vm // NOTE: double check this belongs here }; }; const mapDispatchToProps = (dispatch, ownProps) => ({ cancelFileUpload: loadingState => dispatch(onLoadedProject(loadingState, false, false)), closeFileMenu: () => dispatch(closeFileMenu()), + // transition project state from loading to regular, and close + // loading screen and file menu onLoadingFinished: (loadingState, success) => { dispatch(onLoadedProject(loadingState, ownProps.canSave, success)); dispatch(closeLoadingProject()); dispatch(closeFileMenu()); }, + // show project loading screen onLoadingStarted: () => dispatch(openLoadingProject()), + // step 4: transition the project state so we're ready to handle the new + // project data. When this is done, the project state transition will be + // noticed by componentDidUpdate() requestProjectUpload: loadingState => dispatch(requestProjectUpload(loadingState)) }); // Allow incoming props to override redux-provided props. Used to mock in tests. From 48245be3951a8ba4a32d95b676cffcc0fcea6ae3 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Thu, 30 Jan 2020 14:17:33 -0500 Subject: [PATCH 3/5] reverted project state changes in f2291d96 --- src/reducers/project-state.js | 19 +--- .../reducers/project-state-reducer.test.js | 94 +++++-------------- test/unit/util/project-saver-hoc.test.jsx | 2 +- 3 files changed, 29 insertions(+), 86 deletions(-) diff --git a/src/reducers/project-state.js b/src/reducers/project-state.js index a26d7c971..91e51bbe4 100644 --- a/src/reducers/project-state.js +++ b/src/reducers/project-state.js @@ -17,7 +17,7 @@ const START_AUTO_UPDATING = 'scratch-gui/project-state/START_AUTO_UPDATING'; const START_CREATING_NEW = 'scratch-gui/project-state/START_CREATING_NEW'; const START_ERROR = 'scratch-gui/project-state/START_ERROR'; const START_FETCHING_NEW = 'scratch-gui/project-state/START_FETCHING_NEW'; -const START_LOADING_VM_FILE_UPLOAD = 'scratch-gui/project-state/START_LOADING_VM_FILE_UPLOAD'; +const START_LOADING_VM_FILE_UPLOAD = 'scratch-gui/project-state/START_LOADING_FILE_UPLOAD'; const START_MANUAL_UPDATING = 'scratch-gui/project-state/START_MANUAL_UPDATING'; const START_REMIXING = 'scratch-gui/project-state/START_REMIXING'; const START_UPDATING_BEFORE_CREATING_COPY = 'scratch-gui/project-state/START_UPDATING_BEFORE_CREATING_COPY'; @@ -435,21 +435,10 @@ const onLoadedProject = (loadingState, canSave, success) => { default: return; } - } else { - switch (loadingState) { - case LoadingState.LOADING_VM_WITH_ID: - case LoadingState.LOADING_VM_FILE_UPLOAD: - return { - type: RETURN_TO_SHOWING - }; - case LoadingState.LOADING_VM_NEW_DEFAULT: - return { - type: START_ERROR - }; - default: - return; - } } + return { + type: RETURN_TO_SHOWING + }; }; const doneUpdatingProject = loadingState => { diff --git a/test/unit/reducers/project-state-reducer.test.js b/test/unit/reducers/project-state-reducer.test.js index 6938085a3..2683c5bee 100644 --- a/test/unit/reducers/project-state-reducer.test.js +++ b/test/unit/reducers/project-state-reducer.test.js @@ -92,49 +92,16 @@ test('onFetchedProjectData new loads project data into vm', () => { expect(resultState.projectData).toBe('1010101'); }); -// onLoadedProject: LOADING_VM_WITH_ID - -test('onLoadedProject (LOADING_VM_WITH_ID, true, true) shows with id', () => { +test('onLoadedProject upload, with canSave false, shows without id', () => { const initialState = { - loadingState: LoadingState.LOADING_VM_WITH_ID + loadingState: LoadingState.LOADING_VM_FILE_UPLOAD }; - const action = onLoadedProject(initialState.loadingState, true, true); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); -}); - -test('onLoadedProject (LOADING_VM_WITH_ID, false, true) shows with id', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_WITH_ID - }; - const action = onLoadedProject(initialState.loadingState, true, true); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); -}); - -test('onLoadedProject (LOADING_VM_WITH_ID, false, false), with project id, shows with id', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_WITH_ID, - projectId: '12345' - }; - const action = onLoadedProject(initialState.loadingState, false, false); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); -}); - -test('onLoadedProject (LOADING_VM_WITH_ID, false, false), with no project id, shows without id', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_WITH_ID, - projectId: null - }; - const action = onLoadedProject(initialState.loadingState, false, false); + const action = onLoadedProject(initialState.loadingState, false, true); const resultState = projectStateReducer(initialState, action); expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); -// onLoadedProject: LOADING_VM_FILE_UPLOAD - -test('onLoadedProject(LOADING_VM_FILE_UPLOAD, true, true) prepares to save', () => { +test('onLoadedProject upload, with canSave true, prepares to save', () => { const initialState = { loadingState: LoadingState.LOADING_VM_FILE_UPLOAD }; @@ -143,38 +110,25 @@ test('onLoadedProject(LOADING_VM_FILE_UPLOAD, true, true) prepares to save', () expect(resultState.loadingState).toBe(LoadingState.AUTO_UPDATING); }); -test('onLoadedProject (LOADING_VM_FILE_UPLOAD, false, true) shows without id', () => { +test('onLoadedProject with id shows with id', () => { const initialState = { - loadingState: LoadingState.LOADING_VM_FILE_UPLOAD + loadingState: LoadingState.LOADING_VM_WITH_ID + }; + const action = onLoadedProject(initialState.loadingState, true, true); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); +}); + +test('onLoadedProject new shows without id', () => { + const initialState = { + loadingState: LoadingState.LOADING_VM_NEW_DEFAULT }; const action = onLoadedProject(initialState.loadingState, false, true); const resultState = projectStateReducer(initialState, action); expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); -test('onLoadedProject (LOADING_VM_FILE_UPLOAD, false, false), with project id, shows with id', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_FILE_UPLOAD, - projectId: '12345' - }; - const action = onLoadedProject(initialState.loadingState, false, false); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); -}); - -test('onLoadedProject (LOADING_VM_FILE_UPLOAD, false, false), with no project id, shows without id', () => { - const initialState = { - loadingState: LoadingState.LOADING_VM_FILE_UPLOAD, - projectId: null - }; - const action = onLoadedProject(initialState.loadingState, false, false); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); -}); - -// onLoadedProject: LOADING_VM_NEW_DEFAULT - -test('onLoadedProject (LOADING_VM_NEW_DEFAULT, true, true) shows without id', () => { +test('onLoadedProject new, to save shows without id', () => { const initialState = { loadingState: LoadingState.LOADING_VM_NEW_DEFAULT }; @@ -183,26 +137,26 @@ test('onLoadedProject (LOADING_VM_NEW_DEFAULT, true, true) shows without id', () expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); -test('onLoadedProject (LOADING_VM_NEW_DEFAULT, false, true) shows without id', () => { +test('onLoadedProject with success false, no project id, shows without id', () => { const initialState = { - loadingState: LoadingState.LOADING_VM_NEW_DEFAULT + loadingState: LoadingState.LOADING_VM_WITH_ID, + projectId: null }; - const action = onLoadedProject(initialState.loadingState, false, true); + const action = onLoadedProject(initialState.loadingState, false, false); const resultState = projectStateReducer(initialState, action); expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); -test('onLoadedProject (LOADING_VM_NEW_DEFAULT, false, false) shows error', () => { +test('onLoadedProject with success false, valid project id, shows with id', () => { const initialState = { - loadingState: LoadingState.LOADING_VM_NEW_DEFAULT + loadingState: LoadingState.LOADING_VM_WITH_ID, + projectId: '12345' }; const action = onLoadedProject(initialState.loadingState, false, false); const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.ERROR); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); }); -// doneUpdatingProject - test('doneUpdatingProject with id shows with id', () => { const initialState = { loadingState: LoadingState.MANUAL_UPDATING diff --git a/test/unit/util/project-saver-hoc.test.jsx b/test/unit/util/project-saver-hoc.test.jsx index f5ac7b680..0ec3190bf 100644 --- a/test/unit/util/project-saver-hoc.test.jsx +++ b/test/unit/util/project-saver-hoc.test.jsx @@ -56,7 +56,7 @@ describe('projectSaverHOC', () => { expect(mockedUpdateProject).toHaveBeenCalled(); }); - test('if canSave is already true and we show a project with an id, project will NOT be saved', () => { + test('if canSave is alreatdy true and we show a project with an id, project will NOT be saved', () => { const mockedSaveProject = jest.fn(); const Component = () =>
; const WrappedComponent = projectSaverHOC(Component); From 59f428900c916a3b7a6690bd0ccd073129673be2 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Mon, 3 Feb 2020 11:16:42 -0500 Subject: [PATCH 4/5] update menu-bar test per refactor --- test/integration/menu-bar.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/menu-bar.test.js b/test/integration/menu-bar.test.js index 3f6457e52..c83f1022d 100644 --- a/test/integration/menu-bar.test.js +++ b/test/integration/menu-bar.test.js @@ -75,6 +75,7 @@ describe('Menu bar settings', () => { test('User is not warned before uploading project file over a fresh project', async () => { await loadUri(uri); await clickText('File'); + await clickText('Load from your computer'); const input = await findByXpath('//input[@accept=".sb,.sb2,.sb3"]'); await input.sendKeys(path.resolve(__dirname, '../fixtures/project1.sb3')); // No replace alert since no changes were made @@ -89,6 +90,7 @@ describe('Menu bar settings', () => { await clickText('delete', scope.spriteTile); await clickText('File'); + await clickText('Load from your computer'); const input = await findByXpath('//input[@accept=".sb,.sb2,.sb3"]'); await input.sendKeys(path.resolve(__dirname, '../fixtures/project1.sb3')); await driver.switchTo().alert() From d19cf91205a77857712eb26b94269af905baea77 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 4 Feb 2020 17:30:33 -0500 Subject: [PATCH 5/5] removed stale comments --- src/lib/sb-file-uploader-hoc.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/sb-file-uploader-hoc.jsx b/src/lib/sb-file-uploader-hoc.jsx index 78f68646c..9e34b5768 100644 --- a/src/lib/sb-file-uploader-hoc.jsx +++ b/src/lib/sb-file-uploader-hoc.jsx @@ -157,7 +157,7 @@ const SBFileUploaderHOC = function (WrappedComponent) { }) .catch(error => { log.warn(error); - this.props.intl.formatMessage(messages.loadError); // eslint-disable-line no-alert + this.props.intl.formatMessage(messages.loadError); this.props.onLoadingFinished(this.props.loadingState, false); }) .then(() => { @@ -233,7 +233,7 @@ const SBFileUploaderHOC = function (WrappedComponent) { projectChanged: state.scratchGui.projectChanged, userOwnsProject: ownProps.authorUsername && user && (ownProps.authorUsername === user.username), - vm: state.scratchGui.vm // NOTE: double check this belongs here + vm: state.scratchGui.vm }; }; const mapDispatchToProps = (dispatch, ownProps) => ({