rework alerts

This starts moving alerts to the same subsystem where they are relevant
instead of putting everything in notifications.

So far, only the explorer file in use error is handled like this.
This commit is contained in:
David Lechner
2022-05-20 19:25:18 -05:00
parent 4e88605b16
commit e7366c8afd
31 changed files with 649 additions and 138 deletions
+3 -3
View File
@@ -9,13 +9,13 @@ import { FileMetadata } from '../fileStorage';
import { useFileStorageMetadata } from '../fileStorage/hooks';
import Explorer from './Explorer';
import {
explorerActivateFile,
explorerArchiveAllFiles,
explorerCreateNewFile,
explorerDeleteFile,
explorerDuplicateFile,
explorerExportFile,
explorerImportFiles,
explorerUserActivateFile,
} from './actions';
afterEach(async () => {
@@ -74,7 +74,7 @@ describe('tree item', () => {
userEvent.click(treeItem);
expect(dispatch).toHaveBeenCalledWith(explorerActivateFile('test.file'));
expect(dispatch).toHaveBeenCalledWith(explorerUserActivateFile('test.file'));
});
it('should dispatch action when key is pressed', async () => {
@@ -86,7 +86,7 @@ describe('tree item', () => {
userEvent.click(treeItem);
userEvent.keyboard('{enter}');
expect(dispatch).toHaveBeenCalledWith(explorerActivateFile('test.file'));
expect(dispatch).toHaveBeenCalledWith(explorerUserActivateFile('test.file'));
});
describe('duplicate', () => {
+2 -2
View File
@@ -31,13 +31,13 @@ import { useFileStorageMetadata } from '../fileStorage/hooks';
import { isMacOS } from '../utils/os';
import { TreeItemContext, TreeItemData, renderers } from '../utils/tree-renderer';
import {
explorerActivateFile,
explorerArchiveAllFiles,
explorerCreateNewFile,
explorerDeleteFile,
explorerDuplicateFile,
explorerExportFile,
explorerImportFiles,
explorerUserActivateFile,
} from './actions';
import DeleteFileAlert from './deleteFileAlert/DeleteFileAlert';
import DuplicateFileDialog from './duplicateFileDialog/DuplicateFileDialog';
@@ -373,7 +373,7 @@ const FileTree: React.VoidFunctionComponent<FileTreeProps> = ({ i18n }) => {
canRename={false} // we implement our own rename handler
onFocusItem={(item) => setFocusedItem(item.index)}
onPrimaryAction={(item) =>
dispatch(explorerActivateFile(item.data.fileName))
dispatch(explorerUserActivateFile(item.data.fileName))
}
>
<div className="pb-explorer-file-tree">
+5 -18
View File
@@ -75,33 +75,20 @@ export const explorerDidFailToCreateNewFile = createAction((error: 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',
export const explorerUserActivateFile = createAction((fileName: string) => ({
type: 'explorer.user.action.activateFile',
fileName,
}));
/**
* Indicates that {@link explorerActivateFile} succeeded.
* Indicates that {@link explorerUserActivateFile} completed.
* @param fileName The file name.
*/
export const explorerDidActivateFile = createAction((fileName: string) => ({
type: 'explorer.action.didActivateFile',
export const explorerUserDidActivateFile = createAction((fileName: string) => ({
type: 'explorer.user.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 duplicate a file.
* @param fileName The file name.
@@ -0,0 +1,21 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2022 The Pybricks Authors
import React from 'react';
import { testRender } from '../../../test';
import { fileInUse } from './FileInUseAlert';
it('should be valid', () => {
const callback = jest.fn();
const toast = fileInUse(callback, { fileName: 'test.file' });
// TODO: refactor this to a common function to be used by all alerts
// it should render
const [message] = testRender(<>{toast.message}</>);
expect(message).toBeDefined();
// it should have a dismiss callback
toast.onDismiss?.(false);
expect(callback).toHaveBeenCalledWith('dismiss');
});
+32
View File
@@ -0,0 +1,32 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2022 The Pybricks Authors
import { Intent } from '@blueprintjs/core';
import { useI18n } from '@shopify/react-i18n';
import React from 'react';
import { CreateToast } from '../../i18nToaster';
import { I18nId } from './i18n';
type FileInUseAlertProps = {
fileName: string;
};
const FileInUseAlert: React.VoidFunctionComponent<FileInUseAlertProps> = ({
fileName,
}) => {
// istanbul ignore next: babel-loader rewrites this line
const [i18n] = useI18n();
return <>{i18n.translate(I18nId.FileInUseMessage, { fileName })}</>;
};
export const fileInUse: CreateToast<{ fileName: string }> = (
onAction,
{ fileName },
) => {
return {
message: <FileInUseAlert fileName={fileName} />,
icon: 'error',
intent: Intent.DANGER,
onDismiss: () => onAction('dismiss'),
};
};
+12
View File
@@ -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();
});
});
+6
View File
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2022 The Pybricks Authors
export enum I18nId {
FileInUseMessage = 'fileInUse.message',
}
+7
View File
@@ -0,0 +1,7 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2022 The Pybricks Authors
import { fileInUse } from './FileInUseAlert';
// gathers all of the alert creation functions for passing up to the top level
export default { fileInUse };
+5
View File
@@ -0,0 +1,5 @@
{
"fileInUse": {
"message": "The file '{fileName}' could not be opened. It is already open in another window."
}
}
+24 -8
View File
@@ -5,6 +5,7 @@ import * as browserFsAccess from 'browser-fs-access';
import { FileWithHandle } from 'browser-fs-access';
import { mock } from 'jest-mock-extended';
import { AsyncSaga } from '../../test';
import { alertsShowAlert } from '../alerts/actions';
import {
editorActivateFile,
editorCloseFile,
@@ -12,6 +13,7 @@ import {
editorDidCloseFile,
editorDidFailToActivateFile,
} from '../editor/actions';
import { EditorError } from '../editor/error';
import {
fileStorageCopyFile,
fileStorageDeleteFile,
@@ -30,17 +32,14 @@ import {
} from '../fileStorage/actions';
import { pythonFileExtension } from '../pybricksMicropython/lib';
import {
explorerActivateFile,
explorerArchiveAllFiles,
explorerCreateNewFile,
explorerDeleteFile,
explorerDidActivateFile,
explorerDidArchiveAllFiles,
explorerDidCreateNewFile,
explorerDidDeleteFile,
explorerDidDuplicateFile,
explorerDidExportFile,
explorerDidFailToActivateFile,
explorerDidFailToArchiveAllFiles,
explorerDidFailToCreateNewFile,
explorerDidFailToDeleteFile,
@@ -51,6 +50,8 @@ import {
explorerDuplicateFile,
explorerExportFile,
explorerImportFiles,
explorerUserActivateFile,
explorerUserDidActivateFile,
} from './actions';
import {
deleteFileAlertDidAccept,
@@ -243,25 +244,40 @@ describe('handleExplorerActivateFile', () => {
beforeEach(async () => {
saga = new AsyncSaga(explorer);
saga.put(explorerActivateFile('test.file'));
saga.put(explorerUserActivateFile('test.file'));
await expect(saga.take()).resolves.toEqual(editorActivateFile('test.file'));
});
it('should propagate error', async () => {
it('should alert file in use error', async () => {
const testError = new EditorError('FileInUse', 'test error');
saga.put(editorDidFailToActivateFile('test.file', testError));
await expect(saga.take()).resolves.toEqual(
alertsShowAlert('explorer', 'fileInUse', { fileName: 'test.file' }),
);
await expect(saga.take()).resolves.toEqual(
explorerUserDidActivateFile('test.file'),
);
});
it('should alert unexpected error', async () => {
const testError = new Error('test error');
saga.put(editorDidFailToActivateFile('test.file', testError));
await expect(saga.take()).resolves.toEqual(
explorerDidFailToActivateFile('test.file', testError),
alertsShowAlert('alerts', 'unexpectedError', { error: testError }),
);
await expect(saga.take()).resolves.toEqual(
explorerUserDidActivateFile('test.file'),
);
});
it('should propagate success', async () => {
it('should notify success', async () => {
saga.put(editorDidActivateFile('test.file'));
await expect(saga.take()).resolves.toEqual(
explorerDidActivateFile('test.file'),
explorerUserDidActivateFile('test.file'),
);
});
+22 -16
View File
@@ -4,6 +4,7 @@
import { fileOpen, fileSave } from 'browser-fs-access';
import JSZip from 'jszip';
import { call, put, race, take, takeEvery } from 'typed-redux-saga/macro';
import { alertsShowAlert } from '../alerts/actions';
import {
editorActivateFile,
editorCloseFile,
@@ -11,6 +12,7 @@ import {
editorDidCloseFile,
editorDidFailToActivateFile,
} from '../editor/actions';
import { EditorError } from '../editor/error';
import { getPybricksMicroPythonFileTemplate } from '../editor/pybricksMicroPython';
import {
fileStorageCopyFile,
@@ -38,17 +40,14 @@ import {
} from '../pybricksMicropython/lib';
import { defined, ensureError, timestamp } from '../utils';
import {
explorerActivateFile,
explorerArchiveAllFiles,
explorerCreateNewFile,
explorerDeleteFile,
explorerDidActivateFile,
explorerDidArchiveAllFiles,
explorerDidCreateNewFile,
explorerDidDeleteFile,
explorerDidDuplicateFile,
explorerDidExportFile,
explorerDidFailToActivateFile,
explorerDidFailToArchiveAllFiles,
explorerDidFailToCreateNewFile,
explorerDidFailToDeleteFile,
@@ -59,6 +58,8 @@ import {
explorerDuplicateFile,
explorerExportFile,
explorerImportFiles,
explorerUserActivateFile,
explorerUserDidActivateFile,
} from './actions';
import {
deleteFileAlertDidAccept,
@@ -226,11 +227,11 @@ function* handleExplorerCreateNewFile(): Generator {
* @param action
*/
function* handleExplorerActivateFile(
action: ReturnType<typeof explorerActivateFile>,
action: ReturnType<typeof explorerUserActivateFile>,
): Generator {
yield* put(editorActivateFile(action.fileName));
const { didActivate, didFailToActivate } = yield* race({
const { didFailToActivate } = yield* race({
didActivate: take(
editorDidActivateFile.when((a) => a.fileName === action.fileName),
),
@@ -240,18 +241,23 @@ function* handleExplorerActivateFile(
});
if (didFailToActivate) {
yield* put(
explorerDidFailToActivateFile(
didFailToActivate.fileName,
didFailToActivate.error,
),
);
return;
if (
didFailToActivate.error instanceof EditorError &&
didFailToActivate.error.name === 'FileInUse'
) {
yield* put(
alertsShowAlert('explorer', 'fileInUse', { fileName: action.fileName }),
);
} else {
yield* put(
alertsShowAlert('alerts', 'unexpectedError', {
error: didFailToActivate.error,
}),
);
}
}
defined(didActivate);
yield* put(explorerDidActivateFile(didActivate.fileName));
yield* put(explorerUserDidActivateFile(action.fileName));
}
/** Connects user initiate duplicate file actions to the duplicate file dialog. */
@@ -380,7 +386,7 @@ export default function* (): Generator {
yield* takeEvery(explorerArchiveAllFiles, handleExplorerArchiveAllFiles);
yield* takeEvery(explorerImportFiles, handleExplorerImportFiles);
yield* takeEvery(explorerCreateNewFile, handleExplorerCreateNewFile);
yield* takeEvery(explorerActivateFile, handleExplorerActivateFile);
yield* takeEvery(explorerUserActivateFile, handleExplorerActivateFile);
yield* takeEvery(explorerDuplicateFile, handleExplorerDuplicateFile);
yield* takeEvery(explorerExportFile, handleExplorerExportFile);
yield* takeEvery(explorerDeleteFile, handleExplorerDeleteFile);