explorer: new delete alert dialog

This replaces the delete notification with a new alert dialog. This
makes for better keyboard interaction and forces the user to make a
decision before doing anything else.

Also fixes closing the editor before deleting the file.

src/explorer/Explorer.tsx

@@ -37,6 +37,7 @@ import {
     explorerImportFiles,
     explorerRenameFile,
 } from './actions';
+import DeleteFileAlert from './deleteFileAlert/DeleteFileAlert';
 import { I18nId } from './i18n';
 import NewFileWizard from './newFileWizard/NewFileWizard';
 import RenameFileDialog from './renameFileDialog/RenameFileDialog';
@@ -380,6 +381,7 @@ const Explorer: React.VFC = () => {
             <FileTree i18n={i18n} />
             <NewFileWizard />
             <RenameFileDialog />
+            <DeleteFileAlert />
         </div>
     );
 };

src/explorer/actions.ts

@@ -164,3 +164,24 @@ export const explorerDeleteFile = createAction((fileName: string) => ({
     type: 'explorer.action.deleteFile',
     fileName,
 }));
+
+/**
+ * Action that indicates that {@link explorerDeleteFile} succeeded.
+ * @param fileName The file name.
+ */
+export const explorerDidDeleteFile = createAction((fileName: string) => ({
+    type: 'explorer.action.didDeleteFile',
+    fileName,
+}));
+
+/**
+ * Action that indicates that {@link explorerDeleteFile} failed.
+ * @param fileName The file name.
+ */
+export const explorerDidFailToDeleteFile = createAction(
+    (fileName: string, error: Error) => ({
+        type: 'explorer.action.didFailToDeleteFile',
+        fileName,
+        error,
+    }),
+);

src/explorer/deleteFileAlert/DeleteFileAlert.test.tsx

