From 56e1505f14e33642ac1c62e863a836dd7255d081 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Mon, 30 May 2022 11:26:35 -0500 Subject: [PATCH] Revert "explorer: drop rename feature" This reverts commit 30741b13ef97e4175f6a9163d0d5be476df799ef. --- src/explorer/Explorer.test.tsx | 31 ++++++++++ src/explorer/Explorer.tsx | 29 ++++++++++ src/explorer/actions.ts | 23 ++++++++ src/explorer/i18n.ts | 18 +++--- .../renameFileDialog/RenameFileDialog.tsx | 3 +- src/explorer/sagas.test.ts | 57 +++++++++++++++++++ src/explorer/sagas.ts | 49 ++++++++++++++++ src/explorer/translations/en.json | 2 + 8 files changed, 202 insertions(+), 10 deletions(-) diff --git a/src/explorer/Explorer.test.tsx b/src/explorer/Explorer.test.tsx index 1696f508..96290e7e 100644 --- a/src/explorer/Explorer.test.tsx +++ b/src/explorer/Explorer.test.tsx @@ -15,6 +15,7 @@ import { explorerDuplicateFile, explorerExportFile, explorerImportFiles, + explorerRenameFile, explorerUserActivateFile, } from './actions'; @@ -119,6 +120,36 @@ describe('tree item', () => { }); }); + describe('rename', () => { + it('should dispatch action when button is clicked', async () => { + jest.mocked(useFileStorageMetadata).mockReturnValue([testFile]); + const [explorer, dispatch] = testRender(); + + // 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')); + + // should not propagate to treeitem + expect(dispatch).toHaveBeenCalledTimes(1); + }); + + it('should dispatch action when key is pressed', async () => { + jest.mocked(useFileStorageMetadata).mockReturnValue([testFile]); + const [explorer, dispatch] = testRender(); + + const treeItem = explorer.getByRole('treeitem', { name: 'test.file' }); + + userEvent.click(treeItem); + userEvent.keyboard('{f2}'); + + expect(dispatch).toHaveBeenCalledWith(explorerRenameFile('test.file')); + }); + }); + describe('export', () => { it('should dispatch export action when button is clicked', async () => { jest.mocked(useFileStorageMetadata).mockReturnValue([testFile]); diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index 303e5952..3690394d 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -36,12 +36,14 @@ import { explorerDuplicateFile, explorerExportFile, explorerImportFiles, + explorerRenameFile, explorerUserActivateFile, } from './actions'; import DeleteFileAlert from './deleteFileAlert/DeleteFileAlert'; import DuplicateFileDialog from './duplicateFileDialog/DuplicateFileDialog'; import { I18nId, useI18n } from './i18n'; import NewFileWizard from './newFileWizard/NewFileWizard'; +import RenameFileDialog from './renameFileDialog/RenameFileDialog'; type ActionButtonProps = { /** The DOM id for this instance. */ @@ -101,6 +103,7 @@ const FileActionButtonGroup: React.VoidFunctionComponent const fileName = environment.getItemTitle(item); + const renameButtonId = useId(); const duplicateButtonId = useId(); const exportButtonId = useId(); const deleteButtonId = useId(); @@ -112,6 +115,12 @@ const FileActionButtonGroup: React.VoidFunctionComponent minimal={true} > + dispatch(explorerRenameFile(fileName))} + /> +
  • ${i18n.translate( + I18nId.TreeLiveDescriptorIntroKeybindingsRename, + { key: 'f2' }, + )}
  • ${i18n.translate( I18nId.TreeLiveDescriptorIntroKeybindingsDuplicate, { key: `${isMacOS() ? 'cmd' : 'ctrl'}+d` }, @@ -239,6 +252,13 @@ const renderTreeContainer: typeof renderers.renderTreeContainer = (props) => { const hotKeyActive = isActiveTree; /* && !dnd.isProgrammaticallyDragging && !isRenaming */ + const handleRenameKeyDown = useCallback(() => { + if (focusedItem !== undefined) { + const fileName = environment.getItemTitle(environment.items[focusedItem]); + dispatch(explorerRenameFile(fileName)); + } + }, [environment]); + const handleDuplicateKeyDown = useCallback(() => { if (focusedItem !== undefined) { const fileName = environment.getItemTitle(environment.items[focusedItem]); @@ -262,6 +282,14 @@ const renderTreeContainer: typeof renderers.renderTreeContainer = (props) => { const hotkeys = useMemo( () => [ + { + combo: 'f2', + label: 'Rename', + disabled: !hotKeyActive, + preventDefault: true, + stopPropagation: true, + onKeyDown: handleRenameKeyDown, + }, { combo: 'mod+d', label: 'Duplicate', @@ -379,6 +407,7 @@ const Explorer: React.VFC = () => { + diff --git a/src/explorer/actions.ts b/src/explorer/actions.ts index 22ddf7a6..488a9266 100644 --- a/src/explorer/actions.ts +++ b/src/explorer/actions.ts @@ -89,6 +89,29 @@ export const explorerUserDidActivateFile = createAction((fileName: string) => ({ fileName, })); +/** + * Action that requests to rename a file. + * @param fileName The file name. + */ +export const explorerRenameFile = createAction((fileName: string) => ({ + type: 'explorer.action.renameFile', + fileName, +})); + +/** + * Action that indicates that {@link explorerRenameFile} succeeded. + */ +export const explorerDidRenameFile = createAction(() => ({ + type: 'explorer.action.didRenameFile', +})); + +/** + * Action that indicates that {@link explorerRenameFile} failed. + */ +export const explorerDidFailToRenameFile = createAction(() => ({ + type: 'explorer.action.didFailToRenameFile', +})); + /** * Action that requests to duplicate a file. * @param fileName The file name. diff --git a/src/explorer/i18n.ts b/src/explorer/i18n.ts index f95a1548..e4193ba6 100644 --- a/src/explorer/i18n.ts +++ b/src/explorer/i18n.ts @@ -12,19 +12,21 @@ export function useI18n(): I18n { } export enum I18nId { - HeaderToolbarTitle = 'header.toolbar.title', + HeaderToolbarAddNew = 'header.toolbar.addNew', HeaderToolbarExportAll = 'header.toolbar.exportAll', HeaderToolbarImport = 'header.toolbar.import', - HeaderToolbarAddNew = 'header.toolbar.addNew', + HeaderToolbarTitle = 'header.toolbar.title', + TreeItemDeleteTooltip = 'treeItem.deleteTooltip', + TreeItemDuplicateTooltip = 'treeItem.duplicateTooltip', + TreeItemExportTooltip = 'treeItem.exportTooltip', + TreeItemRenameTooltip = 'treeItem.renameTooltip', TreeLabel = 'tree.label', TreeLiveDescriptorIntroAccessibilityGuide = 'tree.liveDescriptor.intro.accessibilityGuide', - TreeLiveDescriptorIntroNavigation = 'tree.liveDescriptor.intro.navigation', - TreeLiveDescriptorIntroKeybindingsPrimaryAction = 'tree.liveDescriptor.intro.keybindings.primaryAction', + TreeLiveDescriptorIntroKeybindingsDelete = 'tree.liveDescriptor.intro.keybindings.delete', TreeLiveDescriptorIntroKeybindingsDuplicate = 'tree.liveDescriptor.intro.keybindings.duplicate', TreeLiveDescriptorIntroKeybindingsExport = 'tree.liveDescriptor.intro.keybindings.export', - TreeLiveDescriptorIntroKeybindingsDelete = 'tree.liveDescriptor.intro.keybindings.delete', + TreeLiveDescriptorIntroKeybindingsPrimaryAction = 'tree.liveDescriptor.intro.keybindings.primaryAction', + TreeLiveDescriptorIntroKeybindingsRename = 'tree.liveDescriptor.intro.keybindings.rename', + TreeLiveDescriptorIntroNavigation = 'tree.liveDescriptor.intro.navigation', TreeLiveDescriptorSearching = 'tree.liveDescriptor.searching', - TreeItemDeleteTooltip = 'treeItem.deleteTooltip', - TreeItemExportTooltip = 'treeItem.exportTooltip', - TreeItemDuplicateTooltip = 'treeItem.duplicateTooltip', } diff --git a/src/explorer/renameFileDialog/RenameFileDialog.tsx b/src/explorer/renameFileDialog/RenameFileDialog.tsx index 89fecddc..2b645976 100644 --- a/src/explorer/renameFileDialog/RenameFileDialog.tsx +++ b/src/explorer/renameFileDialog/RenameFileDialog.tsx @@ -12,8 +12,7 @@ import { import { useSelector } from '../../reducers'; import FileNameFormGroup from '../fileNameFormGroup/FileNameFormGroup'; import { renameFileDialogDidAccept, renameFileDialogDidCancel } from './actions'; -import { useI18n } from './i18n'; -import { I18nId } from './i18n'; +import { I18nId, useI18n } from './i18n'; const RenameFileDialog: React.VFC = () => { const i18n = useI18n(); diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index f51e1224..293ce5cb 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -24,10 +24,13 @@ import { fileStorageDidFailToDeleteFile, fileStorageDidFailToDumpAllFiles, fileStorageDidFailToReadFile, + fileStorageDidFailToRenameFile, fileStorageDidReadFile, + fileStorageDidRenameFile, fileStorageDidWriteFile, fileStorageDumpAllFiles, fileStorageReadFile, + fileStorageRenameFile, fileStorageWriteFile, } from '../fileStorage/actions'; import { pythonFileExtension } from '../pybricksMicropython/lib'; @@ -46,10 +49,13 @@ import { explorerDidFailToDuplicateFile, explorerDidFailToExportFile, explorerDidFailToImportFiles, + explorerDidFailToRenameFile, explorerDidImportFiles, + explorerDidRenameFile, explorerDuplicateFile, explorerExportFile, explorerImportFiles, + explorerRenameFile, explorerUserActivateFile, explorerUserDidActivateFile, } from './actions'; @@ -69,6 +75,11 @@ import { newFileWizardDidCancel, newFileWizardShow, } from './newFileWizard/actions'; +import { + renameFileDialogDidAccept, + renameFileDialogDidCancel, + renameFileDialogShow, +} from './renameFileDialog/actions'; import explorer from './sagas'; jest.mock('browser-fs-access'); @@ -286,6 +297,52 @@ describe('handleExplorerActivateFile', () => { }); }); +describe('handleExplorerRenameFile', () => { + let saga: AsyncSaga; + + beforeEach(async () => { + saga = new AsyncSaga(explorer); + + saga.put(explorerRenameFile('old.file')); + + await expect(saga.take()).resolves.toEqual(renameFileDialogShow('old.file')); + }); + + it('should dispatch action if canceled', async () => { + saga.put(renameFileDialogDidCancel()); + + await expect(saga.take()).resolves.toEqual(explorerDidFailToRenameFile()); + }); + + describe('should dispatch fileStorageOpenFile action if accepted', () => { + beforeEach(async () => { + saga.put(renameFileDialogDidAccept('old.file', 'new.file')); + + await expect(saga.take()).resolves.toEqual( + fileStorageRenameFile('old.file', 'new.file'), + ); + }); + + it('should dispatch action on fileStorageRenameFile failure', async () => { + saga.put( + fileStorageDidFailToRenameFile('old.file', new Error('test error')), + ); + + await expect(saga.take()).resolves.toEqual(explorerDidFailToRenameFile()); + }); + + it('should dispatch action on fileStorageRenameFile success', async () => { + saga.put(fileStorageDidRenameFile('old.file')); + + await expect(saga.take()).resolves.toEqual(explorerDidRenameFile()); + }); + }); + + afterEach(async () => { + await saga.end(); + }); +}); + describe('handleExplorerDuplicateFile', () => { let saga: AsyncSaga; diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index dd0e5e7b..e44eddfb 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -24,11 +24,14 @@ import { fileStorageDidFailToDeleteFile, fileStorageDidFailToDumpAllFiles, fileStorageDidFailToReadFile, + fileStorageDidFailToRenameFile, fileStorageDidFailToWriteFile, fileStorageDidReadFile, + fileStorageDidRenameFile, fileStorageDidWriteFile, fileStorageDumpAllFiles, fileStorageReadFile, + fileStorageRenameFile, fileStorageWriteFile, } from '../fileStorage/actions'; import { @@ -54,10 +57,13 @@ import { explorerDidFailToDuplicateFile, explorerDidFailToExportFile, explorerDidFailToImportFiles, + explorerDidFailToRenameFile, explorerDidImportFiles, + explorerDidRenameFile, explorerDuplicateFile, explorerExportFile, explorerImportFiles, + explorerRenameFile, explorerUserActivateFile, explorerUserDidActivateFile, } from './actions'; @@ -76,6 +82,11 @@ import { newFileWizardDidCancel, newFileWizardShow, } from './newFileWizard/actions'; +import { + renameFileDialogDidAccept, + renameFileDialogDidCancel, + renameFileDialogShow, +} from './renameFileDialog/actions'; function* handleExplorerArchiveAllFiles(): Generator { try { @@ -302,6 +313,43 @@ function* handleExplorerDuplicateFile( } } +/** Connects user initiate rename file actions to the rename file dialog. */ +function* handleExplorerRenameFile( + action: ReturnType, +): Generator { + yield* put(renameFileDialogShow(action.fileName)); + + const { accepted, canceled } = yield* race({ + accepted: take(renameFileDialogDidAccept), + canceled: take(renameFileDialogDidCancel), + }); + + if (canceled) { + yield* put(explorerDidFailToRenameFile()); + return; + } + + defined(accepted); + + yield* put(fileStorageRenameFile(action.fileName, accepted.newName)); + + const didRename = yield* race({ + succeeded: take( + fileStorageDidRenameFile.when((a) => a.fileName === action.fileName), + ), + failed: take( + fileStorageDidFailToRenameFile.when((a) => a.fileName === action.fileName), + ), + }); + + if (didRename.failed) { + yield* put(explorerDidFailToRenameFile()); + return; + } + + yield* put(explorerDidRenameFile()); +} + function* handleExplorerExportFile( action: ReturnType, ): Generator { @@ -387,6 +435,7 @@ export default function* (): Generator { yield* takeEvery(explorerImportFiles, handleExplorerImportFiles); yield* takeEvery(explorerCreateNewFile, handleExplorerCreateNewFile); yield* takeEvery(explorerUserActivateFile, handleExplorerActivateFile); + yield* takeEvery(explorerRenameFile, handleExplorerRenameFile); yield* takeEvery(explorerDuplicateFile, handleExplorerDuplicateFile); yield* takeEvery(explorerExportFile, handleExplorerExportFile); yield* takeEvery(explorerDeleteFile, handleExplorerDeleteFile); diff --git a/src/explorer/translations/en.json b/src/explorer/translations/en.json index b83a7c35..57f19bf9 100644 --- a/src/explorer/translations/en.json +++ b/src/explorer/translations/en.json @@ -15,6 +15,7 @@ "navigation": "Navigate the tree with the arrow keys. Start typing the name of a file to search for a file. Additional keybindings are available:", "keybindings": { "primaryAction": "{key} to open the file in the code editor", + "rename": "{key} to start renaming the focused file", "duplicate": "{key} to duplicate focused file", "export": "{key} to export the focused file", "delete": "{key} to delete the focused file" @@ -24,6 +25,7 @@ } }, "treeItem": { + "renameTooltip": "Rename {fileName}", "duplicateTooltip": "Duplicate {fileName}", "exportTooltip": "Export {fileName}", "deleteTooltip": "Delete {fileName}"