From 89a2019a9814c6af06a789739d2f11204a537153 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 5 Apr 2022 18:10:06 -0500 Subject: [PATCH] explorer: add action for activating file this simply forwards the action to the editor module --- src/editor/actions.ts | 30 ++++++++ src/explorer/Explorer.test.tsx | 124 ++++++++++++++++++++------------- src/explorer/Explorer.tsx | 6 ++ src/explorer/actions.ts | 31 +++++++++ src/explorer/sagas.test.ts | 41 +++++++++++ src/explorer/sagas.ts | 42 +++++++++++ 6 files changed, 224 insertions(+), 50 deletions(-) diff --git a/src/editor/actions.ts b/src/editor/actions.ts index ff0de423..9a409c5f 100644 --- a/src/editor/actions.ts +++ b/src/editor/actions.ts @@ -27,3 +27,33 @@ export const editorGetValueResponse = createAction((id: number, value: string) = id, value, })); +/** + * Request to activate a file (open or bring to foreground if already open). + * @param fileName The file name. + */ +export const editorActivateFile = createAction((fileName: string) => ({ + type: 'editor.action.activateFile', + fileName, +})); + +/** + * Indicates that {@link editorActivateFile} succeeded. + * @param fileName The file name. + */ +export const editorDidActivateFile = createAction((fileName: string) => ({ + type: 'editor.action.didActivateFile', + fileName, +})); + +/** + * Indicates that {@link editorActivateFile} failed. + * @param fileName The file name. + * @param error The error that was raised. + */ +export const editorDidFailToActivateFile = createAction( + (fileName: string, error: Error) => ({ + type: 'editor.action.didFailToActivateFile', + fileName, + error, + }), +); diff --git a/src/explorer/Explorer.test.tsx b/src/explorer/Explorer.test.tsx index 533f86b1..fdfe0d97 100644 --- a/src/explorer/Explorer.test.tsx +++ b/src/explorer/Explorer.test.tsx @@ -8,6 +8,7 @@ import React from 'react'; import { testRender, uuid } from '../../test'; import Explorer from './Explorer'; import { + explorerActivateFile, explorerArchiveAllFiles, explorerDeleteFile, explorerExportFile, @@ -81,22 +82,16 @@ describe('new file button', () => { }); describe('tree item', () => { - it('should dispatch action when button is clicked', async () => { + it('should dispatch action when clicked', async () => { const [explorer, dispatch] = testRender(, { explorer: { files: [testFile] }, }); - expect( - explorer.queryByRole('dialog', { name: "Rename 'test.file'" }), - ).toBeNull(); + const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); - // NB: this button is intentionally not accessible (by role) since - // there is a keyboard shortcut. - const button = explorer.getByTitle('Rename test.file'); + userEvent.click(treeItem); - userEvent.click(button); - - expect(dispatch).toHaveBeenCalledWith(explorerRenameFile('test.file')); + expect(dispatch).toHaveBeenCalledWith(explorerActivateFile('test.file')); }); it('should dispatch action when key is pressed', async () => { @@ -104,69 +99,98 @@ describe('tree item', () => { explorer: { files: [testFile] }, }); - expect( - explorer.queryByRole('dialog', { name: "Rename 'test.file'" }), - ).toBeNull(); - const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); userEvent.click(treeItem); - userEvent.keyboard('{f2}'); + userEvent.keyboard('{enter}'); - expect(dispatch).toHaveBeenCalledWith(explorerRenameFile('test.file')); + expect(dispatch).toHaveBeenCalledWith(explorerActivateFile('test.file')); }); - it('should dispatch delete action when button is clicked', async () => { - const [explorer, dispatch] = testRender(, { - explorer: { files: [testFile] }, + describe('rename', () => { + it('should dispatch action when button is clicked', async () => { + const [explorer, dispatch] = testRender(, { + explorer: { files: [testFile] }, + }); + + // NB: this button is intentionally not accessible (by role) since + // there is a keyboard shortcut. + const button = explorer.getByTitle('Rename test.file'); + + userEvent.click(button); + + expect(dispatch).toHaveBeenCalledWith(explorerRenameFile('test.file')); }); - // NB: this button is intentionally not accessible (by role) since - // there is a keyboard shortcut. - const button = explorer.getByTitle('Delete test.file'); + it('should dispatch action when key is pressed', async () => { + const [explorer, dispatch] = testRender(, { + explorer: { files: [testFile] }, + }); - userEvent.click(button); + const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); - expect(dispatch).toHaveBeenCalledWith(explorerDeleteFile('test.file')); + userEvent.click(treeItem); + userEvent.keyboard('{f2}'); + + expect(dispatch).toHaveBeenCalledWith(explorerRenameFile('test.file')); + }); }); - it('should dispatch delete action when key is pressed', async () => { - const [explorer, dispatch] = testRender(, { - explorer: { files: [testFile] }, + describe('export', () => { + it('should dispatch export action when button is clicked', async () => { + const [explorer, dispatch] = testRender(, { + explorer: { files: [testFile] }, + }); + + // NB: this button is intentionally not accessible (by role) since + // there is a keyboard shortcut. + const button = explorer.getByTitle('Export test.file'); + + userEvent.click(button); + + expect(dispatch).toHaveBeenCalledWith(explorerExportFile('test.file')); }); - const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); + it('should dispatch export action when key is pressed', async () => { + const [explorer, dispatch] = testRender(, { + explorer: { files: [testFile] }, + }); - userEvent.click(treeItem); - userEvent.keyboard('{del}'); + const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); - expect(dispatch).toHaveBeenCalledWith(explorerDeleteFile('test.file')); + userEvent.click(treeItem); + userEvent.keyboard('{ctrl}e'); + + expect(dispatch).toHaveBeenCalledWith(explorerExportFile('test.file')); + }); }); - it('should dispatch export action when button is clicked', async () => { - const [explorer, dispatch] = testRender(, { - explorer: { files: [testFile] }, + describe('delete', () => { + it('should dispatch delete action when button is clicked', async () => { + const [explorer, dispatch] = testRender(, { + explorer: { files: [testFile] }, + }); + + // NB: this button is intentionally not accessible (by role) since + // there is a keyboard shortcut. + const button = explorer.getByTitle('Delete test.file'); + + userEvent.click(button); + + expect(dispatch).toHaveBeenCalledWith(explorerDeleteFile('test.file')); }); - // NB: this button is intentionally not accessible (by role) since - // there is a keyboard shortcut. - const button = explorer.getByTitle('Export test.file'); + it('should dispatch delete action when key is pressed', async () => { + const [explorer, dispatch] = testRender(, { + explorer: { files: [testFile] }, + }); - userEvent.click(button); + const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); - expect(dispatch).toHaveBeenCalledWith(explorerExportFile('test.file')); - }); + userEvent.click(treeItem); + userEvent.keyboard('{del}'); - it('should dispatch export action when key is pressed', async () => { - const [explorer, dispatch] = testRender(, { - explorer: { files: [testFile] }, + expect(dispatch).toHaveBeenCalledWith(explorerDeleteFile('test.file')); }); - - const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); - - userEvent.click(treeItem); - userEvent.keyboard('{ctrl}e'); - - expect(dispatch).toHaveBeenCalledWith(explorerExportFile('test.file')); }); }); diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index 38cda4d5..45a07689 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -28,6 +28,7 @@ import { isMacOS } from '../utils/os'; import { preventBrowserNativeContextMenu } from '../utils/react'; import { TreeItemContext, TreeItemData, renderers } from '../utils/tree-renderer'; import { + explorerActivateFile, explorerArchiveAllFiles, explorerDeleteFile, explorerExportFile, @@ -336,6 +337,8 @@ const FileTree: React.VoidFunctionComponent = ({ i18n }) => { [treeId, focusedItem], ); + const dispatch = useDispatch(); + return ( {...renderers} @@ -346,6 +349,9 @@ const FileTree: React.VoidFunctionComponent = ({ i18n }) => { liveDescriptors={liveDescriptors} canRename={false} // we implement our own rename handler onFocusItem={(item) => setFocusedItem(item.index)} + onPrimaryAction={(item) => + dispatch(explorerActivateFile(item.data.fileName)) + } >
({ error, })); +/** + * Request to activate a file (open or bring to foreground if already open). + * @param fileName The file name. + */ +export const explorerActivateFile = createAction((fileName: string) => ({ + type: 'explorer.action.activateFile', + fileName, +})); + +/** + * Indicates that {@link explorerActivateFile} succeeded. + * @param fileName The file name. + */ +export const explorerDidActivateFile = createAction((fileName: string) => ({ + type: 'explorer.action.didActivateFile', + fileName, +})); + +/** + * Indicates that {@link explorerActivateFile} failed. + * @param fileName The file name. + * @param error The error that was raised. + */ +export const explorerDidFailToActivateFile = createAction( + (fileName: string, error: Error) => ({ + type: 'explorer.action.didFailToActivateFile', + fileName, + error, + }), +); + /** * Action that requests to rename a file. * @param fileName The file name. diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index b5bc2b57..eb9121a3 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -5,6 +5,11 @@ import * as browserFsAccess from 'browser-fs-access'; import { FileWithHandle } from 'browser-fs-access'; import { mock } from 'jest-mock-extended'; import { AsyncSaga } from '../../test'; +import { + editorActivateFile, + editorDidActivateFile, + editorDidFailToActivateFile, +} from '../editor/actions'; import { fileStorageDidDumpAllFiles, fileStorageDidFailToDumpAllFiles, @@ -21,11 +26,14 @@ import { import { pythonFileExtension } from '../pybricksMicropython/lib'; import { Hub, + explorerActivateFile, explorerArchiveAllFiles, explorerCreateNewFile, + explorerDidActivateFile, explorerDidArchiveAllFiles, explorerDidCreateNewFile, explorerDidExportFile, + explorerDidFailToActivateFile, explorerDidFailToArchiveAllFiles, explorerDidFailToExportFile, explorerDidFailToImportFiles, @@ -180,6 +188,39 @@ describe('handleExplorerCreateNewFile', () => { }); }); +describe('handleExplorerActivateFile', () => { + let saga: AsyncSaga; + + beforeEach(async () => { + saga = new AsyncSaga(explorer); + + saga.put(explorerActivateFile('test.file')); + + await expect(saga.take()).resolves.toEqual(editorActivateFile('test.file')); + }); + + it('should propagate error', async () => { + const testError = new Error('test error'); + saga.put(editorDidFailToActivateFile('test.file', testError)); + + await expect(saga.take()).resolves.toEqual( + explorerDidFailToActivateFile('test.file', testError), + ); + }); + + it('should propagate success', async () => { + saga.put(editorDidActivateFile('test.file')); + + await expect(saga.take()).resolves.toEqual( + explorerDidActivateFile('test.file'), + ); + }); + + afterEach(async () => { + await saga.end(); + }); +}); + describe('handleExplorerRenameFile', () => { let saga: AsyncSaga; diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index bff37894..08739299 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -12,6 +12,11 @@ import { takeEvery, takeLatest, } from 'typed-redux-saga/macro'; +import { + editorActivateFile, + editorDidActivateFile, + editorDidFailToActivateFile, +} from '../editor/actions'; import { getPybricksMicroPythonFileTemplate } from '../editor/pybricksMicroPython'; import { fileStorageDidDumpAllFiles, @@ -37,11 +42,14 @@ import { import { RootState } from '../reducers'; import { defined, ensureError, timestamp } from '../utils'; import { + explorerActivateFile, explorerArchiveAllFiles, explorerCreateNewFile, + explorerDidActivateFile, explorerDidArchiveAllFiles, explorerDidCreateNewFile, explorerDidExportFile, + explorerDidFailToActivateFile, explorerDidFailToArchiveAllFiles, explorerDidFailToCreateNewFile, explorerDidFailToExportFile, @@ -192,6 +200,39 @@ function* handleExplorerCreateNewFile( } } +/** + * Connects user triggered action to editor module. + * @param action + */ +function* handleExplorerActivateFile( + action: ReturnType, +): Generator { + yield* put(editorActivateFile(action.fileName)); + + const { didActivate, didFailToActivate } = yield* race({ + didActivate: take( + editorDidActivateFile.when((a) => a.fileName === action.fileName), + ), + didFailToActivate: take( + editorDidFailToActivateFile.when((a) => a.fileName === action.fileName), + ), + }); + + if (didFailToActivate) { + yield* put( + explorerDidFailToActivateFile( + didFailToActivate.fileName, + didFailToActivate.error, + ), + ); + return; + } + + defined(didActivate); + + yield* put(explorerDidActivateFile(didActivate.fileName)); +} + /** Connects user initiate rename file actions to the rename file dialog. */ function* handleExplorerRenameFile( action: ReturnType, @@ -273,6 +314,7 @@ export default function* (): Generator { yield* takeEvery(explorerArchiveAllFiles, handleExplorerArchiveAllFiles); yield* takeEvery(explorerImportFiles, handleExplorerImportFiles); yield* takeEvery(explorerCreateNewFile, handleExplorerCreateNewFile); + yield* takeEvery(explorerActivateFile, handleExplorerActivateFile); // takeLatest should ensure that if we trigger a new rename before the // previous one is finished, the old one will be canceled. We don't expect // this to happen in practice though.