From fad7b0114af35ec1a111c05ecb935f8f86cdb0ee Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 8 Apr 2022 10:52:54 -0500 Subject: [PATCH] explorer/newFileWizard: refactor This refactors the new file wizard to be more independent. Most of the control is now done through the saga instead of splitting it between react and the sagas. Also fix a few issues while we are touching this: - pressing enter now accepts the dialog - newly created file is now activated in the editor --- src/explorer/Explorer.test.tsx | 13 +- src/explorer/Explorer.tsx | 11 +- src/explorer/actions.ts | 35 +----- .../newFileWizard/NewFileWizard.test.tsx | 62 +++++++++- src/explorer/newFileWizard/NewFileWizard.tsx | 117 +++++++++--------- src/explorer/newFileWizard/actions.ts | 42 +++++++ src/explorer/newFileWizard/reducers.ts | 27 ++++ src/explorer/reducers.ts | 3 +- src/explorer/sagas.test.ts | 38 +++++- src/explorer/sagas.ts | 28 ++++- src/notifications/sagas.test.ts | 1 + src/notifications/sagas.ts | 5 + 12 files changed, 261 insertions(+), 121 deletions(-) create mode 100644 src/explorer/newFileWizard/actions.ts create mode 100644 src/explorer/newFileWizard/reducers.ts diff --git a/src/explorer/Explorer.test.tsx b/src/explorer/Explorer.test.tsx index fdfe0d97..23964453 100644 --- a/src/explorer/Explorer.test.tsx +++ b/src/explorer/Explorer.test.tsx @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors -import { getByLabelText, waitFor } from '@testing-library/dom'; import { cleanup } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; @@ -10,6 +9,7 @@ import Explorer from './Explorer'; import { explorerActivateFile, explorerArchiveAllFiles, + explorerCreateNewFile, explorerDeleteFile, explorerExportFile, explorerImportFiles, @@ -66,18 +66,13 @@ describe('import file button', () => { }); describe('new file button', () => { - it('should show new file wizard', async () => { - const [explorer] = testRender(); + it('should dispatch action when clicked', async () => { + const [explorer, dispatch] = testRender(); const button = explorer.getByTitle('Create a new file'); userEvent.click(button); - - const dialog = explorer.getByRole('dialog', { name: 'Create a new file' }); - expect(dialog).toBeVisible(); - - userEvent.click(getByLabelText(dialog, 'Close')); - await waitFor(() => expect(dialog).not.toBeVisible()); + expect(dispatch).toHaveBeenCalledWith(explorerCreateNewFile()); }); }); diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index 45a07689..f72f860e 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -3,6 +3,7 @@ // A file explorer control. +import './explorer.scss'; import { Button, ButtonGroup, @@ -30,6 +31,7 @@ import { TreeItemContext, TreeItemData, renderers } from '../utils/tree-renderer import { explorerActivateFile, explorerArchiveAllFiles, + explorerCreateNewFile, explorerDeleteFile, explorerExportFile, explorerImportFiles, @@ -38,7 +40,6 @@ import { import { I18nId } from './i18n'; import NewFileWizard from './newFileWizard/NewFileWizard'; import RenameFileDialog from './renameFileDialog/RenameFileDialog'; -import './explorer.scss'; type ActionButtonProps = { /** The icon to use for the button. */ @@ -131,7 +132,6 @@ type HeaderProps = { }; const Header: React.VoidFunctionComponent = ({ i18n }) => { - const [isNewFileWizardOpen, setIsNewFileWizardOpen] = useState(false); const dispatch = useDispatch(); const files = useSelector((s) => s.explorer.files); @@ -155,11 +155,7 @@ const Header: React.VoidFunctionComponent = ({ i18n }) => { setIsNewFileWizardOpen(true)} - /> - setIsNewFileWizardOpen(false)} + onClick={() => dispatch(explorerCreateNewFile())} /> @@ -373,6 +369,7 @@ const Explorer: React.VFC = () => {
+ ); diff --git a/src/explorer/actions.ts b/src/explorer/actions.ts index c252387f..52833867 100644 --- a/src/explorer/actions.ts +++ b/src/explorer/actions.ts @@ -2,27 +2,6 @@ // Copyright (c) 2022 The Pybricks Authors import { createAction } from '../actions'; -import { pythonFileExtension } from '../pybricksMicropython/lib'; - -/** Supported file extensions. */ -type SupportedFileExtension = typeof pythonFileExtension; - -/** Supported hub types. */ -export enum Hub { - /** BOOST Move hub */ - Move = 'movehub', - /** City hub */ - City = 'cityhub', - /** Technic hub */ - Technic = 'technichub', - /** MINDSTORMS Robot Inventor hub */ - Inventor = 'inventorhub', - /** SPIKE Prime hub */ - Prime = 'primehub', - /** SPIKE Essential hub */ - Essential = 'essentialhub', -} - /** * Request to archive (download) all files in the store. */ @@ -71,18 +50,10 @@ export const explorerDidFailToImportFiles = createAction((error: Error) => ({ /** * Action that requests to create a new file. - * @param fileName The requested new file name (without file extension). - * @param fileExtension The file extension (including leading "."). - * @param hub The type of hub this file is for. */ -export const explorerCreateNewFile = createAction( - (fileName: string, fileExtension: SupportedFileExtension, hub: Hub) => ({ - type: 'explorer.action.createNewFile', - fileName, - fileExtension, - hub, - }), -); +export const explorerCreateNewFile = createAction(() => ({ + type: 'explorer.action.createNewFile', +})); /** * Action that indicates that {@link explorerCreateNewFile} succeeded. diff --git a/src/explorer/newFileWizard/NewFileWizard.test.tsx b/src/explorer/newFileWizard/NewFileWizard.test.tsx index f3688d06..7ed89399 100644 --- a/src/explorer/newFileWizard/NewFileWizard.test.tsx +++ b/src/explorer/newFileWizard/NewFileWizard.test.tsx @@ -2,23 +2,73 @@ // Copyright (c) 2022 The Pybricks Authors import { waitFor } from '@testing-library/dom'; +import { cleanup } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { testRender } from '../../../test'; import NewFileWizard from './NewFileWizard'; +import { Hub, newFileWizardDidAccept, newFileWizardDidCancel } from './actions'; -describe('create button', () => { - it('should close the dialog', async () => { - const onClose = jest.fn(); - const [dialog] = testRender(); +afterEach(() => { + cleanup(); +}); + +describe('accept', () => { + it('should dispatch accept action when button is clicked', async () => { + const [dialog, dispatch] = testRender(, { + explorer: { newFileWizard: { isOpen: true } }, + }); const button = dialog.getByLabelText('Create'); // have to type a file name before Create button is enabled - userEvent.type(dialog.getByLabelText('File name'), 'test'); + userEvent.type(dialog.getByRole('textbox', { name: 'File name' }), 'test'); await waitFor(() => expect(button).not.toBeDisabled()); userEvent.click(button); - expect(onClose).toHaveBeenCalled(); + expect(dispatch).toHaveBeenCalledWith( + newFileWizardDidAccept('test', '.py', Hub.Technic), + ); + }); + + it('should dispatch accept action when enter is pressed ', async () => { + const [dialog, dispatch] = testRender(, { + explorer: { newFileWizard: { isOpen: true } }, + }); + + userEvent.type( + dialog.getByRole('textbox', { name: 'File name' }), + 'test{enter}', + ); + + expect(dispatch).toHaveBeenCalledWith( + newFileWizardDidAccept('test', '.py', Hub.Technic), + ); + }); +}); + +describe('cancel', () => { + it('should dispatch cancel when close button is clicked', () => { + const [dialog, dispatch] = testRender(, { + explorer: { newFileWizard: { isOpen: true } }, + }); + + userEvent.click(dialog.getByRole('button', { name: 'Close' })); + + expect(dispatch).toHaveBeenCalledWith(newFileWizardDidCancel()); + }); + + it('should dispatch cancel when escape button is pressed', async () => { + const [dialog, dispatch] = testRender(, { + explorer: { newFileWizard: { isOpen: true } }, + }); + + await waitFor(() => + expect(dialog.getByRole('textbox', { name: 'File name' })).toHaveFocus(), + ); + + userEvent.keyboard('{esc}'); + + expect(dispatch).toHaveBeenCalledWith(newFileWizardDidCancel()); }); }); diff --git a/src/explorer/newFileWizard/NewFileWizard.tsx b/src/explorer/newFileWizard/NewFileWizard.tsx index 3b47487e..52d4b3f3 100644 --- a/src/explorer/newFileWizard/NewFileWizard.tsx +++ b/src/explorer/newFileWizard/NewFileWizard.tsx @@ -10,7 +10,7 @@ import { RadioGroup, } from '@blueprintjs/core'; import { useI18n } from '@shopify/react-i18n'; -import React, { useRef, useState } from 'react'; +import React, { useCallback, useRef, useState } from 'react'; import { useDispatch } from 'react-redux'; import { FileNameValidationResult, @@ -18,28 +18,20 @@ import { validateFileName, } from '../../pybricksMicropython/lib'; import { useSelector } from '../../reducers'; +import { useUniqueId } from '../../utils/react'; import FileNameFormGroup from '../fileNameFormGroup/FileNameFormGroup'; -import { Hub, explorerCreateNewFile } from './../actions'; +import { Hub, newFileWizardDidAccept, newFileWizardDidCancel } from './actions'; import { I18nId } from './i18n'; // This should be set to the most commonly used hub. const defaultHub = Hub.Technic; -type NewFileWizardProps = { - /** Controls if the dialog is open. */ - readonly isOpen: boolean; - /** Called when the dialog is closed. */ - readonly onClose: () => void; -}; - -const NewFileWizard: React.VoidFunctionComponent = ({ - isOpen, - onClose, -}) => { +const NewFileWizard: React.VoidFunctionComponent = () => { // istanbul ignore next: babel-loader rewrites this line const [i18n] = useI18n(); const dispatch = useDispatch(); + const isOpen = useSelector((s) => s.explorer.newFileWizard.isOpen); const [fileName, setFileName] = useState(''); const files = useSelector((s) => s.explorer.files); const fileNameValidation = validateFileName( @@ -51,6 +43,20 @@ const NewFileWizard: React.VoidFunctionComponent = ({ const fileNameInputRef = useRef(null); + const handleSubmit = useCallback( + (e) => { + e.preventDefault(); + dispatch(newFileWizardDidAccept(fileName, pythonFileExtension, hubType)); + }, + [dispatch, fileName, pythonFileExtension, hubType], + ); + + const handleClose = useCallback(() => { + dispatch(newFileWizardDidCancel()); + }, [dispatch]); + + const acceptButtonLabelId = useUniqueId('pybricks-explorer'); + return ( = ({ isOpen={isOpen} onOpening={() => setFileName('')} onOpened={() => fileNameInputRef.current?.focus()} - onClose={onClose} + onClose={handleClose} > -
- - - setHubType(e.currentTarget.value as Hub)} - > - BOOST Move Hub - City Hub - Technic Hub - SPIKE Prime - SPIKE Essential - MINDSTORMS Robot Inventor - - -
-
-
- +
+
+ + + setHubType(e.currentTarget.value as Hub)} + > + BOOST Move Hub + City Hub + Technic Hub + SPIKE Prime + SPIKE Essential + + MINDSTORMS Robot Inventor + + +
-
+
+
+ +
+
+
); }; diff --git a/src/explorer/newFileWizard/actions.ts b/src/explorer/newFileWizard/actions.ts new file mode 100644 index 00000000..7837137d --- /dev/null +++ b/src/explorer/newFileWizard/actions.ts @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { createAction } from '../../actions'; + +import { pythonFileExtension } from '../../pybricksMicropython/lib'; + +/** Supported file extensions. */ +type SupportedFileExtension = typeof pythonFileExtension; + +/** Supported hub types. */ +export enum Hub { + /** BOOST Move hub */ + Move = 'movehub', + /** City hub */ + City = 'cityhub', + /** Technic hub */ + Technic = 'technichub', + /** MINDSTORMS Robot Inventor hub */ + Inventor = 'inventorhub', + /** SPIKE Prime hub */ + Prime = 'primehub', + /** SPIKE Essential hub */ + Essential = 'essentialhub', +} + +export const newFileWizardShow = createAction(() => ({ + type: 'explorer.newFileWizard.action.show', +})); + +export const newFileWizardDidAccept = createAction( + (fileName: string, fileExtension: SupportedFileExtension, hubType: Hub) => ({ + type: 'explorer.newFileWizard.action.didAccept', + fileName, + fileExtension, + hubType, + }), +); + +export const newFileWizardDidCancel = createAction(() => ({ + type: 'explorer.newFileWizard.action.didCancel', +})); diff --git a/src/explorer/newFileWizard/reducers.ts b/src/explorer/newFileWizard/reducers.ts new file mode 100644 index 00000000..d2be76e9 --- /dev/null +++ b/src/explorer/newFileWizard/reducers.ts @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { Reducer, combineReducers } from 'redux'; +import { + newFileWizardDidAccept, + newFileWizardDidCancel, + newFileWizardShow, +} from './actions'; + +/** Controls the new file wizard dialog isOpen state. */ +const isOpen: Reducer = (state = false, action) => { + if (newFileWizardShow.matches(action)) { + return true; + } + + if ( + newFileWizardDidAccept.matches(action) || + newFileWizardDidCancel.matches(action) + ) { + return false; + } + + return state; +}; + +export default combineReducers({ isOpen }); diff --git a/src/explorer/reducers.ts b/src/explorer/reducers.ts index cda4c2e7..89c18690 100644 --- a/src/explorer/reducers.ts +++ b/src/explorer/reducers.ts @@ -10,6 +10,7 @@ import { fileStorageDidRemoveItem, } from '../fileStorage/actions'; +import newFileWizard from './newFileWizard/reducers'; import renameFileDialog from './renameFileDialog/reducers'; export type ExplorerFileInfo = Readonly<{ @@ -51,4 +52,4 @@ const files: Reducer = (state = [], action) => { return state; }; -export default combineReducers({ files, renameFileDialog }); +export default combineReducers({ files, newFileWizard, renameFileDialog }); diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index eb9121a3..3eb3727e 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -25,7 +25,6 @@ import { } from '../fileStorage/actions'; import { pythonFileExtension } from '../pybricksMicropython/lib'; import { - Hub, explorerActivateFile, explorerArchiveAllFiles, explorerCreateNewFile, @@ -35,6 +34,7 @@ import { explorerDidExportFile, explorerDidFailToActivateFile, explorerDidFailToArchiveAllFiles, + explorerDidFailToCreateNewFile, explorerDidFailToExportFile, explorerDidFailToImportFiles, explorerDidFailToRenameFile, @@ -44,6 +44,12 @@ import { explorerImportFiles, explorerRenameFile, } from './actions'; +import { + Hub, + newFileWizardDidAccept, + newFileWizardDidCancel, + newFileWizardShow, +} from './newFileWizard/actions'; import { renameFileDialogDidAccept, renameFileDialogDidCancel, @@ -159,10 +165,28 @@ describe('handleExplorerImportFiles', () => { }); describe('handleExplorerCreateNewFile', () => { - it('should dispatch fileStorage action', async () => { - const saga = new AsyncSaga(explorer); + let saga: AsyncSaga; - saga.put(explorerCreateNewFile('test', pythonFileExtension, Hub.Technic)); + beforeEach(async () => { + saga = new AsyncSaga(explorer); + + saga.put(explorerCreateNewFile()); + + await expect(saga.take()).resolves.toEqual(newFileWizardShow()); + }); + + it('should dispatch error when canceled', async () => { + saga.put(newFileWizardDidCancel()); + + await expect(saga.take()).resolves.toEqual( + explorerDidFailToCreateNewFile( + new DOMException('user canceled', 'AbortError'), + ), + ); + }); + + it('should dispatch fileStorage action', async () => { + saga.put(newFileWizardDidAccept('test', pythonFileExtension, Hub.Technic)); await expect(saga.take()).resolves.toMatchInlineSnapshot(` Object { @@ -182,8 +206,12 @@ describe('handleExplorerCreateNewFile', () => { saga.put(fileStorageDidWriteFile('test.py')); - await expect(saga.take()).resolves.toEqual(explorerDidCreateNewFile()); + await expect(saga.take()).resolves.toEqual(editorActivateFile('test.py')); + await expect(saga.take()).resolves.toEqual(explorerDidCreateNewFile()); + }); + + afterEach(async () => { await saga.end(); }); }); diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index 08739299..6f0e3170 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -61,6 +61,11 @@ import { explorerImportFiles, explorerRenameFile, } from './actions'; +import { + newFileWizardDidAccept, + newFileWizardDidCancel, + newFileWizardShow, +} from './newFileWizard/actions'; import { renameFileDialogDidAccept, renameFileDialogDidCancel, @@ -170,16 +175,27 @@ function* handleExplorerImportFiles(): Generator { } } -function* handleExplorerCreateNewFile( - action: ReturnType, -): Generator { +function* handleExplorerCreateNewFile(): Generator { try { - const fileName = `${action.fileName}${action.fileExtension}`; + yield* put(newFileWizardShow()); + + const { didAccept, didCancel } = yield* race({ + didAccept: take(newFileWizardDidAccept), + didCancel: take(newFileWizardDidCancel), + }); + + if (didCancel) { + throw new DOMException('user canceled', 'AbortError'); + } + + defined(didAccept); + + const fileName = `${didAccept.fileName}${didAccept.fileExtension}`; yield* put( fileStorageWriteFile( fileName, - getPybricksMicroPythonFileTemplate(action.hub) || '', + getPybricksMicroPythonFileTemplate(didAccept.hubType) || '', ), ); @@ -194,6 +210,8 @@ function* handleExplorerCreateNewFile( throw didFailToWrite.error; } + yield* put(editorActivateFile(fileName)); + yield* put(explorerDidCreateNewFile()); } catch (err) { yield* put(explorerDidFailToCreateNewFile(ensureError(err))); diff --git a/src/notifications/sagas.test.ts b/src/notifications/sagas.test.ts index 523fff64..3f86f825 100644 --- a/src/notifications/sagas.test.ts +++ b/src/notifications/sagas.test.ts @@ -139,6 +139,7 @@ test.each([ bleDIServiceDidReceiveFirmwareRevision(firmwareVersion), explorerDidFailToArchiveAllFiles(new DOMException('test message', 'AbortError')), explorerDidFailToImportFiles(new DOMException('test message', 'AbortError')), + explorerDidFailToCreateNewFile(new DOMException('test message', 'AbortError')), explorerDidFailToExportFile( 'test.file', new DOMException('test message', 'AbortError'), diff --git a/src/notifications/sagas.ts b/src/notifications/sagas.ts index 9f8f93d6..b80d14ae 100644 --- a/src/notifications/sagas.ts +++ b/src/notifications/sagas.ts @@ -448,6 +448,11 @@ function* showExplorerFailToImportFiles( function* showExplorerFailToCreateFile( action: ReturnType, ): Generator { + if (action.error.name === 'AbortError') { + // user clicked cancel button - not an error + return; + } + yield* showUnexpectedError(I18nId.ExplorerFailedToCreate, action.error); }