@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2022 The Pybricks Authors
+
+import { cleanup, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+import { testRender } from '../../../test';
+import DeleteFileAlert from './DeleteFileAlert';
+import { deleteFileAlertDidAccept, deleteFileAlertDidCancel } from './actions';
+
+afterEach(() => {
+    cleanup();
+});
+
+describe('accept', () => {
+    it('should dispatch accept action when delete button is clicked', async () => {
+        const [dialog, dispatch] = testRender(<DeleteFileAlert />, {
+            explorer: { deleteFileAlert: { fileName: 'test.file', isOpen: true } },
+        });
+
+        userEvent.click(dialog.getByRole('button', { name: 'Delete' }));
+        expect(dispatch).toHaveBeenCalledWith(deleteFileAlertDidAccept());
+    });
+
+    it('should dispatch accept action when enter is pressed ', async () => {
+        const [dialog, dispatch] = testRender(<DeleteFileAlert />, {
+            explorer: { deleteFileAlert: { fileName: 'test.file', isOpen: true } },
+        });
+
+        await waitFor(() =>
+            expect(dialog.getByRole('button', { name: 'Delete' })).toHaveFocus(),
+        );
+        userEvent.keyboard('{enter}');
+
+        expect(dispatch).toHaveBeenCalledWith(deleteFileAlertDidAccept());
+    });
+});
+
+describe('cancel', () => {
+    it('should dispatch cancel when keep button is clicked', () => {
+        const [dialog, dispatch] = testRender(<DeleteFileAlert />, {
+            explorer: { deleteFileAlert: { fileName: 'test.file', isOpen: true } },
+        });
+
+        userEvent.click(dialog.getByRole('button', { name: 'Keep' }));
+
+        expect(dispatch).toHaveBeenCalledWith(deleteFileAlertDidCancel());
+    });
+
+    it('should dispatch cancel when escape button is pressed', async () => {
+        const [dialog, dispatch] = testRender(<DeleteFileAlert />, {
+            explorer: { deleteFileAlert: { fileName: 'test.file', isOpen: true } },
+        });
+
+        await waitFor(() =>
+            expect(dialog.getByRole('button', { name: 'Delete' })).toHaveFocus(),
+        );
+        userEvent.keyboard('{esc}');
+
+        expect(dispatch).toHaveBeenCalledWith(deleteFileAlertDidCancel());
+    });
+});

src/explorer/deleteFileAlert/DeleteFileAlert.tsx

@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2022 The Pybricks Authors
+
+import { Alert, Classes, Intent } from '@blueprintjs/core';
+import { useI18n } from '@shopify/react-i18n';
+import React, { useCallback } from 'react';
+import { useDispatch } from 'react-redux';
+import { useSelector } from '../../reducers';
+import { deleteFileAlertDidAccept, deleteFileAlertDidCancel } from './actions';
+import { I18nId } from './i18n';
+
+const DeleteFileAlert: React.VoidFunctionComponent = () => {
+    const { isOpen, fileName } = useSelector((s) => s.explorer.deleteFileAlert);
+    const dispatch = useDispatch();
+    // istanbul ignore next: babel rewrites this line
+    const [i18n] = useI18n();
+
+    // a11y: focus primary button when dialog is opened
+    const handleOpened = useCallback((node: HTMLElement) => {
+        // HACK: get the accept button
+        // there doesn't seem to be a nice way to access it via props/ref/etc
+        const button = node.querySelector<HTMLButtonElement>(
+            `button.${Classes.INTENT_DANGER}`,
+        );
+
+        // istanbul ignore if: bug if reached
+        if (!button) {
+            console.error('bug: could not find accept button');
+            return;
+        }
+
+        button.focus();
+    }, []);
+
+    return (
+        <Alert
+            canEscapeKeyCancel={true}
+            canOutsideClickCancel={true}
+            isOpen={isOpen}
+            icon="trash"
+            intent={Intent.DANGER}
+            confirmButtonText={i18n.translate(I18nId.Accept)}
+            cancelButtonText={i18n.translate(I18nId.Cancel)}
+            onConfirm={() => dispatch(deleteFileAlertDidAccept())}
+            onCancel={() => dispatch(deleteFileAlertDidCancel())}
+            onOpened={handleOpened}
+        >
+            {i18n.translate(I18nId.Message, { fileName })}
+        </Alert>
+    );
+};
+
+export default DeleteFileAlert;

src/explorer/deleteFileAlert/actions.ts

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2022 The Pybricks Authors
+
+import { createAction } from '../../actions';
+
+/**
+ * Requests to show the delete file alert dialog.
+ * @param fileName The file name to display to the user.
+ */
+export const deleteFileAlertShow = createAction((fileName: string) => ({
+    type: 'explorer.deleteFileAlert.action.show',
+    fileName,
+}));
+
+/**
+ * Indicates that the user accepted the delete file alert dialog.
+ */
+export const deleteFileAlertDidAccept = createAction(() => ({
+    type: 'explorer.deleteFileAlert.action.didAccept',
+}));
+
+/**
+ * Indicates that the user canceled the delete file alert dialog.
+ */
+export const deleteFileAlertDidCancel = createAction(() => ({
+    type: 'explorer.deleteFileAlert.action.didCancel',
+}));

src/explorer/deleteFileAlert/i18n.test.ts

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2022 The Pybricks Authors
+
+import { lookup } from '../../../test';
+import { I18nId } from './i18n';
+import en from './translations/en.json';
+
+describe('Ensure .json file has matches for I18nId', () => {
+    test.each(Object.values(I18nId))('%s', (id) => {
+        expect(lookup(en, id)).toBeDefined();
+    });
+});

src/explorer/deleteFileAlert/i18n.ts

@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2022 The Pybricks Authors
+
+export enum I18nId {
+    Accept = 'action.accept',
+    Cancel = 'action.cancel',
+    Message = 'message',
+}

src/explorer/deleteFileAlert/reducers.ts

@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2022 The Pybricks Authors
+
+import { Reducer, combineReducers } from 'redux';
+import {
+    deleteFileAlertDidAccept,
+    deleteFileAlertDidCancel,
+    deleteFileAlertShow,
+} from './actions';
+
+/** Controls the delete file alert dialog isOpen state. */
+const isOpen: Reducer<boolean> = (state = false, action) => {
+    if (deleteFileAlertShow.matches(action)) {
+        return true;
+    }
+
+    if (
+        deleteFileAlertDidAccept.matches(action) ||
+        deleteFileAlertDidCancel.matches(action)
+    ) {
+        return false;
+    }
+
+    return state;
+};
+
+/** Controls the file name displayed in the file alert dialog. */
+const fileName: Reducer<string> = (state = '', action) => {
+    if (deleteFileAlertShow.matches(action)) {
+        return action.fileName;
+    }
+
+    return state;
+};
+
+export default combineReducers({ isOpen, fileName });

src/explorer/deleteFileAlert/translations/en.json

@@ -0,0 +1,7 @@
+{
+    "message": "The file '{fileName}' will be permanently deleted. This cannot be undone.",
+    "action": {
+        "accept": "Delete",
+        "cancel": "Keep"
+    }
+}

src/explorer/reducers.ts

@@ -10,6 +10,7 @@ import {
     fileStorageDidRemoveItem,
 } from '../fileStorage/actions';
 
+import deleteFileAlert from './deleteFileAlert/reducers';
 import newFileWizard from './newFileWizard/reducers';
 import renameFileDialog from './renameFileDialog/reducers';
 
@@ -52,4 +53,9 @@ const files: Reducer<readonly ExplorerFileInfo[]> = (state = [], action) => {
     return state;
 };
 
-export default combineReducers({ files, newFileWizard, renameFileDialog });
+export default combineReducers({
+    files,
+    deleteFileAlert,
+    newFileWizard,
+    renameFileDialog,
+});

src/explorer/sagas.test.ts

@@ -4,18 +4,24 @@
 import * as browserFsAccess from 'browser-fs-access';
 import { FileWithHandle } from 'browser-fs-access';
 import { mock } from 'jest-mock-extended';
-import { AsyncSaga } from '../../test';
+import { AsyncSaga, uuid } from '../../test';
 import {
     editorActivateFile,
+    editorCloseFile,
     editorDidActivateFile,
+    editorDidCloseFile,
     editorDidFailToActivateFile,
 } from '../editor/actions';
 import {
+    fileStorageDeleteFile,
+    fileStorageDidDeleteFile,
     fileStorageDidDumpAllFiles,
+    fileStorageDidFailToDeleteFile,
     fileStorageDidFailToDumpAllFiles,
     fileStorageDidFailToReadFile,
     fileStorageDidFailToRenameFile,
     fileStorageDidReadFile,
+    fileStorageDidRemoveItem,
     fileStorageDidRenameFile,
     fileStorageDidWriteFile,
     fileStorageDumpAllFiles,
@@ -28,13 +34,16 @@ import {
     explorerActivateFile,
     explorerArchiveAllFiles,
     explorerCreateNewFile,
+    explorerDeleteFile,
     explorerDidActivateFile,
     explorerDidArchiveAllFiles,
     explorerDidCreateNewFile,
+    explorerDidDeleteFile,
     explorerDidExportFile,
     explorerDidFailToActivateFile,
     explorerDidFailToArchiveAllFiles,
     explorerDidFailToCreateNewFile,
+    explorerDidFailToDeleteFile,
     explorerDidFailToExportFile,
     explorerDidFailToImportFiles,
     explorerDidFailToRenameFile,
@@ -44,6 +53,11 @@ import {
     explorerImportFiles,
     explorerRenameFile,
 } from './actions';
+import {
+    deleteFileAlertDidAccept,
+    deleteFileAlertDidCancel,
+    deleteFileAlertShow,
+} from './deleteFileAlert/actions';
 import {
     Hub,
     newFileWizardDidAccept,
@@ -344,3 +358,73 @@ describe('handleExplorerExportFile', () => {
         await saga.end();
     });
 });
+
+describe('handleExplorerDeleteFile', () => {
+    let saga: AsyncSaga;
+    const testFile = 'test.file';
+
+    beforeEach(async () => {
+        saga = new AsyncSaga(explorer);
+
+        saga.put(explorerDeleteFile(testFile));
+
+        await expect(saga.take()).resolves.toEqual(deleteFileAlertShow(testFile));
+    });
+
+    it('should fail with AbortError if canceled', async () => {
+        saga.put(deleteFileAlertDidCancel());
+
+        await expect(saga.take()).resolves.toEqual(
+            explorerDidFailToDeleteFile(
+                testFile,
+                new DOMException('user canceled', 'AbortError'),
+            ),
+        );
+    });
+
+    it('should fail with AbortError if file was removed before user accept/cancel', async () => {
+        saga.put(
+            fileStorageDidRemoveItem({ path: testFile, uuid: uuid(0), sha256: '' }),
+        );
+
+        // should programmatically cancel the dialog
+        await expect(saga.take()).resolves.toEqual(deleteFileAlertDidCancel());
+
+        await expect(saga.take()).resolves.toEqual(
+            explorerDidFailToDeleteFile(
+                testFile,
+                new DOMException('file was removed', 'AbortError'),
+            ),
+        );
+    });
+
+    describe('accepted', () => {
+        beforeEach(async () => {
+            saga.put(deleteFileAlertDidAccept());
+
+            // should close the editor first
+            await expect(saga.take()).resolves.toEqual(editorCloseFile(testFile));
+            saga.put(editorDidCloseFile(testFile));
+
+            // then delete the file
+            await expect(saga.take()).resolves.toEqual(fileStorageDeleteFile(testFile));
+        });
+
+        it('should propagate error', async () => {
+            const testError = new Error('test error');
+            saga.put(fileStorageDidFailToDeleteFile(testFile, testError));
+            await expect(saga.take()).resolves.toEqual(
+                explorerDidFailToDeleteFile(testFile, testError),
+            );
+        });
+
+        it('should succeed', async () => {
+            saga.put(fileStorageDidDeleteFile(testFile));
+            await expect(saga.take()).resolves.toEqual(explorerDidDeleteFile(testFile));
+        });
+    });
+
+    afterEach(async () => {
+        await saga.end();
+    });
+});

src/explorer/sagas.ts

@@ -14,17 +14,23 @@ import {
 } from 'typed-redux-saga/macro';
 import {
     editorActivateFile,
+    editorCloseFile,
     editorDidActivateFile,
+    editorDidCloseFile,
     editorDidFailToActivateFile,
 } from '../editor/actions';
 import { getPybricksMicroPythonFileTemplate } from '../editor/pybricksMicroPython';
 import {
+    fileStorageDeleteFile,
+    fileStorageDidDeleteFile,
     fileStorageDidDumpAllFiles,
+    fileStorageDidFailToDeleteFile,
     fileStorageDidFailToDumpAllFiles,
     fileStorageDidFailToReadFile,
     fileStorageDidFailToRenameFile,
     fileStorageDidFailToWriteFile,
     fileStorageDidReadFile,
+    fileStorageDidRemoveItem,
     fileStorageDidRenameFile,
     fileStorageDidWriteFile,
     fileStorageDumpAllFiles,
@@ -45,13 +51,16 @@ import {
     explorerActivateFile,
     explorerArchiveAllFiles,
     explorerCreateNewFile,
+    explorerDeleteFile,
     explorerDidActivateFile,
     explorerDidArchiveAllFiles,
     explorerDidCreateNewFile,
+    explorerDidDeleteFile,
     explorerDidExportFile,
     explorerDidFailToActivateFile,
     explorerDidFailToArchiveAllFiles,
     explorerDidFailToCreateNewFile,
+    explorerDidFailToDeleteFile,
     explorerDidFailToExportFile,
     explorerDidFailToImportFiles,
     explorerDidFailToRenameFile,
@@ -61,6 +70,11 @@ import {
     explorerImportFiles,
     explorerRenameFile,
 } from './actions';
+import {
+    deleteFileAlertDidAccept,
+    deleteFileAlertDidCancel,
+    deleteFileAlertShow,
+} from './deleteFileAlert/actions';
 import {
     newFileWizardDidAccept,
     newFileWizardDidCancel,
@@ -328,6 +342,56 @@ function* handleExplorerExportFile(
     }
 }
 
+function* handleExplorerDeleteFile(action: ReturnType<typeof explorerDeleteFile>) {
+    try {
+        yield* put(deleteFileAlertShow(action.fileName));
+
+        const { didCancel, didRemove } = yield* race({
+            didAccept: take(deleteFileAlertDidAccept),
+            didCancel: take(deleteFileAlertDidCancel),
+            didRemove: take(
+                fileStorageDidRemoveItem.when((a) => a.file.path === action.fileName),
+            ),
+        });
+
+        if (didCancel) {
+            throw new DOMException('user canceled', 'AbortError');
+        }
+
+        // automatically cancel the dialog, if the file was removed, e.g. it was
+        // deleted in a different window
+        if (didRemove) {
+            yield* put(deleteFileAlertDidCancel());
+            throw new DOMException('file was removed', 'AbortError');
+        }
+
+        // at this point we know the user accepted
+
+        // have to close editor before deleting, otherwise we get "in use" error
+        yield* put(editorCloseFile(action.fileName));
+        yield* take(editorDidCloseFile.when((a) => a.fileName === action.fileName));
+
+        yield* put(fileStorageDeleteFile(action.fileName));
+
+        const { didFailToDelete } = yield* race({
+            didDelete: take(
+                fileStorageDidDeleteFile.when((a) => a.path === action.fileName),
+            ),
+            didFailToDelete: take(
+                fileStorageDidFailToDeleteFile.when((a) => a.path === action.fileName),
+            ),
+        });
+
+        if (didFailToDelete) {
+            throw didFailToDelete.error;
+        }
+
+        yield* put(explorerDidDeleteFile(action.fileName));
+    } catch (err) {
+        yield* put(explorerDidFailToDeleteFile(action.fileName, ensureError(err)));
+    }
+}
+
 export default function* (): Generator {
     yield* takeEvery(explorerArchiveAllFiles, handleExplorerArchiveAllFiles);
     yield* takeEvery(explorerImportFiles, handleExplorerImportFiles);
@@ -338,4 +402,5 @@ export default function* (): Generator {
     // this to happen in practice though.
     yield* takeLatest(explorerRenameFile, handleExplorerRenameFile);
     yield* takeLatest(explorerExportFile, handleExplorerExportFile);
+    yield* takeLatest(explorerDeleteFile, handleExplorerDeleteFile);
 }